=== philballew_ is now known as philballew [06:51] fwereade: mornin' [06:51] hi rog(?) ;p === Guest7958 is now known as rogpeppe [06:54] fwereade: not sure why it does that. surely noone else is using "rogpeppe"... [06:54] rogpeppe, I *think* I saw your name change spontaneously at some point yesterday [06:54] rogpeppe, I have a couple of CLs up, and I would appreciate your thoughts [06:55] fwereade: i'm having a look right now [06:55] rogpeppe, sweet, tyvm [06:55] fwereade: i saw this: [06:55] [17:25:19] [Notice] -NickServ- rog__!~rog@ool-45754a58.dyn.optonline.net has released your nickname. [06:55] [17:25:20] *** You are now known as Guest7958. [06:55] rogpeppe, omg, it's that nickserv dude again! he's almost as bad as peer! [06:56] (sorry) [07:08] morning [07:37] heya TheMue [07:38] hey, hey, fwereade [07:38] ;) [08:04] TheMue: hiya [08:04] hi roger [08:35] fwereade: ping [08:36] rogpeppe, pong [08:36] fwereade: i'm looking at the filter code, and had a little question [08:36] rogpeppe, go for it [08:37] fwereade: why only receive on the want* channels when there's an event to send? [08:37] fwereade: would it be a problem to always wait on those channels? [08:38] rogpeppe, because receiving on the want channels will unblock the sending channel and we don't want them to send nils [08:38] rogpeppe, and we can't guarantee they won't until we've received a change [08:38] fwereade: but aren't the sending channels themselves nil until an event arrives? [08:38] rogpeppe, happily, all the watchers guarantee us initial events, so it can't be blocked for a long time [08:39] rogpeppe, the action of a receive on a want channel is to un-nil the sending channel [08:40] fwereade: is that necessary? [08:40] fwereade: erm, yes, i see [08:40] rogpeppe, in the context of that code, the condition of wanting to send on an out chan is defined by it being non-nil [08:41] rogpeppe, it means that want* calls can all block for a short time until the filter is ready to deal with them, but the filter should be guaranteed to either become ready or fail, assuming the watchers themselves work as advertised [08:42] rogpeppe, and the failing will itself unblock the want calls [08:43] rogpeppe, and hence, all is sunshine and puppies, I think [08:43] fwereade: i'm definitely starting to see puppies in the sun [08:44] * fwereade feels cheered [08:49] fwereade: one other thing: i'm wondering if it's possible to make the dying status propagate without another step through the watcher [08:50] rogpeppe, I thought about that and decided that having One True Path for the information was the clearest thing I could do [08:50] rogpeppe, it would not be a problem to check-and-set the local life var, and close the channel immediately though [08:51] fwereade: yeah, i was wondering if there was an elegant way to do that [08:51] rogpeppe, it just feels more complex and prone to embarrassing screwup ;) [08:51] fwereade: i do know what you mean [08:51] fwereade: i guess it's just my premature-optimisation head saying "5 more seconds! 5 more seconds!" [08:51] rogpeppe, well, life could actually be a filter field and we could have a separate hey-the-unit-is-dying method [08:52] rogpeppe, it's a reasonable concern, any number of crazy operations could be kicked off in 5 seconds [08:52] fwereade: i was mainly thinking about responsiveness, but that too, yeah [08:52] rogpeppe, if the life-field-plus-method SGTY I can do that, it does feel more ruthlessly efficient in a pleasant sort of way [08:53] rogpeppe, but then... I dunno [08:54] rogpeppe, in terms of sheer simplicity of implementation it's a mere Sync away... [08:55] fwereade: true. i seem to remember it being considered bad practice to call Sync in a watcher though. [08:55] fwereade: but i definitely see the attraction [08:56] rogpeppe, leave some sort of comment for discussion, I imagine niemeyer will have input too ;) [08:56] fwereade: i'm sure of it :-) [08:57] rogpeppe, crack-check on another idea [08:58] rogpeppe, move most of cmd/jujuc/server -- ie the commands, HookContext, and associated types -- into worker/uniter/hook [08:58] fwereade: +1 [08:59] fwereade: it's part of the worker after all, right? [08:59] rogpeppe, I'm quite certain I will do more with the idea but I feel I can't start until I've moved them and absoerbed the impact [08:59] rogpeppe, I have an intuition the the commands themselves may want to move somewhere else [08:59] rogpeppe, but yeah [09:00] fwereade: let me just have a quick look at cmd/jujuc/server again [09:01] fwereade: this is where i come up against the "what things are actually intended to be exported?" thing [09:02] fwereade: looking at http://go.pkgdoc.org/launchpad.net/juju-core/cmd/jujuc/server - which functions are actually part of the package API and which are just there for testing purposes? [09:02] rogpeppe, some of RelationContext's methods may not technically need to be exported [09:02] rogpeppe, and if you want to get technical about it we *could* make all the commands internal-only, but I think that actually ossifies the package [09:03] fwereade: yeah, i was wondering about NewClosePortCommand and friends [09:03] fwereade: i was trying to work out when we'd need them [09:03] rogpeppe, ATM the only client is NewCommand or whatever it is [09:03] fwereade: i'm not sure what you mean by "ossifies" actually [09:04] rogpeppe, adds to the burden of moving parts of it as it becomes more apparent that they're different packages [09:04] fwereade: ok, so everything else other than NewCommand could be private? [09:04] rogpeppe, and HookContext and most if not all its methods [09:04] fwereade: i don't think that's a substantial problem - renaming is easy [09:05] rogpeppe, and RelationContext and several of its methods [09:06] fwereade: this is why i think we should have a more rigorous approach to private/public naming. the important parts of the API are obscured by all the bits that could be private. [09:10] fwereade: but regardless of that, i think it would sit well under worker/uniter [09:11] moin. [09:16] rogpeppe, sorry, got a phone call [09:16] fwereade: np [09:17] fwereade: you've got a filter review [09:17] rogpeppe, tyvm [09:21] fwereade: what does it mean for a relation to be "broken" rather than "departed" (looking at the comment "TODO don't stop until all relations broken.") [09:21] ? [09:21] rogpeppe, has executed the -broken hook (and left the scope) [09:21] rogpeppe, ie when that relation will never again be touched by that unit [09:22] fwereade: ah, so "broken" for a relation is a kinda edge-triggered thing, rather than a steady-state broken like the unit broken thing? [09:22] rogpeppe, yeah, it's an awful name [09:22] fwereade: it certainly confused me :-) [09:23] rogpeppe, I'm not sure there's anyone it doesn't [09:24] fwereade: so the "broken" in this context is like a connection breaking - it doesn't imply anything going wrong at all? [09:24] rogpeppe, yeah, exactly [09:24] jeeze [09:24] fwereade: that's something the docs are going to need to be very careful about... [09:28] fwereade: just looking at modeContext - is there ever a mode that would *not* want to use modeContext? [09:28] davecheney: hiya [09:29] rogpeppe, basicaly no [09:29] fwereade: why not factor it out into the loop that's calling the modes then? [09:29] rogpeppe: hey [09:30] rogpeppe, because the bits that need to know the name (which include the error annotation) can't be used there, because you can't turn a Mode into a name [09:30] fwereade: hmm, the mode name might be a problem [09:39] fwereade: you could always do something like this, i suppose: http://paste.ubuntu.com/1253570/ [09:41] fwereade: i do feel that modeContext feels a bit like a hack in the midst of nice clean code. [09:41] rogpeppe, I feel that modeContext enables a fair amount of that code's cleanliness [09:42] fwereade: if you could get the name from a mode, then you could still have that cleanliness without modeContext AFAICS [09:42] rogpeppe, although, hmm, actually the non-name bits would probably be better if they were stuck in the main loop [09:42] fwereade: i think so [09:43] rogpeppe, sgtm [09:49] woo hoo, just got a 50 pound royalty payment. [09:55] rogpeppe, nice! [09:55] rogpeppe, what for? [09:55] fwereade: a tune i wrote some time ago [09:55] fwereade: someone put it on their album [09:56] rogpeppe, awesome :D [09:56] rogpeppe: +1 [09:57] fwereade: it'll at least pay for my next bow re-hair... [09:57] fwereade: (which is long overdue as it happens) [09:57] rogpeppe, excellent :) [09:57] rogpeppe: where can we d/l your first album? ;) [09:58] TheMue: when i record it. that's predicted to be in NaN years. [09:59] rogpeppe: ok, i plan to get old, but not so old.:( [10:00] rogpeppe: different topic. anything special to know when doing ec2 tests (beside setting the environment with the keys)? [10:00] TheMue: you should set the test timeout, because it takes a while [10:01] TheMue: here's the script i use (i call it "livetest"): [10:01] go test -amazon -test.timeout 2h -gocheck.vv $* >[2=1] | timestamp [10:01] for bash, it would be: [10:01] go test -amazon -test.timeout 2h -gocheck.vv "$@" 2>&1 | timestamp [10:02] rogpeppe: great, thx [10:02] i find the timestamp bit very useful, as i can glance over at the running test and see if it's hanging up unreasonably or progressing normally [10:58] rogpeppe: if i do a "go test" in environ/ec2 and get an error that "juju-sample" is not found, what did i wrong? [11:06] TheMue: hmm, good question, i'll have a look [11:07] TheMue: could you paste the output of go test -gocheck.vv ? [11:07] rogpeppe: sure, one moment [11:09] rogpeppe: http://paste.ubuntu.com/1253687/ [11:10] TheMue: is this running live tests? on trunk? [11:10] rogpeppe: it's a fresh branch of trunk, just to get sure it's non of my changes [11:17] TheMue: this happen even with you're not running with the -amazon flag? [11:17] s/happen/happens/ [11:18] s/with/when/ :-) [11:18] rogpeppe: yes, just a play go test [11:19] rogpeppe: ah, tried it now with -amazon, looks different [11:19] TheMue: i'm wondering if you've got an old copy of the goamz package [11:20] rogpeppe: i'll force an update, to get sure [11:20] TheMue: my version of goamz is on revision 13 [11:21] TheMue: and trunk environs/ec2 passes all tests for me [11:21] TheMue: (N.B. go get -u doesn't work with bzr repositories) [11:22] rogpeppe: uuh, i've got the rev 11 [11:22] rogpeppe: thx, i'll update now and test again [11:22] TheMue: that should fix your problem [11:22] rogpeppe: what GUI tool do you use for diff? [11:22] Aram: qbzr [11:23] Aram: it's not as good as codereview, but much more immediate :-) [11:23] can it help with merges? [11:23] Aram: good morning, BTW! [11:23] (as in, is it a three way diff tool?) [11:23] moin. [11:23] Aram: no. there is a tool around that does, but i can never remember its name [11:24] I'm fine with raw diffs when I need a diff, but merge conflicts really confuse me. [11:24] so I'm searching for a better tool. [11:25] rogpeppe: thx a lot, you've been right, now the rev 13 runs fine [11:25] TheMue: cool [11:25] Aram: moin from here too [11:25] hi. [11:34] fwereade: stupid bzr question: if i've got a full revision id, how can i make a branch from it? [11:34] rogpeppe, sorry, i have no idea [11:34] fwereade: thanks [11:35] ah! done it! [11:57] mramm: morning! [11:58] morning [11:58] How goes? [11:58] mramm: hiya [11:58] hi. [11:58] mramm: pretty good, just about to propose the final piece of the upgrade logic (the --bump-version flag on upgrade-juju) [12:00] awesome [12:01] meh, I'm using p4merge, all these open alternatives suck. [12:02] Aram: i usually find that apart from very occasional cases, doing it inline is fine [12:02] fwereade: relatively trivial: https://codereview.appspot.com/6593053 [12:03] rogpeppe: my brain is simply not adapted for such a task. [12:03] so I have to do it three times before it's right. [12:04] rogpeppe, sorry, just having lunch, I'll do it when I come back [12:04] Aram: i found it hard to start with, but then worked out a technique that seems to work [12:05] Aram: i edit the "this" parts by copying and pasting from the "source" parts. [12:05] Aram: then i delete the source parts [12:05] fwereade: np, enjoy! [12:42] Good morning juju masters! [12:43] niemeyer: heya [12:56] Hmmm.. why am I getting a BADSIG on us.achive [12:56] archive [12:57] niemeyer: time for a short question? [12:59] TheMue: Sure [12:59] niemeyer: fine, thx [13:00] niemeyer: when set-up a new machine we set the juju-group and a machine-individual group [13:00] niemeyer: in ec2 [13:01] TheMue: Hmm.. do we? [13:02] niemeyer: i hope i describe it right, try to make it better understandable [13:03] niemeyer: there's a setUpGroup() for a machine-id. and there a group with the group name ("juju-xyz") and a machine group ("juju-xyz-(machineid)") is set-up [13:04] niemeyer: now i removed the machine group, so all machines share only one group and the ports are opened/closed on it per instance [13:04] TheMue: Ah, okay, you're talking about *security* groups in EC2.. [13:04] niemeyer: sorry, yes, missed to tell more about the context [13:06] TheMue: Ok [13:06] niemeyer: so now the question: shell the IP permissions for the juju-group stay the same (ssh for all plus all ports inside the group)? [13:06] shall [13:08] TheMue: Yeah, sounds ok [13:08] TheMue: Btw, [13:08] niemeyer: ok [13:08] niemeyer: yes? [13:09] TheMue: I'm wondering if we should have that as an option rather than the default behavior [13:09] TheMue: If we keep the firewaller behaving as-is, would it be feasible to have this behavior switchable within the env only? [13:09] niemeyer: would be fine to me as i prefer to start with as few open ports as possible [13:13] niemeyer: let's make it in two CLs. i start with the current default behavior for the juju-group and then, after your lgtm, look how we can make this switchable. ok? [13:13] TheMue: Hmmm.. you mean breaking what's working so that we can then see how to fix it? [13:15] niemeyer: it seems i did not get your idea right. [13:15] niemeyer: could you please tell me what you mean by "switchable"? [13:15] TheMue: I mean being able to select between current behavior and a new behavior [13:16] niemeyer: oh, understand (hopefully). [13:17] niemeyer: i reduced it to the default ports, but you talk about the groups, don't you? [13:17] TheMue: I'm not sure about what "reducing to the default ports" means [13:17] TheMue: And yes, I think we've both been talking about security groups [13:18] niemeyer: I thought about which ports to be open by default. [13:18] TheMue: I don't think we want to change that [13:18] TheMue: Do we? [13:19] niemeyer: no, but after your first sentence "i'm wondering …" i thought so. [13:20] TheMue: Ah, ok.. no, I'm talking about the whole change [13:20] niemeyer: yes, now i got it. sorry i got it wrong the first place. [13:20] TheMue: It's all good [13:21] niemeyer: the firewaller relies on the interface for opening and closing ports. so the change imho can be done without changing the firewaller. [13:22] niemeyer: but the current implementation opens and closes on the machine group [13:22] niemeyer: and in future on the juju group [13:22] niemeyer: so i think this can be done configurable [13:23] TheMue: Right.. so the first CL would be adding such a setting, without any further changes [13:25] niemeyer: ok, and i keep my current branch as a reference for the follow-up ;) [13:25] niemeyer: another question about security groups [13:25] TheMue: Yeah, nothing is lost probably [13:26] niemeyer: if i change a ip permission for the juju group, this does not effect all instances, doesn't it? [13:28] TheMue: If you change a security group, it affects all instances to which the security group is attached [13:29] niemeyer: oh, so instead of a machine security group a service based security group could make more sense, or do i get it wrong? [13:30] TheMue: Security groups can only be attached to a machine when the machine is created [13:32] Sh…, disconnected [13:33] TheMue: Security groups can only be attached to a machine when the machine is created === TheMue_ is now known as TheMue [13:36] niemeyer: Thx, that has been the missing link to the chosen model. Just reading a lot about it to get a better understanding. [13:38] TheMue: np [13:46] TheMue, niemeyer: do you think it still makes sense to have two security groups? i'm thinking that we can make do with only one now. [13:47] niemeyer: --bump-version is back, BTW: https://codereview.appspot.com/6593053/ [13:47] rogpeppe: It depends a bit on the implementation [13:47] rogpeppe: Would be best to have one, but given that it is switchable, I don't know what will end up being easiest [13:47] TheMue: ^ [13:47] rogpeppe: Cheers [13:48] niemeyer: ah, i see. [13:48] niemeyer: yeah, it depends. if you want it switchable at runtime, there's no other option. [13:51] rogpeppe: they need something like gosecuritygroups, more lightweighted than real groups. ;) [13:51] TheMue: all they need is to be able to add security groups after launch [13:51] s/add sec/modify sec/ [13:52] rogpeppe: yes, that would help a lot [13:52] rogpeppe: I'm not considering much runtime at this point [13:53] rogpeppe, TheMue: I'm hoping we can keep the core issue in mind and have that as a trivial yet nice addition soon [13:54] niemeyer: it's interesting actually - we'd need to store the current mode in the environment somewhere - something we don't do anywhere. maybe an additional field in the bucket. [13:54] rogpeppe: We don't need to do anything we're not doing currently. Just a trivial setting that defines the intended behavior [13:54] niemeyer: i mean, we'd need to do that regardless of switching at environment-creation-time or run-time. [13:55] niemeyer: StartInstance needs to know whether it needs to create a new security group or not. [13:56] rogpeppe: StartInstance is in the environment, that has the setting to define its behavior [13:57] niemeyer: yes, assuming noone calls StartInstance from the client side, where the setting might be different from the one that the environment was bootstrapped with. [13:58] rogpeppe: If we randomly use different settings client side and server side, we surely will have many issues [13:59] niemeyer: ok. i actually thought it would work pretty well currently with just the bare minimum of settings on client's side (after bootstrap of course) [14:00] rogpeppe: default-series? [14:01] niemeyer: interesting. tbh i have to say i'm not entirely clear on where and how default-series is used. [14:02] rogpeppe: Okay, so before we dive away, TheMue issue is trivial one as it stands. We must consistently use the environment settings stored in the environment no matter what. [14:02] niemeyer: +1 [14:02] niemeyer: +1 [14:05] lunchtime [14:05] niemeyer: FWIW in the only occurrence of default series being used that i can find, it's taken from the state env config, not the local config. i imagine there are other places where it will come in though. [14:06] TheMue: Enjoy! [14:06] rogpeppe: That seems to agree with what was just stated above [14:08] niemeyer: i probably misunderstood you, sorry. i thought you were saying that things could be mucked up by a client using different env config settings from what we bootstrapped with. i don't *think* we can, currently. [14:09] rogpeppe: They cannot because we're doing exactly what I suggested, which is consistently using the environment settings stored in the environment. [14:09] rogpeppe: This is not a coincidence. [14:13] niemeyer: ah, i see. someone *could* still muck things up by calling StartInstance directly, but none of our stuff will do that. [14:14] rogpeppe: Of course, someone can screw up things arbitrarily by doing arbitrary things.. [14:15] rogpeppe: We give people all the tools to pull an environment configuration from a mailing list and running wild.. [14:16] rogpeppe: Just sent a question to the CL [14:19] niemeyer: i'm not sure what "interacting properly with the --version option" would be [14:20] niemeyer: currently the --version flag does not influence which version is uploaded [14:20] niemeyer: it seems like you might be suggesting that it should [14:23] niemeyer: tbh, i'm thinking that the tool upload functionality might be best off as an entirely separate subcommand [14:23] rogpeppe: Hmm [14:24] rogpeppe: Yeah, you're right [14:24] rogpeppe: --version is about selecting which version to pick [14:24] and if we're uploading we don't really have a choice [14:24] rogpeppe: We should conflict these options explicitly then, I suppose? [14:24] niemeyer: we could do. [14:25] niemeyer: though i don't mind override semantics either [14:25] rogpeppe: It's bogus I think.. it will pretend to be a different version than it actually is [14:26] niemeyer: i'm not sure that will happen. [14:27] niemeyer: s/will/can/ [14:27] rogpeppe: What does --version 10.11.12 --upload-tools --bump-version mean? [14:28] niemeyer: the given version is ignored, the tools are uploaded with a bumped version if necessary [14:28] rogpeppe: There you go.. [14:28] niemeyer: i'm not sure what you mean by "pretending to be a different version". [14:28] niemeyer: what's pretending? [14:28] rogpeppe: Nothing is pretending, because you've ignored the option entirely [14:29] niemeyer: you can also do: --version 10.11.12 --upload-tools [14:29] rogpeppe: Let's just error when the option means nothing [14:29] niemeyer: ok. [14:29] rogpeppe: Unrelated to the current CL, though.. LGTM [14:29] niemeyer: thanks [14:35] niemeyer: apart from that last wrinkle (and major-version upgades, of course), that completes upgrading for the time being, i think. [14:35] rogpeppe: Woohay! [14:36] niemeyer: i'm now moving on to fixing a few bugs that have been assigned to me (in particular some of the sporadic live test failures). other suggestions welcome. [14:37] rogpeppe: We have a pretty high/critical one on the pipeline that could see your attention, actually [14:37] rogpeppe: I've originally invited Aram to look at it, but on a second thought you probably have more context to do it quickly [14:38] niemeyer: ok. let me at it! [14:38] rogpeppe: We're not game to start kicking out the ssh proxying hackery [14:38] s/not/now [14:39] niemeyer: woo. do we think that go.net ssh is up to the task? [14:39] rogpeppe: We don't need ssh at all, hopefully [14:39] niemeyer: even better! [14:40] rogpeppe: The idea is to connect straight onto mongo [14:40] niemeyer: what shall we use for crypto? [14:40] rogpeppe: It'll take a few steps to get there, though [14:40] rogpeppe: Mongo can talk SSL [14:40] niemeyer: cool. [14:40] rogpeppe: mgo can't right now, but I'll fix that before you get there, I'm hoping [14:41] rogpeppe: As a very first step, we have to introduce the idea of a connection password for the agents [14:42] niemeyer: where does the password come from? [14:43] rogpeppe: The agent passwords are automatically generated [14:43] rogpeppe: For the machine agent, we have to hand the password off during the machine creation, so that the new agent can connect once it comes up [14:44] rogpeppe: Unfortunately, we have to do a little trick once the agent comes up the first time, and replace the agent password with a new one that is created locally [14:44] rogpeppe: Because the metadata that we send to the machine is visible to anyone within the machine itself [14:44] niemeyer: perhaps we could have a new agent that's responsible for exchanging one-time passwords for persistent passwords [14:45] (thinking aloud) [14:45] rogpeppe: It's significantly simpler than that [14:45] rogpeppe: It doesn't even have to be a one-time password [14:46] rogpeppe: It's just a "Oh, is that the password I got during creation, okay.. here is a new one I want to use from now on." [14:46] rogpeppe: As long as we do that before the machiner goes on to create units, it's all good [14:46] niemeyer: how do you get the new one? [14:47] rogpeppe: Just create one and change it [14:48] niemeyer: ah, does SSL have provision for changing passwords remotely? [14:48] rogpeppe: Huh.. wires crossed there [14:48] rogpeppe: Forget SSL for the moment [14:49] rogpeppe: This is juju state logic [14:49] rogpeppe: and mongo auth [14:49] niemeyer: ok. [14:50] niemeyer: are we planning to store passwords inside the state? [14:51] rogpeppe: Nope [14:51] niemeyer: sorry, ignore my bleating. i'll shut up until you've explained :-) [14:51] rogpeppe: No, that's it really [14:51] rogpeppe: That's the first step [14:51] niemeyer: could you outline the next steps, so i know where we're heading? [14:52] rogpeppe: How far? :-) [14:52] niemeyer: until we've got equivalent functionality to today [14:52] niemeyer: but via direct connection to mongo [14:52] rogpeppe: We can't drop SSH just yet, but that means we can then introduce SSL more or less easily, and dro pit [14:53] rogpeppe: This is the tricky bit for doing so [14:53] rogpeppe: After that we need some means to define the admin password [14:53] rogpeppe: Which is what that old admin-secret meant [14:54] rogpeppe: So that we can connect straight to mongo [14:54] niemeyer: mongo's authorization is password-based, presumably [14:54] rogpeppe: and then we just enable SSL [14:54] rogpeppe: Yes, user/pass [14:55] rogpeppe: In a future universe, we'll then introduce an HTTPS API to which everyone will talk to [14:55] rogpeppe: Which maps more or less well to that whole model [14:55] niemeyer: and there's a mongo capability to add users and passwords, i guess [14:55] rogpeppe: Yeah, that's already well mapped onto mgo [14:55] niemeyer: so we'd have one user per agent, each with its own password [14:55] rogpeppe: Exactly [14:56] niemeyer: out of interest, which part of the API do you use to do that? [14:57] niemeyer: oh, i see [14:57] rogpeppe: You mean in mgo? [14:57] rogpeppe: http://go.pkgdoc.org/labix.org/v2/mgo#Database [14:57] * rogpeppe grepped for password, not user [14:58] niemeyer: so can anyone change any user? [14:59] rogpeppe: No.. we have to put some thinking on how that will work [15:04] niemeyer: it seems like the model we've got here is something like this: http://paste.ubuntu.com/1254028/ [15:06] rogpeppe: As you spotted above, we'd have one user per agent, each with its own password [15:06] niemeyer: i'm not quite sure how the MA would pass the password securely to other agents. maybe we should just put it in a file. [15:07] niemeyer: after all, if you're root you can get the password anyway. [15:07] rogpeppe: We probably need it in a file either way, since we must know it after it changes [15:07] niemeyer: good point. [15:08] niemeyer: ok, if the above sketch looks right to you, i'm happy. [15:09] rogpeppe: It doesn't because of what I mentioned above [15:09] rogpeppe: As you spotted above, we'd have one user per agent, each with its own password [15:09] rogpeppe: The sketch makes it feel like the password provided is the machine password [15:10] niemeyer: ok, so the MA creates a new mongo user for each agent it starts [15:10] niemeyer: and so, presumably, does the unit agent when it starts subordinates? [15:10] rogpeppe: Yeah, that sounds right [15:11] rogpeppe: This is a bit silly right now, if you think about it, because all those users have the same permissions [15:11] rogpeppe: But it starts to make more sense if we consider that situation won't be like that soon [15:11] niemeyer: yeah, each agent could potentially delegate permissions. [15:12] rogpeppe: Btw, when you say "the MA creates a new mongo user", it's not just a mongo user we're talking about.. these details should be abstracted behind our own API [15:12] niemeyer: yeah, because the MA knows nothing about mongo [15:12] Right [15:13] niemeyer: would this be sufficient? state.State.AddUser(user, pass string) error [15:14] rogpeppe: No.. [15:14] niemeyer: and an extra field in state.Info [15:14] rogpeppe: We have to think through the relationship between user and entity [15:15] niemeyer: a "capability" bitmask or set? [15:16] rogpeppe: Sorry, I just meant that we don't want dangling users around without association to an entity, and we don't want to have to manipulate the needs of authentication and the needs of entity creation/etc independently [15:17] rogpeppe: It should be clear from the API how these things take palce [15:17] place [15:17] niemeyer: ok. by "entity" you mean some type or value in the state? [15:17] niemeyer: or an agent? [15:18] rogpeppe: We've been using entity to refer to unit/service/machine [15:19] rogpeppe: But in this case it's really the subset of those that have an agent ATM [15:19] rogpeppe: So unit/machine [15:20] niemeyer: so the Machine entity is kinda superior to the Unit entity, because an MA can create users for UAs but not vice versa? [15:21] rogpeppe: We won't have the low-level restrictions in place today.. we just need a proper API that makes sense in such a context [15:22] niemeyer: that's what i'm trying to imagine [15:22] rogpeppe: We don't need to "create users" in general [15:22] rogpeppe: We have proper unique keys for the agents [15:22] rogpeppe: We shouldn't have to be manipulating users and passwords in our own API externally to the state model [15:23] rogpeppe: This would add an implicit API on top of our own API that is probalby not necessary [15:26] niemeyer: what about something like this? http://paste.ubuntu.com/1254071/ [15:27] rogpeppe: Yeah, that looks nice [15:28] niemeyer: and then a new "Password" field in state.Info [15:28] niemeyer: (which in actual fact incorporates both the mongodb username and password) [15:29] rogpeppe: I think we can be a bit more explicit: // SetPassword returns a new random password the agent responsible for X should use to communicate with the state servers. Previous passwords are invalidated. [15:30] niemeyer: NewPassword? [15:30] rogpeppe: NewFoo in general returns a new foo, without further consequences [15:30] niemeyer: yeah [15:31] rogpeppe: ReplacePassword maybe [15:31] niemeyer: or ChangePassword [15:31] rogpeppe: SOunds good [15:33] rogpeppe: ChangePassword() (auth string, err error) [15:34] rogpeppe: So we can have Auth in state.Info rather than password, as it may contain the username too [15:34] rogpeppe: Although we never expose those details out of the state itself [15:34] niemeyer: sounds good [15:35] rogpeppe: Hmm.. there's a small detail here, though.. we may need to split that up a bit into separate methods [15:35] niemeyer: i was wondering about this: http://paste.ubuntu.com/1254085/ [15:36] rogpeppe: Ah, no.. we should send the new password.. [15:36] rogpeppe: ChangePassword(password) (auth string, err error) [15:36] rogpeppe: Otherwise there's a race [15:37] rogpeppe: We need to persist the new password before we ask to change, otherwise the agent might be locked out [15:37] niemeyer: ah, interesting, yeah [15:37] rogpeppe: Regarding the past, I'd prefer an explicit API instead [15:37] rogpeppe: unit.ChangePassword.. machine.ChangePassword [15:37] niemeyer: ok [15:38] rogpeppe: No need to guess where it makes sense and where it doesn't [15:38] niemeyer: sounds good [15:39] niemeyer: the nice thing about passing in a password is that the method becomes idempotent. [15:39] rogpeppe: Indeed [15:40] niemeyer: i think SetPassword makes better sense now. [15:40] rogpeppe: +1 [15:40] // SetPassword sets the password the agent responsible for the machine [15:40] // should use to communicate with the state servers. Previous passwords [15:40] // are invalidated. [15:40] func (m *Machine) ChangePassword() (password string, err error) [15:41] func (m *Machine) SetPassword(password string) (auth string, err error) [15:41] I suppose [15:41] niemeyer: yeah c&p error [15:41] fwereade: I mid-way through the filter review.. the conversation here was good, so I didn't make much progress, but will continue after lunch [15:42] niemeyer, no worries [15:42] rogpeppe: I'll step out for lunch and bbiab [15:42] niemeyer: ok cool [15:42] niemeyer, I may be off soon but will try to pop back on later [15:42] fwereade: Sounds good, thanks for the nice progress [15:42] * niemeyer => lunch [15:42] niemeyer, seems to be going ok :) [15:42] niemeyer, cheers [16:14] niemeyer: https://codereview.appspot.com/6596051/ as the first step is in [16:14] cu later [16:41] mramm: Do we have a call in 20 mins? [16:42] niemeyer: Yea the Cloud Consistency Call [16:42] niemeyer: we will be going over the certified public cloud initiative, and some server stuff [16:42] mramm: 'k [16:43] niemeyer: not sure that your attendance is required, if you want to skip I can ping you if something important comes up [16:43] mramm: Sounds good, I'll be around then [16:51] niemeyer: just wondering what's the best thing to do about removing users. i'm thinking that SetPassword("") removes the user for an entity (i can't see that we'll ever want to actually set a blank password) [16:52] rogpeppe: Why would we want to remove a user in such a fashion? [16:52] niemeyer: if a unit gets removed, i think it makes sense to remove the unit's mgo user too [16:52] rogpeppe: Exactly! :) [16:53] niemeyer: erk [16:53] niemeyer: of course. [16:54] rogpeppe: Btw, over lunch I was thinking on our interface.. we'll stumble on problems regarding SetPassword returning auth, for the same reason that we need to pass the password in instead of just using the result [16:54] rogpeppe: I suggest this instead: [16:54] func (m *Machine) SetPassword(password string) error [16:54] struct Info { .....; Principal interface{ SetPassword(string) }; Password string; } [16:55] niemeyer: interesting [16:56] niemeyer: i don't quite see why SetPassword is a problem though, assuming it's deterministic [16:56] rogpeppe: The only change is that the auth result was nonsense [16:57] niemeyer: the Info change above is a pretty major thing. [16:57] rogpeppe: Because? [16:57] niemeyer: currently it's a very simple set of info. [16:58] rogpeppe: Ah, you're right.. we have a chicken and egg [16:58] rogpeppe: Well, we'll need to save the principal as a string then [16:59] niemeyer: alternatively we could just have a Principal method on Machine and Unit [17:00] rogpeppe: For? [17:00] niemeyer: we need to get the user name and password from somewhere. [17:01] rogpeppe: That's not an alternative to the issue above, since we still must tell it what the principal is, and we can't get a password from state [17:01] niemeyer: what about Machine.Auth(password) ? [17:01] rogpeppe: Sorry, feels like we're going backwards in our conclusions [17:02] niemeyer: i'm not sure i've concluded anything yet [17:02] rogpeppe: We already have machine.SetPassword(password).. Auth(password) is no better [17:02] niemeyer: sorry, i'm thinking *in addition* to SetPassword [17:02] rogpeppe: I'm a bit lost [17:02] rogpeppe: What problem are you solving? [17:02] niemeyer: so we'd have SetPassword(password) error; and Auth(password) (auth string, err error) [17:02] rogpeppe: Why? [17:03] niemeyer: so that we can generate the password, save it to disk, then if we crash we can regenerate the required auth info from the password only. [17:03] rogpeppe: All of that seems unrelated to Machine.Auth(password) [17:04] niemeyer: Auth is maybe a bad name. AuthInfo, perhaps. [17:05] rogpeppe: Seems like a complex way to do something simple [17:06] niemeyer: what's your suggestion? [17:06] rogpeppe: The same.. Principal string [17:06] niemeyer: where does that come from? [17:08] rogpeppe: We already have a nice key on PathKey.. we can rename it to something more sensible and reuse [17:08] niemeyer: i was going to suggest that [17:08] niemeyer: i still think that AgentName is a reasonable name for that method. [17:09] niemeyer: BTW there's one thing we haven't talked about [17:09] niemeyer: what does the client do? [17:10] niemeyer: in my implementation so far, i've implemented State.SetPassword [17:10] rogpeppe: +1 on AgentName.. I can't come up with anything better right now [17:10] niemeyer: [17:10] // SetPassword sets the password the administrator [17:10] // should use to communicate with the state servers. [17:11] niemeyer: but actually maybe we don't care much about changing the admin password [17:11] niemeyer: and we could hardwire "admin" as a principal name [17:11] rogpeppe: The thing I dislike about AgentName is that it blocks adding the same interface to other things we'll want to identify in the same manner [17:13] niemeyer: you mean, if we have principals that don't map directly to state objects? [17:13] rogpeppe: No, I mean that the concept of having a unique and intelligible key for entities is very useful [17:14] rogpeppe: EntityName perhaps [17:14] niemeyer: i wrote that, then deleted it [17:14] niemeyer: but it's not too bad, despite being a nasty mouthful. [17:14] rogpeppe: This means we can have "service-foobar" too [17:15] niemeyer: +1 to EntityName [17:15] niemeyer: or even just Name... [17:15] rogpeppe: -1, unless we refactor further [17:15] rogpeppe: We already have that in some places [17:15] niemeyer: oh yeah, we already use it [17:15] niemeyer: darn [17:16] rogpeppe: It's not a bad idea, though [17:16] rogpeppe: We might move to Id() in the other fields [17:17] niemeyer: yeah, Id would work well for how unit.Name etc is used currently. [17:17] niemeyer: and it would fit well with Machine.Id too [17:17] rogpeppe: unit.Id(), service.Id(), and then we have Name() with the full-blown name [17:17] niemeyer: + [17:17] 1 [17:17] ! [17:17] :-) [17:18] niemeyer: ok, that shouldn't be hard to change. [17:18] rogpeppe: We have to do that sooner rather than later, or we'll be stuck with the database schema [17:18] niemeyer: i'll do it now, if you think it's good [17:19] rogpeppe: It sounds like a good move [17:19] niemeyer: what do you think about the client-initiated connection? Info.Principal == "admin" ? [17:19] rogpeppe: Yeah [17:19] rogpeppe: Your idea above regarding "admin" sounds good [17:20] niemeyer: that we have State.SetPassword ? [17:20] rogpeppe: No, that we can hardcode the "admin" user for the moment [17:20] niemeyer: ok, sounds good [17:20] rogpeppe: and that we can do without changing its password for a while [17:20] rogpeppe: I think, but not sure [17:20] niemeyer: then "admin-password" as an environment setting makes total sense [17:20] rogpeppe: Either way, SetAdminPassword would be ok [17:21] niemeyer: which would be a nice change :-) [17:27] robbiew, mramm: Where is the meeting taking place today? [17:28] niemeyer: what meeting? cloud consistency? [17:28] robbiew: Yeah [17:28] niemeyer: meh..you can skip it, unless you wanna hear an update on charm collection [17:28] robbiew: Yeah, that's what I'm curious about [17:28] conf id 80589 63238 [17:29] robbiew: Cheers [17:29] Holy crap [17:29] 17 people.. === philballew_ is now known as philballew [17:57] niemeyer: first stage of Name change: https://codereview.appspot.com/6585053/ [17:57] rogpeppe: Looking [17:58] niemeyer: i'm off for the evening. see ya tomorrow... [17:58] night all! [17:58] rogpeppe: Cool, thanks for the quick turnaround [17:58] rogpeppe: Have a good evening [17:58] niemeyer: np [17:58] niemeyer: static typing FTW [17:58] rogpeppe: Indeed :) [19:20] I'll have a walk outside and come back shortly for more reviews [19:22] niemeyer, very briefly: my issue with using the uniter's existing unit/service is just that I don't want to share them with the filter goroutine [19:23] niemeyer, so I rather think I have to pay the cost of a refresh anyway [19:24] niemeyer, from that perspective, passing a single new watcher into the new goroutine, and leaving it to reconstruct everything it required from there, seemed rather neat [19:25] niemeyer, will be on and off, if you have a quick answer just dump it in the channel and I'll see it soon enough [20:35] fwereade: I half agree [20:36] fwereade: If you want to not share the service, that's certainly reasonable [20:36] fwereade: But the points in the review still feel valid.. you can easily pick the service up from state when starting the filter [20:36] fwereade: and watch it upfront [20:39] fwereade: Or do I misunderstand your point?