wrtpfwereade_: mornin'!06:14
fwereade_wrtp, heyhey06:31
wrtpfwereade_: how's tricks?06:32
fwereade_wrtp, not too bad06:32
wrtpfwereade_: BTW are you getting the initial mail for each codereview proposal?06:33
wrtpfwereade_: 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 general06:34
fwereade_wrtp, maybe something about LP?06:34
wrtpfwereade_: i'm getting the initial '[Merge]' mail, but those [Merge] mails are so noisy i archive them immediately.06:34
wrtpfwereade_: the strange thing is i see any replies06:35
fwereade_wrtp, ah, ok, that is weird then06:35
wrtpfwereade_: so the first thing i'll know about any CL is when someone replies to it06:35
wrtpyeah, it's odd, and quite annoying actually06:35
fwereade_wrtp, yeah, I bet :(06:36
wrtpfwereade_: 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 weirder06:56
wrtpfwereade_: i wonder if there's a way to check in launchpad what messages it's sent me06:57
fwereade_wrtp, I doubt that sort of thing specifically is logger06:59
fwereade_wrtp, I bet we all have a subtly different combination of LP settings, and memberships of various things06:59
fwereade_TheMue, heyhey06:59
wrtpfwereade_: can you think of a CL you've replied to recently that i wasn't involved in?06:59
wrtpTheMue: yo!07:00
fwereade_wrtp, I don't think I have done much of that lately :(07:04
wrtpfwereade_: hmm. TheMue?07:04
TheMuewrtp: What?07:05
wrtpTheMue: (i'm trying to work out why i'm not seeing some emails from launchpad)07:05
wrtpTheMue:  can you think of a CL you've replied to recently that i wasn't involved in?07:05
TheMuewrtp: Have to look, but have to change systems fir it.07:07
wrtpTheMue: any CL that someone else created and you replied earlier than me would do too07:08
TheMuewrtp: If so, then mostly Williams CLs.07:09
wrtpTheMue: one example would be good, if you could, thanks!07:09
wrtpTheMue: here's an updated version of my firewaller sketch, BTW. http://paste.ubuntu.com/1087489/07:13
wrtpTheMue: 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
TheMuewrtp: Great, thx, will today integrate it into the firewaller outline of dave07:15
wrtpTheMue: good plan07:15
wrtpTheMue: 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
TheMuewrtp: That has been my prob with the first approach.07:18
TheMuewrtp: Now I'll use stubs for intermediate CLs07:19
wrtpTheMue: 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
TheMuewrtp: No, the tests shall be the right ones and make the right asserts07:22
wrtpTheMue: yeah, and the problem is that the only observable side-effects of this code are the calls to OpenPort/ClosePort07:22
TheMuewrtp: But while they first may get constants it will get more and more live with each iteration07:22
wrtpTheMue: which won't happen until all the rest is there.07:23
TheMuewrtp: So the tests verify the impl07:23
TheMuewrtp: See the current approach. I've used an interface and a mock for call verification.07:24
wrtpTheMue: 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 AFAICS07:25
wrtpTheMue: i suppose you could put in a fake call to OpenPort for the time being, to be changed later.07:25
TheMuewrtp: 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
wrtpTheMue: 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
TheRealMuewrtp: So, changed client.07:42
=== TheRealMue is now known as TheMue
wrtpTheMue: did you see my message a few seconds ago?07:43
wrtpoh yeah, you should've07:43
TheMuewrtp: About the port manager interface? Yes, just looking at it07:43
wrtpTheMue: in fact, scratch that, i think it's not easy to implement well in ec207:44
TheMuewrtp: So SetPorts() would ensure that only the passed ports are open afterwards?07:45
wrtpTheMue: yeah, but i think it's wrong07:45
wrtpTheMue: i think this is better: http://paste.ubuntu.com/1087535/07:46
TheMuewrtp: 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
wrtpTheMue: i don't think it needs to do that07:46
wrtpTheMue: apart from at the very beginning07:47
TheMuewrtp: ???07:47
TheMuewrtp: The new interface matches exactly to that.07:47
wrtpTheMue: it only needs to do the diff once, when the firewaller starts up, i think.07:47
wrtpTheMue: from then on it knows the "real" provider state, and can just apply OpenPorts and ClosePorts methods as the State changes07:48
wrtpTheMue: i.e. it will need to call Ports once only07:49
TheMuewrtp: Understand, yes.07:49
TheMuewrtp: In todays model the provisioner called individual methods after each service and machine change, so a different approach07:49
wrtpTheMue: 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 times07:50
wrtpTheMue: 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
wrtpfwereade_, TheMue: what do you think should happen to a machine's open ports if it's restarted on a different instance?09:15
wrtpniemeyer: ^09:16
wrtpniemeyer: mornin' BTW09: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, morning09:21
wrtpfwereade_: as currently conceived, the machine's open ports are independent of its instance09:22
wrtpfwereade_: but currently when a new instance is started, it ensures that the machine's group has no permissions09: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* one09:24
wrtpfwereade_: yeah, i think that's right09:24
fwereade_wrtp, however I don;t think we have any other way of telling09:24
fwereade_wrtp, wait a mo09:24
fwereade_wrtp, is it easy to reuse the security group from the original machine?09:25
wrtpfwereade_: i think perhaps it's up to the provisioner to change the port settings for a machine when it starts an instance09:25
wrtpfwereade_: it *doesn* use the security group from the original machine09:25
wrtpfwereade_: but currently it will zero the perms when the instance starts09:26
fwereade_wrtp, ah, but it clears it out?09:26
wrtpfwereade_: i'm thinking that it shouldn't09:26
fwereade_wrtp, hmmmmmmm, so, consider the lifecycle of the unit agent on the new machine09:26
fwereade_wrtp, that will start up, open ports, blah blah09:26
* wrtp is considering09:26
fwereade_wrtp, won't it Just Work already?09:26
fwereade_wrtp, I think it might :)09:27
wrtpfwereade_: it would *if* all the security groups from a previous instance of the environment had been destroyed09:27
wrtpfwereade_: but i'm not sure we can guarantee that09:27
wrtpfwereade_: thus i think that when starting a new instance, the PA should perhaps call Ports and ClosePorts to clear out any old cruft09:28
wrtps/starting a new instance/starting a new machine/09:28
wrtpfwereade_: i *think* that would solve the problem09:29
wrtpniemeyer_: yo!09:29
TheMuewrtp: Kind of CloseAllPorts() sounds good, yes09:29
wrtpTheMue: env.ClosePorts(m, m.Ports())09:29
wrtpTheMue: (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 instance09:30
wrtpTheMue: that's essentially what it does now, behind the scenes09:30
wrtpfwereade_: they are, by the ec2 environ09:30
wrtpfwereade_: but that means we can create a new instance for an existing machine09:30
fwereade_wrtp, ok, so what doesn't already do the Right Thing?09:30
TheMuewrtp: You can't pass m.Ports()09:30
wrtpTheMue: m.Ports would return an error09:31
wrtpoops, env.Ports(m) of course09:31
fwereade_wrtp, if and when that machine needs some ports opened, it'll do an open-port in a hook, and... oh09:31
fwereade_wrtp, the unit will say "meh, those ports are already open, no state changes here!"09:31
wrtpfwereade_: this was the source of my original question09:31
wrtpfwereade_: i'm not sure what behaviour we *want* here09:32
wrtpfwereade_: perhaps a new instance *should* always come up with no open ports09: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 them09:32
fwereade_wrtp, I think new instances should certainly come up with no open ports09:33
fwereade_wrtp, I'm pretty sure the difficulty is at the unit agent level09:33
wrtpfwereade_: given that the PA is starting the new instance, it's in a good position to clear out the ports first09:33
fwereade_wrtp, I thought that always happened already?09:33
TheMuewrtp: Exactly09:33
wrtpfwereade_: it does currently, but i wasn't sure if it should09:33
fwereade_wrtp, I guess that ideally this *should* be done by the PA rather than by the environ09:34
wrtpfwereade_: i'm now thinking the current behaviour is correct09:34
wrtpfwereade_: well, yeah09:34
wrtpfwereade_: that would seem better09:34
fwereade_wrtp, because then we won't need every single environ to mess with ports on instance start09:34
wrtpfwereade_: well, the same code will be executed - it'll just be called from the PA09:35
wrtpahh, but...09:35
fwereade_wrtp, yeah, but we only have to write it once instead of N times09:35
wrtpthis all interacts with the firewall code09:35
fwereade_wrtp, and the firewall code will then get an inaccurate view of state?09:36
wrtpbecause the firewall is keeping track of what ports are supposed to be open09:36
wrtpfwereade_: yeah09: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 machine09:36
TheMuewrtp: that's its task09:36
wrtpfwereade_: actually, i think you're right09:37
wrtpfwereade_: if the UA does it, we have no problems09:37
wrtpfwereade_: and the firewaller doesn't have to keep track of instance ids09:37
fwereade_wrtp, yep, think so09:37
wrtpfwereade_: then *all* port opening and closing is done via the state open/close port09:38
fwereade_wrtp, yeah, exactly09:38
fwereade_wrtp, the firewaller can watch that alone and it'll catch up when it becomes important to09:38
wrtpfwereade_: and the environ won't clear out ports when starting a new instance09:38
fwereade_wrtp, the environs should still clear out ports09:39
fwereade_wrtp, ah wait09:39
wrtpthere's a difficulty09:39
wrtpno, it's ok i think09:40
wrtpfwereade_: the PA clears out ports when it starts a new *machine*09:40
wrtpfwereade_: because machine ids are never reused09:40
wrtpfwereade_: 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 machine09:42
fwereade_wrtp, the UA will clear out the desired ports in state when it comes up09:42
fwereade_wrtp, and at that point the firewaller will be back in sync09:42
wrtpfwereade_: yes, that sounds reasonable09:43
wrtpfwereade_: if *slightly* tricksy09:43
wrtpfwereade_: 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) CloseAllPorts09:45
fwereade_wrtp, StartInstance having to magically mess with ports in every provider STM like a recipe for bugs09:46
wrtpfwereade_: 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 YMMV09:46
wrtpfwereade_: my mileage does :-)09:46
wrtpfwereade_: tbh i'd be happy for the PA to implement that logic09:47
fwereade_wrtp, IMO `env.StartInstance(...` rather than StartInstance(env, ...` not not sufficiently obviously a bug09:47
wrtpfwereade_: it's very straightforward, and then there's no hidden interaction between StartInstance and ClosePorts.09:47
wrtpfwereade_: nothing else calls StartInstance09:48
wrtpfwereade_: so there's really no need for another layer09:48
fwereade_wrtp, we'll see09:48
wrtpfwereade_: for the time being, i'll implement Ports, OpenPorts and ClosePorts, and remove the port closing logic from StartInstance09:49
wrtpfwereade_: i think that should be sufficient09:49
niemeyer_wrtp, fwereade_: Heya!09:50
wrtpniemeyer_: does that sound reasonable?09:50
fwereade_niemeyer_, heyhey09:50
niemeyer_wrtp: Sorry, I wasn't following09:50
niemeyer_wrtp: Breakfast made a lot of sense, though :-)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 happen09:50
* fwereade_ shrugs09:51
wrtpniemeyer_: perhaps you could read over the recent conversation09:51
wrtpfwereade_: only in the context of the provisioner is that true09: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, though09:51
fwereade_wrtp, and on bootstrap?09:51
wrtpniemeyer_: np09:51
wrtpfwereade_: that's just a specialised kind of provisioning :-)09:51
fwereade_wrtp, as you like09:52
wrtpfwereade_: we're only talking about 3 lines of code in two places here...09:52
TheMuewrtp: 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 about09:53
fwereade_wrtp, bug waiting to happen09:53
wrtpfwereade_: 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 ;p09:54
wrtpTheMue: there's one units watcher per machine. then line 36 starts a new watcher for any new unit on a given machine09:54
* wrtp finds that less bugs come from less code09:55
TheMuewrtp: ah, so I misunderstood the plural sentence09:55
fwereade_wrtp, I don't think we're going to convince one another here09:56
wrtpTheMue: yeah by "machine units watcher" i meant a watcher to watch the units on a given machine09:56
TheMuewrtp: That definitely makes more sense09:56
wrtpfwereade_: you're probably right :-|09:57
TheMuewrtp, fwereade_: have to be afk for a moment, once again daddy-daughter-taxi09:57
wrtpfwereade_: 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
wrtpfwereade_: 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 exposing10:00
fwereade_wrtp, but I am not interested in reachitecting anything we don't *have* to at this stage10:00
wrtpfwereade_: the wrong thing to be exposing to what?10:00
fwereade_wrtp, so let's just drop it :)10:00
wrtpfwereade_: np10:00
wrtpfwereade_: i'm interested in your thoughts though10: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 ago10:02
wrtpfwereade_: the common stuff can easily be factored out later while retaining the same environs.Environ interface.10:02
wrtpfwereade_: i think10:02
fwereade_wrtp, we'll worry about that when we have to10:02
wrtpfwereade_: my thought too10:02
niemeyer_fwereade_, wrtp: I'm not sure I undertand what's the last consensus, despite the fact I've read the backlog10:11
wrtpniemeyer_: as i understand it, the consensus is this:10:11
wrtpniemeyer_: 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 to10:12
fwereade_wrtp, niemeyer_: AIUI ;)10:13
wrtpniemeyer_: 2) we change the ec2 StartInstance so that it doesn't automatically clear out machine ports10:13
niemeyer_Looks like there's no consensus indeed10:14
wrtpniemeyer_: we're not in conflict there10:14
wrtpniemeyer_: the question is *where* the provider ports get cleared out10: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
wrtpniemeyer_: 3) make the PA clear out ports when it starts a new instance10:14
wrtpniemeyer_: this is so that there's no behind-the-scenes interaction between StartInstance and the open ports10:15
wrtpniemeyer_: but i'm happy for StartInstance to be documented as closing any machine ports too10:15
wrtpniemeyer_: 4) make the UA clear out state ports on unit start10:16
wrtpniemeyer_: oops.10:16
niemeyer_That seems to turn a trivial issue into a multi-faceted problem on three different entities10:16
wrtpniemeyer_: 3) make the PA clear out ports when it starts a new *machine*.10:16
wrtpniemeyer_: what does?10:17
niemeyer_What's the issue we have?10:17
niemeyer_wrtp: (1) looks good, btw10:18
wrtpniemeyer_: one issue is: what happens when the PA starts a new instance for a machine that had an instance before?10:18
wrtpniemeyer_: open ports are attached to machines, not instances10:18
niemeyer_wrtp: Not exactly10:19
wrtpniemeyer_: and it would be nice if the firewaller doesn't have to track instances10:19
niemeyer_wrtp: Both contain data about it10:19
niemeyer_wrtp: But, ok..10:19
niemeyer_wrtp: The provisioner doesn't care about ports at all10:19
niemeyer_wrtp: It's not its problem10:20
wrtpniemeyer_: there is a problem10:20
niemeyer_wrtp: Ok.. go on10:20
wrtpniemeyer_: if the provisioner starts a new instance for a machine, we want to close all the ports for that machine10:21
wrtpniemeyer_: probably10:21
niemeyer_wrtp: No, we don't10:21
wrtpniemeyer_: although we could choose not to10:21
niemeyer_wrtp: Machines should be started with no open ports10:21
wrtpniemeyer_: fwereade_ thought we should. i'm on the fence.10:21
niemeyer_Bad term10:21
niemeyer_wrtp: Instances should be started with no open ports10:21
wrtpniemeyer_: exactly10:21
niemeyer_wrtp: That's a problem for the environment to solve10:21
wrtpniemeyer_: so we want to close all the ports for the existing machine10:21
wrtpniemeyer_: in the state10:22
niemeyer_wrtp: No!10:22
niemeyer_wrtp: The state is something else!10:22
wrtpniemeyer_: 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 issue10:22
niemeyer_wrtp: Yeah, I was missing indeed.10:23
niemeyer_wrtp: Okay10:23
niemeyer_wrtp: So there are two separate issues10:23
niemeyer_wrtp: One of them concerns management of ports of the instance.. the other about management of ports in the state10:23
niemeyer_wrtp: Which one are we talking about?10:24
wrtpniemeyer_: how they interact with each other10:24
niemeyer_wrtp: Okay, so can we please separate them up10:24
niemeyer_wrtp: Not really.. they are independent issues10:24
wrtpniemeyer_: there is no management of ports of the instance10:25
niemeyer_wrtp: !?10:25
wrtpniemeyer_: there is management of ports of the *machine*10:25
wrtpniemeyer_: OpenPorts(machineId int, ports []state.Port) error10:26
niemeyer_wrtp: Oops10:26
niemeyer_wrtp: Yeah, just now I realize that.. this seems wrong10:26
niemeyer_wrtp: Environ manages instances, not machines10:26
wrtpniemeyer_: 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 yesterday10:27
wrtpniemeyer_: well, currently we name the instance group after the machine id10:27
wrtpniemeyer_: but i think you're suggesting that we don't do that10:27
niemeyer_wrtp: I'm suggesting something else.. I'm suggesting that Environ manages instances, not machines. Do we agree on that?10:28
wrtpniemeyer_: currently the Environ uses the machine id for some things10:29
niemeyer_wrtp: Do we agree on that?10:29
wrtpniemeyer_: "manages", yes.10:29
niemeyer_wrtp: Okay, thanks10:29
wrtpniemeyer_: 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 way10:31
niemeyer_wrtp: It's managing the instance information for sure10:31
wrtpniemeyer_: the ec2 implementation is a fairly close reflection of the python implementation in this respect10:31
niemeyer_wrtp: It's controlling the firewall data associated with the instance10:31
wrtpniemeyer_: currently it's controlling the firewall data associated with the *machine*10:32
niemeyer_wrtp: Not really10:32
wrtpniemeyer_: because the security group is named after the machine, not the instance10:32
niemeyer_wrtp: The firewall data associated the machine is controlled via state10:32
niemeyer_wrtp: It's not doing anything with that10:32
niemeyer_Can't answer that :)10:33
wrtpi could also say "the firewall associated with the machine is controlled by the provider"10:33
niemeyer_wrtp: Machine lives in *State10:34
niemeyer_wrtp: Everythign about the Machine is controlled via *State10:35
wrtpok, s/machine/machine id/10:35
niemeyer_wrtp: Instance is the name we give to a provider's virtual machine10:35
niemeyer_wrtp: Environ.OpenPort manages the Instance, not the Machine10:35
niemeyer_wrtp: That's what I'm trying to  point out10:35
wrtpniemeyer_: in the ec2 implementation that's not true10:35
niemeyer_wrtp: In all environs/* implementations, that is true10:35
wrtpniemeyer_: because the security group is associated with the machine id, not the instance id10:35
niemeyer_wrtp: That's completely irrelevant.. we could name it as "abacate"10:35
wrtpniemeyer_: and there can be several instances for a given machine id10:35
niemeyer_wrtp: It's still associated with the Instance, and it's still managing the firewall for the Instance10:36
niemeyer_wrtp: Machine is an abstraction we decided on for referring to the *State information10:36
wrtpniemeyer_: originally, i had Instance.OpenPort10:36
wrtpniemeyer_: but that seems wrong10:36
niemeyer_wrtp: Yes, and you changed that to make it more convenient10:36
wrtpniemeyer_: not more convenient, more true to reality10: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 detail10:37
niemeyer_wrtp: In the future this will live within the instance10:37
wrtpniemeyer_: in that case, the Environ interface must change10:37
wrtpniemeyer_: because the PA won't be able to control it from outside10:37
niemeyer_wrtp: Yes, it will have to change eventually, by simply losing these methods10:37
wrtpniemeyer_: exactly.10:37
wrtpniemeyer_: 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
wrtpniemeyer_: i certainly didn't disagree that Environ controls Instances10: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 facts10:39
wrtpniemeyer_: the issue of the interaction of machine id and the Environ is a tricky one10:40
wrtpniemeyer_: why are we passing the machine id into StartInstance at all?10:40
niemeyer_wrtp: To get the machine id into the state initialization10:40
niemeyer_wrtp: That was the real reason it got there10:40
wrtpniemeyer_: so maybe it *is* wrong that the instance's security group is named after the machine id10:41
wrtpniemeyer_: (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 machine10:42
wrtpniemeyer_: per machine or per instance?10:42
niemeyer_wrtp: This is completely hidden away from the interface10:42
niemeyer_wrtp: Per instance, sorry10:42
niemeyer_wrtp: I also see it as a nasty hack we have to do for AWS10:42
wrtpniemeyer_: currently, it's *not* unique per instance10:42
wrtpniemeyer_: sure10:42
wrtpniemeyer_: 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
wrtpniemeyer_: we can have multiple instances using the same security group10:43
wrtpniemeyer_: if they're started with the same machine id10:44
wrtpniemeyer_: there's nothing in the Environ API that prevents that10:44
niemeyer_wrtp: Oh, you mean if there are multiple environments in a single AWS account?10:44
niemeyer_wrtp: Fuck, that's a bug10:44
wrtpniemeyer_: no10:44
wrtpniemeyer_: if i call StartInstance(1); StartInstance(1)10:45
niemeyer_wrtp: Oh, wait.. the security group has the env name in it too10:45
wrtpniemeyer_: yeah10:45
niemeyer_wrtp: Oh, sure.. that'd be broken usage of the environment10:45
wrtpniemeyer_: if i do the above, i'll get two instances sharing the same security group10:45
wrtpniemeyer_: not necessarily, if the first instance has stopped for some reason10:46
wrtpniemeyer_: and the PA is restarting it10:46
niemeyer_wrtp: Stopped or destroyed?10:46
wrtpniemeyer_: crashed10:46
niemeyer_wrtp: Crashed and destroyed or crashed and stopped?10:46
niemeyer_wrtp: Both are possible10:46
wrtpniemeyer_: crashed and stopped.10:47
wrtpniemeyer_: 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 alive10:47
niemeyer_wrtp: Yes, it shouldn't10:47
wrtp[11:44:30] <wrtp> niemeyer_: there's nothing in the Environ API that prevents that10:48
wrtpit's just a convention upheld by the PA10: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
wrtpniemeyer_: which is why i thought it might be better to make the environ API better reflect the reality of what's underneath10:48
wrtpniemeyer_: (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, though10:49
wrtpniemeyer_: i'm suggesting that the OpenPorts calls associate ports with machine ids, not instances, because that's what's actually going on underneath10:50
niemeyer_wrtp: "Environ has an unsafe API, thus we can make the API simpler to use" makes no sense to me10:50
wrtpniemeyer_: then the API isn't unsafe10:50
wrtpniemeyer_: 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 direction10:51
niemeyer_wrtp: If our implementation can't control the association between security group and instance correctly, that's a MAJOR PROBLEM.10:51
wrtps/multiple machines/multiple instances/ and yes, that's ok i think10:51
wrtpniemeyer_: 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 baseline10:53
wrtpniemeyer_: 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
wrtpniemeyer_: FWIW the open_port code in the python version takes an instance as an argument but only ever uses the machine id10:55
wrtpniemeyer_: because the instance id can come and go10:55
wrtpniemeyer_: so it's one more thing to track10:56
wrtpniemeyer_: i had a sketch version of the firewaller code that did that10:56
niemeyer_wrtp: We don't have to "track" that, I think.. if the instance id "goes", everything inside it is gone too.10:57
wrtpvs: http://paste.ubuntu.com/1087753/10:58
wrtpniemeyer_: yes, so that firewaller needs to know when that happens, right?10:58
wrtpniemeyer_: and it needs to know when the machine is allocated a new instance id too10: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 ports10:59
niemeyer_wrtp: What will open the ports in the state are the units that run within that instance that was started11:00
niemeyer_wrtp: That's what the firewaller must react to11:00
wrtpniemeyer_: i think i see a problem with that11:03
wrtpniemeyer_: 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: Yes11:04
niemeyer_(I'm supposing, at least)11:04
wrtpniemeyer_: 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 again11:04
wrtpniemeyer_: the new ports and the old ports are identical, so the firewaller might see no change11:05
wrtpniemeyer_: 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 instance11: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
wrtpniemeyer_: i don't think the problem exists if ports are associated with a machine id11:08
wrtpniemeyer_: the problem with the above is that the ports on the new instance won't be opened11:08
niemeyer_wrtp: Again, it is not fine to start the new machine with open ports, despite the fact the unit didn't open it11:09
wrtpniemeyer_: i wasn't suggesting that we did11:10
niemeyer_<wrtp> niemeyer_: the new ports and the old ports are identical, so the firewaller might see no change11:10
niemeyer_wrtp: This exists no matter what the API looks like.. I don't know what you are suggesting then11:10
wrtpniemeyer_: a suggestion is that StartInstance is documented to close all the ports for its machine id11:13
niemeyer_wrtp: s/machine id/started instance/11:13
wrtpniemeyer_: 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
wrtpniemeyer_: suppose we had an implementation of Environ that really did associate ports with instances directly11:17
wrtpniemeyer_: as the API would suggest11:17
wrtpniemeyer_: in that case, we *need* to adjust the firewall settings on a new instance when a machine's instance changes11:18
wrtpniemeyer_: so we'd need to track the instance id for a machine to do that11:19
wrtpniemeyer_: we *could* do that11: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
wrtpniemeyer_: 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 instance11:24
wrtpniemeyer_: 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 ports11:24
wrtpniemeyer_: ah, yes11:24
wrtpniemeyer_: that sounds good11:25
wrtpniemeyer_: there's still a race, but i'm sure that in any reality we know, it'll be won by the firewaller11:25
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 to11:26
wrtpniemeyer_: nevertheless, doing things this way does mean we don't *necessarily* have to track instance ids in the firewaller11:26
wrtpniemeyer_: yeah11:27
wrtpAram: hiya11:27
niemeyer_wrtp: Right, and we can still have the Environ interface operating in terms of instances11:27
wrtpniemeyer_: aargh!11:27
niemeyer_wrtp: Heh11:28
niemeyer_wrtp: Sorry, you won't convince me that11:28
wrtpniemeyer_: if the Environ interface OpenPort and ClosePort methods operate on instances, then we *do* have to track instance ids in the firewaller11:28
niemeyer_is so much better than11:28
niemeyer_id, err := machine.InstanceId()11:29
TheMuewrtp: 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
wrtpenviron.OpenPort(machineId) is operating on a machine! there's no instance there...11:29
niemeyer_wrtp: Gosh..11:29
TheMueAram: Hi11:29
niemeyer_wrtp: This is getting repetitive11:30
wrtpniemeyer_: v sorry11:30
niemeyer_wrtp: Machine leaves in *State11:30
niemeyer_wrtp: To change a *Machine OpenPorts, you need to use the *State11:30
wrtpniemeyer_: that OpenPort sig had "machineId" in it11:30
wrtpniemeyer_: ok, i see your p.o.v.11:30
wrtpniemeyer_: i'll rephrase11:31
niemeyer_wrtp: The Environ manages *Instances11:31
niemeyer_wrtp: Only *Instances..11:31
wrtpenviron.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 name11:31
* TheMue prefers the instance id too as it's the representation of the physical instance11:31
niemeyer_wrtp: As soon as we bless this code base as Good, we'll have a massive number of new providers popping by11:32
wrtpTheMue: 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 this11:32
wrtpniemeyer_: ok...11:32
wrtpniemeyer_: so why aren't you suggesting Instance.OpenPort(port) ?11:33
niemeyer_Power outage.. on batteries11:33
wrtpniemeyer_: crossed fingers11: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 interface11:33
niemeyer_wrtp: We're generally talking to the front wall11:34
niemeyer_wrtp: I definitely wouldn't have a strong argument against Instance.OpenPort, though11:34
wrtpniemeyer_: 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
wrtpniemeyer_: well, in fact, your suggested interface makes that not easy, i think11:35
wrtpniemeyer_: because the provider would have to maintain its own mapping from machine id to instance id11:35
niemeyer_wrtp: Yes11:36
niemeyer_TheMue: Have a good one11:36
wrtpniemeyer_: it would be nice to be able to adjust the open ports for a machine *before* calling StartInstance11:37
niemeyer_wrtp: The provisioner worker can certainly do that11:37
niemeyer_Shit.. no break started to beep11:49
niemeyer_UPS, that is11:49
wrtpniemeyer_: eek11:50
wrtphmm, dubious looking test failure:12:48
wrtpruntime: signal received on thread not created by Go.12:48
wrtpTheMue: i can't reproduce it, sadly12:49
TheMuewrtp: there are two cgos: goyaml and gozk. maybe in there12:49
wrtpi also got a store test failure:12:49
wrtp    s.checkCounterSum(c, []string{"charm-info", curl.Series, curl.Name}, false, 1)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 112:49
wrtpwhich also succeeded when i tried it again12:50
TheMueDoesn't sound good.12:52
wrtpTheMue: just ran all the tests again and they all worked :-(12:57
TheMuewrtp: Monitored your memory, maybe a hanging process12:59
wrtpTheMue: i don't *think* so.13:00
Aramwrtp: also seen that failure.13:01
Aramin gozk, I recall.13:01
Aramhopefully it'll go away as we move away from Zk and cgo.13:01
wrtpAram: +113:02
niemeyerwrtp, Aram, TheMue, fwereade_: I'll attend that MongoDB conference in São Paulo tomorrow, and will be heading there this afternoon13:23
fwereade_niemeyer, cool, have fun13:23
niemeyerfwereade_: Thanks!13:23
wrtpniemeyer: ok, have a good one13:24
Aramniemeyer: have a good one, will you give a talk?13:24
niemeyerAram: Yeah13:24
wrtpniemeyer: did you manage to get your demo thing working?13:24
wrtpniemeyer: with the X million documents...13:24
niemeyerwrtp: It's working, but still requires a few last minute fixes13:24
niemeyerwrtp: and I need slides too13:24
wrtpniemeyer: minor trivialities :-)13:24
niemeyerPlanning on doing that on the trip today.. not smart, but the week was rushy13:24
wrtpniemeyer: i hope your laptop battery is working again13:24
niemeyerwrtp: Yeah, thankfully I got a new one in SF13:25
TheMueniemeyer: Will your slides later be at SlideShare?13:25
niemeyerwrtp: It's great to have a laptop again rather than a mobile desktop :-)13:25
niemeyerTheMue: I'll surely put them up somewhere13:25
wrtpniemeyer: how much did you end up paying?13:25
TheMueniemeyer: Great, appreciate it.13:25
niemeyerwrtp: Just under 100 USD IIRC13:27
wrtpniemeyer: that's loads better than the UK price.13:27
niemeyerwrtp: Yeah, it was really a lot cheaper13:28
niemeyerwrtp: and with the Prime thing, good delivery in a couple of days for free13:28
niemeyerhazmat: I don't have the slides up yet, but I can surely put them up somewhere if you'd like to use them13:29
wrtpniemeyer: initial dummy implementation of port opening, if you have time to have a look: https://codereview.appspot.com/6349097/13:49
wrtpfwereade_: ^13:49
niemeyerwrtp: 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, though13:50
wrtpi thought that meant you'd prefer it to be on machine id13:50
niemeyerwrtp: OpenPorts(machineId int13:51
hazmatniemeyer, that would be great13:51
niemeyerwrtp: That's not instance.OpenPort at all13:51
niemeyerhazmat: COol, will do13:51
wrtpniemeyer: indeed, it sounded like you'd prefer for it *not* to be Instance.OpenPort13: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 interface13:52
niemeyerwrtp: Yes, I'd prefer for it to be Environ.OpenPort indeed.. environ.OpenPort(inst, ports)13:52
wrtpniemeyer: 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
niemeyerwrtp: LOL13:53
niemeyer<wrtp> [12:34:31] <niemeyer_> wrtp: I definitely wouldn't have a strong argument against Instance.OpenPort, though13:53
wrtpniemeyer: 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
niemeyerwrtp: Those two things are completely unrelated13:54
* TheMue would like Instance.OpenPorts(ports) or Environ.OpenPorts(instanceId, ports)13:54
niemeyer        StopInstances([]Instance) error13:55
niemeyer        Destroy(insts []Instance) error13:55
TheMuewrtp: Yep, then Environ.OpenPorts(instance, ports)13:55
wrtpniemeyer: [12:28:38] <niemeyer_> environ.OpenPort(machineId) is so much better than [...] environ.OpenPort(instance)13:55
wrtpniemeyer: those are only like that because they take multiple instances13:56
wrtpniemeyer: if you could only stop a single instance at a time, i'd have done Stop()13:56
niemeyerwrtp: Heh13:57
niemeyerwrtp: You're doing the journalistic thing of quoting part of a sentence and inverting its meaning13:57
niemeyerJul 12 08:28:30 <niemeyer_>     wrtp: Sorry, you won't convince me that13:57
niemeyerJul 12 08:28:37 <niemeyer_>     environ.OpenPort(machineId)13:57
niemeyerJul 12 08:28:48 <niemeyer_>     is so much better than13:57
wrtpniemeyer: i think i must have understood the wrong meaning to start with!13:57
niemeyerJul 12 08:29:00 <niemeyer_>     id, err := machine.InstanceId()13:57
niemeyerJul 12 08:29:07 <niemeyer_>     environ.OpenPort(instance)13:57
niemeyerwrtp: That's what I said.13:58
wrtpi don't see the connection between the last two lines13:58
niemeyerwrtp: Doesn't matter.. it's still not ok to invert the meaning of what I said13:58
wrtpniemeyer: that wasn't my intention, honest!13:58
wrtpniemeyer: you seemed to like OpenPort taking a machine id13:59
wrtpniemeyer: rather than an instance id13:59
wrtpniemeyer: so that's what i implemented13:59
niemeyerwrtp: Okay, so let's try to agree on things rather than manipulating quotes13:59
niemeyerwrtp: The environment deals with instances13:59
wrtpniemeyer: do you like "environ.OpenPort(machineId)" or not?13:59
niemeyerwrtp: not machines13:59
niemeyerwrtp: Giving an environment a machine id for it to figure which instance is associated with is wrong13:59
niemeyerwrtp: Forget the implementation of EC2.. we're designing the API14:00
wrtpniemeyer: ok, so i totally don't understand what you meant by that sentence above...14:00
niemeyerwrtp: Forget it14:00
wrtpniemeyer: done14:00
wrtpniemeyer: i'll put OpenPort as a method on Instance as i'd first intended, ages ago14:00
niemeyerwrtp: Let's try to understand each other rather than quotes from the past14:01
TheMuewrtp: To keep the environment consistent it should handle with instances, no machine ids.14:01
niemeyerwrtp: That sounds fine14:01
niemeyerwrtp: and elegant in fact14:01
* TheMue likes a consistent interface (not in the sense of a go interface)14:01
wrtpniemeyer: the very first cut at environs/interface.go had it that way :-)14:01
wrtpniemeyer: it's not *quite* so easy for the firewaller to mock though.14:02
niemeyerwrtp: If the firewaller has a machine, it can query the instance from the environment and use it14:03
wrtpniemeyer: sure, it's not hard. in fact, if it just uses the dummy environ for tests, it might be trivial.14:04
wrtpniemeyer: oh, there's a problem14:06
wrtpniemeyer: currently i can't find the machine id from the instance14:06
niemeyerwrtp: Okay, I was waiting for that.. let's see if it's what I think14:06
niemeyerwrtp: Yeah, bingo14:06
wrtpniemeyer: i *can* do it, of course14:06
wrtpniemeyer: by querying the security groups for the instance14:07
niemeyerwrtp: We can definitely have a parameter of machineId in the instance to make the implementation simpler14:07
niemeyerwrtp: We can definitely have a parameter of machineId in the instance method to make the implementation simpler14:07
wrtpniemeyer: in which method, sorry?14:07
niemeyerwrtp: Instance.OpenPort14:08
wrtpniemeyer: i guess14:08
TheMueniemeyer: So Instance.OpenPorts(machineId, ports)?14:08
niemeyerwrtp: We're arriving in the same design we have in Python14:08
wrtpTheMue: yeah14:08
niemeyerwrtp: Except the method is on Instance14:08
TheMueniemeyer: Indeed14:08
niemeyerWhich sounds fine14:08
TheMueAh, Mark is there14:09
wrtpniemeyer, TheMue: here's what i've got currently14:10
niemeyerwrtp: Cool.. I'd still prefer to not have machineId there, but that'd be unnecessary purism at this point14:11
niemeyerwrtp: s/should/must/14:13
niemeyerwrtp: Otherwise, +1 to move forward with it14:13
wrtpniemeyer: i think that in dummy, i'll put the port map inside the instance, just to keep us honest.14:14
wrtpniemeyer: (in the proposal, the port map was in the state, to match ec2's implementation)14:14
wrtpniemeyer: does that sound reasonable?14:14
niemeyerwrtp: +114:14
wrtpniemeyer, TheMue, fwereade_: hopefully this does the job: https://codereview.appspot.com/6349097/14:37
TheMuewrtp: First quick look is fine to me14:39
wrtpTheMue: thanks14:40
fwereade_wrtp, indeed, looks sensible I think, I'll try to give it a closer look a bit later14:53
niemeyerwrtp: LGTM with a point for consideration14:59
wrtpniemeyer: if we change that, i should change it all the way through.15:01
wrtpniemeyer: the intention was that the ops channel says that an operation is about to happen, not that it *has* happened15:01
wrtpniemeyer: thus it's ok to send on the channel outside of the lock15:01
wrtpniemeyer: thanks for the review BTW!15:02
niemeyerwrtp: I think it's problematic for the reason stated15:02
niemeyerwrtp: and we already have operations within the lock elsewhere15:02
wrtpniemeyer: yeah, i hadn't noticed that StartInstance sends within the lock.15:04
wrtpniemeyer: i was wary of deadlock, but it's probably ok15:04
niemeyerwrtp: If we ever face a deadlock, it'd probably be safer to do it afterwards, rather than before15:05
niemeyerwrtp: So that waits that get unblocked and act on state find what they exepct15:05
wrtpniemeyer: maybe i should defer the channel send15:05
wrtpniemeyer: so that it happens even when there's an error15:06
niemeyerwrtp: Well, or just unlock earlier15:06
wrtpniemeyer: yeah, that looks ok given that hardly anything actually returns an error.15:07
niemeyerwrtp: Yeah, and if it panics we'll know it15:07
wrtpniemeyer: the only awkward bit is the "environment is already bootstrapped" error15:08
wrtpniemeyer: which will have to duplicate the send15:08
wrtpniemeyer: because some tests rely on receiving the operation even though there's an error. perhaps that should change though.15:09
wrtpniemeyer: 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
niemeyerwrtp: Sounds good15:14
niemeyerAlright guys, lunch time15:23
niemeyerI'll head to São Paulo after lunhc15:23
Aramhave fun niemeyer.15:27
niemeyerAram: Cheers!15:28
* niemeyer leaves.. see you soon15:28
wrtpfwereade_: am looking at your "train wreck" BTW15:51
fwereade_wrtp, cheers15:51
wrtpfwereade_: ping17:29
fwereade_wrtp, talk if you wish, but I'm in a meeting, expect slow replies17:30
wrtpfwereade_: k17:30
wrtpfwereade_: i think that ec2.environProvider.ComposeConfig could be quite a bit simpler17:31
wrtpfwereade_: and other bits too, but in general, i think it's really not such a train wreck17:33
fwereadewrtp, thanks, good to hear, I'm calling it a day now though :)18:30
wrtpfwereade: ok, have a good evening.18:31
wrtpfwereade: BTW Embassytown is awesome18:31

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!