/srv/irclogs.ubuntu.com/2012/09/04/#juju-dev.txt

niemeyerdavecheney: The use cases we will dictate what's the best API00:04
niemeyerwe have00:04
davecheneyi'll leave it as a TODO for now00:10
davecheneyit's only one extra line00:10
niemeyerdavecheney: It certainly makes sense from a high-level perspective00:17
niemeyerdavecheney: I don't know how we use it, though00:17
davecheney% juju ssh mongodb/2 -- uptime00:49
davecheneyWarning: Permanently added 'ec2-184-72-139-96.compute-1.amazonaws.com,184.72.139.96' (ECDSA) to the list of known hosts. 00:49:01 up 21 min,  1 user,  load average: 0.08, 0.03, 0.0500:49
niemeyerdavecheney: WOah!01:25
davecheneyniemeyer: there is a small issue currently01:30
davecheneyyou need the --'s to tell gnuflag to stop consuming arguments01:30
davecheneybut it works01:30
wrtp fwereade__: mornin'05:13
fwereade__wrtp, heyhey05:13
wrtpfwereade__: how's it going?05:13
fwereade__wrtp, good, I'm pretty sure05:13
wrtpfwereade__: great!05:14
fwereade__wrtp, the upgrader stuff is now (I think) approaching reviewable05:14
fwereade__wrtp, 3rd attempt :)05:14
wrtpfwereade__: well, it's not easy stuff...05:15
fwereade__wrtp, it has helped me to see a specific area I need to work on though... I've realised that, these days, once something's merged I'm *much* more resistant to even considering changing it05:15
wrtpfwereade__: interesting. i should watch out for that too.05:16
fwereade__wrtp, I suspect it has to do with the amount of effort we put into getting things right the first time05:16
fwereade__wrtp, but it took me an annoyingly long time to see that hook state and charm state (where state here implies local, persistent) are really just aspects of *uniter* state05:17
fwereade__wrtp, and that life gets much easier if they're integrated05:17
wrtpfwereade__: that is true. but all that initial effort is usually predicated on some imagined use cases, and if those don't pan out, things might be better changin.g.05:17
fwereade__wrtp, indeed so05:17
wrtpfwereade__: i find a source of endless wonder from the way that getting an abstraction right can make whole swathes of difficulty just drop away.05:18
wrtpfwereade__: i have this feeling that *somewhere* in there are the *real* "design patterns"05:20
fwereade__wrtp, haha, yeah -- it's a great feeling when it happens05:21
wrtpfwereade__: can i run an idea past you?05:26
fwereade__wrtp, ofc05:26
wrtpfwereade__: the next stage with agent upgrading is to do unit upgrade, which should be trivial. but the next after that is the provisioning agent, which gustavo suggested a while ago might require a whole bunch of infrastructure in state. e.g. NewProvisioner, Provisioner.AssignToMachine etc etc05:27
fwereade__wrtp, hmmm, interesting05:28
wrtpfwereade__: it seems to me that almost all of the latter is currently covered by state.Unit05:28
fwereade__wrtp, yeah, things-managed-by-a-machine-agent sounds like a category worthy of investigation05:29
wrtpfwereade__: and that i should be able to side step all of it by getting the bootstrap machine to start a hidden service ("juju-provisioner" say) and have the machine agent know about that special service and start the agent directly rather than starting a unit agent for it.05:29
fwereade__wrtp, gut says +1 to first bit, -1 to second bit05:30
wrtpfwereade__: i wondered about reserving all service names starting "juju-*"05:30
fwereade__wrtp, that definitely sgtm05:30
wrtpfwereade__: i think it's important that the provisioner not be started by a unit agent05:31
fwereade__wrtp, why is that so?05:31
wrtpfwereade__: because the whole reason for this is to have a place to put the agent version for upgrades05:31
wrtpfwereade__: and if it's started by the unit agent, the unit agent uses that05:31
fwereade__wrtp, I think you need to expand on "a place to put the agent version for upgrades" before I can follow properly05:32
wrtpfwereade__: ok, so each agent puts their current running version in the state so that you can see what's upgraded to what05:33
fwereade__wrtp, ok, yep05:33
wrtpfwereade__: we've got {Machine,Unit}.SetAgentVersion05:33
wrtpfwereade__: but nothing for Provisioner, because there's no such abstraction in State currently.05:34
wrtpfwereade__: i don't see this as a special-case hack for the provisioner BTW; i see it as a way of being able to add *any* new agent at minimal cost.05:34
wrtpfwereade__: for instance, separating the firewaller from the provisioner would be trivial.05:35
wrtpfwereade__: actually, i don't think the machine agent itself would need any logic for this - i think container.Deploy(unit) could see that the service is juju-* and deploy "jujud \1"05:36
fwereade__wrtp, I'm still generally +1 on the idea that juju components should be services wherever practical05:36
fwereade__wrtp, but this feels like a digression slightly05:37
wrtpfwereade__: yeah. the controversial part of this is that there would be a unit without a unit agent.05:37
fwereade__wrtp, I think I'm missing something, because I'm still (dogmatically?) opposed to the concept and the reason it's necessary has not yet clicked05:38
wrtpfwereade__: is it possible that we could use the charm upgrade mechanism to upgrade the provisioning agent?05:38
fwereade__wrtp, ha, that hadn't clicked... but offhand *perhaps* no reason why not05:39
wrtpfwereade__: it seems a little inconsistent, given that machine agents are done differently.05:39
fwereade__wrtp, although the *charm* itself wouldn't be changing much I'd imagine05:39
fwereade__wrtp, seems more like a service-config thing really05:40
wrtpfwereade__: indeed.05:40
wrtpfwereade__: ah, that's interesting.05:40
wrtpfwereade__: BTW is there anything that the unit agent is *required* to do, other than maintain its presence node?05:41
fwereade__wrtp, eg I think there are a couple of charms that let you switch between stable and bleeding edge for example05:41
fwereade__wrtp, restate please, in a sense it's required to do everything it does ;)05:41
fwereade__wrtp, set unit status and charm version in state05:42
fwereade__wrtp, and "participate in relations"05:42
wrtpfwereade__: assume no relations05:42
fwereade__wrtp, the rest doesn't touch external state IIRC05:42
fwereade__wrtp, then presence, unit status, charm version05:42
wrtpfwereade__: i'm wondering if a way to look at it could be that the provisioning agent is its own unit agent.05:43
fwereade__wrtp, I'm still not following something fundamental05:43
fwereade__wrtp, I had this vague understanding that each individual agent was watching juju version and upgrading itself?05:43
wrtpfwereade__: yes, that's right. juju version is now global.05:44
wrtpfwereade__: and it sets something in the state to say when it's upgraded.05:44
wrtpfwereade__: which status prints out, for example.05:45
wrtpfwereade__: and while we'll need to watch when we do major-version upgrades.05:45
fwereade__wrtp, yep, ok05:45
wrtpfwereade__: i'm not sure how that would work if the PA was started by a uniter05:46
wrtpfwereade__: but if the PA is its own uniter, it's trivial.05:46
fwereade__wrtp, yeah, without some out-of-band signalling mechanism to the charm it's problematic05:46
wrtpfwereade__: the problem is really that we don't have any out-of-band signalling mechanism *from* the charm, right?05:47
fwereade__wrtp, the idea *feels* like ++confusion not --confusion... but it could be that point when you're pulling on a tangled mass of slinky just before it reconfigures back into the neat form05:48
fwereade__wrtp, I can live with that perspective too :)05:48
wrtpfwereade__: thing is, i *think* that in terms of lines of code, it will be almost negligible05:48
wrtpfwereade__: like three or four lines in container, a couple of lines in status to exclude juju-* services from printing by default, and a little stuff in jujud bootstrap-init to actually set up the service.05:49
wrtpfwereade__: and that's what attracts me to the idea.05:50
wrtpfwereade__: and the alternative previously suggested is to build an entirely new edifice in state with many methods and tests, just to do something we already *almost* do.05:51
fwereade__wrtp, I still feel like unitless services (or reimplementations of the uniter) are kinda smelly05:51
fwereade__wrtp, there are definitely good things floating around in this space though05:51
fwereade__wrtp, wait a mo05:52
fwereade__wrtp, is there much meaningful distinction between the environment config and the uniter's service config?05:52
fwereade__wrtp, it suddenly feel like they're (almost) exactlythe same thing05:53
wrtpfwereade__: it's a *config.Config, not a *state.ConfigNode05:53
fwereade__wrtp, couple of conceptual levels up though05:53
wrtpfwereade__: but that's only a recent thing05:53
fwereade__wrtp, what "is" the environment config if not the config of the juju service?05:54
wrtpfwereade__: that seems like a nice way to look at it05:54
wrtpfwereade__: that would mean we could put the machine agent under this umbrella too05:54
wrtpmaybe05:54
fwereade__wrtp, I think it *might* be, but I suspect the ramifications will be somewhat hefty even if it is05:54
fwereade__wrtp, hmm, I had still been thinking of the MA as the root of everything, maybe we can take it a step further but it feels wrong somehow05:55
wrtpfwereade__: ooh, that might actually solve some problems down the line05:55
wrtpfwereade__: like... the machine agent doesn't need to know environment secrets, but the provisioner does.05:56
fwereade__wrtp, one MA per machine which is responsible for starting the things that should run on that machine -- everything else is services05:56
wrtpfwereade__: perhaps you could think of the MA as just one unit on a machine. it's a bit special, because it's started by starting the instance, but maybe...05:57
wrtpfwereade__: but yes, that's what i was thinking.05:57
fwereade__wrtp, it does all definitely sound worthy of investigation :)06:01
wrtpfwereade__: but that does imply uniterless units, i think06:01
fwereade__wrtp, indeed, we need an escape hatch *somewhere* in order to do things that nicely06:02
wrtpfwereade__: unless the *uniter* is the first thing to run on a machine06:02
wrtpfwereade__: can a hook do config-set, BTW?06:02
fwereade__wrtp, nope, which also may be a problem06:02
fwereade__wrtp, the only bits of state a unit agent even *can* write are status, charm, presence, and relation stuff06:03
wrtpfwereade__: it's not really a problem. i kinda feel that each unit should have its own config actually.06:03
fwereade__wrtp, and I'm starting to like the idea that the MA could be a unit of the juju-machines service06:04
wrtpfwereade__: but that probably goes against the whole concept of homegeneous units06:04
fwereade__wrtp, but I'm a little concerned about that depth of the rabbit hole06:04
wrtpfwereade__: yeah, that's definitely a bigger thing than i was suggesting.06:04
fwereade__wrtp, I'm mainly just worried that this whole area of investigation is just the innocuous entryway to a rabbit megalopolis that we aren't really in a position to explore right now ;)06:05
wrtpfwereade__: i think i'll give the PA idea a play and see what the code looks like.06:05
fwereade__wrtp, cool06:05
wrtpfwereade__: thanks for the discussion - i now know the points to be careful about when proposing the idea!06:08
fwereade__wrtp, haha, glad it was useful :)06:08
wrtpdavecheney: hiya06:23
wrtpdavecheney: if you're feeling so inclined, i'm looking for reviews of https://codereview.appspot.com/6490067/06:25
davecheneywrtp: OH MY GOD06:26
davecheneytoday has been such a drama06:26
davecheneyat one point I had three sepearate tradesemen here06:26
davecheneythe tiler, manged to short out my power06:26
wrtpdavecheney: lol06:26
davecheneythen the eletrician had to some06:26
davecheneyand then the guy from the power compnay because the eletrician noticed the voltage was a bit high06:26
davecheneyand now the flooring guy just turned up to do a moisture test06:27
davecheneyanyway06:27
davecheneydrama is over06:27
davecheneyi wish I was bak in lisbon06:27
davecheneymost of the renovations happened when we were there06:27
wrtpdavecheney: do you know this song? http://www.youtube.com/watch?v=zyeMFSzPgGc06:27
davecheneywrtp: i don't think this is a situation for levity06:28
wrtpdavecheney: you're probably right. but i've found that song to be spot on many times :-)06:29
davecheneylol06:29
davecheneywrtp: i'll review your CL in two secs06:31
wrtpdavecheney: thanks06:31
davecheneyjust submitting one that has been waiting for my intertubes to return06:31
wrtpdavecheney: there's a follow-up too06:31
davecheneywrtp: i've been enjoying watching your cleanups06:32
davecheneyrefactor !!06:32
wrtpdavecheney: thanks06:32
davecheneywrtp: for you https://codereview.appspot.com/6499071/06:32
wrtpdavecheney: do it when you see it, i reckon06:32
davecheneywrtp: absolutely06:32
wrtpdavecheney: i haven't looked at more than the title, but definite +1 - i thought of that before. but... i wondered if there was a particular reason for it.06:33
wrtpdavecheney: i.e. is there a time when it's valid to have an assigned machine id but no actual machine06:33
wrtp?06:33
wrtpdavecheney: "06:34
wrtpAll the non test consumers of this06:34
wrtpmethod are actually after a *Machine, not the id anyway06:34
davecheneywrtp: i can't see how, you have to pass a *state.Machine to unit.AssingToMachine06:34
wrtp"06:34
wrtpnot *quite* true - status.go only uses the id :-)06:34
davecheneystatus.go can suck it up06:35
wrtplol06:35
davecheneyfirewaller also uses it, but it is a reasonable change06:35
davecheneyi ran across this writing juju ssh06:35
wrtpdavecheney: can you remove a machine without removing the units assigned to it?06:35
davecheneywrtp: this is overall too much code to turn a unit name into an insatnce06:36
davecheneyhttp://paste.ubuntu.com/1185173/06:36
davecheneywrtp: i don't know the answer to that question06:36
wrtpdavecheney: it's an important question to answer in the context of that CL06:37
wrtpdavecheney: if you can, then it's a reason for AssignedMachineId06:37
wrtpdavecheney: or... maybe we say it can return nil.06:37
wrtpor an error06:37
davecheney        return u.st.Machine(keySeq(machineKey))06:38
davecheneywill return the error06:38
wrtpdavecheney: seems reasonable.06:39
fwereade__wrtp, davecheney: IIRC you cannot remove machines until they're empty06:41
davecheneythat sounds reasonable06:41
fwereade__wrtp, davecheney: and AFAIK there is nothing in place that allows users to move units from one machine to another, but AIUI that will not necessarily be the case06:41
fwereade__always06:41
wrtpdavecheney: i think we should have State.Unit - then your code could be: http://paste.ubuntu.com/1185191/06:43
davecheneywrtp: that was infact the first method I reached for06:44
davecheneywrtp: and wouldn't the units' name be "unit/0"06:45
davecheneyso I wouldn't have to split c.Target at all06:45
wrtpdavecheney: indeed.06:45
wrtpdavecheney: but... i'm not entirely sure.06:45
wrtpdavecheney: in fact, yeah, that's right06:45
davecheneyfunc (s *State) Unit(name string) (unit *Unit, err error) Unit returns a unit by name.06:46
davecheneyDOH!06:46
davecheneyi'm a fuckwit06:46
davecheneyi think I didn't use that initially because I had the id of the unit/NUM06:46
wrtpdavecheney: oh yeah, i just looked for that and missed it06:46
davecheneyand I was frustrated that i had an int, but needed a string06:46
davecheneystill, i need to get from Unit -> assigned Machine -> Instance06:47
wrtpdavecheney: yeah. i think returning a *Machine makes total sense.06:47
wrtpdavecheney: i can't think of any case where the id is useful or even significant by itself.06:47
davecheneyand Machine.Id() does not return an err06:48
davecheneyso it is much easier to use than state.Machine(id)06:48
davecheneywhich does06:48
wrtpdavecheney: indeed.06:48
wrtpdavecheney: yup06:48
davecheneywrtp: http://paste.ubuntu.com/1185220/07:04
davecheneygetting better, and I can reduce it further with my other CL07:04
wrtpdavecheney: not entirely sure07:05
wrtpdavecheney: the Unit request can fail because of more than just the name being malformed,07:05
wrtpdavecheney: i think that strings.Index(name, "/') > 0 might be a better test07:05
wrtpdavecheney: that way you don't incur the round trip when it's not a unit07:06
davecheneywrtp: will do, the python code also included that logic07:06
davecheneywrtp: speaking of round trips07:06
davecheneyadding mstate.Info07:06
davecheneywe're going to reuse the UseSSH logic to establish a tunned to mgo07:06
davecheneybut in the future there exists a possibility of using TLS directly07:06
wrtpdavecheney: i'd love that ssh tunnelling code to disappear07:07
wrtpdavecheney: it was a right pain to write, and i still don't fully trust it07:07
davecheneygiven we control the mgo code, it might be easier to use the go.crypto ssh package07:07
davecheneybut that is bordering on out of scope07:08
davecheneyi am very confident of the tcp forwarding code, several of us have been banging on that for months07:08
davecheneywrtp: the state already checks the unit name for us07:10
davecheney% juju ssh mysql/q07:10
davecheneyerror: cannot get unit "mysql/q": "mysql/q" is not a valid unit name07:10
davecheneywrtp: btw +1 to your change to always make a state connection when we open juju.Conn07:11
wrtpdavecheney: good point.07:11
davecheneyanyway, i'll leave that CL for the moment07:12
davecheneythe tests for it are going to be a days work07:12
davecheneywe have to mock *everything*07:12
davecheneywrtp: sorry, still reading your CL07:17
wrtpdavecheney: which CL is that?07:17
davecheneythe one you pasted me 30 mins ago07:18
wrtpdavecheney: i mean, the one you're gonna leave07:18
davecheneyoh, juju ssh07:19
davecheneyi haven't proposed that yet07:19
davecheneywill finalise it tomorrow07:19
davecheneyi'm sort of jumping between a few at the moment07:20
davecheneyin good news, juju deploy is getting very solid07:21
davecheneyjuju bootstrap ; juju deploy mysql ; juju deploy mongodb ; juju add-unit mongodb ; juju add-unit -n2 mysql ; juju status07:21
davecheneyjust works07:21
wrtpdavecheney: cool07:40
davecheneywrtp: in reusing the UseSSH code from state/ssh.go07:58
davecheneydo you think it is worth moving it into another package07:58
davecheneyor just copy it to mstate as we're nix'ing state soon ?07:58
wrtpdavecheney: the latter07:58
davecheneywrtp: roger, roger07:58
wrtpdavecheney: i want that code to go!07:58
wrtpdavecheney: and giving it its own package is not the way forward :-)07:59
davecheneywrtp: no, we don't want to give it an endorsement07:59
davecheneywrtp: https://bugs.launchpad.net/mgo/+bug/104567808:05
wrtpdavecheney: +1.08:07
wrtpdavecheney: or even an io.ReadWriteCloser08:07
davecheneysure, if mgo doesn't need to know Local/RemoteAddr08:07
Arammoin.08:11
davecheneyhey08:12
wrtpdavecheney: those changes could be handled in a separate CL, but it's only 9 lines of changes total, with no impact on other code, and they were both as a result of problems i encountered when putting this CL together.08:31
davecheneywrtp: i'd like to see them split out, but it's not a strong objection08:32
wrtpdavecheney: ok, i'll split 'em out. there's no dependency on them i think.08:32
wrtpdavecheney: BTW i suspect you might have been the one that used Errorf in opRecvTimeout. any particular reason for it?08:33
Aramanyone seen mramm lately?08:37
TheMuegood morning08:39
Arammorning.08:39
fwereade__right, I think I might actually be happy with the charm upgrades at last -- taking a short break before polish and propose08:45
fwereade__(good mornings to those I have not thus far addressed)08:45
niemeyerGooood morning rockstars10:14
fwereade__niemeyer, heyhey10:19
TheMueniemeyer: hiya10:20
fwereade__huh:10:52
fwereade__runtime: signal received on thread not created by Go.10:52
fwereade__FAILlaunchpad.net/juju-core/state21.458s10:52
fwereade__anyone seen that before?10:52
davecheneyfwereade__: yes10:52
davecheneynot in state10:53
fwereade__davecheney, is it something I did? :)10:53
Aramno.10:53
Aramit happens sometimes.10:53
fwereade__is there a bug for it?10:53
davecheneyit's an artifact of the gozk c bindings10:53
Aramit's poor zookeeper cgo interraction.10:53
davecheneyfwereade__: probably should raise one10:53
fwereade__ah, ok, cool, I'll go and do that10:54
Aramthere is one bug in the Go tracker.10:54
fwereade__Aram, for gozk?10:54
davecheneyAram: there is a googler who is playing in that area at the moment10:54
Aramnot for gozk, for cgo.10:54
Aramcgo is at fault.10:54
fwereade__Aram, ahh, ok10:54
Aramnothing we can do, sadly.10:54
fwereade__Aram, very well, I shall wait for state to go away entirely then :)10:55
davecheneyjolly good, carry on10:55
davecheneyfwereade__: longer version, it is an argument about who owns the signal handler10:56
davecheneyif a signal is delivered to a thread while in c code, the go signal handler doesn't have the proper registers setup to handle it10:56
davecheneyso it has to panic10:56
fwereade__davecheney, good to know, thank you10:58
davecheneyfwereade__: there isn't much of a solution at the moment, apart from 'don't use so much cgo you darn kids'10:58
davecheneyc'mon, lets get this show on the road10:59
wrtpdavecheney: ah! i'd wondered where that came from.10:59
niemeyerParty time!11:00
niemeyerInvites sent11:03
Aramhttps://plus.google.com/hangouts/_/3016131d787ecda60f236d1dec4e32264e3353ad?authuser=0&hl=en11:03
niemeyerdavecheney, fwereade__, wrtp, TheMue, Aram11:03
Aram"google can't load CSS", wow, never seen that one.11:04
davecheneyhttps://bugs.launchpad.net/juju-core/+bug/104515111:29
davecheneyniemeyer: Aram ^^11:29
davecheneystate.waitForInitialised11:29
Aramthat doesn't require a watcher though.11:30
TheMuelunchtime11:51
fwereade__niemeyer, https://codereview.appspot.com/6500072/ -- I hope this one meets with your approval :)12:26
fwereade__niemeyer, the only way I could see to split it was uncomfortably artificial -- without the upgrade modes in place, the changes to the other modes looked unpleasantly arbitrary to me12:27
Aramis there a way to turn line numbers off in codereview?12:27
niemeyerfwereade__: Sure.. I'll have a look12:31
niemeyerfwereade__: There some low-hanging fruits to split there, but I'll look as-is12:31
fwereade__niemeyer, indeed, I guess a couple of types could indeed have been implemented independently without affecting anything else, but then I'd end up with both used and unused code for doing the same thing, and that confuses me ;)12:33
niemeyer_fwereade__: Cool, I'd just appreciate if things were a bit more broken down overall.. your branches have been consistently large12:34
niemeyer_fwereade__: This not only tricky for review, but also creates that frustration where it feels like the branch is always coming back because it's all in the same hunk12:34
fwereade__niemeyer_, indeed, I know they have been less than ideal lately, I shall try to do better :)12:35
wrtpniemeyer_: i don't think of it as a trivial point. concurrent code is not easy to grok, and every extra select is another bifurcation point that makes things a little harder to grasp. that was the motivation behind my original comments. i apologise for the diversion though.12:38
niemeyer_wrtp: I apologize, but all you're doing is insistently complaining that I'm writing down safer concurrent code.12:48
niemeyer_wrtp: Read my original point and ponder about it for half a second.12:48
wrtpniemeyer_: depends what we mean by "safer" i guess. i like being able to trace an obvious no-conditions path through the code. with the select, a reply can be lost, which isn't obvious; i think it's easier to verify that replies are always sent. i appreciate that this is a matter of taste though.12:55
niemeyer_wrtp: I hope I was clear by what I meant by safer in the review.12:55
wrtp"if the other side decides to abort in the middle of handling this due to Dying" seems a bit like "if you forget to call file.Close" to me. yes you can forget it, but it's easy to verify, whereas concurrent states aren't quite as easy.12:57
wrtpniemeyer_: ^12:58
fwereade__late lunch, bbiab12:59
wrtpfwereade__: hmm lunch, good idea12:59
niemeyer_wrtp: No, it means that if we do any operation in the middle that can fail, exactly as we already have in other cases such as refresh, we have an unperceived deadlock in the code.13:03
=== niemeyer_ is now known as niemeyer
niemeyerwrtp: That's writing safer concurrent code. If you're unhappy with that, I'll have to apologize.13:05
wrtpniemeyer: it depends where you see the locus of control, i think. with RPC-style channel comms like this, i think it's reasonable to think of sending the RPC request as handing control to the central routine. then it's up to that routine to reply appropriately if something fails.13:07
wrtpniemeyer: both approaches are "safe". one has slightly simpler code which is easier to verify IMHO.13:07
niemeyerwrtp: It's sensible to think like this, except it's wrong in some cases, and thus not safe.13:10
fwereade__niemeyer, am I right to think it is acceptable for the UA to read from environment config to determine the provider type?13:28
niemeyerfwereade__: Yes, I think it's fine for us to do that rigth now13:31
fwereade__niemeyer, cool, cheers13:31
niemeyerfwereade__: The UA shouldn't have access to the whole environment config, but when we fix that we can do whatever13:31
fwereade__niemeyer, yeah, I understand we don't want it to have access long-term13:32
TheMueniemeyer: one question about the usage of your presence.Watcher. i need Alive() for Unit.AgentAlive(). is it intended the every "user" of such a watcher creates an own one or shall State provide one watcher, e.g. unitw?13:41
niemeyerTheMue: Single instance for State13:41
TheMueniemeyer: ok13:42
TheMueniemeyer: currently i have no overview. do we need watchers for all collections?13:42
niemeyerTheMue: We need the watchers we'll actually use13:43
niemeyerTheMue: The watchers being used by workers are a good guideline to start with13:43
TheMueniemeyer: ok, i have to scan the code13:43
TheMueniemeyer: thx13:43
niemeyerTheMue: np13:44
niemeyerTheMue: Are you busy or would you have a moment?13:58
niemeyerTheMue: Would just like to run an idea I exchanged with wrtp the other day, for the next time you get blocked13:59
TheMueniemeyer: i'm integrating your presence in unit.AgentAlive() and co.13:59
TheMueniemeyer: ok, i'm listening13:59
niemeyerTheMue: Right now we have the handling of environment somewhat hardcoded in each different worker13:59
niemeyerTheMue: It'd be nice to have a shared way to ensure a worker is handling the environment loading and reloading basically correctly14:00
niemeyerTheMue: One idea we came up with was to have an observer channel being passed into the worker14:00
TheMueniemeyer: yeah, i've seen that wrtp has been already active regarding provisioner and firewaller14:01
niemeyerTheMue: If not nil, we send the environment into the channel every time the local "<w>.environ" property is set14:01
niemeyerTheMue: So that we can create a shared test suite that is run against any worker14:01
niemeyerTheMue: Makes sense?14:02
wrtpniemeyer: i thought we decided that that wasn't a great way to go14:02
TheMueniemeyer: a peep whole ;)14:02
TheMueniemeyer: yes, sounds good14:02
TheMuewrtp: why not?14:03
niemeyerAug 31 10:11:06 <rogpeppe>      niemeyer: perhaps you could mention to TheMue the thoughts you had on the worker tests, re: the above provisioner_test.go CL.14:03
niemeyerAug 31 10:11:36 <niemeyer>      rogpeppe: Will do14:03
* wrtp goes back to look14:03
niemeyerwrtp: My memories and this makes it feel like we agreed on something else14:03
niemeyerwrtp: But I'm happy to talk agan14:03
niemeyeragain14:03
TheMueniemeyer: the way right now with the op codes and the secret doesn't feel very good14:05
niemeyerTheMue: Yeah, it's a good interim solution14:05
TheMueniemeyer: exactly14:05
wrtpniemeyer: hmm, i remembered more discussion than that14:05
niemeyerTheMue: But it'd be nice to have a way we can reuse for all the workers, instead of having to hack it together14:05
niemeyerwrtp: It was a G+ call14:05
TheMueniemeyer: absolutely14:05
niemeyerwrtp: Still, I have vivid memories of the agreement14:06
* wrtp wishes we could archive G+ calls14:06
niemeyerwrtp: I recall even you saying you'd prefer to have the channel passing in through the New function14:06
niemeyerwrtp: While my original proposal was to have a method14:06
TheMueniemeyer: my favorite would be a method to set the channel. so it's nil by default and would be only set in tests.14:09
TheMueniemeyer: could get a neat interface EnvironTester { SetEnvironChan(ch) }14:10
* wrtp is still trying to remember the context in detail14:10
wrtpTheMue: the problem with that is that it has to interact with the worker to set the channel.14:11
wrtpTheMue: and it would probably need a channel to do that, so we don't gain anything.14:11
niemeyerwrtp++14:11
TheMuewrtp: sh.., yes14:12
wrtpniemeyer: i can't remember if we decided it was best to have a chan of environs.Environ or of *config.Config14:14
wrtpah, i remember!14:14
niemeyerwrtp: It was environ the whole time, IIRC. The point was ensuring that it was properly loaded14:15
TheMueniemeyer: thx btw, will go in. the other one is WIP, i need to discuss something for it with you14:15
niemeyerTheMue: Sure, what's up?14:15
TheMueniemeyer: so far the agent alive flag has been a zk node below the entity node14:15
TheMueniemeyer: now we don't have that hierarchy.14:16
niemeyerTheMue: Indeed14:16
wrtpniemeyer: my confusion had been about the discussion we had about an *incoming* channel14:16
wrtpniemeyer: i didn't read your initial comment above properly, doh!14:16
niemeyerwrtp: Ah, no worries14:16
TheMueniemeyer: shall i add a collectiong for alive like for config nodes, with own namesspaces "m/..." and "u/..."?14:17
TheMueniemeyer: so presence.Watcher and Pinger would act on this collection.14:18
niemeyerTheMue: Sorry, I kind of know what you're talking about, but given the way you phrase it I'm not sure14:19
TheMueniemeyer: oh, sorry, will rephrase it.14:19
TheMueniemeyer: the base question is, what shall be watched/pinged for Machine.AgentAlive and Unit.AgentAlive? We don't have own nodes for it anymore.14:20
niemeyerTheMue: As a side note, I quickly agreed with Aram that we'll move towards using "m#...", "u#...", etc14:20
niemeyerTheMue: This avoids the ambiguity of "m/1" being both a unit name and a resource key14:21
TheMueniemeyer: ok, will change it in my code.14:21
niemeyerTheMue: Okay, if I understand your question, for machine 1 we should watch/ping "m#1"14:22
TheMueniemeyer: yes, exactly. or u#wordpress/1.14:23
niemeyerTheMue: +114:23
TheMueniemeyer: cheers, will do it for units first and then machines14:24
TheMueniemeyer: and both use the same collection14:24
niemeyerTheMue: Btw, the pinger should live in a separate MongoDB database14:25
niemeyerTheMue: "presence", probably14:25
niemeyerTheMue: This means it gets its own write lock14:25
niemeyerTheMue: Which is relevant given how much traffic this may end up happening, and how unrelated to the activity in "juju" it is14:25
TheMueniemeyer: hmm, ok, have to take a look how that works.14:26
niemeyerTheMue: No big deal.. just a different session.DB14:26
TheMueniemeyer: but it sounds reasonable14:26
TheMueniemeyer: ok14:26
* TheMue loves mgo14:26
niemeyerTheMue: As another idea, I think we may end up having a type entity interface { entityKey() string }14:27
TheMueniemeyer: +114:27
niemeyerTheMue: That unifies all things "C#ID"14:27
niemeyerOr most, anyway14:27
wrtpniemeyer: upgrade-juju command now works: https://codereview.appspot.com/649808514:28
niemeyerwrtp: Woohay!14:28
niemeyerwrtp: Did you actually see the thing working live?14:28
wrtpniemeyer: i'll just try it now. i have the utmost confidence :-)14:29
niemeyerwrtp: Man, this is so awesome14:29
wrtpniemeyer: ('cos it actually doesn't do much)14:29
* wrtp goes for a drink while the amazon test runs14:31
wrtpjeeze it's slow14:42
wrtp*still* uploading the same set of tools.14:47
wrtpmaybe it's hung. you just canna tell.14:50
niemeyerhttps://codereview.appspot.com/6490067/diff/3018/cmd/juju/cmd_test.go15:11
niemeyerwrtp: On that CL,15:11
niemeyerwrtp: time.Sleep(500 * time.Millisecond)15:11
wrtpniemeyer: yeah, looks spurious15:11
niemeyerwrtp: Weren't you complaining about timings just yesterday?15:11
niemeyer:)15:11
niemeyerwrtp: Let's please not do this15:11
wrtpniemeyer: i'm just removing it - i think it was from a debug session15:11
niemeyerwrtp: I can imagine why you had it there15:12
niemeyerwrtp: I don't think the error reordering is entirely right either..15:12
wrtpniemeyer: it makes for a much more useful diagnostic15:13
niemeyerwrtp: Ah, you've added buffering to opc15:13
wrtpniemeyer: if the command has died with an error, you get to see the error rather than a "timed out" message15:13
wrtpniemeyer: yeah15:13
niemeyerwrtp: Okay, I guess that should work15:13
wrtpniemeyer: i made the change because the diagnostics were terrible when it failed...15:13
niemeyerwrtp: +115:13
wrtpniemeyer: will remove the sleep pronto15:14
niemeyerwrtp: Can we bump a bit further along the 20 number? Like 256?15:14
niemeyerwrtp: We'll never see the wasted memory, and it may save us precious debugging time some day15:14
wrtpniemeyer: sure. although given that it's only 2 in practice...15:14
niemeyerwrtp: Okay, 64 then ;)15:15
niemeyerwrtp: The point is really to get unrealistically large15:15
niemeyerwrtp: So we don't have to debug a most extraneous bug some day15:16
wrtpniemeyer: i thought 20 already was, but will change. 50 perhaps. can't really see why a power of two is useful here.15:16
niemeyerwrtp: Although, the whole logic will probably need some caring for :(15:16
niemeyerwrtp: Since it has hardcoded ops15:16
wrtpniemeyer: that's probably true. but not in scope for this CL15:17
wrtpniemeyer: i've wondered about giving dummy.Listen a mask of operations we're interested in.15:17
wrtpniemeyer: then hardcoded ops would be just fine, i think.15:18
niemeyerwrtp: I'd rather aim for more useful tests..15:20
niemeyerc.Check((<-opc).(dummy.OpPutFile).Env, Equals, "peckham")15:20
niemeyerWhat is this telling us really?15:20
niemeyerThat a method was called, in whatever circumstances, with whatever arguments..15:20
niemeyerTHUS, IT MUST WORK!15:20
wrtpniemeyer: agreed. we should test that the tools now exist in the environment15:21
* niemeyer <= mocking disbeliever15:21
wrtpniemeyer: and that we can connect to the state, etc.15:21
wrtpniemeyer: that's much easier with the infrastructure we have now actually15:21
niemeyerwrtp: Yeah15:21
wrtpniemeyer: i'll file a bug15:22
niemeyerwrtp: Cheers.. something for us to prize, if nothing else :)15:22
wrtpniemeyer: done. also, sleep removed and buffer size increased.15:30
niemeyerwrtp: Cheers15:31
fwereade__wrtp, i'm running a machine agent on EC2 in the hope that it will deploy a unit agent for me, but it appears to be stuck opening state... does this sound familiar to you?15:40
wrtpfwereade__: how long have you waited?15:40
fwereade__wrtp, maybe 10 mins15:41
wrtpfwereade__: wait another 5 mins perhaps15:41
fwereade__wrtp, why would that happen?15:41
wrtpfwereade__: what do the log messages say?15:41
fwereade__wrtp, 2012/09/04 15:28:32 JUJU state: opening state; zookeeper addresses: ["ec2-107-20-89-226.compute-1.amazonaws.com:2181"]15:42
fwereade__wrtp, nothing after that15:42
wrtpfwereade__: have you tried sshing to the machine?15:42
fwereade__wrtp, that's where I got the log from; is there something else I should be looking for?15:42
wrtpfwereade__: ah, so that message is from the machine agent log?15:43
fwereade__wrtp, yeah15:43
wrtpfwereade__: did you run the initial bootstrap with --verbose --debug?15:43
fwereade__wrtp, gah, no15:43
wrtpfwereade__: ISTR that that flag propagates through to the bootstrap machine and thence to the machines it starts15:44
wrtpfwereade__: have you tried sshing to the machine it's trying to connect to?15:45
wrtpfwereade__: in fact, have you tried juju status?15:45
fwereade__wrtp, since it's the state machine I'm implicitly sshing to with every command, no15:45
fwereade__wrtp, status is fine I think15:45
wrtpfwereade__: hmm, what about sshing to the machine agent's machine, then trying to ssh from that machine to the address listed above15:46
wrtpfwereade__: perhaps it can't use the external address internally or something15:46
fwereade__wrtp, ah-HA, security groups15:50
fwereade__wrtp, opened 2181, stuff is happening15:50
fwereade__wrtp, but...15:50
fwereade__2012/09/04 15:49:43 JUJU deploying unit etherpad-lite/015:50
fwereade__2012/09/04 15:49:43 JUJU cannot deploy unit etherpad-lite/0: cannot find executable: exec: "jujud": executable file not found in $PATH15:50
fwereade__2012/09/04 15:49:43 JUJU machiner waiting for change15:50
fwereade__wrtp, and I need to be off :(15:50
wrtpfwereade__: hmm, isn't 2181 open for everything?15:51
fwereade__wrtp, I have every hope that I will be on later...15:51
fwereade__wrtp, juju-amazon just had 2215:51
wrtpfwereade__: interesting. i'll check.15:51
wrtpfwereade__: and cloud-init should set the PATH15:51
wrtpfwereade__: good stuff anyway!15:51
wrtpfwereade__: environs/ec2/ec2.go:664: // TODO authorize internal traffic15:54
wrtp:-)15:54
niemeyerwrtp: Sent a review..16:06
niemeyerwrtp: It looks great in general16:06
wrtpniemeyer: brill, thanks16:06
niemeyerwrtp: There are a couple of things to sort out, one of them is connected to the test stuff we were just talking16:07
niemeyerwrtp: The other is about environ config changes16:07
wrtpniemeyer: yeah, i wondered how you'd take that16:07
niemeyerwrtp: Unfortunately I'll have to step out for lunch to avoid a fight here, but I'll be back soon :)16:07
wrtp:-)16:07
wrtpniemeyer: i wanted to avoid timeouts16:07
niemeyerwrtp: Sure.. that's a great goal.. but there are many ways to do it16:08
niemeyerAnyway, lunch16:08
niemeyerbiab16:08
wrtpniemeyer: enjoy!16:08
TheMueniemeyer: presence integration in unit works great *hug*16:29
wrtpfwereade__: i've got a fix for the internal traffic TODO. will propose tomorrow.17:35
* wrtp is off for the night. see y'all tomorrow17:35
niemeyerwrtp: Night!18:04
fwereade__wrtp, you rock :)18:56
niemeyerfwereade__: ping19:17
fwereade_niemeyer, pong20:43
niemeyerfwereade_: yo20:44
fwereade_niemeyer, how's it going?20:44
niemeyerfwereade_: Was just wondering if you had any input on the ForceRefresh conversation with TheMue20:44
fwereade_niemeyer, sorry, no20:44
niemeyerfwereade_: But no to worry, it's late for you20:44
fwereade_niemeyer, cath and laura are asleep, I'm weighing my options -- if I can help then I'm happy to20:45
niemeyerfwereade_: Thanks, but that was all I had at least.. I was wondering if you had any insight from the uniter perspective on the issue, but it's not a big deal either way20:47
fwereade_niemeyer, I was just thinking that I could ofc break down that CL I proposed earlier into (1) replace state files (2) add Deployer, extend GitDir (3) dump Manager, use Deployer, implement upgrades20:49
fwereade_niemeyer, if you haven't already spent time on it then that might be a sensible use of my time before you get up tomorrow20:49
fwereade_niemeyer, although none of the breaks there are quite as clean as I would hope20:50
niemeyerfwereade_: I haven't spent time on it, and I wouldn't mind waiting until tomorrow morning as the plate is full ATM20:50
niemeyerfwereade_: This will likely improve your progress on it too20:51
niemeyerfwereade_: As we'll surely be able to starting merging bits sooner rather than later20:51
niemeyerstart20:51
fwereade_niemeyer, hopefully so -- deployer is a nicely separable chunk, at the very least20:51
fwereade_niemeyer, (and that then leads me to consider another possible partitioning...)20:52
* fwereade_ just wishes we had a good multi-prereq story20:53
niemeyerfwereade_: =120:54
niemeyer+120:54

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!