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