=== bcsaller1 is now known as bcsaller [06:41] * davecheney waves [07:07] davecheney, fwereade, TheMue: mornin' all [07:07] wrtp, davecheney, TheMue: heyhey [07:07] wrtp: morning [07:08] wrtp: i reapplied the local ec2 tests and was pleased to discover none of the tests were broken [07:08] but that was just pure luck [07:08] as I there were blindly being changed [07:08] davecheney: great! [07:08] wrtp: you may not think so when you discover why I needed to add UseLocalStateInfo [07:09] davecheney: why's that? [07:09] (to LiveTests, presumably?) [07:09] wrtp: ec2test is hard coded to hand back DNS names machines in the form i-NNN.example.com [07:11] davecheney: well ec2test is there to be changed for tests' convenience... [07:11] davecheney: but maybe there's no convenient way of changing it [07:12] wrtp: I tried for a while [07:12] given how inception like jujutest is [07:12] there is no way to easily access the underly ec2test [07:12] or even know it is being used [07:14] have a good evening [07:14] i've gotta fly [10:44] hello. [10:48] Aram: Hi [10:49] Aram: Took a deeper look into mstate and really like it. [10:49] great :). [10:52] * TheMue diggs into environs to get a better idea of where get_machine_provider() is or will be in Go and to better integrate a new firewall approach into the provisioning agent [12:13] TheMue: ping === Aram2 is now known as Aram [13:03] * Aram is off for a few hours. [13:08] wrtp: pong [13:09] TheMue: i wonder if we could have a chat about the firewall code [13:09] TheMue: not right now though... i've just got involved in fixing another bug [13:09] wrtp: for sure [13:09] TheMue: 15 minutes or so [13:10] ? [13:10] wrtp: ok, I'm currently start with smaller chunks [13:10] TheMue: i think it's worth working out what the overall structure might look like (without actually doing it) [13:11] wrtp: yeah, there have to be changes from the old approach [13:11] TheMue: currently you've got several independent agents all doing their own thing, and i think that's potentially problematic [13:11] TheMue: i'm wondering whether it might be better to funnel all events into a central goroutine that keeps track of the state and issues port open/close requests. [13:21] Good morning all [13:28] niemeyer: hello, just had half a doomsday here. lots of rain. [13:28] TheMue: Heya [13:28] TheMue: Woohay :) [13:43] So, have to step out shortly for the dentist, they just called me if I wonna come earlier. Should not last long. [13:48] TheMue: Awesome, good luck there [13:50] niemeyer: small bug fix for you. should fix the charm store upload process. https://codereview.appspot.com/6344105 [13:50] niemeyer: good morning, BTW! [13:51] wrtp: Heya [13:53] wrtp: Neat! [13:56] g'morning [13:57] wrtp, cool [13:58] fwiw.. i think there two charms that applies to atm, there's a larger listing of other charms that don't appear in the charm store here.. http://jujucharms.com/tools/store-missing [13:58] niemeyer, does the charm store require maintainer? [13:58] hazmat: Not yet [14:00] hmm.. ok for several charms that's the only thing clint's lint/proof tool reports, so its unclear what the issue is with them [14:00] niemeyer, how do you like gce? [14:01] hazmat: Great stuff [14:39] niemeyer: what happens currently if two charms in the same container each open the same port? [14:39] hazmat: ^ [14:40] i suppose i should really ask what *should* happen in that case? [14:40] wrtp: They conflict [14:40] wrtp: and will always continue to conflict [14:40] niemeyer: there's an error? [14:40] wrtp: A single container is a single port namespace [14:40] niemeyer: open-port fails? [14:40] wrtp: Oh, no, that should work [14:40] at a juju level currently there is not, at a system level the port binding is an error [14:41] wrtp: Well.. I don't know if it "should" work, but I bet it "will" work [14:41] hazmat: so a charm shouldn't open-port until it's actually bound the socket? [14:41] wrtp, not nesc. [14:41] niemeyer: i quite like the idea that a given port is "owned" by a particular unit. [14:42] niemeyer: then open-port by another unit would give an error [14:42] wrtp, it could be reserving port for future exposed usage [14:42] wrtp: +1 [14:42] wrtp: Specifically in the case of subordinates, right? [14:43] niemeyer: absolutely [14:43] wrtp: Cool, makes sense [14:43] niemeyer: i've been going over the firewall semantics [14:43] sounds good, detect errors structurally instead of runtime undetected failures. [14:43] niemeyer: and that would make sense. [14:43] hazmat: yeah [14:48] shoud ports be part of the units metadata then, instead of in an arbitrary hook [14:49] so its owned from the getgo [14:49] imbrandon: that's a much-discussed question... [14:49] imbrandon, that discussion viewpoint is part of the ml archive on this topic [14:49] ahh [14:50] imbrandon: i wasn't actually suggesting that though [14:50] to date though, nothing is actually using dynamic ports [14:50] imbrandon: i intended to suggest that open-port would take ownership of a given port, if possible. [14:50] right, not much does , incomming wise iirc [14:51] wrtp: right, but what if it cant, the charm would need logic to handle that right ? [14:51] and maybe try another port [14:51] imbrandon: yup. [14:52] imbrandon: if you're deploying two charms which want to use the same port, there's no way around that [14:52] right [15:04] TheMue, niemeyer, fwereade_: here's a pseudocode sketch of a slightly different approach to the firewall management code: http://paste.ubuntu.com/1086303/ [15:04] *click* [15:04] wrtp: Can you talk me through it? [15:05] niemeyer: ok [15:05] wrtp: Is this a worker.. what's unit/machine/etc [15:05] niemeyer: so, we've got one central goroutine that has a coherent idea of the current state of the system (with regard to ports) [15:06] niemeyer: this is to be started by the provisioning agent. [15:06] wrtp, that looks broadly sensible to me [15:06] wrtp: Okay, so it is a worker [15:06] niemeyer: yeah. [15:07] wrtp: we have two kinds of service changes: adding/removing and exposed flag. [15:07] niemeyer: and it *probably* will work ok when run concurrently with itself, assuming a sensible implementation of Open and ClosePort in the provider [15:07] wrtp: machine/unit/etc are local structs, I assume, rather than representing changes to state.Unit/etc [15:07] wrtp, I presume portManager is something separate, with state, that worries about EC2 errors and suchlike and keeps retrying on errors? [15:07] niemeyer: yes [15:07] wrtp: cool [15:07] wrtp: Re-reading with that info [15:07] niemeyer: portManager was my name for the main loop [15:08] niemeyer: but it would be restarted on errors, yes [15:08] wrtp, it was also the thing that had OpenPort and ClosePort called on it [15:08] fwereade_: oh, sorry, i've got two portManagers! [15:08] wrtp, if that's an env I'm a little uncertain [15:09] fwereade_: no, portManager is intended to be an environs.Instance [15:09] wrtp, ah-ha, ok, sorry [15:09] there is actually a problem [15:10] wrtp, but still... any errors there will surely mean that we have to keep retrying, there, until we succeed... right? [15:10] fwereade_: i guess so. [15:10] wrtp: sounds good so far, only the missing differentiation between adding/removing and exposing of services [15:11] wrtp: The data coming from the change on line 38 looks curious [15:11] wrtp, that feels a little icky to me but not enough to sink the concept :) [15:11] niemeyer: yes, i glossed over that bit [15:12] niemeyer: since we're waiting for many watchers at once, we have a goroutine for each watcher that adds context to the change passed on the channel, then sends to a single channel. [15:12] niemeyer: so where the pseudocode says "add port watcher...", it implies setting up a goroutine to do that too [15:13] niemeyer: but those goroutines don't mess with the state at all [15:14] the main problem i can see currently is that there needs to be another phase at the start [15:15] where we need to interrogate the currently open ports and close them if they need to be. [15:16] fwereade_: it's possible that we might want another layer, being a proxy for a machine, that deals with retrying port changes for that machine. [15:17] wrtp, yeah, something like that [15:20] wrtp: right now the real state is retrieved from the provider and compared to the state informations [15:20] TheMue: if OpenPort and ClosePort are idempotent, i'm not sure that's necessary. [15:20] s/idem/each idem/ [15:21] wrtp: would be the better solution, indeed [15:22] it's entirely possible that this scheme is crackful though. i just thought i'd give it as a talking point. [15:23] one thing that's not currently taken into account is that the instance for a machine can change [15:23] wrtp: today the fw is notificated when services are added. those get an exposed watcher. if exposed, a unit watcher is set up. and those are watching the units ports. *sigh* deeply nested. [15:23] wrtp: Looks very sensible [15:24] TheMue: i didn't see any point in watching services that have no machines, so i add the service watcher only when necessary [15:24] niemeyer: thanks [15:24] wrtp: sounds reasonable [15:31] wrtp: Have you seen this: https://codereview.appspot.com/6333067/ [15:31] niemeyer: no. will look. [15:32] wrtp: Cool, it's good to sync up with Dave on that, since they both seem to be overlapping [15:32] niemeyer: looks pretty compatible to me [15:33] niemeyer: i *think* the environ watching would go inside the same loop [15:34] wrtp: It is compatible so far for sure. I'm just saying that they're both supposed to implement the same functionality, so synchronizing is important [15:34] wrtp: Or we'll end up with two people working on the same thing [15:35] niemeyer: definitely. i wasn't actually proposing to write this code - TheMue is there already. [15:35] wrtp: Perfect, thanks [15:35] niemeyer: this was borne out of my looking at TheMue's initial stab, which was invaluable for me to see what actually needed to be done. [15:36] wrtp: Super [15:36] wrtp: Thanks for diving into this. Very useful. [15:36] niemeyer: If the firewall is only used by provisioning, is it worth to create an own service? [15:36] TheMue: hopefully this will be useful input to your next steps, and perhaps we have a better idea of what we might be aiming for [15:36] wrtp: Yes, thx. [15:37] TheMue: i think it should be a file within the provisioning agent [15:37] niemeyer: There are two connection points in the provisioner. [15:37] s/a file/implemented in a file/ [15:38] wrtp: The PA starts the provisioner. And there is a loop where today the machines are watched. In the Py code here also services are watched. [15:39] wrtp: So I would see it as a non-exported type for the provisioner (same package, own file). [15:39] TheMue: yup [15:39] TheMue: that's what i was trying to suggest [15:39] wrtp: h5 [15:39] TheMue: h5 [15:41] TheMue: It is worth creating a *worker*, yes [15:41] wrtp: I'd prefer to have this as an independent worker [15:41] wrtp: It's functionality is completely unrelated to the rest of the provisioner [15:41] niemeyer: a separate executable? [15:42] wrtp: No [15:42] A different worker, not a different agent [15:42] niemeyer: a separate goroutine with the PA? [15:42] niemeyer: (that's what i had envisaged) [15:42] s/with the/within the/ [15:42] wrtp: Yes, and a different package under juju-core/worker/firewaller [15:43] wrtp: I only disagreed with "a file within the provisioning agnet" [15:43] niemeyer: ah, i hadn't seen juju-core/worker [15:43] niemeyer: presumably a CL waiting to land [15:43] wrtp: It's currently named juju-core/service, but that's wrong and we should rename ASAP [15:43] wrtp: No, we've agreed that was the best nomenclature, and Dave had stuff in progress that he wanted to push forward without distractions. Sounded sensible [15:44] niemeyer: yes, that all sounds very sensible [15:44] niemeyer: now i understand what you mean by "worker" :-) [15:44] * TheMue too [15:48] niemeyer: Today the notification about added/removed services or machines is done by the PA (in Py). The according code fragments in Go are in the provisioner worker. [15:49] niemeyer: So should the provisioner call those two exported methods in future too or better setup own watchers to work standalone? [15:49] here's a version with logic for dealing with instance ids coming and going: http://paste.ubuntu.com/1086373/ [15:50] TheMue: i think they'd each set up their own watchers [15:50] TheMue: it's a little less efficient, but nicer structurally [15:50] wrtp: sounds more clear, yes. better maintainable [15:51] TheMue: Sorry, I don't get the question [15:51] TheMue: As a hint, though, the Python structure of provisioner+firewaller is not an example to be followerd [15:51] followed [15:52] niemeyer: Yes, that's what I've done with the first approach. [15:53] niemeyer: Do so the FW only needs two public methods for the PA. [15:53] niemeyer: But it leads to this nested structure. [15:53] niemeyer: So as an own worker with own watchers it's definitely better. [15:54] TheMue: Yeah, that's the direction we should follow [15:54] TheMue: and *small branches*, with *tests*, please! [15:57] niemeyer: Yeah, I only have still problems to propose something when it does nothing. And the nested structure of todays FW lead fast to this amount of code, sorry. [15:57] niemeyer: So I now will already propose code with stubs. [15:59] TheMue: Thanks a lot [15:59] TheMue: As a hint, try to write tests with the code, rather than getting it ready and then testing [16:00] That's lunch time.. biab [17:19] niemeyer: do you think we should do the same thing with security groups as the python version (i.e. one security group per instance) ? [17:19] niemeyer: i've been wondering about ways to do better (e.g. one security group per combination of ports) [17:20] niemeyer: the latter uses as many security groups as machines in the worst case, but the usual case would be many fewer, i think. [17:22] wrtp: There's no way to do different [17:22] wrtp: Security groups can only be defined at instance creation time [17:23] niemeyer: oh darn it! i'd forgotten that [17:31] wrtp: I wish it worked as well [17:34] niemeyer: originally i had environs.Instance.OpenPort and ClosePort, but i'm thinking that doesn't map very well to how it works. perhaps environs.Environ.OpenPort(machineId int) would be better. [17:34] niemeyer: thoughts? [17:35] anyway, gotta go. see y'all tomorrow! [17:56] wrtp: In case you read this, yes, that's how it currently works in Python too [20:45] flaviamissi: Hey! [20:46] niemeyer: hi! [20:46] flaviamissi: Just now I was looking at the auth problem [20:46] flaviamissi: I think I can reproduce it [20:47] flaviamissi: Should be fixed in a moment hopefully [20:47] niemeyer: good! I debugged it, but I couldn't get anywhere... [20:47] niemeyer: you have something that can be causing this problem in mind? [20:50] flaviamissi: Not yet, but should have something in a bit :) [20:51] flaviamissi: It's not hard to find this kind of inconsistency between EC2 & similars [20:51] unfortunately [20:51] flaviamissi: Even between *regions* of EC2, sometimes there are inconsistencies [20:52] niemeyer: hmmm, first time i've seen something like that, good to know though [20:53] niemeyer: I'm really curious about what is causing that problem, if you can let me know when you find something, I would really appreciate that :) [20:57] flaviamissi: Oh yeah, I'll certainly let you know [20:58] niemeyer: thanks :) [21:04] flaviamissi: It's the path [21:04] flaviamissi: The endpoint path, more specifically [21:05] I'll have a fix in a moment [21:08] Works! [21:08] niemeyer: putz! [21:08] :) [21:08] niemeyer: We didnt think about it... [21:09] niemeyer: really great. Thanks a log [21:09] lot* [21:09] flaviamissi: My pleasure [21:35] niemeyer: is the change in trunk yet? [21:40] niemeyer: well, i'll leave now, when you merge it with trunk i'll try it :) [21:47] Come on Launchpad.. why you don't like me [23:08] Dinner time