[00:14] davecheney: Quite possibly.. config needs love [00:17] niemeyer: basically it looks like the config-get command is returning null to the hook [00:17] not the default value [00:17] looking at the tests in cmd/jujuc/server/config-get_test.go [00:17] it looks like that is the cause [00:17] ie, none of the default options in the dummy charm are being reported by config-get [00:17] davecheney: It's state that needs to change, both to support multiple charms and to integrate default handling [00:17] davecheney: jujuc is merely reporting what it finds [00:18] riiiiiiiiiigh [00:18] but isn't that logic in the charm package ? [00:18] * davecheney raises an issue [00:19] davecheney: No, the charm package manages charm bundles, directories, etc, and reports the defaults in the charm itself correctly [00:19] davecheney: But state needs to incorporate those details [00:19] right [00:19] is there an issue reported for this already ? [00:22] https://bugs.launchpad.net/juju-core/+bug/1060537 [00:22] ^ will close if it is a duplicate [00:26] niemeyer: sorry to keep asking about this [00:26] but are you sure this is a problem in the state package [00:26] looking at state/charm.go, it imports charm, an includes a *charm.Config in the document [00:27] davecheney: Look at Service.Config [00:27] niemeyer: thank you [00:27] davecheney: np [00:28] niemeyer: ah, right [00:28] it just returns a *state.ConfigNode with no consideration for any defaults provided by the charm [00:30] niemeyer: cmd/juju/get.go:end [00:30] if this logic is correct, should I move it into state ? [00:31] davecheney: We should implement this together with the multi-config handling that we talked about in the sprint [00:31] davecheney: Since this will mean we'll have the charm at hand, among other things [00:32] you will have to refresh my memory [00:32] or, if others understand this better, i will move on to something else [00:33] davecheney: There's some coverage of the problem in the list IIRC [00:33] ok, will re-read the archive [00:34] davecheney: I thought you had talked to William during the sprint about this as well [00:34] yes, but it is not ringing any bells [00:34] possibly i have a different name for the problem [00:35] davecheney: Okay, there was some misscommunication then.. this is what I've been asking you about for the past couple of weeks whenever I ask "How is the config stuff going" :) [00:37] definitly a misscommunication [00:37] i was working on the user side juju get [00:37] davecheney: Cool, no worries [00:38] davecheney: The basic idea, as you can dig in the archives for background, is that when we upgrade, the configuration has to be upgraded as well [00:38] right, yes, https://lists.ubuntu.com/archives/juju-dev/2012-September/000195.html [00:38] davecheney: Because the new charm may have different options [00:38] incompatible charm upgrades [00:39] davecheney: Then, we have an issue: what do we do during the upgrade [00:40] davecheney: This isn't an atomic operation.. old charms cannot deal with new config [00:40] davecheney: new charms cannot deal with old config [00:40] davecheney: and they're both running [01:40] davechen1y: Makes sense? [01:42] niemeyer: yes, i think i understand the problem [01:42] but the solution looks compilcated [01:50] davechen1y: Yeah, it requires some consideration indeed [01:50] davechen1y: I suggest just leaving it aside for the moment then, and continuing the good progress [01:51] niemeyer: understood [01:52] davechen1y: It'll probably require more synchronization than we'll have available in the next couple of weeks due to the timezone differences, so it seems to make more sense for you to focus on tasks you're comfortable with the approach and direction [01:53] yes, i agree [01:53] i don't want to tackle that [01:53] you and william are in the best position to handle it [02:12] davechen1y: Alright, time for some sleep.. have a good day there [07:02] :135 [09:14] moin. [09:24] rogpeppe: https://codereview.appspot.com/6598052/ [09:24] LGTM [09:25] i've been using it all day, and no charms have barfed on it [09:25] davecheney: cool, thanks. i wanted an extra look because i made some significant changes after your previous review [09:25] davecheney: i was kinda hoping for a review from niemeyer, but i think i'll submit anyway [09:26] davecheney: i'm not too happy about the difference between -u=foo and --u=foo but that's GNU flags for ya [09:29] rogpeppe: what is the difference between -u=foo and --u=foo ? [09:29] maybe i'm a bsd zealot, but -u=foo isn't a valid syntax [09:29] davecheney: it is [09:29] although I realise that plan9/flag treat them as identical [09:29] davecheney: -u associates the value "=foo" with the u flag [09:29] ahhm yes [09:29] davecheney: --u associates the value "foo" [09:30] dont you also introduce things like [09:30] -flag=5 [09:30] which is -f -l -a -g=5 [09:30] davecheney: yeah, that works [09:30] and also [09:30] -uwang [09:31] davecheney: that works too [09:31] which could be -u -w -a -n -g, and also -u => wang [09:31] this shit is cazy [09:31] davecheney: yeah, you can't tell without looking at the flag [09:31] davecheney: it's a real pity that flag parsing is context-sensitive [09:31] davecheney: (that's true for go's flag syntax too, of course) [09:32] rogpeppe: if you are happy with what exists now, then i'd +1 for comitting it now so I can hammer on folks to go get -u their installs [09:33] we're at the point that if you squint, the mysql gem is usable [09:33] davecheney: FWIW plan 9's flag package didn't allow multi-letter flags at all... [09:33] davecheney: cool! [09:33] davecheney: ok, will submit [09:33] davecheney: gustavo can moan later if he wants [09:33] yeah, I very much disklike multi letter flags. [09:34] Aram: i'm not sure. [09:34] Aram: it depends how many flags you've got [09:35] Aram: i very much dislike unix-style commands that have many single-letter flags [09:35] Aram: of course, i've got used to ls by now :-) [09:35] rogpeppe: Aram it is what it is, whatever the charms expect, we have to support [09:35] I dislike commands that have too many flags as well. [09:35] davecheney: submitted [09:35] bam! [09:35] davecheney: of course. [09:36] Aram: but i share your feelings [09:36] python commands tend to be very verbose in their input [09:37] davecheney: did you get a t-shirt? [09:38] rogpeppe: i did indeed [09:38] a lovely green one [09:38] davecheney: me too. very tasteful. [09:38] i think you've seen niemeyer model it [09:38] w00t [09:39] davecheney: ah, you're right. i thought i'd seen it before. [09:39] coming to a google io near you [09:40] where do you get these tshirts? [09:43] Aram: i think adg sent them [09:43] a long time ago, back in march from memory, they asked for a shipping address [09:43] unless this was an unrelated act of generosity [09:43] send to whom? to an elite cabal? :) [09:44] Aram: i don't know who was on the list, i'll find out [09:46] heh, Microsoft released the source of the Z3 theorem prover under a license that can be characterized as: "We couldn't figure out how to make money with it, but if you do, give it to me." [09:48] Aram: rofl [09:48] the 'come in spinner' licence [11:06] Aram, fwereade: how important do you think it is that state.Initialize is idempotent? [11:06] rogpeppe, I think it's important but my only strong motivation is an obscure sense of aesthetics [11:06] Aram, fwereade: 'cos the changes i'm making will mean that the second time you call it, it will give an error [11:07] rogpeppe, that sounds fine too, at least it's explicit :) [11:07] fwereade: cool [11:07] rogpeppe: tell me more about this [11:07] we do have explicit tests that assert that this is ok [11:07] however I can't think of a real world case where this wouldn't be an error [11:07] davecheney: we're going to run mongodb in auth mode [11:07] davecheney: Initialize is responsible for setting up the first user (admin) [11:08] i think the 'it's ok to double init' logic was part of the 'wait for init' logic [11:08] which probably makes little sense [11:08] davecheney: subsequent connections will need to provide authorization [11:08] rogpeppe: right, so first in sets the key [11:08] davecheney: so if you call Initialize twice, the second call won't have the correct authorization info, so it'll fail [11:08] davecheney: yeah [11:08] after that it's password or GTFO [11:08] davecheney: yup [11:08] i think that is a reasonable restriction [11:08] +1 [11:08] davecheney: cool [11:09] urgh - i just ended up writing a test harness for ssh by hand [11:09] http://codereview.appspot.com/6601043/ [11:09] kind wish we had gocheck for go.crypto packages [11:09] /s/ssh/sshd [11:10] certainly {SetUp,TearDown}Test [11:13] davecheney: i don't really understand why sshtest is a package. it doesn't seem to export any symbols. [11:14] i want the func tests to be in another packaged [11:14] because, honestly, they are going to fail more often [11:14] because of various environmental issues [11:15] davecheney: won't they be run exactly as often because people will always do test ./... ? [11:16] it's mainly for the builders [11:16] you make a good point that it is uncommon to split tests across two packages [11:17] davecheney: it seems odd to me, tbh [11:17] rogpeppe: noted [11:18] davecheney: at the very least there should be a package comment that says "this package consists of tests only" [11:19] davecheney: and i think there's no point in having non-test files. they're just confusing. i looked for a while to try to find what the package was about. [11:19] rogpeppe: good call [11:19] i will make that change [11:22] rogpeppe: I don't care if state.Initialize is idempotent. I wish it would be, but I wouldn'd mind if it is not. [11:23] Aram: i don't see any situation in which we want to call Initialize twice on the same mongo (apart from if we've reset everything in tests) [12:55] Morning all! [12:59] niemeyer, heyhey [13:02] niemeyer: morning :D [13:03] fwereade, andrewsmedina: Heyas [13:07] niemeyer: yo! [13:08] niemeyer: new SetAdminPassword CL: https://codereview.appspot.com/6586070/ [13:08] rogpeppe: Heya [13:22] rogpeppe: Sent a review [13:22] niemeyer: thanks [13:23] rogpeppe: np [13:25] niemeyer: s.State.Service won't work as a check because s.State now has admin perms. but i guess if i did the SetAdminPassword in a different connection, that might work. [13:25] niemeyer: alternatively, perhaps i could produce a better error message from state.Open [13:25] niemeyer: that depends on the mgo errors though [13:27] rogpeppe: We can just try to talk to the database then.. I think s.Session will not be authenticated [13:28] niemeyer: yeah, that'll probably work. [13:28] rogpeppe: Otherwise, hmm [13:28] rogpeppe: It actually sounds like a good idea to have a sane user message in the open case [13:28] rogpeppe: Since that's really what people will see [13:29] niemeyer: that's what i thought. [13:29] niemeyer: it depends if there's a reproducible error code though [13:29] rogpeppe: We can do that without introducing new logic [13:29] niemeyer: yeah, i'd look at the Code in the error returned from the same operation [13:30] rogpeppe: Cool.. I don't recall what the code is, but hopefully there is a well defined on [13:30] e [13:30] niemeyer: i'll have a look. this page is not great: http://www.mongodb.org/display/DOCS/Error+Codes [13:32] rogpeppe: Easiest is to simply enable mgo debugging while running that test [13:32] rogpeppe: Or just print out the error, realy [13:32] really [13:32] niemeyer: i was gonna print the error %#v [13:44] niemeyer: done. https://codereview.appspot.com/6586070 [13:58] rogpepper: Sorry, forgot to ping back.. that's ready [13:59] Hmm.. getting spam from the Corinthia hotel in Lisbon.. how unfortunate [14:21] Aram: Watcher is so much better, thank you [14:21] niemeyer: thanks. [14:21] indeed it is [14:21] I'm doing the SUW now [14:22] and SRW afterwards [14:42] niemeyer: i think we might need two admin accounts [14:42] s/accounts/users/ [14:43] niemeyer: because there's a difficulty with our proposed method of sending the hash of the password - the remote side never gets to know the actual admin secret [14:44] rogpeppe: In a call, but will be with you in a moment [14:44] niemeyer: np [15:15] rogpeppe: SO [15:15] niemeyer: SO [15:15] :) [15:16] niemeyer: i have a possible solution [15:16] rogpeppe: I suspect we can handle that nicely by setting the password on the first connection, as we vaguely alluded to yesterday [15:16] niemeyer: once we've done that, how does the server side connect to the state? [15:16] niemeyer: (it doesn't know the admin secret) [15:17] rogpeppe: And it shouldn't, I believe [15:17] niemeyer: indeed [15:17] niemeyer: so the question remains [15:17] rogpeppe: That's the case we covered before we went into the admin password [15:19] niemeyer: to put it more clearly, perhaps: when the bootstrap machine reboots, what password does the PA use to reconnect to the state? [15:19] rogpeppe: The machine agent should use the machine password [15:20] niemeyer: ah, ok, so nothing on the bootstrap machine needs access to admin privileges [15:20] niemeyer: that makes sense. [15:21] rogpeppe: Yes, its password should give access to everything it needs [15:21] niemeyer: so... how would you feel about jujud bootstrap-state being responsible for starting the initial machine agent? [15:21] rogpeppe: Of course, nowadays that's all mostly silly since passwords give access to the whole database, but once we front the API with a server, we'll be able to cut down privileges [15:22] niemeyer: it's not *that* silly - the admin account does have more privileges than most [15:22] rogpeppe: I'd feel like that's unnecessary and redundant.. we have to handle the case of a machine getting its password anyway [15:22] niemeyer: yes, but someone has to create the password for the new machine [15:23] niemeyer: and i think that should probably be bootstrap-state [15:23] rogpeppe: Yes, that's a good task for bootstrap-state.. [15:23] rogpeppe: After all, it creates the machine in the first place [15:24] niemeyer: so if bootstrap-state is writing stuff specifically for the machine agent, it would seem reasonable for it to actually start the agent too [15:24] niemeyer: so we haven't got too much action-at-a-distance going on [15:26] rogpeppe: Don't see the leap [15:26] rogpeppe: bootstrap-state has to create a machine and set its password, because it's bootstrapping [15:26] rogpeppe: Every single time a machine is started, we need to run the machine agent.. that's not special [15:27] rogpeppe: No reason to put special code inside the bootstrap-state command to do things that are common [15:27] niemeyer: it has to do more than set its password - it has to write a file containing the machine agent's password to disk, i think [15:27] rogpeppe: I don't think so.. the machine agent itself can write its own file to disk [15:27] niemeyer: how does the machine agent find out its own password? [15:27] rogpeppe: Like it generally would anyway [15:28] rogpeppe: How will it find its own password when we're creating a machine that is not the bootstrap machine? [15:29] niemeyer: the PA writes a cloudinit that creates the file holding a random password [15:29] rogpeppe: Why are we creating a file? [15:30] niemeyer: because want the MA to be able to restart and find its key again [15:30] niemeyer: and we don't want the key in the upstart file [15:30] rogpeppe: The machine agent itself can create its own file [15:30] rogpeppe: Why not? [15:33] niemeyer: i guess it's no harm. i was thinking that we didn't want to have the client create the initial MA key, but perhaps it's ok. we're actually only vulnerable if we run units on the bootstrap machine, which we can't do until the MA has changed its password and started work. [15:33] rogpeppe: Exactly [15:33] niemeyer: so we pass the initial password as a flag to the MA, which is ignored if the key file already exists. [15:34] rogpeppe: and we need to hand off the password anyway in the general case, since otherwise the machine won [15:34] 't be able to talk back [15:34] rogpeppe: Yeah, --initial-password [15:34] rogpeppe: So explicitly something that won't be around for long [15:35] niemeyer: i'm thinking there's no point in doing the two-stage thing for unit agents, but what do you think? [15:36] rogpeppe: I think it wouldn't hurt to do the same mechanism, and may have relevant semantics in specific cases [15:36] niemeyer: ok [15:36] rogpeppe: For example, it means the principal unit loses access to the subordinate unit after it's started [15:36] niemeyer: interesting point, yes [15:37] niemeyer: although i'm not sure you lose access to a connection if its password is changed. [15:39] rogpeppe: This aspect will only be relevant once we have a frontend API instead of direct connection to the database.. right now the passwords will be interchangeable [15:42] niemeyer: here's a sketch of my understanding of this: http://paste.ubuntu.com/1258227/ [15:43] rogpeppe: On bootstrap-state, I suppose we should set the admin password first thing [15:44] rogpeppe: What's pbkdf and what's the benefit of using it? [15:46] niemeyer: http://go.pkgdoc.org/code.google.com/p/go.crypto/pbkdf2 [15:46] rogpeppe: 2) I think we only need one initial password to bootstrap-state and the machine agent [15:46] niemeyer: i'm not sure that works [15:47] niemeyer: the benefit of using pbkdf2 is that noone can crack the password even if they've got access to the cloudinit file [15:48] niemeyer: ha, yeah, maybe you're right about needing only one password [15:50] rogpeppe: You mean it'll be more expensive to crack, okay [15:50] niemeyer: yeah, like unfeasible expensive, i believe [15:51] rogpeppe: You're slowing down the cost count-times, specifically.. that's different from infeasible [15:52] niemeyer: yeah. but if the cost count times are slowed down enough (imagine 1 second per test) it becomes infeasible for the indefinite future. [15:52] niemeyer: well, ok 5 years :-) [15:53] rogpeppe: Cool, +1 [15:53] niemeyer: cool [15:54] niemeyer: this is my latest version of the sketch: http://paste.ubuntu.com/1258245/ [15:54] rogpeppe: Let's embed the function instead of importing the package [15:54] niemeyer: why not import the package? it's supported by the core Go team [15:55] rogpeppe: Because it means another external import and repository in exchange for 25 lines of code [15:56] niemeyer: ok. how about launchpad.net/juju-core/go.crypto/pbkdf2 ? [15:56] niemeyer: so the origin is obvious [15:56] niemeyer: and we can easily import other packages from go.crypto as necessary [15:57] rogpeppe: juju-core/thirdparty/pbkdf2? [15:57] rogpeppe: If we start to depend on more stuff from go.crypto, that'd justify using the external package itself [15:58] niemeyer: i'd prefer to keep go.crypto in the path (particular as pbkdf2 is such a non-obvious name) [15:58] niemeyer: but i don't feel too strongly [15:58] rogpeppe: There's content inside it indicating its origin [15:58] niemeyer: ok [15:59] rogpeppe: That also defines the pattern.. if have other similar needs, or depend on poorly maintained external packages, we just stick them in there [15:59] if we [16:00] niemeyer: how about we put a comment at the top of the file // Original at code.google.com/p/go.crypto/pbkdf2 [16:00] ? [16:00] rogpeppe: +1 [16:01] niemeyer: so, does my final version of the sketch LGTY? [16:01] rogpeppe: Sorry, haven't seen it.. [16:01] * niemeyer looks [16:01] rogpeppe: On bootstrap-state, I suppose we should set the admin password first thing [16:01] rogpeppe: ? [16:02] niemeyer: i think it's good to set the admin password as the last thing actually [16:02] niemeyer: so that people can't connect until we've set up all the state we want to [16:03] niemeyer: it gives us an atomic initialize again, without needing to try [16:03] rogpeppe: That's why I think we should set it first.. ? [16:03] rogpeppe: Ah, no, I see.. [16:04] rogpeppe: Sounds good [16:04] niemeyer: cool. [16:04] niemeyer: anything else? [16:05] rogpeppe: Yeah, we'll need to retry on unauth, I suppose [16:06] niemeyer: i don't think you can connect at all until SetAdminPassword is called [16:06] rogpeppe: Have you tested that? [16:06] niemeyer: yeah, although not formally [16:07] rogpeppe: Formally!? :-) [16:07] niemeyer: i didn't write a test for it [16:07] rogpeppe: Have you run mongo with auth and tried to connect? [16:07] niemeyer: yes [16:07] rogpeppe: Okay [16:07] rogpeppe: What happens? [16:07] niemeyer: i got connection refused forever [16:08] niemeyer: well, for 10 minutes until the connect timed out [16:08] niemeyer: i was wondering about the other way around actually: does mongodb connect fail immediately if it gets an unauth error? [16:09] rogpeppe: Interesting.. just did a trivial test and it connects fine.. [16:09] niemeyer: we don't want the first connection to take 10 minutes because it's constantly retrying after it's been told it's unauthorized [16:09] niemeyer: from another machine? [16:09] rogpeppe: and it says it's unauthorized [16:10] "$err" : "unauthorized db:test ns:test.test lock type:0 client:192.168.157.104", [16:10] niemeyer: that *was* from a different machine, right? [16:10] niemeyer: maybe my diagnosis of the failure before was wrong. [16:11] rogpeppe: If I connected to the local address, it would work, no? [16:11] niemeyer: the auth access is based on your source address, i believe [16:12] niemeyer: but, yeah, i'd expect it to fail if you connected to 192.168... rather than 127.1 [16:13] rogpeppe: It doesn't fail, and you get an unauthorized [16:13] niemeyer: hmm, i wonder what was going on in my failed test then [16:14] rogpeppe: Were you testing on EC2? [16:14] niemeyer: yes. [16:14] niemeyer: i was just running the usual ec2 live tests [16:14] rogpeppe: Did you open the security group port? [16:15] niemeyer: i removed the --auth flag from environs/cloudinit and the live tests worked fine [16:16] rogpeppe: Sorry, I don't see how that's related to the issue [16:16] rogpeppe: live tests work fine if you remove --auth, because they currently work fine.. (?) [16:17] niemeyer: sure. but adding --auth made the tests hang for 10 minutes trying to connect [16:17] niemeyer: it didn't fail immediately with an unauth error [16:17] rogpeppe: If you want to test the behavior of --auth, just fire a mongodb and try to connect to it.. inferring behavior because live tests broke anyhow isn't a good way to verify behavior [16:18] niemeyer: true nuff [16:18] niemeyer: i'll bootstrap an instance and play with it [16:19] rogpeppe: I can test this as easily as running mongod, and trying to connect to the machine ip [16:19] rogpeppe: (the machine ip, no 127.0.0.1) [16:19] niemeyer: i've only got one machine :-) [16:19] not [16:19] rogpeppe: This test only needs a single machine [16:20] niemeyer: actually it occurs to me that the external connection will be connecting from 127.1 to 127.1 anyway, because we're using ssh port forwarding [16:25] rogpeppe: Besides that, looks the scheme looks good [16:26] rogpeppe: Still wanna talk about the location for the password, but that's a trivial for later [16:26] niemeyer: ok, cool [16:26] rogpeppe: Thanks for the write up.. nice to debate like this [16:26] niemeyer: np. i needed it to get the pieces straight in my head. [16:27] I'm heading to lunch, biab [17:00] * niemeyer respawns [17:48] Aram: ping [17:49] niemeyer: pong [17:49] Aram: Yo [17:49] hi [17:50] Aram: Just sent one last round.. please let me know if it sounds reasonable [17:50] ok, reading now [17:51] Aram: Just sent a last note regarding the copying.. we don't need to touch the database as I suggested before [17:51] niemeyer: the old SetPassword CL is back, with better tests. i have confidence that it's really working now :-) https://codereview.appspot.com/6587060 [17:54] i'm off for the night. see y'all tomorrow! [17:56] rogpeppe: Thanks, and have a good evening! [18:05] rogpeppe: Awesome tests [18:38] niemeyer: changed the watcher. [18:38] I'm off now [18:38] Aram: Have a good evening [18:39] thanks [18:41] niemeyer: oops, I fucked up something [18:41] Aram: Ah, Alive I suppose [18:41] yeah. [18:41] that breaks two tests [18:41] hmm [18:41] Aram: Yeah, the original was good [18:42] I'll revert it [18:42] Aram: LGTM with that reverted and assuming all the tests pass [18:42] thanks [18:42] Aram: Thanks! [19:11] fwereade_: ping [19:44] niemeyer: ping [19:45] fss: Yo [19:46] niemeyer: I'm adding support for some IAM actions on goamz, would you be interested in merging it? :) [19:47] fss: Definitely [19:50] niemeyer: cool. I will split the changes that I've already made and send them to you in parts [19:50] fss: Brilliant, thank you [20:03] niemeyer, pong [20:09] fwereade_: Heya [20:09] niemeyer, heyhey [20:09] fwereade_: Ended up just sending a note in the review so you can process at a better time [20:10] niemeyer, lovely, cheers, bit distracted tonight :) [20:11] fwereade_: Nice :) [20:39] OOPS: 102 passed, 2 FAILED, 3 FIXTURE-PANICKED, 26 MISSED [20:39] --- FAIL: TestPackage (22.84 seconds) [20:39] FAIL [20:39] FAIL launchpad.net/juju-core/state 22.885s [20:39] niemeyer, is this known? ^^ [20:40] niemeyer, the first panic is "need to login" [20:43] niemeyer, that is on trunk, and matches what I get on my branch; my changes merge cleanly with whatever broke, AFAICT, so I don't think I'm making anything worse by submitting :) [20:44] fwereade_: Oh nos [20:44] fwereade_: Hold on [20:45]