| wrtp | fwereade_: mornin'! | 06:14 |
|---|---|---|
| fwereade_ | wrtp, heyhey | 06:31 |
| wrtp | fwereade_: how's tricks? | 06:32 |
| fwereade_ | wrtp, not too bad | 06:32 |
| wrtp | fwereade_: BTW are you getting the initial mail for each codereview proposal? | 06:33 |
| wrtp | fwereade_: i'm not, and i can't see what's going wrong. i've just been through all my mail filter patterns and can't see any likely candidate. | 06:33 |
| fwereade_ | wrtp, hmm, yes, it looks like I am in general | 06:34 |
| fwereade_ | wrtp, maybe something about LP? | 06:34 |
| wrtp | fwereade_: i'm getting the initial '[Merge]' mail, but those [Merge] mails are so noisy i archive them immediately. | 06:34 |
| wrtp | fwereade_: the strange thing is i see any replies | 06:35 |
| fwereade_ | wrtp, ah, ok, that is weird then | 06:35 |
| wrtp | fwereade_: so the first thing i'll know about any CL is when someone replies to it | 06:35 |
| wrtp | yeah, it's odd, and quite annoying actually | 06:35 |
| fwereade_ | wrtp, yeah, I bet :( | 06:36 |
| wrtp | fwereade_: hmm, the odd thing is i do see *some* initial emails. Aram's and gustavo's seem to come through ok. | 06:56 |
| fwereade_ | wrtp, heh that's even weirder | 06:56 |
| wrtp | fwereade_: i wonder if there's a way to check in launchpad what messages it's sent me | 06:57 |
| fwereade_ | wrtp, I doubt that sort of thing specifically is logger | 06:59 |
| TheMue | Morning | 06:59 |
| fwereade_ | wrtp, I bet we all have a subtly different combination of LP settings, and memberships of various things | 06:59 |
| fwereade_ | TheMue, heyhey | 06:59 |
| wrtp | fwereade_: can you think of a CL you've replied to recently that i wasn't involved in? | 06:59 |
| wrtp | TheMue: yo! | 07:00 |
| fwereade_ | wrtp, I don't think I have done much of that lately :( | 07:04 |
| wrtp | fwereade_: hmm. TheMue? | 07:04 |
| TheMue | wrtp: What? | 07:05 |
| wrtp | TheMue: (i'm trying to work out why i'm not seeing some emails from launchpad) | 07:05 |
| wrtp | TheMue: can you think of a CL you've replied to recently that i wasn't involved in? | 07:05 |
| TheMue | wrtp: Have to look, but have to change systems fir it. | 07:07 |
| wrtp | TheMue: any CL that someone else created and you replied earlier than me would do too | 07:08 |
| TheMue | wrtp: If so, then mostly Williams CLs. | 07:09 |
| wrtp | TheMue: one example would be good, if you could, thanks! | 07:09 |
| wrtp | TheMue: here's an updated version of my firewaller sketch, BTW. http://paste.ubuntu.com/1087489/ | 07:13 |
| wrtp | TheMue: i realised that tracking machine instance ids was unnecessary, because the OpenPort/ClosePort functions should operate on Environ with a machine id rather than on an environs.Instance. | 07:14 |
| TheMue | wrtp: Great, thx, will today integrate it into the firewaller outline of dave | 07:15 |
| wrtp | TheMue: good plan | 07:15 |
| wrtp | TheMue: i'm not sure of a good way to build it up bit by bit though. i can't think of any useful tests for the intermediate stages. | 07:18 |
| TheMue | wrtp: That has been my prob with the first approach. | 07:18 |
| TheMue | wrtp: Now I'll use stubs for intermediate CLs | 07:19 |
| wrtp | TheMue: i suppose you could put hooks in for the intermediate stages, to be removed later. but writing tests only to delete them later seems a bit wrong. | 07:21 |
| TheMue | wrtp: No, the tests shall be the right ones and make the right asserts | 07:22 |
| wrtp | TheMue: yeah, and the problem is that the only observable side-effects of this code are the calls to OpenPort/ClosePort | 07:22 |
| TheMue | wrtp: But while they first may get constants it will get more and more live with each iteration | 07:22 |
| wrtp | TheMue: which won't happen until all the rest is there. | 07:23 |
| TheMue | wrtp: So the tests verify the impl | 07:23 |
| TheMue | wrtp: See the current approach. I've used an interface and a mock for call verification. | 07:24 |
| wrtp | TheMue: the first stage AFAICS is to get the machine watcher done. but there are no calls to anything when the machine watcher fires, so no verification to do AFAICS | 07:25 |
| wrtp | TheMue: i suppose you could put in a fake call to OpenPort for the time being, to be changed later. | 07:25 |
| TheMue | wrtp: Yep, that's what I meant. The Py firewall gets a provider and I reduced it to the three methods of the PortManager interface. Will see how I can temoporarilly inject something similar here and remove it later. | 07:27 |
| wrtp | TheMue: i'm doing the port manager implementation in ec2, and i think the Environ port manager interface might look something like this: http://paste.ubuntu.com/1087531/ | 07:42 |
| TheRealMue | wrtp: So, changed client. | 07:42 |
| === TheRealMue is now known as TheMue | ||
| wrtp | TheMue: did you see my message a few seconds ago? | 07:43 |
| wrtp | oh yeah, you should've | 07:43 |
| TheMue | wrtp: About the port manager interface? Yes, just looking at it | 07:43 |
| wrtp | TheMue: in fact, scratch that, i think it's not easy to implement well in ec2 | 07:44 |
| TheMue | wrtp: So SetPorts() would ensure that only the passed ports are open afterwards? | 07:45 |
| wrtp | TheMue: yeah, but i think it's wrong | 07:45 |
| wrtp | TheMue: i think this is better: http://paste.ubuntu.com/1087535/ | 07:46 |
| TheMue | wrtp: Today the FirewallManager retrieves the current real state, compares it to the wanted state and builds the diffs to explicitely open and close the ports. | 07:46 |
| wrtp | TheMue: i don't think it needs to do that | 07:46 |
| wrtp | TheMue: apart from at the very beginning | 07:47 |
| TheMue | wrtp: ??? | 07:47 |
| TheMue | wrtp: The new interface matches exactly to that. | 07:47 |
| wrtp | TheMue: it only needs to do the diff once, when the firewaller starts up, i think. | 07:47 |
| wrtp | TheMue: from then on it knows the "real" provider state, and can just apply OpenPorts and ClosePorts methods as the State changes | 07:48 |
| wrtp | TheMue: i.e. it will need to call Ports once only | 07:49 |
| TheMue | wrtp: Understand, yes. | 07:49 |
| TheMue | wrtp: In todays model the provisioner called individual methods after each service and machine change, so a different approach | 07:49 |
| wrtp | TheMue: yes - the difference is that, if you follow my sketch, we keep track of the state in one place, so we know exactly what's going on at all times | 07:50 |
| wrtp | TheMue: BTW the reason that SetPorts is a bad idea is that the ec2 primitives are AuthorizeSecurityGroup and RevokeSecurityGroup - there's no way to set the entire set of permissions of a security group at once. | 07:52 |
| wrtp | fwereade_, TheMue: what do you think should happen to a machine's open ports if it's restarted on a different instance? | 09:15 |
| wrtp | niemeyer: ^ | 09:16 |
| wrtp | niemeyer: mornin' BTW | 09:17 |
| fwereade_ | wrtp, hmm, given that that *can* happen... are we not in a situation where we *cannot* depend on our local state to know what ports are really open? | 09:21 |
| fwereade_ | wrtp, this is not a solution but it does maybe feel like a fundamental problem with the maintain-local-state approach... | 09:21 |
| fwereade_ | niemeyer, morning | 09:21 |
| wrtp | fwereade_: as currently conceived, the machine's open ports are independent of its instance | 09:22 |
| wrtp | fwereade_: but currently when a new instance is started, it ensures that the machine's group has no permissions | 09:23 |
| fwereade_ | wrtp, I think that is a restatement of the problem? | 09:23 |
| fwereade_ | wrtp, the *obvious* answer is to put a watch on a machine's instance ID, but I'm not sure it's a *good* one | 09:24 |
| wrtp | fwereade_: yeah, i think that's right | 09:24 |
| fwereade_ | wrtp, however I don;t think we have any other way of telling | 09:24 |
| fwereade_ | wrtp, wait a mo | 09:24 |
| fwereade_ | wrtp, is it easy to reuse the security group from the original machine? | 09:25 |
| wrtp | fwereade_: i think perhaps it's up to the provisioner to change the port settings for a machine when it starts an instance | 09:25 |
| wrtp | fwereade_: it *doesn* use the security group from the original machine | 09:25 |
| wrtp | s/doesn/does/ | 09:25 |
| wrtp | fwereade_: but currently it will zero the perms when the instance starts | 09:26 |
| fwereade_ | wrtp, ah, but it clears it out? | 09:26 |
| wrtp | yeah | 09:26 |
| wrtp | fwereade_: i'm thinking that it shouldn't | 09:26 |
| fwereade_ | wrtp, hmmmmmmm, so, consider the lifecycle of the unit agent on the new machine | 09:26 |
| fwereade_ | wrtp, that will start up, open ports, blah blah | 09:26 |
| * wrtp is considering | 09:26 | |
| fwereade_ | wrtp, won't it Just Work already? | 09:26 |
| fwereade_ | wrtp, I think it might :) | 09:27 |
| wrtp | fwereade_: it would *if* all the security groups from a previous instance of the environment had been destroyed | 09:27 |
| wrtp | fwereade_: but i'm not sure we can guarantee that | 09:27 |
| wrtp | fwereade_: thus i think that when starting a new instance, the PA should perhaps call Ports and ClosePorts to clear out any old cruft | 09:28 |
| wrtp | s/starting a new instance/starting a new machine/ | 09:28 |
| wrtp | fwereade_: i *think* that would solve the problem | 09:29 |
| wrtp | niemeyer_: yo! | 09:29 |
| TheMue | wrtp: Kind of CloseAllPorts() sounds good, yes | 09:29 |
| wrtp | TheMue: env.ClosePorts(m, m.Ports()) | 09:29 |
| wrtp | TheMue: (with error checking of course :-]) | 09:29 |
| fwereade_ | wrtp, I'm confused: I thought you said that the perms *were* zeroed whenever we start an instance | 09:30 |
| wrtp | TheMue: that's essentially what it does now, behind the scenes | 09:30 |
| wrtp | fwereade_: they are, by the ec2 environ | 09:30 |
| wrtp | fwereade_: but that means we can create a new instance for an existing machine | 09:30 |
| fwereade_ | wrtp, ok, so what doesn't already do the Right Thing? | 09:30 |
| TheMue | wrtp: You can't pass m.Ports() | 09:30 |
| wrtp | TheMue: m.Ports would return an error | 09:31 |
| wrtp | oops, env.Ports(m) of course | 09:31 |
| fwereade_ | wrtp, if and when that machine needs some ports opened, it'll do an open-port in a hook, and... oh | 09:31 |
| fwereade_ | wrtp, the unit will say "meh, those ports are already open, no state changes here!" | 09:31 |
| wrtp | fwereade_: this was the source of my original question | 09:31 |
| wrtp | fwereade_: i'm not sure what behaviour we *want* here | 09:32 |
| wrtp | fwereade_: perhaps a new instance *should* always come up with no open ports | 09:32 |
| fwereade_ | wrtp, so actually we need the unit agent to realise when *it* is being started on a fresh machine, and clear out its own ports to begin with, so the Right Thing happens when the hooks reopen them | 09:32 |
| fwereade_ | wrtp, I think new instances should certainly come up with no open ports | 09:33 |
| fwereade_ | wrtp, I'm pretty sure the difficulty is at the unit agent level | 09:33 |
| wrtp | fwereade_: given that the PA is starting the new instance, it's in a good position to clear out the ports first | 09:33 |
| fwereade_ | wrtp, I thought that always happened already? | 09:33 |
| TheMue | wrtp: Exactly | 09:33 |
| wrtp | fwereade_: it does currently, but i wasn't sure if it should | 09:33 |
| fwereade_ | wrtp, I guess that ideally this *should* be done by the PA rather than by the environ | 09:34 |
| wrtp | fwereade_: i'm now thinking the current behaviour is correct | 09:34 |
| wrtp | fwereade_: well, yeah | 09:34 |
| wrtp | fwereade_: that would seem better | 09:34 |
| fwereade_ | wrtp, because then we won't need every single environ to mess with ports on instance start | 09:34 |
| wrtp | fwereade_: well, the same code will be executed - it'll just be called from the PA | 09:35 |
| wrtp | ahh, but... | 09:35 |
| fwereade_ | wrtp, yeah, but we only have to write it once instead of N times | 09:35 |
| wrtp | this all interacts with the firewall code | 09:35 |
| fwereade_ | wrtp, and the firewall code will then get an inaccurate view of state? | 09:36 |
| wrtp | because the firewall is keeping track of what ports are supposed to be open | 09:36 |
| wrtp | fwereade_: yeah | 09:36 |
| fwereade_ | wrtp, I don't *think* it matters if we depend on the UA doing the Right Thing wrt closing all ports when it starts up on a fresh machine | 09:36 |
| TheMue | wrtp: that's its task | 09:36 |
| wrtp | fwereade_: actually, i think you're right | 09:37 |
| wrtp | fwereade_: if the UA does it, we have no problems | 09:37 |
| wrtp | fwereade_: and the firewaller doesn't have to keep track of instance ids | 09:37 |
| fwereade_ | wrtp, yep, think so | 09:37 |
| wrtp | fwereade_: then *all* port opening and closing is done via the state open/close port | 09:38 |
| fwereade_ | wrtp, yeah, exactly | 09:38 |
| fwereade_ | wrtp, the firewaller can watch that alone and it'll catch up when it becomes important to | 09:38 |
| wrtp | fwereade_: and the environ won't clear out ports when starting a new instance | 09:38 |
| fwereade_ | wrtp, the environs should still clear out ports | 09:39 |
| fwereade_ | wrtp, ah wait | 09:39 |
| wrtp | yeah | 09:39 |
| wrtp | there's a difficulty | 09:39 |
| wrtp | no, it's ok i think | 09:40 |
| wrtp | fwereade_: the PA clears out ports when it starts a new *machine* | 09:40 |
| wrtp | fwereade_: because machine ids are never reused | 09:40 |
| wrtp | fwereade_: thus we ensure that all new machines start in a known state, and the firewaller can track them properly. | 09:41 |
| fwereade_ | wrtp, I think that clearing them at *instance* start time is the Right Thing: I don't see any benefit to keeping the ports open while launching the machine | 09:42 |
| fwereade_ | wrtp, the UA will clear out the desired ports in state when it comes up | 09:42 |
| fwereade_ | wrtp, and at that point the firewaller will be back in sync | 09:42 |
| wrtp | fwereade_: yes, that sounds reasonable | 09:43 |
| wrtp | fwereade_: if *slightly* tricksy | 09:43 |
| wrtp | fwereade_: i think StartInstance should document that the ports for the machine id will be closed when it's called. | 09:44 |
| fwereade_ | wrtp, honestly this feels like a job for an Environ struct type that tells an internal EC2-or-whatever env to (1) StartInstance, (2) CloseAllPorts | 09:45 |
| fwereade_ | wrtp, StartInstance having to magically mess with ports in every provider STM like a recipe for bugs | 09:46 |
| wrtp | fwereade_: s/struct type/function/ ? | 09:46 |
| fwereade_ | wrtp, IMO having the real environ accessible for people to mess with is similarly a recipe for bugs, but YMMV | 09:46 |
| wrtp | fwereade_: my mileage does :-) | 09:46 |
| wrtp | fwereade_: tbh i'd be happy for the PA to implement that logic | 09:47 |
| fwereade_ | wrtp, IMO `env.StartInstance(...` rather than StartInstance(env, ...` not not sufficiently obviously a bug | 09:47 |
| wrtp | fwereade_: it's very straightforward, and then there's no hidden interaction between StartInstance and ClosePorts. | 09:47 |
| wrtp | fwereade_: nothing else calls StartInstance | 09:48 |
| wrtp | afaik | 09:48 |
| wrtp | fwereade_: so there's really no need for another layer | 09:48 |
| fwereade_ | wrtp, we'll see | 09:48 |
| wrtp | fwereade_: for the time being, i'll implement Ports, OpenPorts and ClosePorts, and remove the port closing logic from StartInstance | 09:49 |
| wrtp | fwereade_: i think that should be sufficient | 09:49 |
| niemeyer_ | wrtp, fwereade_: Heya! | 09:50 |
| wrtp | niemeyer_: does that sound reasonable? | 09:50 |
| fwereade_ | niemeyer_, heyhey | 09:50 |
| niemeyer_ | wrtp: Sorry, I wasn't following | 09:50 |
| niemeyer_ | wrtp: Breakfast made a lot of sense, though :-) | 09:50 |
| wrtp | lol | 09:50 |
| fwereade_ | wrtp, IMO any situation in which you're saying "a call to Foo must be immediately followed by a call to Bar" is a bug waiting to happen | 09:50 |
| * fwereade_ shrugs | 09:51 | |
| wrtp | niemeyer_: perhaps you could read over the recent conversation | 09:51 |
| wrtp | fwereade_: only in the context of the provisioner is that true | 09:51 |
| niemeyer_ | wrtp: Will definitely do, but I have to take Ale to work first, as she's starting in a few minutes.. I'll be back soon, though | 09:51 |
| fwereade_ | wrtp, and on bootstrap? | 09:51 |
| wrtp | niemeyer_: np | 09:51 |
| wrtp | fwereade_: that's just a specialised kind of provisioning :-) | 09:51 |
| fwereade_ | wrtp, as you like | 09:52 |
| wrtp | fwereade_: we're only talking about 3 lines of code in two places here... | 09:52 |
| TheMue | wrtp: line 24 of http://paste.ubuntu.com/1087489/, how would you manage the watcher of all units for a machine? based on your outline I only see one per machine. | 09:52 |
| fwereade_ | wrtp, one of which you apparently forgot about | 09:53 |
| fwereade_ | wrtp, bug waiting to happen | 09:53 |
| wrtp | fwereade_: there are many bugs waiting to happen :-) | 09:53 |
| fwereade_ | wrtp, which is why in general I prefer to make the ones I can see unpossible ;p | 09:54 |
| wrtp | TheMue: there's one units watcher per machine. then line 36 starts a new watcher for any new unit on a given machine | 09:54 |
| wrtp | fwereade_: | 09:54 |
| * wrtp finds that less bugs come from less code | 09:55 | |
| TheMue | wrtp: ah, so I misunderstood the plural sentence | 09:55 |
| fwereade_ | wrtp, I don't think we're going to convince one another here | 09:56 |
| wrtp | TheMue: yeah by "machine units watcher" i meant a watcher to watch the units on a given machine | 09:56 |
| TheMue | wrtp: That definitely makes more sense | 09:56 |
| wrtp | fwereade_: you're probably right :-| | 09:57 |
| TheMue | wrtp, fwereade_: have to be afk for a moment, once again daddy-daughter-taxi | 09:57 |
| wrtp | fwereade_: there's just as many bugs waiting to happen with a Environ shim BTW - you'd need to remember to insert the shim that does the StartInstance magic. | 09:59 |
| wrtp | fwereade_: and then that shim wouldn't strictly implement environs.Environ, because it would do magic on StartInstance... | 09:59 |
| fwereade_ | wrtp, yeah, environs.Environ is IMO the wrong thing to be exposing | 10:00 |
| fwereade_ | wrtp, but I am not interested in reachitecting anything we don't *have* to at this stage | 10:00 |
| wrtp | fwereade_: the wrong thing to be exposing to what? | 10:00 |
| fwereade_ | wrtp, so let's just drop it :) | 10:00 |
| wrtp | fwereade_: np | 10:00 |
| wrtp | fwereade_: i'm interested in your thoughts though | 10:00 |
| fwereade_ | wrtp, they're not really relevant at this stage -- we made the decision to stick all the common stuff into the EC2 provider ages ago | 10:02 |
| wrtp | fwereade_: the common stuff can easily be factored out later while retaining the same environs.Environ interface. | 10:02 |
| wrtp | fwereade_: i think | 10:02 |
| fwereade_ | wrtp, we'll worry about that when we have to | 10:02 |
| wrtp | fwereade_: my thought too | 10:02 |
| niemeyer_ | fwereade_, wrtp: I'm not sure I undertand what's the last consensus, despite the fact I've read the backlog | 10:11 |
| wrtp | niemeyer_: as i understand it, the consensus is this: | 10:11 |
| wrtp | niemeyer_: 1) we add these methods to environs.Environ: http://paste.ubuntu.com/1087700/ | 10:12 |
| fwereade_ | wrtp, niemeyer_: clear all provider ports on instance start; clear all state ports on UA start; firewaller will catch up when it needs to | 10:12 |
| fwereade_ | wrtp, niemeyer_: AIUI ;) | 10:13 |
| wrtp | niemeyer_: 2) we change the ec2 StartInstance so that it doesn't automatically clear out machine ports | 10:13 |
| niemeyer_ | LOL | 10:13 |
| niemeyer_ | Looks like there's no consensus indeed | 10:14 |
| wrtp | niemeyer_: we're not in conflict there | 10:14 |
| wrtp | niemeyer_: the question is *where* the provider ports get cleared out | 10:14 |
| niemeyer_ | "clear all provider ports on instance start" != "change the ec2 StartInstance so that it doesn't automatically clear out machine ports" | 10:14 |
| wrtp | niemeyer_: 3) make the PA clear out ports when it starts a new instance | 10:14 |
| wrtp | niemeyer_: this is so that there's no behind-the-scenes interaction between StartInstance and the open ports | 10:15 |
| wrtp | niemeyer_: but i'm happy for StartInstance to be documented as closing any machine ports too | 10:15 |
| wrtp | niemeyer_: 4) make the UA clear out state ports on unit start | 10:16 |
| niemeyer_ | Huh | 10:16 |
| wrtp | niemeyer_: oops. | 10:16 |
| niemeyer_ | That seems to turn a trivial issue into a multi-faceted problem on three different entities | 10:16 |
| wrtp | niemeyer_: 3) make the PA clear out ports when it starts a new *machine*. | 10:16 |
| wrtp | niemeyer_: what does? | 10:17 |
| niemeyer_ | 1+2+3+4 | 10:17 |
| niemeyer_ | What's the issue we have? | 10:17 |
| niemeyer_ | wrtp: (1) looks good, btw | 10:18 |
| wrtp | niemeyer_: one issue is: what happens when the PA starts a new instance for a machine that had an instance before? | 10:18 |
| wrtp | niemeyer_: open ports are attached to machines, not instances | 10:18 |
| niemeyer_ | wrtp: Not exactly | 10:19 |
| wrtp | niemeyer_: and it would be nice if the firewaller doesn't have to track instances | 10:19 |
| niemeyer_ | wrtp: Both contain data about it | 10:19 |
| niemeyer_ | wrtp: But, ok.. | 10:19 |
| niemeyer_ | wrtp: The provisioner doesn't care about ports at all | 10:19 |
| niemeyer_ | wrtp: It's not its problem | 10:20 |
| wrtp | niemeyer_: there is a problem | 10:20 |
| niemeyer_ | wrtp: Ok.. go on | 10:20 |
| wrtp | niemeyer_: if the provisioner starts a new instance for a machine, we want to close all the ports for that machine | 10:21 |
| wrtp | niemeyer_: probably | 10:21 |
| niemeyer_ | wrtp: No, we don't | 10:21 |
| wrtp | niemeyer_: although we could choose not to | 10:21 |
| niemeyer_ | wrtp: Machines should be started with no open ports | 10:21 |
| wrtp | niemeyer_: fwereade_ thought we should. i'm on the fence. | 10:21 |
| niemeyer_ | Sorry | 10:21 |
| niemeyer_ | Bad term | 10:21 |
| niemeyer_ | wrtp: Instances should be started with no open ports | 10:21 |
| wrtp | niemeyer_: exactly | 10:21 |
| niemeyer_ | wrtp: That's a problem for the environment to solve | 10:21 |
| wrtp | niemeyer_: so we want to close all the ports for the existing machine | 10:21 |
| wrtp | niemeyer_: in the state | 10:22 |
| niemeyer_ | wrtp: No! | 10:22 |
| niemeyer_ | wrtp: The state is something else! | 10:22 |
| wrtp | niemeyer_: so we'd have a machine in the state with open ports, but the instance would have no open ports? | 10:22 |
| niemeyer_ | wrtp: This was just about the environment itself.. | 10:22 |
| niemeyer_ | wrtp: I think I'm missing the actual issue | 10:22 |
| niemeyer_ | wrtp: Yeah, I was missing indeed. | 10:23 |
| niemeyer_ | wrtp: Okay | 10:23 |
| niemeyer_ | wrtp: So there are two separate issues | 10:23 |
| niemeyer_ | wrtp: One of them concerns management of ports of the instance.. the other about management of ports in the state | 10:23 |
| wrtp | yup | 10:24 |
| niemeyer_ | wrtp: Which one are we talking about? | 10:24 |
| wrtp | both | 10:24 |
| wrtp | niemeyer_: how they interact with each other | 10:24 |
| niemeyer_ | wrtp: Okay, so can we please separate them up | 10:24 |
| niemeyer_ | wrtp: Not really.. they are independent issues | 10:24 |
| wrtp | niemeyer_: there is no management of ports of the instance | 10:25 |
| niemeyer_ | wrtp: !? | 10:25 |
| wrtp | niemeyer_: there is management of ports of the *machine* | 10:25 |
| wrtp | niemeyer_: OpenPorts(machineId int, ports []state.Port) error | 10:26 |
| niemeyer_ | wrtp: Oops | 10:26 |
| niemeyer_ | wrtp: Yeah, just now I realize that.. this seems wrong | 10:26 |
| niemeyer_ | wrtp: Environ manages instances, not machines | 10:26 |
| wrtp | niemeyer_: so we manufacture a new security group every time we start a new instance? | 10:27 |
| niemeyer_ | wrtp: Is there any other way? | 10:27 |
| niemeyer_ | wrtp: We talkeda about this yesterday | 10:27 |
| wrtp | niemeyer_: well, currently we name the instance group after the machine id | 10:27 |
| wrtp | niemeyer_: but i think you're suggesting that we don't do that | 10:27 |
| niemeyer_ | wrtp: I'm suggesting something else.. I'm suggesting that Environ manages instances, not machines. Do we agree on that? | 10:28 |
| wrtp | niemeyer_: currently the Environ uses the machine id for some things | 10:29 |
| niemeyer_ | wrtp: Do we agree on that? | 10:29 |
| wrtp | niemeyer_: "manages", yes. | 10:29 |
| niemeyer_ | wrtp: Okay, thanks | 10:29 |
| wrtp | niemeyer_: but setting the ports for a machine isn't "managing" the machine, i think. | 10:30 |
| niemeyer_ | wrtp: So the question is what interface makes sense given that. I understand the ec2 implementation does certain things a given way | 10:31 |
| niemeyer_ | wrtp: It's managing the instance information for sure | 10:31 |
| wrtp | niemeyer_: the ec2 implementation is a fairly close reflection of the python implementation in this respect | 10:31 |
| niemeyer_ | wrtp: It's controlling the firewall data associated with the instance | 10:31 |
| wrtp | niemeyer_: currently it's controlling the firewall data associated with the *machine* | 10:32 |
| niemeyer_ | wrtp: Not really | 10:32 |
| wrtp | niemeyer_: because the security group is named after the machine, not the instance | 10:32 |
| niemeyer_ | wrtp: The firewall data associated the machine is controlled via state | 10:32 |
| niemeyer_ | wrtp: It's not doing anything with that | 10:32 |
| wrtp | ? | 10:32 |
| niemeyer_ | ? | 10:32 |
| niemeyer_ | Can't answer that :) | 10:33 |
| wrtp | :-) | 10:33 |
| wrtp | i could also say "the firewall associated with the machine is controlled by the provider" | 10:33 |
| niemeyer_ | wrtp: Machine lives in *State | 10:34 |
| niemeyer_ | wrtp: Everythign about the Machine is controlled via *State | 10:35 |
| wrtp | ok, s/machine/machine id/ | 10:35 |
| niemeyer_ | wrtp: Instance is the name we give to a provider's virtual machine | 10:35 |
| niemeyer_ | wrtp: Environ.OpenPort manages the Instance, not the Machine | 10:35 |
| niemeyer_ | wrtp: That's what I'm trying to point out | 10:35 |
| wrtp | niemeyer_: in the ec2 implementation that's not true | 10:35 |
| niemeyer_ | wrtp: In all environs/* implementations, that is true | 10:35 |
| wrtp | niemeyer_: because the security group is associated with the machine id, not the instance id | 10:35 |
| niemeyer_ | wrtp: That's completely irrelevant.. we could name it as "abacate" | 10:35 |
| wrtp | niemeyer_: and there can be several instances for a given machine id | 10:35 |
| niemeyer_ | wrtp: It's still associated with the Instance, and it's still managing the firewall for the Instance | 10:36 |
| niemeyer_ | wrtp: Machine is an abstraction we decided on for referring to the *State information | 10:36 |
| wrtp | niemeyer_: originally, i had Instance.OpenPort | 10:36 |
| wrtp | niemeyer_: but that seems wrong | 10:36 |
| niemeyer_ | wrtp: Yes, and you changed that to make it more convenient | 10:36 |
| wrtp | niemeyer_: not more convenient, more true to reality | 10:36 |
| niemeyer_ | wrtp: The fact we use machine ids on the security group name is an implementation detail.. the fact we have to *use* security groups like that is also an implementation detail | 10:37 |
| niemeyer_ | wrtp: In the future this will live within the instance | 10:37 |
| wrtp | niemeyer_: in that case, the Environ interface must change | 10:37 |
| wrtp | niemeyer_: because the PA won't be able to control it from outside | 10:37 |
| niemeyer_ | wrtp: Yes, it will have to change eventually, by simply losing these methods | 10:37 |
| wrtp | niemeyer_: exactly. | 10:37 |
| wrtp | niemeyer_: so for the time being, this is what we've got - the ports for an instance are managed by changing attributes associated with the machine id, not the instance. | 10:38 |
| niemeyer_ | wrtp: But disagreeing that Environ controls Instances is a blocker on that conversation.. we StartInstance, we StopInstance, we ask for Instance and AllInstances.. | 10:38 |
| wrtp | niemeyer_: i certainly didn't disagree that Environ controls Instances | 10:39 |
| niemeyer_ | wrtp: That may be the best way to move forward, but it's hard to get there if we can't even agree on obvious facts | 10:39 |
| wrtp | niemeyer_: the issue of the interaction of machine id and the Environ is a tricky one | 10:40 |
| wrtp | niemeyer_: why are we passing the machine id into StartInstance at all? | 10:40 |
| niemeyer_ | wrtp: To get the machine id into the state initialization | 10:40 |
| niemeyer_ | wrtp: That was the real reason it got there | 10:40 |
| wrtp | niemeyer_: so maybe it *is* wrong that the instance's security group is named after the machine id | 10:41 |
| wrtp | niemeyer_: (but it's going to be a bit awkward to come up with new unique names) | 10:42 |
| niemeyer_ | wrtp: I see it merely as an implementation convenience as we need something unique per machine | 10:42 |
| wrtp | niemeyer_: per machine or per instance? | 10:42 |
| niemeyer_ | wrtp: This is completely hidden away from the interface | 10:42 |
| niemeyer_ | wrtp: Per instance, sorry | 10:42 |
| niemeyer_ | wrtp: I also see it as a nasty hack we have to do for AWS | 10:42 |
| wrtp | niemeyer_: currently, it's *not* unique per instance | 10:42 |
| wrtp | niemeyer_: sure | 10:42 |
| wrtp | niemeyer_: i agree it's not great. | 10:43 |
| niemeyer_ | wrtp: Surely it must be unique.. a security group name is unique, and we have one security group per instance.. | 10:43 |
| wrtp | niemeyer_: we can have multiple instances using the same security group | 10:43 |
| wrtp | niemeyer_: if they're started with the same machine id | 10:44 |
| wrtp | niemeyer_: there's nothing in the Environ API that prevents that | 10:44 |
| niemeyer_ | wrtp: Oh, you mean if there are multiple environments in a single AWS account? | 10:44 |
| niemeyer_ | wrtp: Fuck, that's a bug | 10:44 |
| wrtp | niemeyer_: no | 10:44 |
| wrtp | niemeyer_: if i call StartInstance(1); StartInstance(1) | 10:45 |
| niemeyer_ | wrtp: Oh, wait.. the security group has the env name in it too | 10:45 |
| wrtp | niemeyer_: yeah | 10:45 |
| niemeyer_ | wrtp: Oh, sure.. that'd be broken usage of the environment | 10:45 |
| wrtp | niemeyer_: if i do the above, i'll get two instances sharing the same security group | 10:45 |
| wrtp | niemeyer_: not necessarily, if the first instance has stopped for some reason | 10:46 |
| wrtp | niemeyer_: and the PA is restarting it | 10:46 |
| niemeyer_ | wrtp: Stopped or destroyed? | 10:46 |
| wrtp | niemeyer_: crashed | 10:46 |
| niemeyer_ | wrtp: Crashed and destroyed or crashed and stopped? | 10:46 |
| niemeyer_ | wrtp: Both are possible | 10:46 |
| wrtp | niemeyer_: crashed and stopped. | 10:47 |
| wrtp | niemeyer_: but you're going to say that the PA won't restart in that case, right? | 10:47 |
| niemeyer_ | wrtp: That means the instance is still alive | 10:47 |
| niemeyer_ | wrtp: Yes, it shouldn't | 10:47 |
| wrtp | [11:44:30] <wrtp> niemeyer_: there's nothing in the Environ API that prevents that | 10:48 |
| wrtp | it's just a convention upheld by the PA | 10:48 |
| niemeyer_ | wrtp: Sure.. I don't undertand what your underlying point is, though. This is our backyard.. are you suggesting we implement a second *State within the environment? | 10:48 |
| niemeyer_ | wrtp: Just so we can control the correct use of *State? | 10:48 |
| wrtp | niemeyer_: which is why i thought it might be better to make the environ API better reflect the reality of what's underneath | 10:48 |
| wrtp | niemeyer_: (and as a bonus, it makes the firewaller code easier) | 10:49 |
| niemeyer_ | wrtp: Sorry, I can buy that.. that's completely unrelated to the point above, though | 10:49 |
| wrtp | niemeyer_: i'm suggesting that the OpenPorts calls associate ports with machine ids, not instances, because that's what's actually going on underneath | 10:50 |
| niemeyer_ | wrtp: "Environ has an unsafe API, thus we can make the API simpler to use" makes no sense to me | 10:50 |
| wrtp | niemeyer_: then the API isn't unsafe | 10:50 |
| wrtp | niemeyer_: because it's not trying to pretend that ports are associated directly with instances. | 10:51 |
| niemeyer_ | wrtp: How come? You're saying we can have multiple machines associated with a single security group, and then we associated ports with whatever machine is using, and that's fine!? | 10:51 |
| niemeyer_ | wrtp: This is going in a pretty strange direction | 10:51 |
| niemeyer_ | wrtp: If our implementation can't control the association between security group and instance correctly, that's a MAJOR PROBLEM. | 10:51 |
| wrtp | s/multiple machines/multiple instances/ and yes, that's ok i think | 10:51 |
| wrtp | niemeyer_: our implementation controls the association between security group and port, not between instance and port. that's the basic difficulty. | 10:53 |
| niemeyer_ | wrtp: Agreed. Cool, that's a good baseline | 10:53 |
| wrtp | niemeyer_: the "bonus" above is that the firewaller wouldn't need to keep track of every machine's allocated instance, because all it needs to change is the ports associated with a *machine* not an instance. | 10:54 |
| niemeyer_ | wrtp: machine.InstanceId()? How is doing that any harder than using a machine id? | 10:55 |
| wrtp | niemeyer_: FWIW the open_port code in the python version takes an instance as an argument but only ever uses the machine id | 10:55 |
| wrtp | niemeyer_: because the instance id can come and go | 10:55 |
| wrtp | niemeyer_: so it's one more thing to track | 10:56 |
| wrtp | niemeyer_: i had a sketch version of the firewaller code that did that | 10:56 |
| wrtp | http://paste.ubuntu.com/1087751/ | 10:57 |
| niemeyer_ | wrtp: We don't have to "track" that, I think.. if the instance id "goes", everything inside it is gone too. | 10:57 |
| wrtp | vs: http://paste.ubuntu.com/1087753/ | 10:58 |
| wrtp | niemeyer_: yes, so that firewaller needs to know when that happens, right? | 10:58 |
| wrtp | niemeyer_: and it needs to know when the machine is allocated a new instance id too | 10:58 |
| niemeyer_ | wrtp: No, the firewaller only needs to know when a new port is opened or closed, and act accordingly to open and close ports in the environment, as far as I understand it. | 10:59 |
| niemeyer_ | wrtp: StartInstance creates an instance with zero open ports | 10:59 |
| niemeyer_ | wrtp: What will open the ports in the state are the units that run within that instance that was started | 11:00 |
| niemeyer_ | wrtp: That's what the firewaller must react to | 11:00 |
| wrtp | niemeyer_: i think i see a problem with that | 11:03 |
| wrtp | niemeyer_: so... an instance gets destroyed; the PA notices that and allocates a new instance. the firewaller is thus far blissfully unaware, right? | 11:03 |
| niemeyer_ | wrtp: Yes | 11:04 |
| niemeyer_ | (I'm supposing, at least) | 11:04 |
| wrtp | niemeyer_: on the new instance, the UA starts up, notices that some ports are open that shouldn't be, so it closes them. it then runs the start hook, which opens some ports again | 11:04 |
| wrtp | niemeyer_: the new ports and the old ports are identical, so the firewaller might see no change | 11:05 |
| wrtp | niemeyer_: but given that we're operating on the instance, not the machine, that would mean we won't have changed the firewall settings for the new instance | 11:05 |
| niemeyer_ | wrtp: This problem exists irrespective of what the API looks like. Semantically, these ports should be closed. It's not fine to say "Oh, it's the same machine so I'll just leave stuff open.." | 11:07 |
| wrtp | niemeyer_: i don't think the problem exists if ports are associated with a machine id | 11:08 |
| wrtp | niemeyer_: the problem with the above is that the ports on the new instance won't be opened | 11:08 |
| niemeyer_ | wrtp: Again, it is not fine to start the new machine with open ports, despite the fact the unit didn't open it | 11:09 |
| wrtp | niemeyer_: i wasn't suggesting that we did | 11:10 |
| niemeyer_ | <wrtp> niemeyer_: the new ports and the old ports are identical, so the firewaller might see no change | 11:10 |
| niemeyer_ | wrtp: This exists no matter what the API looks like.. I don't know what you are suggesting then | 11:10 |
| wrtp | niemeyer_: a suggestion is that StartInstance is documented to close all the ports for its machine id | 11:13 |
| niemeyer_ | wrtp: s/machine id/started instance/ | 11:13 |
| wrtp | niemeyer_: no, that's the difficulty. | 11:14 |
| niemeyer_ | wrtp: Okay, clearly I'm missing what it is.. what is the difficulty in closing the ports for an instance? | 11:14 |
| * wrtp tries to get his head in order :-) | 11:16 | |
| wrtp | niemeyer_: suppose we had an implementation of Environ that really did associate ports with instances directly | 11:17 |
| wrtp | niemeyer_: as the API would suggest | 11:17 |
| wrtp | niemeyer_: in that case, we *need* to adjust the firewall settings on a new instance when a machine's instance changes | 11:18 |
| wrtp | niemeyer_: so we'd need to track the instance id for a machine to do that | 11:19 |
| wrtp | niemeyer_: we *could* do that | 11:20 |
| niemeyer_ | wrtp: We do track instance id for machines. We also do need to adjust the firewall setting on the new instance, both to ensure it has no ports open when it started, and also to ensure that the units get a clean state when they get up. | 11:20 |
| niemeyer_ | wrtp: Both of these are already true, and we're not even talking about the API. | 11:21 |
| wrtp | niemeyer_: the PA and the UA can do that between them. the PA to adjust the ports before it starts a new instance, the UA to make the state hold what it should. | 11:23 |
| niemeyer_ | wrtp: That sounds good.. the PA should simply zero out all ports on *state* specifically (not the Environ) when starting the instance | 11:24 |
| wrtp | niemeyer_: one problem with having the firewaller doing it is that it the machine will be started and running before the firewaller gets around to closing the ports | 11:24 |
| wrtp | niemeyer_: ah, yes | 11:24 |
| wrtp | niemeyer_: that sounds good | 11:25 |
| wrtp | niemeyer_: there's still a race, but i'm sure that in any reality we know, it'll be won by the firewaller | 11:25 |
| Aram | hey, | 11:26 |
| niemeyer_ | wrtp: Indeed. We can be pragmatic for now and just have a TODO in the code saying it'd be extremely weird to have that race happening. | 11:26 |
| niemeyer_ | wrtp: It's also easy to avoid the race in the future when we choose to | 11:26 |
| wrtp | niemeyer_: nevertheless, doing things this way does mean we don't *necessarily* have to track instance ids in the firewaller | 11:26 |
| wrtp | niemeyer_: yeah | 11:27 |
| wrtp | Aram: hiya | 11:27 |
| niemeyer_ | wrtp: Right, and we can still have the Environ interface operating in terms of instances | 11:27 |
| wrtp | niemeyer_: aargh! | 11:27 |
| niemeyer_ | wrtp: Heh | 11:28 |
| niemeyer_ | wrtp: Sorry, you won't convince me that | 11:28 |
| niemeyer_ | environ.OpenPort(machineId) | 11:28 |
| wrtp | niemeyer_: if the Environ interface OpenPort and ClosePort methods operate on instances, then we *do* have to track instance ids in the firewaller | 11:28 |
| niemeyer_ | is so much better than | 11:28 |
| niemeyer_ | id, err := machine.InstanceId() | 11:29 |
| niemeyer_ | environ.OpenPort(instance) | 11:29 |
| TheMue | wrtp: When the PA sets the instance id for a machine this could be watched so we know the old one for closing and the new one for opening. | 11:29 |
| wrtp | environ.OpenPort(machineId) is operating on a machine! there's no instance there... | 11:29 |
| niemeyer_ | wrtp: Gosh.. | 11:29 |
| TheMue | Aram: Hi | 11:29 |
| niemeyer_ | wrtp: This is getting repetitive | 11:30 |
| wrtp | niemeyer_: v sorry | 11:30 |
| niemeyer_ | wrtp: Machine leaves in *State | 11:30 |
| niemeyer_ | lives | 11:30 |
| niemeyer_ | wrtp: To change a *Machine OpenPorts, you need to use the *State | 11:30 |
| wrtp | niemeyer_: that OpenPort sig had "machineId" in it | 11:30 |
| wrtp | niemeyer_: ok, i see your p.o.v. | 11:30 |
| wrtp | niemeyer_: i'll rephrase | 11:31 |
| niemeyer_ | wrtp: The Environ manages *Instances | 11:31 |
| niemeyer_ | wrtp: Only *Instances.. | 11:31 |
| wrtp | environ.OpenPort(machineId) isn't operating on an instance! there's no instance there... | 11:31 |
| niemeyer_ | wrtp: It's an implementation detail that an AWS security group uses a Machine.Id in its name | 11:31 |
| * TheMue prefers the instance id too as it's the representation of the physical instance | 11:31 | |
| niemeyer_ | wrtp: As soon as we bless this code base as Good, we'll have a massive number of new providers popping by | 11:32 |
| wrtp | TheMue: the difficulty is that the underlying environ isn't actually changing an attribute on the instance... | 11:32 |
| niemeyer_ | wrtp: Despite this being a silly argument, I don't want to screw up on that interface, because then I'll have to argue with everybody else about this | 11:32 |
| wrtp | niemeyer_: ok... | 11:32 |
| wrtp | niemeyer_: so why aren't you suggesting Instance.OpenPort(port) ? | 11:33 |
| niemeyer_ | Power outage.. on batteries | 11:33 |
| wrtp | niemeyer_: crossed fingers | 11:33 |
| niemeyer_ | wrtp: As I mentioned earlier, it *sounds* to me like the implementation is simpler, given the nature of an Environ is, to have it on its interface | 11:33 |
| niemeyer_ | wrtp: We're generally talking to the front wall | 11:34 |
| niemeyer_ | wrtp: I definitely wouldn't have a strong argument against Instance.OpenPort, though | 11:34 |
| wrtp | niemeyer_: say we had a new provider that *did* associate ports directly with instances, our implementation could break, i think. | 11:34 |
| * TheMue 's wife calls for lunchtime, but thankfully I've catched up the discussion until here. It's very helpful. | 11:35 | |
| wrtp | niemeyer_: well, in fact, your suggested interface makes that not easy, i think | 11:35 |
| wrtp | niemeyer_: because the provider would have to maintain its own mapping from machine id to instance id | 11:35 |
| niemeyer_ | wrtp: Yes | 11:36 |
| niemeyer_ | TheMue: Have a good one | 11:36 |
| wrtp | niemeyer_: it would be nice to be able to adjust the open ports for a machine *before* calling StartInstance | 11:37 |
| niemeyer_ | wrtp: The provisioner worker can certainly do that | 11:37 |
| niemeyer_ | Shit.. no break started to beep | 11:49 |
| niemeyer_ | UPS, that is | 11:49 |
| wrtp | niemeyer_: eek | 11:50 |
| wrtp | hmm, dubious looking test failure: | 12:48 |
| wrtp | runtime: signal received on thread not created by Go. | 12:48 |
| wrtp | FAILlaunchpad.net/juju-core/state8.222s | 12:48 |
| TheMue | iiihks | 12:48 |
| wrtp | TheMue: i can't reproduce it, sadly | 12:49 |
| TheMue | wrtp: there are two cgos: goyaml and gozk. maybe in there | 12:49 |
| wrtp | i also got a store test failure: | 12:49 |
| wrtp | server_test.go:63: | 12:49 |
| wrtp | s.checkCounterSum(c, []string{"charm-info", curl.Series, curl.Name}, false, 1) | 12:49 |
| wrtp | server_test.go:83: | 12:49 |
| wrtp | c.Errorf("counter sum for %#v is %d, want %d", key, sum, expected) | 12:49 |
| wrtp | ... Error: counter sum for []string{"charm-info", "oneiric", "wordpress"} is 0, want 1 | 12:49 |
| wrtp | which also succeeded when i tried it again | 12:50 |
| wrtp | :-( | 12:50 |
| TheMue | Doesn't sound good. | 12:52 |
| wrtp | TheMue: just ran all the tests again and they all worked :-( | 12:57 |
| TheMue | wrtp: Monitored your memory, maybe a hanging process | 12:59 |
| TheMue | ? | 12:59 |
| wrtp | TheMue: i don't *think* so. | 13:00 |
| Aram | wrtp: also seen that failure. | 13:01 |
| Aram | in gozk, I recall. | 13:01 |
| Aram | hopefully it'll go away as we move away from Zk and cgo. | 13:01 |
| wrtp | Aram: +1 | 13:02 |
| niemeyer | wrtp, Aram, TheMue, fwereade_: I'll attend that MongoDB conference in São Paulo tomorrow, and will be heading there this afternoon | 13:23 |
| fwereade_ | niemeyer, cool, have fun | 13:23 |
| niemeyer | fwereade_: Thanks! | 13:23 |
| wrtp | niemeyer: ok, have a good one | 13:24 |
| Aram | niemeyer: have a good one, will you give a talk? | 13:24 |
| niemeyer | Aram: Yeah | 13:24 |
| wrtp | niemeyer: did you manage to get your demo thing working? | 13:24 |
| wrtp | niemeyer: with the X million documents... | 13:24 |
| niemeyer | wrtp: It's working, but still requires a few last minute fixes | 13:24 |
| niemeyer | wrtp: and I need slides too | 13:24 |
| wrtp | niemeyer: minor trivialities :-) | 13:24 |
| niemeyer | Planning on doing that on the trip today.. not smart, but the week was rushy | 13:24 |
| wrtp | niemeyer: i hope your laptop battery is working again | 13:24 |
| niemeyer | wrtp: Yeah, thankfully I got a new one in SF | 13:25 |
| TheMue | niemeyer: Will your slides later be at SlideShare? | 13:25 |
| niemeyer | wrtp: It's great to have a laptop again rather than a mobile desktop :-) | 13:25 |
| niemeyer | TheMue: I'll surely put them up somewhere | 13:25 |
| wrtp | niemeyer: how much did you end up paying? | 13:25 |
| TheMue | niemeyer: Great, appreciate it. | 13:25 |
| niemeyer | wrtp: Just under 100 USD IIRC | 13:27 |
| wrtp | niemeyer: that's loads better than the UK price. | 13:27 |
| niemeyer | wrtp: Yeah, it was really a lot cheaper | 13:28 |
| niemeyer | wrtp: and with the Prime thing, good delivery in a couple of days for free | 13:28 |
| niemeyer | hazmat: I don't have the slides up yet, but I can surely put them up somewhere if you'd like to use them | 13:29 |
| wrtp | niemeyer: initial dummy implementation of port opening, if you have time to have a look: https://codereview.appspot.com/6349097/ | 13:49 |
| wrtp | fwereade_: ^ | 13:49 |
| niemeyer | wrtp: Didn't we just agree to do it with *Instance? | 13:50 |
| wrtp | [12:34:31] <niemeyer_> wrtp: I definitely wouldn't have a strong argument against Instance.OpenPort, though | 13:50 |
| wrtp | i thought that meant you'd prefer it to be on machine id | 13:50 |
| niemeyer | wrtp: OpenPorts(machineId int | 13:51 |
| hazmat | niemeyer, that would be great | 13:51 |
| niemeyer | wrtp: That's not instance.OpenPort at all | 13:51 |
| niemeyer | hazmat: COol, will do | 13:51 |
| wrtp | niemeyer: indeed, it sounded like you'd prefer for it *not* to be Instance.OpenPort | 13:51 |
| wrtp | [12:33:54] <niemeyer_> wrtp: As I mentioned earlier, it *sounds* to me like the implementation is simpler, given the nature of an Environ is, to have it on its interface | 13:52 |
| niemeyer | wrtp: Yes, I'd prefer for it to be Environ.OpenPort indeed.. environ.OpenPort(inst, ports) | 13:52 |
| wrtp | niemeyer: huh? if it's just a method that takes an instance as argument, why not just have it as a method on that instance? | 13:53 |
| niemeyer | wrtp: LOL | 13:53 |
| niemeyer | <wrtp> [12:34:31] <niemeyer_> wrtp: I definitely wouldn't have a strong argument against Instance.OpenPort, though | 13:53 |
| TheMue | funny | 13:53 |
| wrtp | niemeyer: if it's environ.OpenPort, i think it should take a machine id. if it takes an instance, then i think it should be an instance method. | 13:54 |
| niemeyer | wrtp: Those two things are completely unrelated | 13:54 |
| * TheMue would like Instance.OpenPorts(ports) or Environ.OpenPorts(instanceId, ports) | 13:54 | |
| niemeyer | StopInstances([]Instance) error | 13:55 |
| niemeyer | Destroy(insts []Instance) error | 13:55 |
| niemeyer | etc | 13:55 |
| TheMue | ah | 13:55 |
| TheMue | wrtp: Yep, then Environ.OpenPorts(instance, ports) | 13:55 |
| wrtp | niemeyer: [12:28:38] <niemeyer_> environ.OpenPort(machineId) is so much better than [...] environ.OpenPort(instance) | 13:55 |
| wrtp | niemeyer: those are only like that because they take multiple instances | 13:56 |
| wrtp | niemeyer: if you could only stop a single instance at a time, i'd have done Stop() | 13:56 |
| niemeyer | wrtp: Heh | 13:57 |
| niemeyer | wrtp: You're doing the journalistic thing of quoting part of a sentence and inverting its meaning | 13:57 |
| niemeyer | Jul 12 08:28:30 <niemeyer_> wrtp: Sorry, you won't convince me that | 13:57 |
| niemeyer | Jul 12 08:28:37 <niemeyer_> environ.OpenPort(machineId) | 13:57 |
| niemeyer | Jul 12 08:28:48 <niemeyer_> is so much better than | 13:57 |
| wrtp | niemeyer: i think i must have understood the wrong meaning to start with! | 13:57 |
| niemeyer | Jul 12 08:29:00 <niemeyer_> id, err := machine.InstanceId() | 13:57 |
| niemeyer | Jul 12 08:29:07 <niemeyer_> environ.OpenPort(instance) | 13:57 |
| niemeyer | wrtp: That's what I said. | 13:58 |
| wrtp | i don't see the connection between the last two lines | 13:58 |
| niemeyer | wrtp: Doesn't matter.. it's still not ok to invert the meaning of what I said | 13:58 |
| wrtp | niemeyer: that wasn't my intention, honest! | 13:58 |
| wrtp | niemeyer: you seemed to like OpenPort taking a machine id | 13:59 |
| wrtp | niemeyer: rather than an instance id | 13:59 |
| wrtp | niemeyer: so that's what i implemented | 13:59 |
| niemeyer | wrtp: Okay, so let's try to agree on things rather than manipulating quotes | 13:59 |
| niemeyer | wrtp: The environment deals with instances | 13:59 |
| wrtp | niemeyer: do you like "environ.OpenPort(machineId)" or not? | 13:59 |
| niemeyer | wrtp: not machines | 13:59 |
| niemeyer | wrtp: Giving an environment a machine id for it to figure which instance is associated with is wrong | 13:59 |
| niemeyer | wrtp: Forget the implementation of EC2.. we're designing the API | 14:00 |
| wrtp | niemeyer: ok, so i totally don't understand what you meant by that sentence above... | 14:00 |
| niemeyer | wrtp: Forget it | 14:00 |
| wrtp | niemeyer: done | 14:00 |
| wrtp | niemeyer: i'll put OpenPort as a method on Instance as i'd first intended, ages ago | 14:00 |
| niemeyer | wrtp: Let's try to understand each other rather than quotes from the past | 14:01 |
| TheMue | wrtp: To keep the environment consistent it should handle with instances, no machine ids. | 14:01 |
| niemeyer | wrtp: That sounds fine | 14:01 |
| niemeyer | wrtp: and elegant in fact | 14:01 |
| * TheMue likes a consistent interface (not in the sense of a go interface) | 14:01 | |
| wrtp | niemeyer: the very first cut at environs/interface.go had it that way :-) | 14:01 |
| wrtp | niemeyer: it's not *quite* so easy for the firewaller to mock though. | 14:02 |
| niemeyer | wrtp: If the firewaller has a machine, it can query the instance from the environment and use it | 14:03 |
| wrtp | niemeyer: sure, it's not hard. in fact, if it just uses the dummy environ for tests, it might be trivial. | 14:04 |
| wrtp | niemeyer: oh, there's a problem | 14:06 |
| wrtp | niemeyer: currently i can't find the machine id from the instance | 14:06 |
| niemeyer | wrtp: Okay, I was waiting for that.. let's see if it's what I think | 14:06 |
| niemeyer | wrtp: Yeah, bingo | 14:06 |
| wrtp | niemeyer: i *can* do it, of course | 14:06 |
| wrtp | niemeyer: by querying the security groups for the instance | 14:07 |
| niemeyer | wrtp: We can definitely have a parameter of machineId in the instance to make the implementation simpler | 14:07 |
| niemeyer | Sorry | 14:07 |
| niemeyer | wrtp: We can definitely have a parameter of machineId in the instance method to make the implementation simpler | 14:07 |
| wrtp | niemeyer: in which method, sorry? | 14:07 |
| niemeyer | wrtp: Instance.OpenPort | 14:08 |
| wrtp | niemeyer: i guess | 14:08 |
| TheMue | niemeyer: So Instance.OpenPorts(machineId, ports)? | 14:08 |
| niemeyer | wrtp: We're arriving in the same design we have in Python | 14:08 |
| wrtp | TheMue: yeah | 14:08 |
| niemeyer | wrtp: Except the method is on Instance | 14:08 |
| TheMue | niemeyer: Indeed | 14:08 |
| niemeyer | Which sounds fine | 14:08 |
| TheMue | Ah, Mark is there | 14:09 |
| wrtp | niemeyer, TheMue: here's what i've got currently | 14:10 |
| wrtp | http://paste.ubuntu.com/1088025/ | 14:10 |
| niemeyer | wrtp: Cool.. I'd still prefer to not have machineId there, but that'd be unnecessary purism at this point | 14:11 |
| niemeyer | wrtp: s/should/must/ | 14:13 |
| niemeyer | wrtp: Otherwise, +1 to move forward with it | 14:13 |
| wrtp | niemeyer: i think that in dummy, i'll put the port map inside the instance, just to keep us honest. | 14:14 |
| wrtp | niemeyer: (in the proposal, the port map was in the state, to match ec2's implementation) | 14:14 |
| wrtp | niemeyer: does that sound reasonable? | 14:14 |
| niemeyer | wrtp: +1 | 14:14 |
| wrtp | niemeyer, TheMue, fwereade_: hopefully this does the job: https://codereview.appspot.com/6349097/ | 14:37 |
| TheMue | wrtp: First quick look is fine to me | 14:39 |
| wrtp | TheMue: thanks | 14:40 |
| fwereade_ | wrtp, indeed, looks sensible I think, I'll try to give it a closer look a bit later | 14:53 |
| niemeyer | wrtp: LGTM with a point for consideration | 14:59 |
| wrtp | niemeyer: if we change that, i should change it all the way through. | 15:01 |
| wrtp | niemeyer: the intention was that the ops channel says that an operation is about to happen, not that it *has* happened | 15:01 |
| wrtp | niemeyer: thus it's ok to send on the channel outside of the lock | 15:01 |
| wrtp | niemeyer: thanks for the review BTW! | 15:02 |
| niemeyer | wrtp: I think it's problematic for the reason stated | 15:02 |
| niemeyer | wrtp: and we already have operations within the lock elsewhere | 15:02 |
| wrtp | niemeyer: yeah, i hadn't noticed that StartInstance sends within the lock. | 15:04 |
| wrtp | niemeyer: i was wary of deadlock, but it's probably ok | 15:04 |
| niemeyer | wrtp: If we ever face a deadlock, it'd probably be safer to do it afterwards, rather than before | 15:05 |
| niemeyer | wrtp: So that waits that get unblocked and act on state find what they exepct | 15:05 |
| niemeyer | expect | 15:05 |
| wrtp | niemeyer: maybe i should defer the channel send | 15:05 |
| wrtp | niemeyer: so that it happens even when there's an error | 15:06 |
| niemeyer | wrtp: Well, or just unlock earlier | 15:06 |
| wrtp | niemeyer: yeah, that looks ok given that hardly anything actually returns an error. | 15:07 |
| niemeyer | wrtp: Yeah, and if it panics we'll know it | 15:07 |
| wrtp | niemeyer: the only awkward bit is the "environment is already bootstrapped" error | 15:08 |
| wrtp | niemeyer: which will have to duplicate the send | 15:08 |
| wrtp | niemeyer: because some tests rely on receiving the operation even though there's an error. perhaps that should change though. | 15:09 |
| wrtp | niemeyer: tbh, i'm going to try sending within the lock. it makes the code easier to maintain, and there's no reason it should deadlock. | 15:12 |
| niemeyer | wrtp: Sounds good | 15:14 |
| niemeyer | Alright guys, lunch time | 15:23 |
| niemeyer | I'll head to São Paulo after lunhc | 15:23 |
| Aram | have fun niemeyer. | 15:27 |
| niemeyer | Aram: Cheers! | 15:28 |
| * niemeyer leaves.. see you soon | 15:28 | |
| wrtp | fwereade_: am looking at your "train wreck" BTW | 15:51 |
| fwereade_ | wrtp, cheers | 15:51 |
| wrtp | fwereade_: ping | 17:29 |
| fwereade_ | wrtp, talk if you wish, but I'm in a meeting, expect slow replies | 17:30 |
| wrtp | fwereade_: k | 17:30 |
| wrtp | fwereade_: i think that ec2.environProvider.ComposeConfig could be quite a bit simpler | 17:31 |
| wrtp | fwereade_: and other bits too, but in general, i think it's really not such a train wreck | 17:33 |
| fwereade | wrtp, thanks, good to hear, I'm calling it a day now though :) | 18:30 |
| wrtp | fwereade: ok, have a good evening. | 18:31 |
| wrtp | fwereade: BTW Embassytown is awesome | 18:31 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!