[04:49] davecheney: hiya [04:52] davecheney: FYI here's a sketch of the possible firewall-management code: http://paste.ubuntu.com/1089246/ [04:52] davecheney: (we discussed it when you weren't around) [05:22] * davecheney reads wrtp [05:44] davecheney: actually, i realise that i didn't update it for the most recent version of the OpenPorts API: http://paste.ubuntu.com/1089282/ [05:47] wrtp: on da phone, hang on [05:48] davecheney: np [05:55] wrtp: that propsal looks good [05:55] davecheney: cool [05:56] open question, are you, i and frank all trying to eat the same elephant ? [05:56] this is simlar to the branch be proposed on wednesday [05:56] davecheney: i hope so [05:57] davecheney: this was a response to frank's proposal [05:57] he was approaching it from the state side, which sort of makes sense because in the py version there is a state.FirewallManager [05:57] davecheney: i'm not quite sure why he did it actually, as it's really just part of the PA, but it was useful for focusing all the issues in one place [05:57] davecheney: ah! [05:58] but it does exactly that, although simpler [05:58] davecheney: i hadn't quite got that it was in state [05:58] for every machine or unit, it registers a callback (it == firewall manager) [05:58] then does (stuff) when that callback fires [05:58] davecheney: the main difference is that in the python version, it checks the "real" state every time. [05:58] in some ways in twisted it's simpler [05:58] in our version we need to start and manage a watcher goroutine [05:59] davecheney: whereas this CL keeps track of the state and only issues OpenPort/ClosePort requests at well-defined times [05:59] this is the <-serviceChange bit ? [06:00] davecheney: yeah. there's only one central goroutine that manages the whole thing [06:00] wrtp: here is the problem i see [06:00] davecheney: i had difficulty keeping track of all the potential state interactions when there were multiple independent goroutines operating on it. [06:00] if we have N port watchers, how do we funnel all those change messages into a single channel ? [06:01] davecheney: easy [06:01] davecheney: just start a goroutine for each watcher [06:01] can we pass a channel to watchPort ? [06:01] yup, goroutine that watches the watcher and forwardst he messages (means more tomb.Tombs ..) [06:01] davecheney: which prepends some data and sends on a central channel [06:01] davecheney: i think we can get away without any more tombs [06:01] wrtp: i'd like to try [06:02] nothing wrong with tomb [06:02] but less of everything is better, when there are going to be a lot of these port watchers [06:02] davecheney: the only thing wrong with tomb is that it's more heavyweight than necessary sometimes [06:02] wrtp: i have to step out for a few hours, but will be online from ~8/9 my time this evening [06:02] davecheney: ok [06:02] davecheney: i think that all the mux goroutines can watch the same tomb [06:03] wrtp: that would be excellent, because the most likely cause of them wanting to die is the firewaller goroutine exiting [06:03] also, http://codereview.appspot.com/6333067/ [06:04] i imagine this would be a valid chassis for your proposal ? [06:04] davecheney: absolutely [06:04] davecheney: frank is now working on integrating my sketch with your chassis [06:04] wrtp: could I ask for some of your time today to get a run through of that chassis [06:05] davecheney: definitely [06:05] it doesn't do anyhthing at the moment so the amount of review should be minimal [06:05] especially at is a carbon copy of the provisioner [06:05] which makes me wonder if some of the dulication can be factored out [06:05] davecheney: it looks fine to me at first glance [06:05] but that is not important right now [06:06] wrtp: just got an email from minux, he sent me a photo out front of building 47 on the google campus with some of the go team [06:06] davecheney: cool. link? [06:07] i'll forward it [06:10] wrtp: in ya gmail [06:11] davecheney: rob looks very blues brothers [06:11] davecheney: thanks, BTW [06:19] wrtp: have a good one, be online later [06:20] davecheney: how long? [06:20] * wrtp can't remember time zones! [06:21] davecheney: np, just worked it out [06:21] davecheney: i'll be around [07:30] fwereade_, TheMue: mornin' [07:30] wrtp, TheMue: similarly :) [07:31] wrtp, so, what am I doing in ComposeConfig that I don't need to? :) [07:31] fwereade_: i have an idea, but i haven't quite been able to make it work yet [07:32] wrtp, yeah, my experience has been that it's a bit harder than it sounds like it should be ;) [07:32] * fwereade_ looks sadly at this morning's conflicts [07:32] fwereade_: here's what i what i'm trying: http://paste.ubuntu.com/1089407/ [07:33] wrtp, hmm, interesting [07:34] fwereade_: but there's a problem. [07:34] wrtp, I felt that we were doing so much with the maps that the internal types just complicated matters [07:34] fwereade_: both the provider config and the config config require fields that have defaults provided by the other. [07:35] fwereade_: so the initial config.Compose fails because it hasn't got a default-series [07:35] wrtp, yeah, indeed, it's fiddly [07:37] fwereade_: i like the security that the internal type buys us [07:38] fwereade_: it doesn't seem right that core after the initial configuration is still type coercing [07:38] s/core/code/ [07:39] wrtp, agreed, very much so; but converting back and forth between dicts and internal types *all the damn time*, because the composition api needs dicts (or at least divt-backed configs) seems even worse [07:40] fwereade_: i'm not sure [07:40] wrtp, and niemeyer seems very convinced that an enviroment config "really is" a map [07:41] fwereade_: outside of a provider, yes [07:41] wrtp, I consider that argument to be utterly invalid, it's as much a map as every single other type in the system [07:41] fwereade_: inside a provider, it's a known thing [07:41] wrtp, that is a serialization format, surely? just like for every other type in the system that we serialize..? [07:43] fwereade_: the difficulty is that it's not just a serialisation format - it's a serialisation that we want to process, rather than just unmarshal. [07:43] wrtp, fwereade_ : Oh, sorry, morning from my side too. Just had received my malt #29, a Laphroaig Cask Strength Batch 004 *yummy* [07:43] TheMue: don't drink it all this morning! [07:44] wrtp: hehe, no, will open it this evening [07:44] wrtp, well, I see no good reason not to just marshal/unmarshal the type as a real type once we know what it is, and to handle *all* the funky processing at user input time [07:44] TheMue, nice [07:45] fwereade_: what about secrets? [07:46] fwereade_: i.e. how can we marshal the type without some info? [07:46] fwereade_: also, SetAttr [07:47] fwereade_: we need to be able to set environment attributes, but just marshalling and unmarshalling doesn't easily allow that [07:47] wrtp, I'm not sure I follow your concerns [07:48] wrtp, the secrets thing maybe needs a little bit of rearrangement [07:48] wrtp, but then I've been asked not to think about secrets for now anyway [07:49] wrtp, and the type could surely handle user-defined config files, and env-set command data, in the same way? [07:50] fwereade_: part of the cause of the current issues, i think, is the way that concerns about the config are divided between the config package and the provider package [07:51] fwereade_: if the provider did it all, i don't think there would be so much of a problem. [07:51] wrtp, wouldn't that just mean that we need to duplicate everything in config across every provider? [07:51] wrtp, I am maybe taking this whole situation a little personally [07:52] wrtp, because the very first thing I did when I joined the team was to unpick the ec2 provider so we could actually use the common bits in orchestra [07:52] wrtp, and it was not easy [07:52] fwereade_: of course, you'd need to be careful about consistency across providers, but that's true in any case [07:53] fwereade_: i don't think it would mean that we'd need to duplicate everything. [07:53] wrtp, and what I see in go is a wilful refusal to heed the lessons of the python port -- it feels like a deliberate decision was made to replicate a known mistake in the python :/ [07:53] fwereade_: there would be some duplication, yes, but anything substantial could be factored out and put in the environs package [07:54] fwereade_: that's always been the intention [07:54] fwereade_: if the providers are in charge, they can choose what shared bits to use from the environs package [07:54] fwereade_: (readAuthorizedKeys being one example) [07:55] s/the intention/my intention/ [07:56] s/the environs package/any package we choose to put shared bits in/ [07:56] wrtp, I just don't see how it's a good idea from any perspective except the let's-just-hack-it-together-quickly one [07:57] wrtp, which I thought it was our avowed intention to avoid [07:57] fwereade_: which code duplication are you particularly thinking of here? [08:00] wrtp, all the bootstrap/could-init stuff, default-series and authorized-keys, things like we discussed yesterday with ports handling on instance start [08:00] wrtp, the prts handling I agree was not well factored in the python [08:00] fwereade_: prts? [08:00] wrtp, sorry, ports [08:00] ah [08:02] fwereade_: it has always been my intention to move environs/ec2/cloudinit.go to environs/cloudinit.go when it's needed [08:02] fwereade_: it was designed to be used by any package (hence the "providerType" field) [08:02] s/package/provider/ [08:02] fwereade_: same with readAuthorizedKeys [08:02] wrtp, and so what we end up with is bits of common code mixed in with provider-specific code, and a big nasty surprise waiting for us when we try to actually use them elsewhere [08:03] wrtp, I suspect you are going to say it's ok to duplicate all the structural code (ie doing the right things at the right time) across the providers [08:03] fwereade_: i think we end up with providers having a coherent interface and using what is necessary to fulfil that interface [08:03] fwereade_: the tests will test that we've got the structural stuff right [08:04] fwereade_: and it will all end up nicely clear and understandable, i hope [08:05] wrtp, I hope you're right [08:05] wrtp, IMO you've severely underestimated the subtleties [08:06] fwereade_: maybe i have. i put my faith in encapsulation :-) [08:06] wrtp, and duplication :/ [08:07] wrtp: Did you thought about this way http://paste.ubuntu.com/1089445/ to send all machnine unit changes to the firewaller? [08:07] fwereade_: we're guessing about that. yes, there will be code similarities between providers, but i'm hoping there won't be significant duplication [08:07] wrtp: So the main loop of the firewaller handles all like you porposed. [08:08] TheMue: not quite [08:08] TheMue: one mo [08:08] wrtp, sorry, I'm just feeling generally negative about things because, wtf, this whole config saga [08:09] fwereade_: i totally understand [08:09] wrtp, honestly I'm thinking it's all sunk costs, fuck it, just use a map throughout [08:10] * wrtp resists, but is perhaps weakening. seems a pity but... [08:12] wrtp, the trouble is this is *another* week pissed up the wall [08:12] fwereade_: agreed [08:12] fwereade_: let's just do it [08:13] wrtp, "let's return actual configs from State.EnvironConfig"; "sounds good"; for {"ok, I'll give it a go"; "no, not like that"} :/ [08:14] fwereade_: :-( [08:17] wrtp, and all I wanted to do was to fix state.Initialize so I could fix the high-priority default-series bug that was assigned to me, pulling me away from the unit agent stuff [08:18] TheMue: something like this perhaps? http://paste.ubuntu.com/1089457/ [08:18] fwereade_: yeah, it's gone a bit out of control [08:19] TheMue: it's important that the changes on the channel include the information about what the change is about. [08:23] TheMue: slightly more accurate: http://paste.ubuntu.com/1089462/ [08:23] wrtp: ah, thx [08:24] wrtp: funnily it will drive the whole stuff into the same complexity direction as before, but with a cleaner interface [08:24] TheMue: the important difference is that the state is only managed in one goroutine [08:25] TheMue: stuff like that machine loop goroutine is just a conduit - it does nothing active [08:25] TheMue: and that, i think, makes everything easier to reason about [08:26] wrtp: yip [08:27] TheMue: http://www.youtube.com/watch?v=SPPS8vExLmE :-) [08:31] wrtp: ;) yip [10:26] hey. [10:27] Aram: yo [10:41] fwereade_: ping [10:44] Aram: Hi [10:47] fwereade_: i know it's way too late, but once i thought of this idea, i had to try it out. it feels much nicer to me, but it might be total crack. http://paste.ubuntu.com/1089617/ [10:50] wrtp, thanks -- I'll try to look at it this pm, but cath's away and I'll be with laura :( (well, :) as well, but you know ;)) [10:50] fwereade_: if you want to declare your train wrecked and you like this idea, i'd be ok to take the flak from here if you like. [10:50] fwereade_: ok [10:51] wrtp, it's appreciated, I'll think on it :) [10:51] wrtp, thanks :) [10:52] fwereade_: that code passes the dummy tests BTW [10:53] fwereade_: although i haven't added a test for default-series [12:26] Hmm, starting with useful tests without blowing the whole stuff up isn't simple. [14:04] fwereade_, davecheney, TheMue: here's an configuration alternative. it feels quite a bit cleaner to me than env-config-trainwreck. i wonder what you think: https://codereview.appspot.com/6343107/ [14:05] i *think* it provides as much flexibility as the other, but doesn't require nearly as much messing around with maps [14:06] the heart of the proposal is in https://codereview.appspot.com/6343107/diff/1/environs/interface.go [14:16] wrtp: Could you enlighten me, what is this "authorized-keys" stuff in config.go? [14:17] TheMue: authorized-keys is a standard environment setting across all providers [14:17] TheMue: it's also possible to use authorized-keys-path, which gives the name of a file containing the authorized keys [14:18] TheMue: the authorized keys are ssh keys [14:18] wrtp: per config entry? [14:18] TheMue: yeah [14:18] wrtp: ok, understand [14:18] TheMue: that is, each config entry can have a different set of auth keys [14:19] wrtp: How variable is the config scheme? [14:19] TheMue: as variable as we want to make it [14:20] wrtp: In the sense of "per software version", not in the sense of "how much possibilities this software has", sorry. [14:20] TheMue: currently the fields i've put into config.go are a good guess at common functionality [14:20] TheMue: i guess we'd want to keep things backwardly compatible [14:21] wrtp: ok, otherwise I would use direct yaml or json marshalling of a strcut scheme representing the configuration [14:21] wrtp: but if we need this generic approach it't ok [14:22] TheMue: we can't do that, because each provider has a different set of attributes [14:23] TheMue: config.go is an attempt to factor out some common functionality between providers. [14:23] wrtp: so the config types have to be defined per provider. today he also as to know, which configuration variables he retrieves from the config, so it's kinda hard coded [14:24] TheMue: only the common functionality is hard coded [14:24] wrtp: and the names of variables a provider accesses [14:24] TheMue: but each provider knows which attributes it supports, of course [14:24] wrtp: that's what I mean, exactly [14:25] TheMue: how could the provider get its configuration data otherwise? [14:26] wrtp: Kind of ReadConfig(filename, &myTopStruct), internally simply using an unmarshalling of e.g. json [14:27] TheMue: but myTopStruct has to hard code the fields names in just the same way [14:27] s/fields/field/ [14:27] TheMue: but also, there are several sections in the file, each with a different provider and a different set of attributes. i don't know how the above could work with that [14:28] wrtp: yes, indeed, only the generic part is simpler [14:28] wrtp: in both ways we can have runtime errors, yes [14:29] TheMue: what would the type of myTopStruct look like? [14:29] wrtp: that depends on the provider [14:29] TheMue: you don't know the provider before you read the file [14:30] TheMue: we're using the schema package to do a similar kind of thing. [14:30] TheMue: it doesn't marshal to a struct sadly [14:30] s/marshal/unmarshal/ [14:42] wrtp: ok, I see [14:48] " Aram: technically both can modify the struct, but one will be modifying copies, the other one will be modifying the "original"" [14:48] I hate such redundant and pointless "corrections" on IRC. [14:48] read between the lines! [14:54] Aram: *lol* === jimbaker` is now known as jimbaker [16:05] TheMue, Aram, fwereade_: a code review if anyone cares for it: https://codereview.appspot.com/6344113 [16:05] TheMue: it's the implementation of OpenPorts etc that the firewaller code will use [16:21] rogpeppe, I'm very unlikely to get to that, but I really like the approach you took on the config stuff [16:21] fwereade_: really? cool. [16:22] rogpeppe, I think it has unexamined fiddlinesses (in the review) but it's still going to be way way nicer [16:22] fwereade_: i felt it was a bit of a last-ditch attempt, but after trying to work with it, i was tending to agree with you about the train wreck [16:22] fwereade_: ah, a review! you're a lovely man. [16:23] fwereade_: it definitely needs lots more tests - i didn't want to waste any more time on it if it was a dead end. [16:23] rogpeppe, IMO, s/last-ditch attempt/fantastic diving save/ [16:30] fwereade_: BTW should authorized-keys be optional in general? [16:30] rogpeppe, I think we always need it [16:31] rogpeppe, otherwise we can't connect to state [16:31] fwereade_: but should the fields be optional? [16:31] fwereade_: (assuming we can fetch something from $HOME) [16:31] rogpeppe, input-wise, yes, but a Config with empty authorizedKeys should be an error I think [16:32] fwereade_: agreed [16:32] fwereade_: ah, and that's why i put authorized-keys everywhere [16:33] fwereade_: because we can't rely on any being in the testing user's home [16:33] rogpeppe, yeah [16:34] rogpeppe, that has tripped up a fair number of users I know about [16:34] rogpeppe, so, probably, a whole bunch more that never said anything ;) [16:35] fwereade_: it's awkward to make the dummy provider add a authorized-keys default [16:37] fwereade_: i wonder if we should set $HOME=/tmp/something in every test suite [16:37] rogpeppe, +1 [16:38] rogpeppe, c.MkDir() anyway [16:38] fwereade_: yeah [16:39] func init() {os.Setenv("HOME", "/dsvfkdsalewnfcldsakj") } :-) [16:39] haha [16:40] fwereade_: should default-series be optional? [16:40] fwereade_: (defaulting to environs.CurrentSeries, if so, presumably) [16:40] rogpeppe, I think that's OK to get from there, yeah [16:51] fwereade_: " [16:51] it shouldn't [16:51] even be possible to look at a Conn's Environ's config, because it is likely to [16:51] be wrong. [16:51] " [16:51] what do you mean by that? [17:01] i'm off for the weekend. [17:01] have a great weekend everyone!