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