[00:11] niemeyer: thanks for your review [00:11] davecheney: My pleasure [00:12] the reason the firewaller watches the environ is I imagined that would be how it woudl talk to the (unknown) firewalling service [00:12] as the Environ in this case represents ec2 [00:12] niemeyer: in the pythonic version, the provider managed openport/closeport [00:13] davecheney: Duh.. it is obvious indeed, sorry [00:14] niemeyer: well, the facility doesn't exist on environ, yet [00:14] davecheney: Which facility? [00:15] open port/close port [00:15] it's there in goamz [00:15] but there is no generic interface in environs [00:16] davecheney: Ah, I see, yep [00:17] davecheney: It kind of sucks that we'll have to redo this soon to be machine-based [00:17] davecheney: But it seems like the right thing to do, rather than get into a trip in unknown territory before we actually get things working at all [00:18] niemeyer: right o [00:18] niemeyer: i'm going to raise a bug on juju-core to define the open port / close port interface on environ [00:18] if that is ok [00:18] davecheney: E.g. if we moved onto the machine agent, we actually have to watch relations too, at the machine level, so that we alter the firewall allowing them to go in and out [00:19] davecheney: Sounds great [00:19] davecheney: Thanks [00:19] niemeyer: the reason it's a new type, not an addition to the Provisioner, was a hope that the service could be moved between agents [00:19] but i don't know how realisitic that will be [00:19] davecheney: I love the fact you splitted it out [00:20] davecheney: It was a bad decision on the original implementation [00:20] davecheney: Even if we can't move it as-is, we can move something, or we can even erase it by itself if nothing else [00:20] yup [00:25] davecheney: Hmm [00:25] davecheney: I don't think we should allow the environment name to change [00:26] davecheney: We that value internally in the environment itself [00:26] We use [00:26] niemeyer: ok, i'll do it another way [00:26] davecheney: E.g. security groups [00:26] it wasn't being set at all [00:26] davecheney: It feels slightly bad that we do this [00:26] davecheney: It would be better to have that as an external property of the environment [00:27] davecheney: But it's helpful to tell the user about what those machines he's running are about [00:27] niemeyer: the current thing that is blocking bootstrap is there are no tools in my s3 bucket [00:27] i can't see anything to generate those [00:27] davecheney: Oh [00:27] 012/06/27 10:27:45 JUJU findTools searching for {{0 0 0} precise amd64} in [] [00:27] error: cannot start bootstrap instance: cannot find juju tools that would work on the specified instance: no compatible tools found [00:28] davecheney: bootstrap -upload-tools? [00:28] davecheney: bootstrap --upload-tools? [00:28] davecheney: I think you're right, though [00:29] davecheney: We do need to set the environment name, at least once [00:29] 2012/06/27 10:28:50 JUJU environs: putting tools tools/juju-0.0.0-precise-amd64.tgz [00:29] niemeyer: i'll find a nicer way to do it [00:29] davecheney: I feel like your CL is a good step forward, actually [00:29] davecheney: I'm probably mixing problems up [00:29] davecheney: We should prevent the user from replacing the name, but that's not the way to do it [00:30] niemeyer: the name is an unfortunately property that comes out of the yaml file [00:31] davecheney: I'll file a bug, and LGTM your change [00:31] niemeyer: ta, just a small cosmetic i found [00:31] davecheney: This may well be what's preventing it from running [00:32] 2012/06/27 10:31:34 JUJU environs/ec2: started instance "i-acd0d3d5" [00:35] niemeyer: dude, it bootstrapped ! [00:35] davecheney: Woah! [00:35] niemeyer: i'm going to pull this branch apart into smaller pieces, but i think it's working [00:35] no idea if there is a PA at the other end yet [00:35] also, juju status would be nice to have :) [00:35] davecheney: +1 :) [00:36] niemeyer: time for a celebratory cup of tea [00:36] davecheney: Feel free to start looking at it actually, if you ever get blocked/bothered [00:36] s/bothered/bored/ [00:36] niemeyer: ok [00:36] niemeyer: i'm not going to step on franks toes ? [00:36] screw it, i'll just email juju dev and claim it [00:38] niemeyer: 2012/06/27 10:34:27 JUJU environs: putting tools tools/juju-0.0.0-precise-amd64.tgz [00:38] error: cannot upload tools: cannot write file "tools/juju-0.0.0-precise-amd64.tgz" to control bucket: Your socket connection to the server was not read from or written to within the timeout period. Idle connections will be closed. [00:38] ^ kinda unreliable, but that is australia for ya [00:38] maybe i'll try a different availability zone [00:38] davecheney: Definitely not stepping on Frank's.. he's not looking at status ATM [00:38] sorry, william [00:38] * davecheney facepalm [00:39] davecheney: That error was kind of weird [00:39] davecheney: I don't think anyone is doing status yet [00:39] davecheney: I've not seen that before.. worth investigating [00:40] niemeyer: anyway, time for a break [00:46] Woohay.. new battery arrived [00:46] It's time for me to leave too [00:46] I'm late, actually [00:46] Dinner with Go folks tonight.. back later [05:04] niemeyer: how was dinner ? [05:04] * davecheney is a tad jealous [05:05] davecheney: It was awesome indeed [05:05] niemeyer: i think i figured out why juju can't bootstrap in other avail zones [05:05] davecheney: Not the full team, unfortunately [05:05] niemeyer: i bet rsc wasn't there [05:06] he's elusive [05:06] davecheney: Yeah, we had Rob and Andrew [05:06] davecheney: He lives elsewhere.. I bet he'd join if he was around [05:06] he lives in boston right ? [05:11] niemeyer: can I commit rogers branch ? or should I merge it into one I own to lbox propose ? [05:12] davecheney: The right thing would be to branch from it and continue, or possibly even break it down in smaller chunks [05:12] davecheney: Yeah, he lives in Boston, or next to it I think [05:12] niemeyer: i'll try to break it into two chunks, it's only 100 lines [05:12] actually less than 90 [05:13] davecheney: Yeah, but it's also incomplete, right? [05:13] it's very close [05:13] [ 102.246904] init: juju-provision-agent main process (4356) terminated with status 127 [05:13] [ 102.246937] init: juju-provision-agent main process ended, respawning [05:13] ^ jujud isn't in the path, that is pretty much the only bit missing [05:14] davecheney: I had the impression it was a spike mostly.. rog mentioned it had no testing [05:14] davecheney: I confess to not have looked into it, though [05:15] he's done some tests, > 50% of the change is in livetests [05:22] davecheney: Ah, nice [05:22] davecheney: I wonder what he was referring to then [05:23] maybe it was the environ name problem [05:23] davecheney: Well, you're in a much better position than I am [05:23] since I fixed that it's been working pretty well for me [05:23] davecheney: Whatever you feel should go to review, I'll be happy to look at [05:23] ok [05:24] niemeyer: did my diagnosis of the s3 problem make sense ? [05:25] niemeyer: ubuntu@domU-12-31-39-0C-58-F1:~$ pgrep jujud -lf [05:25] 4539 /var/lib/juju/tools/juju-0.0.0-precise-amd64/jujud provisioning --zookeeper-servers localhost:2181 --log-file /var/log/juju/provision-agent.log [05:37] davecheney: I think it does.. it's definitely an error we'll want to fix [05:38] i think it should be pretty easy to add support for that [05:38] davecheney: My surprise was with the fact Amazon refuses to take a request on a region-specific endpoint because the payload doesn't mention the region-specific endpoint name *as well* [05:39] davecheney: Woah? Provisioner running? [05:39] yes, almost [05:39] i think i got initzk working as well (path issues) [05:39] but it's blocked waiting on the right value for /environment [05:43] niemeyer: [zk: localhost:2181(CONNECTED) 2] ls / [05:43] [services, charms, relations, zookeeper, initialized, machines, units] [05:43] initzk worked, just missing /environment [05:44] niemeyer: i don't understand why location constraint has to be xml, why can't it just be a header [05:44] davecheney: That's expected [05:45] davecheney: /environment comes from juju deploy [05:45] davecheney: Well.. it doesn't even need to be a header [05:45] davecheney: The endpoint URL itself is already a great indicator [05:45] davecheney: There's no way to be talking to that endpoint URL and not wanting to.. well.. talk to it [05:45] niemeyer: i thought there was [05:46] davecheney: Kind of strange.. either a very awkward decision, or I'm missing something more interesting [05:46] there is a note in one page I found that said there is a generic s3 location you can talk to [05:46] but they recommend not using it, because it will internally redirect your request [05:46] davecheney: Sure, but we're not using it, AFAIK [05:46] either way, it's a bit of a wart [05:47] niemeyer: so, juju deploy will be responsible for keeping /environment up to date ? [06:08] davecheney: Yeah [06:08] davecheney: Well, hopefully not up-to-date [06:08] davecheney: Only the first time [06:08] davecheney: THe Python version updates it every time, but that's a mistake [06:09] niemeyer: ok, so i should not make the bootstrap process seed /environment [06:11] davecheney: It's tricky because to do that we'd have to wait until the environment bootstrapped [06:11] davecheney: Chicken and egg [06:11] davecheney: We don't want to send credentials over via user-data [06:15] niemeyer: right, so to confirm, the cloud-init script is not secure ? [06:29] davecheney: Right, it puts thing up in an address that can be openly accessed from within the machine [06:40] niemeyer, davecheney: heyhey [06:40] fwereade: howdy [06:40] niemeyer, perhaps I'm missing something re: https://codereview.appspot.com/6335057/diff/3001/state/presence/presence.go#newcode397 [06:42] niemeyer, but it seems to me that we can only guarantee absence notifications from the watches if we don't stop the childLoops at all [06:42] bah [06:43] ok, I'm off to take laura to nursery [06:45] yeah, i'm off soon as well [06:54] davecheney: Yo [06:54] davecheney: Please feel free to tackle the location bug if you're up for it [06:54] davecheney: I'll certainly have a low attention level in the next few days with I/O going by [06:56] Time to head to bed now [06:56] davecheney: Have a good time there, and talk to you tomorrow [06:56] Night all [07:31] Morning. [07:33] TheMue, heyhey [10:45] TheMue, any objection to me just straight-up deleting relationUnitWatcher? after the meeting yesterday we have a better plan for doing essentially the same stuff [10:46] fwereade: No problem, so I'll just don't touch it when doing the Deleted/Removed change. [10:47] TheMue, I consider this trivial enough to go straight in with a cursory review; sound sane? [10:47] TheMue, I'll be proposing in a couple of mins... [10:47] fwereade: Does the mimic also may relate the ServiceUnitsWatcher? [10:47] TheMue, sorry, can't parse [10:48] fwereade: The way you will do it in future, will this be interesting for the service units watcher too? [10:50] TheMue, ah sorry; I don't *think* so [10:50] TheMue, I don't think that should be considering agent presence [10:50] TheMue, but I may not have thought it through properly [10:50] TheMue, should it? [10:50] fwereade: OK, I'll take a look when you proposed it. ;) [10:51] fwereade: No, it's only that both are interested in units, but in different contexts. [10:51] TheMue, yeah, this is specifically unit *relation* presence [10:51] fwereade: ic [10:51] TheMue, (the better plan is to use the forthcoming presence.ChildrenWatcher) [10:59] TheMue, https://codereview.appspot.com/6351046 [11:00] TheMue, should be trivial :) [11:01] fwereade: LGTM, those pure red ones are pretty simple. ;) [11:01] TheMue, cheers; agree ok to submit directly? :) [11:02] fwereade: If the removed watcher isn't yet used then it's ok. [11:02] TheMue, don't worry, I ran the tests :) [11:02] TheMue, submitting now [11:04] TheMue, btw, if you have another couple of moments [11:04] TheMue, it seems to me that the watcher use in cmd/jujud is a bit inconsistent/idiosyncratic [11:05] TheMue, I would appreciate your opinion on whether it's justified, or if we should clean it up [11:05] TheMue, by which I mainly mean moving stopWatcher and mustErr into state/watcher, and using them consistently [11:06] fwereade: i take a look [11:07] TheMue, cheers [11:07] fwereade: hmm, do you have a pointer for me? [11:09] TheMue, cmd/jujud/machine.go:93 [11:09] TheMue, I'm pretty sure that it's wrong to let a tomb.ErrStillRunning slip through [11:09] TheMue, and in line 80 we discard errors [11:10] TheMue, I haven't looked at provisioner properly but I'm pretty sure it suffers similar problems [11:12] TheMue, it may or may not be justifiable to rearrange NewMachiner such that it follows the usual model -- ie start the watcher before we start the loop [11:13] TheMue, IMO it's a cleaner way of doing things, and is anyway better in the context of juju because it's more like the model we use throughout state [11:13] fwereade: yes, this uncommon way may lead to misunderstandings [11:13] TheMue, cool, I'll have a go at that; we can see what niemeyer thinks when he;s around [11:14] fwereade: yes [11:14] fwereade: lunchtime [11:14] TheMue, cool, enjoy [11:41] hi. [11:47] Aram, heyhey [11:49] TheMue, it crosses my mind that *actually* the PA has it right wrt the watcher... there doesn't ever seem to be a good reason to create a subwatcher outside the loop [11:50] TheMue, or even to keep it in a field on a, er, superwatcher [11:51] re [11:51] Aram: Hi [11:52] fwereade: Maybe, it only puzzles me if something is different than in most/all other places. [11:52] TheMue, I can't see anywhere a watcher field is used outside a loop method [12:21] fwereade: Maybe it's my old OO as well as Erlang thinking that I do most initialization in a constructor and use the loop method only as loop. [12:22] TheMue, IMO a field that's used in only one method should not really be a field ;) [12:22] TheMue, anyway, I'm experimenting with what might be a noticeable simplification of the stuff in state/watcher.go, I'll let you know if it works out :) [12:23] fwereade: But then I would not call the method loop(). I often name them backend() in my projects. ;) [12:23] TheMue, I would personally gravitate towards run() :) [12:24] fwereade: That's the job of the whole program: come on, run, Forrest ... [12:24] TheMue, haha [12:46] TheMue, making this change has made some of the state watchers start to look a bit inconsistent to me: an awful lot of them seem to me to be at risk of sending "changes" that don't actually represent changes [12:47] TheMue, I rather consider that to be poor behaviour; opinion? [12:49] TheMue, basically all of them seem to do what FlagWatcher did before I suggested changing it yesterday [12:51] TheMue, excluding the Machine ones, that is [12:51] fwereade: Hmm, maybe they should be fixed then one-by-one in small changes. [12:51] TheMue, yeah, that sounds sensible [12:52] fwereade: We've done it this way for the error messages in state. [12:52] TheMue, sorry, expand please [12:52] TheMue, ah, how we make the changes [12:53] fwereade: +1 [12:53] fwereade: yep [12:53] fwereade: I think the should be fixed to work exactly as expeted. [12:53] TheMue, cool [12:54] TheMue, I need to think about what I've done a little more but it feels like a big win ATM [12:54] fwereade: great [12:54] TheMue, I'll see what niemeyer thinks before I start hanging more changes off it though ;) [12:55] fwereade: I'm just doing the last changes for the service unit watcher. niemeyer had a very good hint how to test it more elegant. [12:55] TheMue, yeah, I saw that [12:55] TheMue, sounds good [12:55] fwereade: it's so much clearer now. [12:55] TheMue, sweet -- that style does make my head hurt a bit sometimes :) [12:56] fwereade: it still uses test tables, but more simple ones [12:57] TheMue, perfect [12:57] fwereade: will now propose it and then start a new branch for the deleted to removed change [12:57] * fwereade worries that that change will be a real hassle to merge with his current work [12:58] TheMue, can you wait a few mins? I'll propose what I have -wip so you can (1) tell me if it's sane and (2) decide whether the conflicts will be worth the hassle [13:02] fwereade: Which change you mean, the first one or the deleted/removed one? [13:02] TheMue, added/removed [13:02] TheMue, (sorry, remind me what the first one is?) [13:04] fwereade: The fixes after the review of the service units watcher. [13:04] fwereade: They are proposed now. [13:04] TheMue, I'm happy to suck up the conflicts on that one myself [13:04] fwereade: The work on the deleted-to-be-named-removed will start now. [13:05] TheMue, but Added/Removed might hurt [13:05] fwereade: This is why I will do it in an extra branch. There are influences in more packages than just state. [13:06] TheMue, true [13:06] fwereade: But I'll wait for your proposal. [13:08] TheMue, https://codereview.appspot.com/6347045 [13:09] TheMue, the reason I worry slightly is that it totally guts state/watcher.go [13:10] TheMue, actually, looking at it, I doubt Added/Removed will hurt that much [13:10] TheMue, but I'd still appreciate your preliminary opinion [13:10] TheMue, the important bit is the changes to state/watcher.go, most of the rest can probably be a separate CL [13:11] fwereade: In the moment the field is renamed in watcher.go any user of this field has to be renamed too. [13:12] fwereade: So it may be better type by type instead file by file. [13:12] TheMue, you mean the Added/Removed change? [13:12] TheMue, my CL does not alter observable behaviour AFAICT [13:12] fwereade: Yes [13:12] TheMue, cool [13:14] fwereade: The other one - don't raise events for unwanted content changes - is a different topic and should be discussed with niemeyer [13:14] TheMue, yeah, I haven't even started on that [13:14] fwereade: Maybe you should file it first. [13:15] TheMue, hopefully I'll get a chance to see him and discuss it tonight [13:15] fwereade: So which change of mine do you think could conflict yours? [13:15] TheMue, I decided it probably wouldn't hurt too much in the end [13:15] TheMue, there will be conflicts but they'll be trivial [13:16] TheMue, feared they might be ugly before I looked at the actual diff [13:17] fwereade: ah, ok, understand [13:17] TheMue, anyway, does that change look worth it to you? [13:18] fwereade: i'm not yet through, one moment [13:20] TheMue, MW and MUW are not finished there [13:20] TheMue, you should be able to ignore them and still have a clear idea of what's involved [13:27] fwereade: Mostly LGTM, I only have troubles with the panic() in MustErr(). [13:27] TheMue, that was already there :) [13:28] fwereade: Maybe, but not by me. ;) [13:28] TheMue, the idea is that if a subwatcher's channel is closed while the watcher is reading from it, it indicates a fundamental logic error [13:28] fwereade: The question is: Is this situation simply an error itself and recoverable or do we really need to tear down the program? [13:29] TheMue, it means that we, as coders, are demonstrably on crack [13:29] TheMue, ie not recoverable IMO [13:29] fwereade: If it can't happen by any runtime situation then I'm fine with hit. [13:29] it [13:29] TheMue, that's the idea [13:33] fwereade: This is what I really like in Erlang. Here a supervisor would recognize this crash and start the needed tasks to recover the system (which sometimes is only the restart and registration of a coroutine (process) and sometimes may be the stopping and restarting of large process trees). [13:34] TheMue, but if the error is "the programmers are demonstrably failing basic logic" then I think it's better to fail hard AAP [13:34] fwereade: supervisors can also supervise other ones, so it cascades if one fails during restart [13:34] ASAP [13:35] fwereade: yes, indeed, if it is definitely not recoverable and a logical error [13:36] TheMue, if a watcher has closed its change channel while its owner is still making use of it, that is IMO evidence of fundamental crackfulness [13:37] aujuju: Security groups creation in EC2 [14:37] hey folks, does anyone know who's responsible for maintaining http://jujucharms.com/ ? [14:42] mthaddon, that's hazmat's [14:43] james_w: ah, thx