[00:00] niemeyer, that one should be a trivial, it's the State.Relation(...Endpoint) -> State.EndpointsRelation(...Endpoint) + state.Relation(int) [00:00] niemeyer, that IIRC we agreed on a few days ago [00:00] fwereade: +1 [00:03] niemeyer: cool [00:05] niemeyer: % juju bootstrap --upload-tools [00:05] error: cannot start bootstrap instance: cannot make user data: invalid machine configuration: initial password is empty [00:05] submitting branch now [00:05] well, for review [00:06] niemeyer, how pleasing, a palindromic CL id: https://codereview.appspot.com/6601066 [00:07] fwereade: Wow, beautiful :) [00:07] davecheney: Hmm, ok.. curious about the cause [00:08] niemeyer: looking for it now [00:08] fwereade: Looking already [00:08] at least we know where the problem is now [00:08] niemeyer, great -- and https://codereview.appspot.com/6650043 shouldn't be too controversial either [00:08] fwereade: nicely straightforward [00:09] fwereade: Cool, looking [00:09] niemeyer: password := e.Config().AdminSecret() [00:09] if password != "" { [00:09] password = trivial.PasswordHash(password) [00:09] } [00:09] ec2/bootstrap [00:10] davecheney: Hmm.. that looks okay..? [00:10] AdminSecret() must be returning "" [00:10] * fwereade calmly resists the temptation to submit tonight, and really really goes off to bed, he can look forward to it in the morning :) [00:12] fwereade: Hmm.. one last question? :) [00:12] * fwereade hasn't really gone [00:12] * fwereade is sadly predictable :) [00:12] fwereade: ROTFL [00:12] niemeyer: ahh, found it [00:13] i was missing an admin-secret key for one of my environments [00:13] but the schema didn't complain [00:13] fwereade: Why is it not sending the plain slice it gets? [00:14] fwereade: It seems to be sending all relations it ever received at all times [00:14] niemeyer, it clears them out when it sends [00:14] niemeyer, f.relations = nil [00:14] niemeyer, then it collects a set of ids it's seen until someone receives again [00:15] fwereade: Ahh, I see [00:15] niemeyer, I didn;t want to just collect by appending because dupes would be icky [00:15] fwereade: I suggest keeping the form as a list of ids still [00:16] fwereade: But that's minor.. it looks pleasantly straightforward too [00:16] niemeyer: from ec2/config.go [00:16] "admin-secret": schema.String(), // Unused. Here just for compatibility. [00:16] fwereade: Have a good sleep! [00:16] ^ clearly, this isn't true [00:16] fwereade: and thanks [00:16] niemeyer, all I think I'll ever want to do is iterate over it [00:16] niemeyer, and it feels a bit icky to (potentially) merge into a list without dupes every change [00:16] davecheney: Indeed, but that shouldn't, I think, be affecting anything [00:17] niemeyer, what's the advantage of a list? [00:17] niemeyer: "admin-secret": schema.Omit, [00:17] fwereade: I'm willing to bet this is orders of magnitude faster and consumes less space in the real cases [00:17] means AccessSecret always retuns a value [00:17] and that value is "" [00:18] niemeyer, I guess this is indeed not really one of the N=100000 cases [00:18] fwereade: not only that, but real cases will generally see no appends at all, and allocating and handling a sequence of bytes is pretty friendly to the hardware [00:19] niemeyer, all true -- thanks :) [00:19] fwereade: np, and have a good night! :-) [00:19] niemeyer, consider it done... but tomorrow ;) [00:19] niemeyer, davecheney, gn [00:19] fwereade: gn [00:19] davecheney: Isn't that the "default" value? [00:19] yup, found it in a few places [00:20] which means a valid config can have a "" for the inital password [00:21] davecheney: It can, but that's not related to this logic [00:21] davecheney: and I thought you had said you didn't have an empty admin-secret? [00:21] niemeyer: that was my fault [00:22] i didn't read my environments.yaml closely enough [00:22] that key was missing from some envs [00:22] davecheney: Ah, okay, so without the admin secret indeed it will fail I suppose [00:22] just confirming now [00:22] ok, yes, with an admin-secret it is working [00:22] davecheney: We should prevent that, but ec2 is not the right place.. that should happen in environs/config/config.go [00:22] davecheney: Woohay! That's great news. [00:22] niemeyer: already fixing [00:23] niemeyer: and i've added double underpants in cloudinit [00:39] niemeyer: before I get too far into this [00:39] is this statement correct -> admin-secret is now a required configuration item ? [00:52] davecheney: I think so [00:52] davecheney: Assuming it all works well [00:52] niemeyer: ok, i'll propose this and we can discuss it with rog tonight [00:53] well, once I get all the tests passing [06:32] davecheney: morning! [06:34] morning [06:35] morning [06:44] davecheney: not yet time for dinner? ;) [06:44] TheMue: soon [06:45] still fighting with tests [06:45] https://codereview.appspot.com/6659048 [06:46] hmmmp, network === TheMue_ is now known as TheMue [06:50] rogpeppe: i've hit a roadblock with admin-secret [06:50] niemeyer is pretty sure it is now a required configuartion item [06:51] davecheney: oh dear. tell me about it [06:51] davecheney: fuck, yes it is [06:51] davecheney: alternatively i *could* change things so it would work without it [06:51] rogpeppe: https://bugs.launchpad.net/juju-core/+bug/1065750 [06:51] ^ long version [06:52] davecheney: is there some problem you're having even when you *do* specify admin-secret? [06:52] nope, it works then [06:52] but without something liek https://codereview.appspot.com/6659048 (unfinished) [06:52] davecheney: cool, i'll just make it mandatory and all problems will go away :-) [06:52] you can happily deploy a non working config [06:53] rogpeppe: yes, but if you make it manditory, then things like BootstrapConfig stop working [06:53] davecheney: yeah, it was my intention to make it mandatory - it slide off my todo list, i'm sorry. [06:53] davecheney: why's that? [06:53] s/slide/slid [06:53] // We never want to push admin-secret to the cloud. [06:53] delete(m, "admin-secret") [06:53] m["agent-version"] = tools.Number.String() [06:53] return config.New(m) [06:53] if m is lacking "admin-secret" [06:53] config.New() will fail [06:54] hmm [06:54] so it's mandatory at one level, but not at another [06:54] unpossible [06:54] i'd forgotten that wrinkle [06:55] i think probably juju.NewConn (?) should fail if there's no admin secret, [06:56] davecheney: but config should continue to work without it [06:56] well, my first effort was https://codereview.appspot.com/6659048/patch/2001/3003 [06:56] but that just spiraled out of control trying to make all the tests pass [06:56] davecheney: yeah, that won't work [06:56] davecheney: you've gotta stop the problem at source [06:58] davecheney: i'm happy to fix this problem if you haven't done so already. it's all my fault. [06:59] rogpeppe: have a crack, i'm done for hte day [06:59] the diff is just getting longer and longer [06:59] davecheney: ok, thanks for taking a look [06:59] davecheney: can you push your latest attempt? [07:00] rogpeppe: i just did [07:00] davecheney: cool, so https://codereview.appspot.com/6659048 is it? [07:01] y [07:32] rogpeppe, TheMue: I imagine, but would like to confirm, that none of you are implementing service or relation lifecycle stuff? [07:33] fwereade__: correct in my case at least [07:33] fwereade__: morning, BTW! [07:33] rogpeppe, cheers [07:33] rogpeppe, heyhey :) [07:33] fwereade__: yes, i'm doing firewaller stuff right now [07:33] TheMue, thanks [07:33] fwereade__, rogpeppe: morning [07:33] TheMue, (morning :)) [08:33] * fwereade__ has just spotted where he broke the uniter [08:45] rogpeppe, am I to understand that we can't deploy things atm for authy reasons? [08:45] fwereade__: no. [08:45] fwereade__: but you do need to add an admin-secret to your environ config [08:45] rogpeppe, ah, ok, I've always had one of them lying around anyway ;p [08:46] fwereade__: i'm just trying to work out where to put the checks to ensure that it's set [08:46] fwereade__: config.Config is unfortunately *not* the right place [08:47] * fwereade__ has always maintained that an input config and an internal config are not the same thing, and that the hassle of pretending they are is not wrth it [08:47] * rogpeppe tends to agree, but we have what we have now. [08:49] * fwereade__ wishes rogpeppe good luck [08:51] fwereade__: part of the problem is that it's quite hard to enable passwords for local tests... but maybe it's ok now since my "restart mongo on unauthorised-access-failure" hack [08:52] TheMue: hiya [08:52] rogpeppe: ;) [09:37] moin. [09:49] Aram, heyhey [09:50] TheMue, I'm a bit confused, would you confirm for me what security groups I should be seeing with a bootstrap + deploy mongodb? [10:04] fwereade__: today one named "juju-" and one per machine named "juju--". [10:05] TheMue, ok, cool -- but I am confused, because none of them seem to have any rules; and yet I can connect to state just fine: is it immediately apparent that I am being stupid? [10:07] fwereade__: the juju-group afaik has all ports open between each other and 22 to the world. the other group should have those ports worldwide open which are needed. [11:29] rogpeppe, can I get a trivial LGTM on https://codereview.appspot.com/6656051 [11:30] fwereade_: looking [11:30] yummy, had a good lunch, kraut [11:31] fwereade_: not *that* trivial :-) [11:31] rogpeppe, the code change really really is -- and, bother, I thought the test changes would be clear [11:32] fwereade_: the code change really is (although i'm not sure i see the implications - why didn't it crash before?); the test changes need a little thought though. [11:32] rogpeppe, it did, I think [11:32] rogpeppe, I believe this was what was hitting davecheny 2 nights ago [11:32] fwereade_: ah. have you got a test that failed before and now passes? [11:33] rogpeppe, yes; the expanded config-changed test [11:33] rogpeppe, in it, config-changed now records the output of config-get, and we verify that we see what we should [11:36] fwereade_: by "verify lack of hooks" do you mean that you're verifying lack of hook *executions* ? [11:36] rogpeppe, ha, yes [11:36] fwereade_: ok, that makes more sense [11:37] fwereade_: i'm still really enjoying your little test DSL BTW [11:37] rogpeppe, yeah, it makes me happy too :) [11:37] fwereade_: who needs LISP, eh? :-) [11:37] rogpeppe, haha :) [11:41] fwereade_: what does node.Update do? [11:41] fwereade_: scratch that [11:41] fwereade_: i just read the code, doh! [11:42] rogpeppe, ha, yeah, I find myself doing that :) [11:42] fwereade_: i mean i actually read the code for changeConfig.step properly and noticed the little "s" arg to Update... [11:42] rogpeppe, ha, yeah [11:43] fwereade_: so config-get never worked at all before? [11:43] fwereade_: anyway, LGTM [11:44] fwereade_: i'll leave it up to you to judge whether it's trivial enough to submit now [11:44] rogpeppe, it did until I did a half-baked (agreed) post-LGTM change the other day [11:45] fwereade_: ahhh [11:45] rogpeppe, since it fixes my own monstrous howler, I feel it is very important that it be sumbmitted asap :) [11:45] rogpeppe, and I'm preety sure the test changes are all positive [11:45] fwereade_: go for it [11:45] rogpeppe, cheers [11:46] fwereade_: a similarly almost-trivial CL: https://codereview.appspot.com/6601067 [11:48] rogpeppe, LGTM -- need to be off for lunch now though [12:10] lunch too [13:50] I need some information about the firewaller and machiner. [13:51] I need to understand better what to replace change.Remove with. [13:51] Dying, Dead, or (Dead OR Removed). [13:51] rogpeppe: fwereade_: TheMue ^ ^ [13:52] Aram: for which watcher? [13:52] rogpeppe: Machine(Principal)?UnitsWatcher. [13:53] I used Dead OR Remove, and tests pass. [13:53] but I am not sure if that's right or not. [13:53] actually I used Dead OR Remove OR Unassigned [13:53] Aram: i would prefer dead to keep it consistent with the lifecycle [13:53] Aram, AIUI that is what we need there, yes [13:54] Aram, but I had a vague feeling that we wanted to implement all the life-group watchers as sending lists of ids? [13:54] fwereade_: Dead OR Remove OR Unassigned seems about right [13:54] fwereade_: that's what I did, I reimplemented the units watcher to send list of ids. [13:54] rogpeppe, agree, I think, haven't 100% thought through unassigned [13:55] Aram, ah, excellent -- I thought the idea was that we should just be sending every life change, rather than filtering those at the watcher level? [13:56] fwereade_: I'm not sure I follow. you want the watcher to return map[Life][]string instead of []string? [13:56] Aram, not at all [13:57] Aram, above, I got the impression that you were not sending changes from alive to dying; was that wrong? [13:58] this is the behavior now: [13:58] / MachinePrincipalUnitsWatcher notifies about assignments and lifecycle [13:58] / changes for all principal units assigned to a machine. [13:58] / [13:58] / The first event emitted contains the unit names of all principal [13:58] / units currently assigned to the machine, irrespective of their life [13:58] / state. From then on, a new event is emitted whenever a unit is assigned [13:58] / to or unassigned from the machine, or the lifecycle of a unit that is [13:58] / currently assigned to the machine changes. [13:58] / [13:58] / After a unit is found to be Dead, no further event will include it. [13:58] [13:58] so the watcher fires both when life is changed, and when is a change in assignment, reglardless of lifecycle. [13:58] fwereade_: i'm having second thoughts about that earlier CL. here's an alternative: https://codereview.appspot.com/6601068/ [13:59] fwereade_: i'm seriously considering adding a NoBootstrap field to JujuConnSuite, but fear it might be condemned as crackful. [14:01] rogpeppe, not especially keen myself, I think I liked the earlier one more [14:01] * Aram loved $CDPATH [14:01] s [14:01] fwereade_: the problem is i need to do exactly the same thing in CmdSuite [14:02] fwereade_: and it seems a bit silly to duplicate *almost* all of JujuConnSuite's functionality in two places, when we actually want something only slightly less than it does [14:02] rogpeppe, hmm, good point -- I think I'd prefer a NoBootstrap field to what you have there [14:02] rogpeppe, but I can't predict niemeyer's response [14:02] fwereade_: ok, that's useful thanks [14:03] fwereade_: maybe i should just bite the bullet and do the duplication [14:03] rogpeppe, I suspect that will give yu the smoothest path through review [14:42] fwereade_: done: https://codereview.appspot.com/6660048 [14:55] holy cow, bazaar really doesn't have the option of reverting a commit. [14:55] you need to do some merge magic [14:57] Aram: bzr uncommit ? [14:58] rogpeppe: that only removes the last commits, and it really removes them, I don't want that, I want to undo an arbitrary change bu applying the reverse diff and commit the new undo operation. [14:58] Aram: yeah, you're out of luck there. [14:58] Aram: i've used patch for that before [15:00] you can do a merge . -r -X..-X+1 [15:01] Aram: i didn't know merge accepted revno ranges [15:06] rogpeppe: yeah, it worked just fine [15:45] rogpeppe: fwereade_: TheMue: PTAL at the firewaller and machiner changes: https://codereview.appspot.com/6625069 [15:46] firewaller and machiner tests pass with those changes and the new watcher. [15:52] Aram: in firewaller, when are units added after watcher changes? [15:53] Aram: ah, found, is ok [15:53] Aram: missed the line [16:08] Amazon doesn't like the diacritical marks in my name. [16:08] wtf. [16:08] I can't set a name if it's not ASCII [16:12] "We could not verify the address you have provided. If you are sure this is a correct address, please try again." [16:12] how is amazon veryfying my address and why? [16:12] fuck you. [16:12] already spent to much time on this. [16:12] this should not have taken more than 30 seconds. [16:19] fwereade_, Aram: trivial (code movement only): https://codereview.appspot.com/6651062/ [16:19] Aram: i'm sorry i'm not sure i'll have time to go through the watcher changes today. i'll try though. [16:24] rogpeppe: don't worry. [16:24] * Aram is done for today. [16:24] cheers everyone, have a great weekend. [16:30] fwereade_: can i get a quick LGTM on the above branch please? [16:31] niemeyer: ^ [16:31] https://codereview.appspot.com/6651062/ [16:31] rogpeppe: On holiday, and on a call [16:31] niemeyer: np at all [16:31] niemeyer: hope you're having a good day! [16:31] rogpeppe: Hello, btw :) [16:31] niemeyer: likewise [16:32] TheMue: ping [16:37] rogpeppe: pong [16:37] TheMue: any chance of a quick LGTM on the above CL? [16:37] TheMue: code movement only, no code changes [16:37] rogpeppe: already looking [16:38] rogpeppe: you got it ;) [16:38] TheMue: ta! [16:43] TheMue, fwereade_: here's another one if you fancy having a look: https://codereview.appspot.com/6653050/ [16:48] rogpeppe: thumbs up [16:48] TheMue: thanks [17:00] so, i'm off. have a nice weekend [17:22] me too, have a great weekend everyone! [17:24] mramm: I have a good chuckle when I'm in a phone meeting with Mark S. and then there's that message saying "The LEADER has left the conference!" and all goes silent at the end.. [17:24] rogpeppe: Have a good one [17:25] * niemeyer steps out for more family activities.. [17:26] Have a good weekend all [17:27] niemeyer: have a great weekend [17:27] niemeyer: I never saw it that way, but it is great. [17:59] would be a great prank to have IS change that message to 'Elvis has left the building'