niemeyer | davecheney: The use cases we will dictate what's the best API | 00:04 |
---|---|---|
niemeyer | we have | 00:04 |
davecheney | i'll leave it as a TODO for now | 00:10 |
davecheney | it's only one extra line | 00:10 |
niemeyer | davecheney: It certainly makes sense from a high-level perspective | 00:17 |
niemeyer | davecheney: I don't know how we use it, though | 00:17 |
davecheney | % juju ssh mongodb/2 -- uptime | 00:49 |
davecheney | Warning: 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.05 | 00:49 |
niemeyer | davecheney: WOah! | 01:25 |
davecheney | niemeyer: there is a small issue currently | 01:30 |
davecheney | you need the --'s to tell gnuflag to stop consuming arguments | 01:30 |
davecheney | but it works | 01:30 |
wrtp | fwereade__: mornin' | 05:13 |
fwereade__ | wrtp, heyhey | 05:13 |
wrtp | fwereade__: how's it going? | 05:13 |
fwereade__ | wrtp, good, I'm pretty sure | 05:13 |
wrtp | fwereade__: great! | 05:14 |
fwereade__ | wrtp, the upgrader stuff is now (I think) approaching reviewable | 05:14 |
fwereade__ | wrtp, 3rd attempt :) | 05:14 |
wrtp | fwereade__: 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 it | 05:15 |
wrtp | fwereade__: 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 time | 05: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* state | 05:17 |
fwereade__ | wrtp, and that life gets much easier if they're integrated | 05:17 |
wrtp | fwereade__: 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 so | 05:17 |
wrtp | fwereade__: 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 |
wrtp | fwereade__: 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 happens | 05:21 |
wrtp | fwereade__: can i run an idea past you? | 05:26 |
fwereade__ | wrtp, ofc | 05:26 |
wrtp | fwereade__: 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 etc | 05:27 |
fwereade__ | wrtp, hmmm, interesting | 05:28 |
wrtp | fwereade__: it seems to me that almost all of the latter is currently covered by state.Unit | 05:28 |
fwereade__ | wrtp, yeah, things-managed-by-a-machine-agent sounds like a category worthy of investigation | 05:29 |
wrtp | fwereade__: 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 bit | 05:30 |
wrtp | fwereade__: i wondered about reserving all service names starting "juju-*" | 05:30 |
fwereade__ | wrtp, that definitely sgtm | 05:30 |
wrtp | fwereade__: i think it's important that the provisioner not be started by a unit agent | 05:31 |
fwereade__ | wrtp, why is that so? | 05:31 |
wrtp | fwereade__: because the whole reason for this is to have a place to put the agent version for upgrades | 05:31 |
wrtp | fwereade__: and if it's started by the unit agent, the unit agent uses that | 05:31 |
fwereade__ | wrtp, I think you need to expand on "a place to put the agent version for upgrades" before I can follow properly | 05:32 |
wrtp | fwereade__: ok, so each agent puts their current running version in the state so that you can see what's upgraded to what | 05:33 |
fwereade__ | wrtp, ok, yep | 05:33 |
wrtp | fwereade__: we've got {Machine,Unit}.SetAgentVersion | 05:33 |
wrtp | fwereade__: but nothing for Provisioner, because there's no such abstraction in State currently. | 05:34 |
wrtp | fwereade__: 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 |
wrtp | fwereade__: for instance, separating the firewaller from the provisioner would be trivial. | 05:35 |
wrtp | fwereade__: 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 practical | 05:36 |
fwereade__ | wrtp, but this feels like a digression slightly | 05:37 |
wrtp | fwereade__: 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 clicked | 05:38 |
wrtp | fwereade__: 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 not | 05:39 |
wrtp | fwereade__: 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 imagine | 05:39 |
fwereade__ | wrtp, seems more like a service-config thing really | 05:40 |
wrtp | fwereade__: indeed. | 05:40 |
wrtp | fwereade__: ah, that's interesting. | 05:40 |
wrtp | fwereade__: 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 example | 05: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 state | 05:42 |
fwereade__ | wrtp, and "participate in relations" | 05:42 |
wrtp | fwereade__: assume no relations | 05:42 |
fwereade__ | wrtp, the rest doesn't touch external state IIRC | 05:42 |
fwereade__ | wrtp, then presence, unit status, charm version | 05:42 |
wrtp | fwereade__: 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 fundamental | 05:43 |
fwereade__ | wrtp, I had this vague understanding that each individual agent was watching juju version and upgrading itself? | 05:43 |
wrtp | fwereade__: yes, that's right. juju version is now global. | 05:44 |
wrtp | fwereade__: and it sets something in the state to say when it's upgraded. | 05:44 |
wrtp | fwereade__: which status prints out, for example. | 05:45 |
wrtp | fwereade__: and while we'll need to watch when we do major-version upgrades. | 05:45 |
fwereade__ | wrtp, yep, ok | 05:45 |
wrtp | fwereade__: i'm not sure how that would work if the PA was started by a uniter | 05:46 |
wrtp | fwereade__: 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 problematic | 05:46 |
wrtp | fwereade__: 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 form | 05:48 |
fwereade__ | wrtp, I can live with that perspective too :) | 05:48 |
wrtp | fwereade__: thing is, i *think* that in terms of lines of code, it will be almost negligible | 05:48 |
wrtp | fwereade__: 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 |
wrtp | fwereade__: and that's what attracts me to the idea. | 05:50 |
wrtp | fwereade__: 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 smelly | 05:51 |
fwereade__ | wrtp, there are definitely good things floating around in this space though | 05:51 |
fwereade__ | wrtp, wait a mo | 05: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 thing | 05:53 |
wrtp | fwereade__: it's a *config.Config, not a *state.ConfigNode | 05:53 |
fwereade__ | wrtp, couple of conceptual levels up though | 05:53 |
wrtp | fwereade__: but that's only a recent thing | 05:53 |
fwereade__ | wrtp, what "is" the environment config if not the config of the juju service? | 05:54 |
wrtp | fwereade__: that seems like a nice way to look at it | 05:54 |
wrtp | fwereade__: that would mean we could put the machine agent under this umbrella too | 05:54 |
wrtp | maybe | 05:54 |
fwereade__ | wrtp, I think it *might* be, but I suspect the ramifications will be somewhat hefty even if it is | 05: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 somehow | 05:55 |
wrtp | fwereade__: ooh, that might actually solve some problems down the line | 05:55 |
wrtp | fwereade__: 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 services | 05:56 |
wrtp | fwereade__: 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 |
wrtp | fwereade__: but yes, that's what i was thinking. | 05:57 |
fwereade__ | wrtp, it does all definitely sound worthy of investigation :) | 06:01 |
wrtp | fwereade__: but that does imply uniterless units, i think | 06:01 |
fwereade__ | wrtp, indeed, we need an escape hatch *somewhere* in order to do things that nicely | 06:02 |
wrtp | fwereade__: unless the *uniter* is the first thing to run on a machine | 06:02 |
wrtp | fwereade__: can a hook do config-set, BTW? | 06:02 |
fwereade__ | wrtp, nope, which also may be a problem | 06:02 |
fwereade__ | wrtp, the only bits of state a unit agent even *can* write are status, charm, presence, and relation stuff | 06:03 |
wrtp | fwereade__: 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 service | 06:04 |
wrtp | fwereade__: but that probably goes against the whole concept of homegeneous units | 06:04 |
fwereade__ | wrtp, but I'm a little concerned about that depth of the rabbit hole | 06:04 |
wrtp | fwereade__: 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 |
wrtp | fwereade__: i think i'll give the PA idea a play and see what the code looks like. | 06:05 |
fwereade__ | wrtp, cool | 06:05 |
wrtp | fwereade__: 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 |
wrtp | davecheney: hiya | 06:23 |
wrtp | davecheney: if you're feeling so inclined, i'm looking for reviews of https://codereview.appspot.com/6490067/ | 06:25 |
davecheney | wrtp: OH MY GOD | 06:26 |
davecheney | today has been such a drama | 06:26 |
davecheney | at one point I had three sepearate tradesemen here | 06:26 |
davecheney | the tiler, manged to short out my power | 06:26 |
wrtp | davecheney: lol | 06:26 |
davecheney | then the eletrician had to some | 06:26 |
davecheney | and then the guy from the power compnay because the eletrician noticed the voltage was a bit high | 06:26 |
davecheney | and now the flooring guy just turned up to do a moisture test | 06:27 |
davecheney | anyway | 06:27 |
davecheney | drama is over | 06:27 |
davecheney | i wish I was bak in lisbon | 06:27 |
davecheney | most of the renovations happened when we were there | 06:27 |
wrtp | davecheney: do you know this song? http://www.youtube.com/watch?v=zyeMFSzPgGc | 06:27 |
davecheney | wrtp: i don't think this is a situation for levity | 06:28 |
wrtp | davecheney: you're probably right. but i've found that song to be spot on many times :-) | 06:29 |
davecheney | lol | 06:29 |
davecheney | wrtp: i'll review your CL in two secs | 06:31 |
wrtp | davecheney: thanks | 06:31 |
davecheney | just submitting one that has been waiting for my intertubes to return | 06:31 |
wrtp | davecheney: there's a follow-up too | 06:31 |
davecheney | wrtp: i've been enjoying watching your cleanups | 06:32 |
davecheney | refactor !! | 06:32 |
wrtp | davecheney: thanks | 06:32 |
davecheney | wrtp: for you https://codereview.appspot.com/6499071/ | 06:32 |
wrtp | davecheney: do it when you see it, i reckon | 06:32 |
davecheney | wrtp: absolutely | 06:32 |
wrtp | davecheney: 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 |
wrtp | davecheney: i.e. is there a time when it's valid to have an assigned machine id but no actual machine | 06:33 |
wrtp | ? | 06:33 |
wrtp | davecheney: " | 06:34 |
wrtp | All the non test consumers of this | 06:34 |
wrtp | method are actually after a *Machine, not the id anyway | 06:34 |
davecheney | wrtp: i can't see how, you have to pass a *state.Machine to unit.AssingToMachine | 06:34 |
wrtp | " | 06:34 |
wrtp | not *quite* true - status.go only uses the id :-) | 06:34 |
davecheney | status.go can suck it up | 06:35 |
wrtp | lol | 06:35 |
davecheney | firewaller also uses it, but it is a reasonable change | 06:35 |
davecheney | i ran across this writing juju ssh | 06:35 |
wrtp | davecheney: can you remove a machine without removing the units assigned to it? | 06:35 |
davecheney | wrtp: this is overall too much code to turn a unit name into an insatnce | 06:36 |
davecheney | http://paste.ubuntu.com/1185173/ | 06:36 |
davecheney | wrtp: i don't know the answer to that question | 06:36 |
wrtp | davecheney: it's an important question to answer in the context of that CL | 06:37 |
wrtp | davecheney: if you can, then it's a reason for AssignedMachineId | 06:37 |
wrtp | davecheney: or... maybe we say it can return nil. | 06:37 |
wrtp | or an error | 06:37 |
davecheney | return u.st.Machine(keySeq(machineKey)) | 06:38 |
davecheney | will return the error | 06:38 |
wrtp | davecheney: seems reasonable. | 06:39 |
fwereade__ | wrtp, davecheney: IIRC you cannot remove machines until they're empty | 06:41 |
davecheney | that sounds reasonable | 06: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 case | 06:41 |
fwereade__ | always | 06:41 |
wrtp | davecheney: i think we should have State.Unit - then your code could be: http://paste.ubuntu.com/1185191/ | 06:43 |
davecheney | wrtp: that was infact the first method I reached for | 06:44 |
davecheney | wrtp: and wouldn't the units' name be "unit/0" | 06:45 |
davecheney | so I wouldn't have to split c.Target at all | 06:45 |
wrtp | davecheney: indeed. | 06:45 |
wrtp | davecheney: but... i'm not entirely sure. | 06:45 |
wrtp | davecheney: in fact, yeah, that's right | 06:45 |
davecheney | func (s *State) Unit(name string) (unit *Unit, err error) Unit returns a unit by name. | 06:46 |
davecheney | DOH! | 06:46 |
davecheney | i'm a fuckwit | 06:46 |
davecheney | i think I didn't use that initially because I had the id of the unit/NUM | 06:46 |
wrtp | davecheney: oh yeah, i just looked for that and missed it | 06:46 |
davecheney | and I was frustrated that i had an int, but needed a string | 06:46 |
davecheney | still, i need to get from Unit -> assigned Machine -> Instance | 06:47 |
wrtp | davecheney: yeah. i think returning a *Machine makes total sense. | 06:47 |
wrtp | davecheney: i can't think of any case where the id is useful or even significant by itself. | 06:47 |
davecheney | and Machine.Id() does not return an err | 06:48 |
davecheney | so it is much easier to use than state.Machine(id) | 06:48 |
davecheney | which does | 06:48 |
wrtp | davecheney: indeed. | 06:48 |
wrtp | davecheney: yup | 06:48 |
davecheney | wrtp: http://paste.ubuntu.com/1185220/ | 07:04 |
davecheney | getting better, and I can reduce it further with my other CL | 07:04 |
wrtp | davecheney: not entirely sure | 07:05 |
wrtp | davecheney: the Unit request can fail because of more than just the name being malformed, | 07:05 |
wrtp | davecheney: i think that strings.Index(name, "/') > 0 might be a better test | 07:05 |
wrtp | davecheney: that way you don't incur the round trip when it's not a unit | 07:06 |
davecheney | wrtp: will do, the python code also included that logic | 07:06 |
davecheney | wrtp: speaking of round trips | 07:06 |
davecheney | adding mstate.Info | 07:06 |
davecheney | we're going to reuse the UseSSH logic to establish a tunned to mgo | 07:06 |
davecheney | but in the future there exists a possibility of using TLS directly | 07:06 |
wrtp | davecheney: i'd love that ssh tunnelling code to disappear | 07:07 |
wrtp | davecheney: it was a right pain to write, and i still don't fully trust it | 07:07 |
davecheney | given we control the mgo code, it might be easier to use the go.crypto ssh package | 07:07 |
davecheney | but that is bordering on out of scope | 07:08 |
davecheney | i am very confident of the tcp forwarding code, several of us have been banging on that for months | 07:08 |
davecheney | wrtp: the state already checks the unit name for us | 07:10 |
davecheney | % juju ssh mysql/q | 07:10 |
davecheney | error: cannot get unit "mysql/q": "mysql/q" is not a valid unit name | 07:10 |
davecheney | wrtp: btw +1 to your change to always make a state connection when we open juju.Conn | 07:11 |
wrtp | davecheney: good point. | 07:11 |
davecheney | anyway, i'll leave that CL for the moment | 07:12 |
davecheney | the tests for it are going to be a days work | 07:12 |
davecheney | we have to mock *everything* | 07:12 |
davecheney | wrtp: sorry, still reading your CL | 07:17 |
wrtp | davecheney: which CL is that? | 07:17 |
davecheney | the one you pasted me 30 mins ago | 07:18 |
wrtp | davecheney: i mean, the one you're gonna leave | 07:18 |
davecheney | oh, juju ssh | 07:19 |
davecheney | i haven't proposed that yet | 07:19 |
davecheney | will finalise it tomorrow | 07:19 |
davecheney | i'm sort of jumping between a few at the moment | 07:20 |
davecheney | in good news, juju deploy is getting very solid | 07:21 |
davecheney | juju bootstrap ; juju deploy mysql ; juju deploy mongodb ; juju add-unit mongodb ; juju add-unit -n2 mysql ; juju status | 07:21 |
davecheney | just works | 07:21 |
wrtp | davecheney: cool | 07:40 |
davecheney | wrtp: in reusing the UseSSH code from state/ssh.go | 07:58 |
davecheney | do you think it is worth moving it into another package | 07:58 |
davecheney | or just copy it to mstate as we're nix'ing state soon ? | 07:58 |
wrtp | davecheney: the latter | 07:58 |
davecheney | wrtp: roger, roger | 07:58 |
wrtp | davecheney: i want that code to go! | 07:58 |
wrtp | davecheney: and giving it its own package is not the way forward :-) | 07:59 |
davecheney | wrtp: no, we don't want to give it an endorsement | 07:59 |
davecheney | wrtp: https://bugs.launchpad.net/mgo/+bug/1045678 | 08:05 |
wrtp | davecheney: +1. | 08:07 |
wrtp | davecheney: or even an io.ReadWriteCloser | 08:07 |
davecheney | sure, if mgo doesn't need to know Local/RemoteAddr | 08:07 |
Aram | moin. | 08:11 |
davecheney | hey | 08:12 |
wrtp | davecheney: 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 |
davecheney | wrtp: i'd like to see them split out, but it's not a strong objection | 08:32 |
wrtp | davecheney: ok, i'll split 'em out. there's no dependency on them i think. | 08:32 |
wrtp | davecheney: BTW i suspect you might have been the one that used Errorf in opRecvTimeout. any particular reason for it? | 08:33 |
Aram | anyone seen mramm lately? | 08:37 |
TheMue | good morning | 08:39 |
Aram | morning. | 08:39 |
fwereade__ | right, I think I might actually be happy with the charm upgrades at last -- taking a short break before polish and propose | 08:45 |
fwereade__ | (good mornings to those I have not thus far addressed) | 08:45 |
niemeyer | Gooood morning rockstars | 10:14 |
fwereade__ | niemeyer, heyhey | 10:19 |
TheMue | niemeyer: hiya | 10:20 |
fwereade__ | huh: | 10:52 |
fwereade__ | runtime: signal received on thread not created by Go. | 10:52 |
fwereade__ | FAILlaunchpad.net/juju-core/state21.458s | 10:52 |
fwereade__ | anyone seen that before? | 10:52 |
davecheney | fwereade__: yes | 10:52 |
davecheney | not in state | 10:53 |
fwereade__ | davecheney, is it something I did? :) | 10:53 |
Aram | no. | 10:53 |
Aram | it happens sometimes. | 10:53 |
fwereade__ | is there a bug for it? | 10:53 |
davecheney | it's an artifact of the gozk c bindings | 10:53 |
Aram | it's poor zookeeper cgo interraction. | 10:53 |
davecheney | fwereade__: probably should raise one | 10:53 |
fwereade__ | ah, ok, cool, I'll go and do that | 10:54 |
Aram | there is one bug in the Go tracker. | 10:54 |
fwereade__ | Aram, for gozk? | 10:54 |
davecheney | Aram: there is a googler who is playing in that area at the moment | 10:54 |
Aram | not for gozk, for cgo. | 10:54 |
Aram | cgo is at fault. | 10:54 |
fwereade__ | Aram, ahh, ok | 10:54 |
Aram | nothing we can do, sadly. | 10:54 |
fwereade__ | Aram, very well, I shall wait for state to go away entirely then :) | 10:55 |
davecheney | jolly good, carry on | 10:55 |
davecheney | fwereade__: longer version, it is an argument about who owns the signal handler | 10:56 |
davecheney | if a signal is delivered to a thread while in c code, the go signal handler doesn't have the proper registers setup to handle it | 10:56 |
davecheney | so it has to panic | 10:56 |
fwereade__ | davecheney, good to know, thank you | 10:58 |
davecheney | fwereade__: there isn't much of a solution at the moment, apart from 'don't use so much cgo you darn kids' | 10:58 |
davecheney | c'mon, lets get this show on the road | 10:59 |
wrtp | davecheney: ah! i'd wondered where that came from. | 10:59 |
niemeyer | Party time! | 11:00 |
niemeyer | Invites sent | 11:03 |
Aram | https://plus.google.com/hangouts/_/3016131d787ecda60f236d1dec4e32264e3353ad?authuser=0&hl=en | 11:03 |
niemeyer | davecheney, fwereade__, wrtp, TheMue, Aram | 11:03 |
Aram | "google can't load CSS", wow, never seen that one. | 11:04 |
davecheney | https://bugs.launchpad.net/juju-core/+bug/1045151 | 11:29 |
davecheney | niemeyer: Aram ^^ | 11:29 |
davecheney | state.waitForInitialised | 11:29 |
Aram | that doesn't require a watcher though. | 11:30 |
TheMue | lunchtime | 11: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 me | 12:27 |
Aram | is there a way to turn line numbers off in codereview? | 12:27 |
niemeyer | fwereade__: Sure.. I'll have a look | 12:31 |
niemeyer | fwereade__: There some low-hanging fruits to split there, but I'll look as-is | 12: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 large | 12: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 hunk | 12:34 |
fwereade__ | niemeyer_, indeed, I know they have been less than ideal lately, I shall try to do better :) | 12:35 |
wrtp | niemeyer_: 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 |
wrtp | niemeyer_: 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 |
wrtp | niemeyer_: ^ | 12:58 |
fwereade__ | late lunch, bbiab | 12:59 |
wrtp | fwereade__: hmm lunch, good idea | 12: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 | ||
niemeyer | wrtp: That's writing safer concurrent code. If you're unhappy with that, I'll have to apologize. | 13:05 |
wrtp | niemeyer: 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 |
wrtp | niemeyer: both approaches are "safe". one has slightly simpler code which is easier to verify IMHO. | 13:07 |
niemeyer | wrtp: 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 |
niemeyer | fwereade__: Yes, I think it's fine for us to do that rigth now | 13:31 |
fwereade__ | niemeyer, cool, cheers | 13:31 |
niemeyer | fwereade__: The UA shouldn't have access to the whole environment config, but when we fix that we can do whatever | 13:31 |
fwereade__ | niemeyer, yeah, I understand we don't want it to have access long-term | 13:32 |
TheMue | niemeyer: 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 |
niemeyer | TheMue: Single instance for State | 13:41 |
TheMue | niemeyer: ok | 13:42 |
TheMue | niemeyer: currently i have no overview. do we need watchers for all collections? | 13:42 |
niemeyer | TheMue: We need the watchers we'll actually use | 13:43 |
niemeyer | TheMue: The watchers being used by workers are a good guideline to start with | 13:43 |
TheMue | niemeyer: ok, i have to scan the code | 13:43 |
TheMue | niemeyer: thx | 13:43 |
niemeyer | TheMue: np | 13:44 |
niemeyer | TheMue: Are you busy or would you have a moment? | 13:58 |
niemeyer | TheMue: Would just like to run an idea I exchanged with wrtp the other day, for the next time you get blocked | 13:59 |
TheMue | niemeyer: i'm integrating your presence in unit.AgentAlive() and co. | 13:59 |
TheMue | niemeyer: ok, i'm listening | 13:59 |
niemeyer | TheMue: Right now we have the handling of environment somewhat hardcoded in each different worker | 13:59 |
niemeyer | TheMue: It'd be nice to have a shared way to ensure a worker is handling the environment loading and reloading basically correctly | 14:00 |
niemeyer | TheMue: One idea we came up with was to have an observer channel being passed into the worker | 14:00 |
TheMue | niemeyer: yeah, i've seen that wrtp has been already active regarding provisioner and firewaller | 14:01 |
niemeyer | TheMue: If not nil, we send the environment into the channel every time the local "<w>.environ" property is set | 14:01 |
niemeyer | TheMue: So that we can create a shared test suite that is run against any worker | 14:01 |
niemeyer | TheMue: Makes sense? | 14:02 |
wrtp | niemeyer: i thought we decided that that wasn't a great way to go | 14:02 |
TheMue | niemeyer: a peep whole ;) | 14:02 |
TheMue | niemeyer: yes, sounds good | 14:02 |
TheMue | wrtp: why not? | 14:03 |
niemeyer | Aug 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 |
niemeyer | Aug 31 10:11:36 <niemeyer> rogpeppe: Will do | 14:03 |
* wrtp goes back to look | 14:03 | |
niemeyer | wrtp: My memories and this makes it feel like we agreed on something else | 14:03 |
niemeyer | wrtp: But I'm happy to talk agan | 14:03 |
niemeyer | again | 14:03 |
TheMue | niemeyer: the way right now with the op codes and the secret doesn't feel very good | 14:05 |
niemeyer | TheMue: Yeah, it's a good interim solution | 14:05 |
TheMue | niemeyer: exactly | 14:05 |
wrtp | niemeyer: hmm, i remembered more discussion than that | 14:05 |
niemeyer | TheMue: But it'd be nice to have a way we can reuse for all the workers, instead of having to hack it together | 14:05 |
niemeyer | wrtp: It was a G+ call | 14:05 |
TheMue | niemeyer: absolutely | 14:05 |
niemeyer | wrtp: Still, I have vivid memories of the agreement | 14:06 |
* wrtp wishes we could archive G+ calls | 14:06 | |
niemeyer | wrtp: I recall even you saying you'd prefer to have the channel passing in through the New function | 14:06 |
niemeyer | wrtp: While my original proposal was to have a method | 14:06 |
TheMue | niemeyer: 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 |
TheMue | niemeyer: could get a neat interface EnvironTester { SetEnvironChan(ch) } | 14:10 |
* wrtp is still trying to remember the context in detail | 14:10 | |
wrtp | TheMue: the problem with that is that it has to interact with the worker to set the channel. | 14:11 |
wrtp | TheMue: and it would probably need a channel to do that, so we don't gain anything. | 14:11 |
niemeyer | wrtp++ | 14:11 |
TheMue | wrtp: sh.., yes | 14:12 |
wrtp | niemeyer: i can't remember if we decided it was best to have a chan of environs.Environ or of *config.Config | 14:14 |
wrtp | ah, i remember! | 14:14 |
niemeyer | wrtp: It was environ the whole time, IIRC. The point was ensuring that it was properly loaded | 14:15 |
TheMue | niemeyer: thx btw, will go in. the other one is WIP, i need to discuss something for it with you | 14:15 |
niemeyer | TheMue: Sure, what's up? | 14:15 |
TheMue | niemeyer: so far the agent alive flag has been a zk node below the entity node | 14:15 |
TheMue | niemeyer: now we don't have that hierarchy. | 14:16 |
niemeyer | TheMue: Indeed | 14:16 |
wrtp | niemeyer: my confusion had been about the discussion we had about an *incoming* channel | 14:16 |
wrtp | niemeyer: i didn't read your initial comment above properly, doh! | 14:16 |
niemeyer | wrtp: Ah, no worries | 14:16 |
TheMue | niemeyer: shall i add a collectiong for alive like for config nodes, with own namesspaces "m/..." and "u/..."? | 14:17 |
TheMue | niemeyer: so presence.Watcher and Pinger would act on this collection. | 14:18 |
niemeyer | TheMue: Sorry, I kind of know what you're talking about, but given the way you phrase it I'm not sure | 14:19 |
TheMue | niemeyer: oh, sorry, will rephrase it. | 14:19 |
TheMue | niemeyer: 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 |
niemeyer | TheMue: As a side note, I quickly agreed with Aram that we'll move towards using "m#...", "u#...", etc | 14:20 |
niemeyer | TheMue: This avoids the ambiguity of "m/1" being both a unit name and a resource key | 14:21 |
TheMue | niemeyer: ok, will change it in my code. | 14:21 |
niemeyer | TheMue: Okay, if I understand your question, for machine 1 we should watch/ping "m#1" | 14:22 |
TheMue | niemeyer: yes, exactly. or u#wordpress/1. | 14:23 |
niemeyer | TheMue: +1 | 14:23 |
TheMue | niemeyer: cheers, will do it for units first and then machines | 14:24 |
TheMue | niemeyer: and both use the same collection | 14:24 |
niemeyer | TheMue: Btw, the pinger should live in a separate MongoDB database | 14:25 |
niemeyer | TheMue: "presence", probably | 14:25 |
niemeyer | TheMue: This means it gets its own write lock | 14:25 |
niemeyer | TheMue: Which is relevant given how much traffic this may end up happening, and how unrelated to the activity in "juju" it is | 14:25 |
TheMue | niemeyer: hmm, ok, have to take a look how that works. | 14:26 |
niemeyer | TheMue: No big deal.. just a different session.DB | 14:26 |
TheMue | niemeyer: but it sounds reasonable | 14:26 |
TheMue | niemeyer: ok | 14:26 |
* TheMue loves mgo | 14:26 | |
niemeyer | TheMue: As another idea, I think we may end up having a type entity interface { entityKey() string } | 14:27 |
TheMue | niemeyer: +1 | 14:27 |
niemeyer | TheMue: That unifies all things "C#ID" | 14:27 |
niemeyer | Or most, anyway | 14:27 |
wrtp | niemeyer: upgrade-juju command now works: https://codereview.appspot.com/6498085 | 14:28 |
niemeyer | wrtp: Woohay! | 14:28 |
niemeyer | wrtp: Did you actually see the thing working live? | 14:28 |
wrtp | niemeyer: i'll just try it now. i have the utmost confidence :-) | 14:29 |
niemeyer | wrtp: Man, this is so awesome | 14:29 |
wrtp | niemeyer: ('cos it actually doesn't do much) | 14:29 |
* wrtp goes for a drink while the amazon test runs | 14:31 | |
wrtp | jeeze it's slow | 14:42 |
wrtp | *still* uploading the same set of tools. | 14:47 |
wrtp | maybe it's hung. you just canna tell. | 14:50 |
niemeyer | https://codereview.appspot.com/6490067/diff/3018/cmd/juju/cmd_test.go | 15:11 |
niemeyer | wrtp: On that CL, | 15:11 |
niemeyer | wrtp: time.Sleep(500 * time.Millisecond) | 15:11 |
wrtp | niemeyer: yeah, looks spurious | 15:11 |
niemeyer | wrtp: Weren't you complaining about timings just yesterday? | 15:11 |
niemeyer | :) | 15:11 |
niemeyer | wrtp: Let's please not do this | 15:11 |
wrtp | niemeyer: i'm just removing it - i think it was from a debug session | 15:11 |
niemeyer | wrtp: I can imagine why you had it there | 15:12 |
niemeyer | wrtp: I don't think the error reordering is entirely right either.. | 15:12 |
wrtp | niemeyer: it makes for a much more useful diagnostic | 15:13 |
niemeyer | wrtp: Ah, you've added buffering to opc | 15:13 |
wrtp | niemeyer: if the command has died with an error, you get to see the error rather than a "timed out" message | 15:13 |
wrtp | niemeyer: yeah | 15:13 |
niemeyer | wrtp: Okay, I guess that should work | 15:13 |
wrtp | niemeyer: i made the change because the diagnostics were terrible when it failed... | 15:13 |
niemeyer | wrtp: +1 | 15:13 |
wrtp | niemeyer: will remove the sleep pronto | 15:14 |
niemeyer | wrtp: Can we bump a bit further along the 20 number? Like 256? | 15:14 |
niemeyer | wrtp: We'll never see the wasted memory, and it may save us precious debugging time some day | 15:14 |
wrtp | niemeyer: sure. although given that it's only 2 in practice... | 15:14 |
niemeyer | wrtp: Okay, 64 then ;) | 15:15 |
niemeyer | wrtp: The point is really to get unrealistically large | 15:15 |
niemeyer | wrtp: So we don't have to debug a most extraneous bug some day | 15:16 |
wrtp | niemeyer: i thought 20 already was, but will change. 50 perhaps. can't really see why a power of two is useful here. | 15:16 |
niemeyer | wrtp: Although, the whole logic will probably need some caring for :( | 15:16 |
niemeyer | wrtp: Since it has hardcoded ops | 15:16 |
wrtp | niemeyer: that's probably true. but not in scope for this CL | 15:17 |
wrtp | niemeyer: i've wondered about giving dummy.Listen a mask of operations we're interested in. | 15:17 |
wrtp | niemeyer: then hardcoded ops would be just fine, i think. | 15:18 |
niemeyer | wrtp: I'd rather aim for more useful tests.. | 15:20 |
niemeyer | c.Check((<-opc).(dummy.OpPutFile).Env, Equals, "peckham") | 15:20 |
niemeyer | What is this telling us really? | 15:20 |
niemeyer | That a method was called, in whatever circumstances, with whatever arguments.. | 15:20 |
niemeyer | THUS, IT MUST WORK! | 15:20 |
wrtp | niemeyer: agreed. we should test that the tools now exist in the environment | 15:21 |
* niemeyer <= mocking disbeliever | 15:21 | |
wrtp | niemeyer: and that we can connect to the state, etc. | 15:21 |
wrtp | niemeyer: that's much easier with the infrastructure we have now actually | 15:21 |
niemeyer | wrtp: Yeah | 15:21 |
wrtp | niemeyer: i'll file a bug | 15:22 |
niemeyer | wrtp: Cheers.. something for us to prize, if nothing else :) | 15:22 |
wrtp | niemeyer: done. also, sleep removed and buffer size increased. | 15:30 |
niemeyer | wrtp: Cheers | 15: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 |
wrtp | fwereade__: how long have you waited? | 15:40 |
fwereade__ | wrtp, maybe 10 mins | 15:41 |
wrtp | fwereade__: wait another 5 mins perhaps | 15:41 |
fwereade__ | wrtp, why would that happen? | 15:41 |
wrtp | fwereade__: 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 that | 15:42 |
wrtp | fwereade__: 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 |
wrtp | fwereade__: ah, so that message is from the machine agent log? | 15:43 |
fwereade__ | wrtp, yeah | 15:43 |
wrtp | fwereade__: did you run the initial bootstrap with --verbose --debug? | 15:43 |
fwereade__ | wrtp, gah, no | 15:43 |
wrtp | fwereade__: ISTR that that flag propagates through to the bootstrap machine and thence to the machines it starts | 15:44 |
wrtp | fwereade__: have you tried sshing to the machine it's trying to connect to? | 15:45 |
wrtp | fwereade__: 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, no | 15:45 |
fwereade__ | wrtp, status is fine I think | 15:45 |
wrtp | fwereade__: hmm, what about sshing to the machine agent's machine, then trying to ssh from that machine to the address listed above | 15:46 |
wrtp | fwereade__: perhaps it can't use the external address internally or something | 15:46 |
fwereade__ | wrtp, ah-HA, security groups | 15:50 |
fwereade__ | wrtp, opened 2181, stuff is happening | 15:50 |
fwereade__ | wrtp, but... | 15:50 |
fwereade__ | 2012/09/04 15:49:43 JUJU deploying unit etherpad-lite/0 | 15: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 $PATH | 15:50 |
fwereade__ | 2012/09/04 15:49:43 JUJU machiner waiting for change | 15:50 |
fwereade__ | wrtp, and I need to be off :( | 15:50 |
wrtp | fwereade__: 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 22 | 15:51 |
wrtp | fwereade__: interesting. i'll check. | 15:51 |
wrtp | fwereade__: and cloud-init should set the PATH | 15:51 |
wrtp | fwereade__: good stuff anyway! | 15:51 |
wrtp | fwereade__: environs/ec2/ec2.go:664: // TODO authorize internal traffic | 15:54 |
wrtp | :-) | 15:54 |
niemeyer | wrtp: Sent a review.. | 16:06 |
niemeyer | wrtp: It looks great in general | 16:06 |
wrtp | niemeyer: brill, thanks | 16:06 |
niemeyer | wrtp: There are a couple of things to sort out, one of them is connected to the test stuff we were just talking | 16:07 |
niemeyer | wrtp: The other is about environ config changes | 16:07 |
wrtp | niemeyer: yeah, i wondered how you'd take that | 16:07 |
niemeyer | wrtp: 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 |
wrtp | niemeyer: i wanted to avoid timeouts | 16:07 |
niemeyer | wrtp: Sure.. that's a great goal.. but there are many ways to do it | 16:08 |
niemeyer | Anyway, lunch | 16:08 |
niemeyer | biab | 16:08 |
wrtp | niemeyer: enjoy! | 16:08 |
TheMue | niemeyer: presence integration in unit works great *hug* | 16:29 |
wrtp | fwereade__: i've got a fix for the internal traffic TODO. will propose tomorrow. | 17:35 |
* wrtp is off for the night. see y'all tomorrow | 17:35 | |
niemeyer | wrtp: Night! | 18:04 |
fwereade__ | wrtp, you rock :) | 18:56 |
niemeyer | fwereade__: ping | 19:17 |
fwereade_ | niemeyer, pong | 20:43 |
niemeyer | fwereade_: yo | 20:44 |
fwereade_ | niemeyer, how's it going? | 20:44 |
niemeyer | fwereade_: Was just wondering if you had any input on the ForceRefresh conversation with TheMue | 20:44 |
fwereade_ | niemeyer, sorry, no | 20:44 |
niemeyer | fwereade_: But no to worry, it's late for you | 20:44 |
fwereade_ | niemeyer, cath and laura are asleep, I'm weighing my options -- if I can help then I'm happy to | 20:45 |
niemeyer | fwereade_: 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 way | 20: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 upgrades | 20: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 tomorrow | 20:49 |
fwereade_ | niemeyer, although none of the breaks there are quite as clean as I would hope | 20:50 |
niemeyer | fwereade_: I haven't spent time on it, and I wouldn't mind waiting until tomorrow morning as the plate is full ATM | 20:50 |
niemeyer | fwereade_: This will likely improve your progress on it too | 20:51 |
niemeyer | fwereade_: As we'll surely be able to starting merging bits sooner rather than later | 20:51 |
niemeyer | start | 20:51 |
fwereade_ | niemeyer, hopefully so -- deployer is a nicely separable chunk, at the very least | 20:51 |
fwereade_ | niemeyer, (and that then leads me to consider another possible partitioning...) | 20:52 |
* fwereade_ just wishes we had a good multi-prereq story | 20:53 | |
niemeyer | fwereade_: =1 | 20:54 |
niemeyer | +1 | 20:54 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!