| davecheney | shitballs, I deleted by ssh branch before pushing it | 03:26 |
|---|---|---|
| davecheney | crap -- have to start again | 03:26 |
| wrtp | davecheney: aw shit, i've managed to avoid doing that so far, but i've come close | 06:36 |
| wrtp | davecheney: morning BTW! | 06:37 |
| wrtp | fwereade: hiya | 06:37 |
| fwereade | wrtp, heyhey | 06:38 |
| wrtp | davecheney: i wanted to use that branch too | 06:38 |
| fwereade | davecheney, morning | 06:38 |
| fwereade | wrtp, sorry I haven't reviewed it properly yet, I'll do that right now | 06:39 |
| wrtp | fwereade: np. that would be great, ta! | 06:39 |
| davecheney | wrtp: s'ok, rewritten already | 06:41 |
| wrtp | fwereade: is there are reason that state.Initialize doesn't take a *config.Config ? | 06:41 |
| wrtp | s/are/a/ | 06:41 |
| davecheney | wrtp: maybe that is my failt | 06:41 |
| davecheney | fault | 06:41 |
| fwereade | wrtp, I don't think so, no | 06:41 |
| davecheney | wrtp: also, since I patched that issue in the TLS lib | 06:41 |
| fwereade | wrtp, ie I think it probably should | 06:41 |
| davecheney | not a single EOF from etc | 06:41 |
| wrtp | fwereade: i was just looking at if len(c.EnvConfig) == 0 and thinking that it should probably be checking that the config was accepted by the config package | 06:42 |
| davecheney | also, if you are following tip, cgo is broken again, http://codereview.appspot.com/6501101/ | 06:42 |
| davecheney | ^ apply this for general satisfaction | 06:42 |
| wrtp | davecheney: i haven't followed tip for a while now, as i haven't been working on go core stuff. there is something i've been meaning to do though, and i may have some time in the next few days as carmen's away for a bit. | 06:44 |
| davecheney | wrtp: some awesome imrpovements to 6g coming | 06:45 |
| davecheney | at least 15% improvement across the board | 06:45 |
| davecheney | and substantial reduction in stack size | 06:45 |
| wrtp | davecheney: which bit are you referring to? | 06:45 |
| wrtp | davecheney: (i've been following golang-dev, but skimming it and missing quite a bit) | 06:45 |
| davecheney | http://codereview.appspot.com/6501110/ | 06:46 |
| davecheney | http://codereview.appspot.com/6494107/ | 06:46 |
| davecheney | and so forth | 06:46 |
| davecheney | filtering out all the LEAQ's | 06:46 |
| davecheney | issue 1914 | 06:46 |
| wrtp | davecheney: great. yeah, there's so much potential optimisation. | 06:47 |
| wrtp | davecheney: i'm really looking forward to russ's Grand Optimisation branch whenever it might arrive | 06:47 |
| davecheney | wrtp: this one is nice too http://codereview.appspot.com/6506089/ | 06:47 |
| davecheney | the big improvements are in stack usage | 06:49 |
| davecheney | reflect.Value.call consumes a kilobyte of stack on x64! | 06:49 |
| wrtp | davecheney: that will help a lot | 06:49 |
| wrtp | davecheney: woah | 06:49 |
| davecheney | remy has shaved 1/4 off that | 06:49 |
| davecheney | but it's stupid large | 06:49 |
| davecheney | so any time that method comes near you, it's a stack split for sure | 06:50 |
| davecheney | and an expensive one | 06:50 |
| wrtp | davecheney: i'm not too surprised - it's a complex piece of code | 06:50 |
| davecheney | nah, it's just long | 06:50 |
| wrtp | davecheney: things will be better when the runtime knows what size of stack is likely to be necessary. | 06:50 |
| davecheney | if it was split up | 06:50 |
| davecheney | the stack could be reused because it works top to bottom | 06:50 |
| davecheney | wrtp: i hope that is more than a pipe dream | 06:51 |
| wrtp | davecheney: i don't see why it shouldn't work. a best-case stack size calculation should not be out of reach | 06:51 |
| davecheney | wrtp: lp:~dave-cheney/juju-core/093-cmd-juju-ssh/ | 06:51 |
| wrtp | davecheney: another optimisation i thought would be nice is to cache the function value from the itab when an interface value doesn't change. | 06:53 |
| davecheney | wrtp: daniel morosing has been working on that one for a while | 06:53 |
| wrtp | davecheney: that could save quite a bit in tight loops calling interfaces | 06:53 |
| davecheney | wrtp: http://codereview.appspot.com/6351090/ ?maybe? | 06:54 |
| wrtp | davecheney: i don't think so | 06:55 |
| wrtp | davecheney: that's about interface type conversion; i'm just talking about interface calling | 06:55 |
| davecheney | wrtp: does taht require runtime support ? | 06:55 |
| davecheney | i'm guessing it does, but for some reason I can't think of where it is in the runtime | 06:56 |
| wrtp | davecheney: i don't think so | 06:56 |
| wrtp | davecheney: the compiler generates the code to call methods | 06:56 |
| davecheney | which must take the interface type, and look it up in a table to get the func addr of the i impl | 06:57 |
| davecheney | anyone heard from mark lately ? | 06:58 |
| fwereade | wrtp, sorry, missed you; you may be right, I was trying to avoid getting sidetracked into fixing too much in one CL though :) | 06:59 |
| fwereade | wrtp, reviewed https://codereview.appspot.com/6501106/ | 06:59 |
| wrtp | davecheney: yeah, probably best in another CL | 06:59 |
| wrtp | fwereade: thanks! | 07:00 |
| wrtp | fwereade: i used VarDir because it was niemeyer's suggestion originally ("all directories are juju directories") | 07:00 |
| fwereade | wrtp, you probably won't agree with it all | 07:00 |
| wrtp | fwereade: but i'd personally be very happy to use JujuDir. | 07:00 |
| wrtp | fwereade: in fact i agree it's a better name | 07:01 |
| fwereade | wrtp, cool :) | 07:01 |
| fwereade | wrtp, we'll have to see how it does then :)_ | 07:01 |
| fwereade | wrtp, JujuRoot, perhaps? | 07:01 |
| wrtp | fwereade: how about Machiner.simpleContainer ? | 07:02 |
| fwereade | wrtp, (btw, sorry, I feel the review comments skewed rather negative, the "lots to like" is the overriding impression though) | 07:02 |
| wrtp | fwereade: we can't stick with container.Simple as a global | 07:02 |
| wrtp | fwereade: as we need to be able to pass in VarDir | 07:03 |
| fwereade | wrtp, well, outside of testing, when do we need to do that? | 07:03 |
| wrtp | fwereade: always | 07:03 |
| wrtp | fwereade: there's no global VarDir any more | 07:03 |
| wrtp | fwereade: and you can invoke commands with --juju-dir | 07:04 |
| wrtp | fwereade: (which works now, BTW) | 07:04 |
| fwereade | wrtp, hmm, I think I have an odd perspective on this, but I did a bunch of agent stuff over the w/e, and I'm (say) 90% sure that an Agent type is (1) a good thing and (2) the appropriate home for JujuDir | 07:05 |
| fwereade | wrtp, so ISTM that it's not actually the container's responsibility (but that initDir and logDir are) | 07:05 |
| wrtp | fwereade: i'm not sure what you mean by "the ... home for JujuDir" | 07:06 |
| wrtp | fwereade: it's just a parameter | 07:06 |
| fwereade | wrtp, it's a parameter used in several places, all of which IMO become nicer once an Agent type is added | 07:07 |
| wrtp | fwereade: so an Agent would be a parameter to the Container? | 07:08 |
| fwereade | wrtp, perhaps it would help if I propose -wip my current branch (which is very very unfinished but I hope instructive) | 07:08 |
| fwereade | wrtp, I *think* so, yes, it comes out quite nice IMO | 07:08 |
| wrtp | fwereade: i'll maybe give you a glance at another unfinished branch of mine to get your thoughts too | 07:10 |
| fwereade | wrtp, cool -- I'm a little worried that we may be about to collide actually, a lot of the ugliness in https://codereview.appspot.com/6500095 will evaporate once your VarDir branch is merged | 07:11 |
| fwereade | wrtp, the underlying insight that led me in this direction is that all the agents really *are* the same in that they should all be named, because they do in fact all correspond to a single state entity, and that messing around with --machine-id and --unit-name and not-having-one-for-provisioning is actually a side-effect of missing this insight | 07:15 |
| fwereade | wrtp, ofc it may, as always, be crack | 07:15 |
| wrtp | fwereade: i'm absorbing it | 07:16 |
| wrtp | fwereade: i was thinking about something a little similar in some ways actually | 07:16 |
| wrtp | fwereade: i wondered if we could have a single "jujud agent <agent-name>" command | 07:16 |
| fwereade | wrtp, I've been keeping that out of mind | 07:17 |
| fwereade | wrtp, because I think it is *probably* right but that we are not quite there yet | 07:17 |
| fwereade | wrtp, once we have upgraders for everything, and (if it passes muster) agent.Agent, it will I think be a good time to move the upgrading task out of the Kind-specific tasks, and make it all happen at the Agent level | 07:18 |
| wrtp | fwereade: the one thing i'm not sure about is having a single agent.Run for all the agents. | 07:19 |
| fwereade | wrtp, ISTM that the list-of-tasks abstraction is an excellent one | 07:19 |
| wrtp | fwereade: i think that's the kind of direction i was heading with my "runner" package and it was deemed crackful | 07:19 |
| fwereade | wrtp, bah | 07:19 |
| fwereade | wrtp, my feeling is that list-of-tasks is *exactly* the thing that differentiates multiple agents | 07:20 |
| wrtp | fwereade: it depends whether we think that *all* agents *always* will be a simple set of concurrent tasks | 07:20 |
| fwereade | wrtp, that STM to be the assumption underlying the worker package, and it seems to have served us well so far | 07:21 |
| wrtp | fwereade: i'm not sure that's true actually | 07:21 |
| wrtp | fwereade: the workers can be used in any way | 07:21 |
| wrtp | fwereade: they *happen* to implement the same interface, but there's no requirement that they do so | 07:22 |
| wrtp | fwereade: i do like the the factoring-out of the UpgradedError logic though. | 07:24 |
| wrtp | fwereade: BTW why does this code deserve its own package? it seems like it would still live well in cmd/jujud | 07:24 |
| wrtp | fwereade: ah! but you want to pass Agents around to other packages. | 07:25 |
| fwereade | wrtp, yep, exactly | 07:25 |
| fwereade | wrtp, like I say, might all be crack, but it seems like a small amount of code that is useful in several places | 07:25 |
| wrtp | fwereade: i'm not sure. i think that simply passing JujuDir as a parameter work well for many of the methods | 07:26 |
| wrtp | s/work/works/ | 07:26 |
| wrtp | fwereade: and agent name when appropriate, i guess | 07:27 |
| fwereade | wrtp, and agent kind... | 07:27 |
| fwereade | wrtp, and frequently state info... | 07:27 |
| wrtp | fwereade: when do we need agent kind? | 07:27 |
| fwereade | wrtp, any time we want to get a tools dir (according to the agent-foo-123, provisioning-whatever naming scheme I thought we discussed on friday) | 07:28 |
| wrtp | fwereade: that comes from the agent name, no? | 07:28 |
| fwereade | wrtp, ah, ok, sorry, I misunderstood which name you were talking about | 07:28 |
| wrtp | fwereade: i'm thinking that every agent has a unique name | 07:29 |
| fwereade | er sorry s/agent-foo/unit-foo/ | 07:29 |
| wrtp | fwereade: i think i'd be happier if the agent package had stuff for agent identification and location only, and all the *actual* agent logic lived elsewhere. | 07:30 |
| fwereade | wrtp, I am also thinking that, but I've called it "badge" because ISTM that the *name* is the name of the attached state entity, and the agent itself is somewhat different | 07:30 |
| wrtp | fwereade: when would you need "name" instead of "badge"? | 07:30 |
| fwereade | wrtp, if one takes the Agent abstraction seriously it becomes useful to pass a *Agent into (eg) uniter, instead of the name/dir bits, and then the unit name is directly accessible from there | 07:32 |
| wrtp | fwereade: but the uniter knows the *Unit? and the name comes from that, no? or perhaps the name is something else and i'm misunderstanding | 07:32 |
| fwereade | wrtp, no, the Uniter has never been started with a *Unit | 07:33 |
| wrtp | fwereade: why not, out of interest? | 07:33 |
| wrtp | fwereade: it would seem logical | 07:33 |
| fwereade | wrtp, there's no reason to? | 07:33 |
| fwereade | wrtp, we have available a state and a name, and we need the state anyway | 07:34 |
| fwereade | wrtp, it's just smearing the state setup across the uniter and its client to no apparent benefit | 07:35 |
| fwereade | wrtp, s/it's/pregetting the unit is/ | 07:35 |
| wrtp | fwereade: i think that when the uniter does upgrades, the caller of the uniter will need to get the unit anyway | 07:36 |
| wrtp | fwereade: because it needs to be passed into the upgrader. | 07:36 |
| wrtp | fwereade: same as the MA | 07:36 |
| fwereade | wrtp, sure, but the MA takes the wrong params itself | 07:36 |
| fwereade | wrtp, where's the state info? | 07:36 |
| fwereade | wrtp, (another Agent field, you'll notice :)) | 07:37 |
| wrtp | fwereade: i'm not sure. these are things that individual agents need, but it feels you're making them into universal parameters, and i'm not sure that's necessary. | 07:38 |
| wrtp | fwereade: and, um, i think that's a red herring in fact. | 07:38 |
| wrtp | fwereade: we'll still need to get the *Unit before calling the Uniter | 07:39 |
| fwereade | wrtp, go on? | 07:39 |
| wrtp | fwereade: so we may as well pass the *Unit into the uniter | 07:39 |
| fwereade | wrtp, is it just that we want one to create the upgrader? | 07:40 |
| wrtp | fwereade: yes. but if we've already got one, passing it into the Uniter seems like a fine thing. why pass in a name when you've already got the thing itself? | 07:40 |
| wrtp | fwereade: BTW with your current arrangement, you *can't* pass a *agent.Agent into the Uniter | 07:42 |
| fwereade | wrtp, ISTM like a nicer interface to pass a *State and a name than a *State and a *Unit | 07:42 |
| fwereade | wrtp, you just talking about package dependencies? | 07:42 |
| wrtp | fwereade: yeah | 07:42 |
| fwereade | wrtp, yeah, doesn't feel insurmountable to me | 07:42 |
| wrtp | fwereade: why a name nicer than the thing it's naming? | 07:43 |
| wrtp | fwereade: i *think* it's inherent to this approach. the Agent calls the worker, which needs the Agent. cyclic dependency. | 07:43 |
| wrtp | s/why a/why is a/ :-) | 07:44 |
| fwereade | wrtp, surely it's just a matter of re-extracting an agent conf type and passing that around? | 07:44 |
| wrtp | fwereade: so then we have *another* package, just to contain that type? | 07:45 |
| fwereade | wrtp, which probably helps to address the run-mixed-with-info concerns | 07:45 |
| wrtp | fwereade: why not factor out all the run stuff, and make the agent type *solely* concerned with agent storage? | 07:45 |
| wrtp | fwereade: then there's no problem | 07:45 |
| fwereade | wrtp, I think that's the type that holds most of the methods -- it's just Run and Stop that move elsewhere | 07:46 |
| wrtp | fwereade: agreed | 07:46 |
| wrtp | fwereade: in the end, i think you've got something like: type Agent {Name string; JujuDir string} | 07:47 |
| fwereade | wrtp, what about kind and state info? | 07:47 |
| wrtp | fwereade: (i *think* putting the StateInfo in there is mixing concerns) | 07:47 |
| fwereade | wrtp, name me an agent that doesn't need a state info | 07:47 |
| wrtp | fwereade: the firewaller :-) | 07:48 |
| wrtp | fwereade: i know it's not its own agent, but it could be | 07:48 |
| fwereade | wrtp, so is your contention that if 4 things use something, and a 5th doesn't, it is apropriate to write 5 separate code paths rather than suffer the shame and indignity of an unused param? ;p | 07:49 |
| wrtp | fwereade: it's not really a matter of "which agent's don't need it" but "why is it living in this package, which is actually only to do with agent storage?" | 07:49 |
| wrtp | fwereade: i don't see that this would require any extra code paths | 07:49 |
| fwereade | wrtp, ah, hmm, I don;t think it is just concerned with agent storage, I think Conf is an imortant part | 07:49 |
| wrtp | fwereade: Conf? | 07:50 |
| fwereade | wrtp, Agent.Conf, which means we can write a Deploy wethod that works | 07:50 |
| fwereade | wrtp, well, in concert with other things, it does | 07:50 |
| fwereade | wrtp, rather than having the crazy doesn't-even-work-and-is-not-consistent-with-cloudinit Deploy we currently have | 07:51 |
| wrtp | fwereade: an explicit StateInfo param to Deploy seems quite reasonable to me | 07:51 |
| wrtp | fwereade: i'm already most of the way through fixing that | 07:51 |
| fwereade | wrtp, and to the machine agent, and to the unit agent, and (I presume) to the provisioning agent as well? | 07:52 |
| wrtp | fwereade: absolutely. | 07:52 |
| fwereade | wrtp, sorry s/agent/worker/g | 07:52 |
| wrtp | fwereade: i don't think an extra parameter is a problem | 07:52 |
| wrtp | fwereade: especially as it makes it obvious that this worker connects to the state | 07:53 |
| wrtp | fwereade: it would *not* be a parameter to the upgrader or to the firewaller | 07:53 |
| fwereade | wrtp, what? that's not why we pass it at all | 07:53 |
| wrtp | fwereade: no? | 07:53 |
| wrtp | fwereade: why do we pass it? | 07:53 |
| fwereade | wrtp, we pass it to the things that themselves need to set up new agents one way or another | 07:53 |
| wrtp | fwereade: indeed - things that need to connect to the state | 07:54 |
| fwereade | wrtp, the *State is handled outside and shared by many worker, and that really does need to be passed to everything | 07:54 |
| fwereade | wrtp, an MA doesn't connect to the state | 07:54 |
| fwereade | wrtp, sorry a Machiner | 07:54 |
| wrtp | fwereade: it starts things that do | 07:54 |
| fwereade | wrtp, yeah, exactly | 07:55 |
| wrtp | fwereade: so, indirectly, yes, it does. | 07:55 |
| fwereade | wrtp, I would like it if we used a definition of "connects to the state" that involved, y'know, opening a connection to the state ;p | 07:55 |
| wrtp | fwereade: lol | 07:55 |
| fwereade | wrtp, but yes, anyway, I see your perspective | 07:56 |
| wrtp | fwereade: i think of it like a capability | 07:56 |
| fwereade | wrtp, this is kinda by the by anyway | 07:57 |
| fwereade | wrtp, I'm explicitly *not* proposing a common worker creation interface taking an agent, because I felt it would lead to derails ;) | 07:58 |
| wrtp | fwereade: i'm just objecting to the fact we're stuffing StateInfo inside the agent package, when *nothing* in the agent package uses it. it's solely to avoid us passing an extra parameter. | 07:58 |
| fwereade | wrtp, Conf uses it... | 07:58 |
| fwereade | wrtp, and IMO it's a really nice thing to be able to take the exact same object and run it, or install it, or generate the scripts required to install it | 07:59 |
| fwereade | wrtp, it feels like that's what an agent "is" | 07:59 |
| fwereade | wrtp, you need exactly the same information to run one, and to generate a conf for it | 07:59 |
| wrtp | fwereade: i'm not sure that Conf lives inside agent. | 08:00 |
| wrtp | fwereade: i think it lives inside container. | 08:00 |
| wrtp | fwereade: i've had some thought as to how container should work | 08:00 |
| fwereade | wrtp, interesting | 08:01 |
| wrtp | fwereade: that's what i was planning to work on this morning | 08:01 |
| wrtp | fwereade: but your thought of "to be able to take the exact same object and run it, or install it, or generate the scripts required to install it" is exactly where i was coming from. | 08:02 |
| fwereade | wrtp, if your position is that Container is a good place for this then I am very happy to stand back and let you get on with it | 08:02 |
| wrtp | fwereade: i was planning to filch a pattern of yours from the unit agent testing, which i rather liked | 08:02 |
| fwereade | wrtp, because my spidey-sense kept saying "use container in cloudinit", but I couldn't figure out how to | 08:03 |
| wrtp | fwereade: i *think* it is | 08:03 |
| wrtp | fwereade: that is my plan | 08:03 |
| fwereade | wrtp, sweet | 08:03 |
| wrtp | fwereade: the plan is for container to be able to generate shell scripts as well as running Deploy | 08:03 |
| fwereade | wrtp, ok, I just need to figure out what I can do that doesn't conflict with you | 08:03 |
| wrtp | fwereade: ok, sorry, i didn't realise you were so deep into this area | 08:04 |
| wrtp | fwereade: the global VarDir was a prelude | 08:04 |
| fwereade | wrtp, because, well, I really want to make the UA just run a Uniter, but the prospect of essentially writing more duped tests inside jujud bugged me enough to go looking for abstractions :) | 08:04 |
| fwereade | wrtp, I think we have actually been for a couple of days, but I think we're coming at the problem from opposite ends so it hasn't been entirely apparent | 08:05 |
| wrtp | fwereade: yeah, i know what you mean. but i think we can write shared tests without shoehorning them all into the same code. | 08:05 |
| fwereade | wrtp, it is extremely reassuring to me that we seem to be in broad agreement about the general problems depsite our differing perspectives :) | 08:06 |
| wrtp | fwereade: agreed | 08:06 |
| wrtp | fwereade: like two ends of an inductive proof coming together... | 08:07 |
| fwereade | wrtp, yeah :) | 08:07 |
| wrtp | fwereade: now we just have to make the terms match | 08:07 |
| fwereade | wrtp, the trouble is that getting a running Uniter is blocked on getting Container to run a unit agent | 08:09 |
| wrtp | fwereade: ok, i'll try and get it out very soon | 08:09 |
| fwereade | wrtp, I guess I can at least just write a really dumb unit agent + test, on top of your VarDir branch | 08:09 |
| fwereade | wrtp, that should be pretty independent | 08:10 |
| wrtp | fwereade: sounds like a reasonable way to go | 08:10 |
| fwereade | wrtp, cool | 08:10 |
| fwereade | wrtp, I look forward to seeing what you do with container | 08:11 |
| Aram | moin. | 08:13 |
| wrtp | fwereade: here's the likely crackful branch i alluded to earlier. it gives us PA upgrading. but at what cost? what d'ya think? https://codereview.appspot.com/6493101/ | 08:13 |
| fwereade | wrtp, haha, I'll take a look | 08:18 |
| fwereade | wrtp, I think I'm -1 on that, it feels like too much special-casing in state | 08:24 |
| fwereade | wrtp, I think it would be better to either charm the provisioner (which I don;t think is a good use of our time right now) or to build the provisioner-deploying directly into machiner | 08:25 |
| fwereade | wrtp, I thought it was agreed a while ago that we'd be adding a provisioner field to state.machine somehow, and I was expecting to use that (in the absence of a nice charmy way to do it) | 08:26 |
| fwereade | wrtp, (and *that* then makes me think that, hell, the MA itself should probably just run the PA tasks if it's configured to be a provisioner machine, and drop the whole idea of a separate agent | 08:27 |
| fwereade | ) | 08:27 |
| wrtp | fwereade: i wanted to go in that kind of direction, but niemeyer thinks that the PA should have its own entity in state | 08:28 |
| fwereade | wrtp, foiled again :( | 08:28 |
| wrtp | fwereade: which implies loads more mechanism | 08:28 |
| fwereade | wrtp, indeed | 08:28 |
| wrtp | fwereade: which i'm really reluctant to do, because it's actually *identical* to what Unit does | 08:29 |
| wrtp | fwereade: except there's no charm for an agent | 08:29 |
| wrtp | fwereade: hence my AgentService | 08:29 |
| fwereade | wrtp, yeah, I understand, I just think it's basically inferior to a `provisioner bool` field in state.Machien | 08:30 |
| fwereade | wrtp, or at least the underlying state if not that type | 08:30 |
| wrtp | fwereade: except it's much more flexible than that | 08:30 |
| wrtp | fwereade: it means PAs are independent of MAs | 08:30 |
| wrtp | fwereade: ... well... | 08:31 |
| wrtp | fwereade: an MA must start a PA, as with any unit | 08:31 |
| wrtp | (non-subordinate unit) | 08:31 |
| fwereade | wrtp, I'm not sure -- a separate provisioning state entity would be, but I'm suspicious that the AgentService thing feels unhelpfully cross-cutting | 08:31 |
| wrtp | fwereade: you're probably right. | 08:32 |
| wrtp | fwereade: the other thing it offers is the capability to add any new agent of our choice for free. | 08:32 |
| wrtp | fwereade: *currently* we only have three agent types, but that may well change. | 08:32 |
| wrtp | fwereade: i would be happy to just add a bool to a Machine for now. | 08:33 |
| fwereade | wrtp, sure, I'm just not convinced that we can predict the circumstances that might lead us to change well enough to call it right | 08:33 |
| fwereade | wrtp, that would be my choice, indeed, but I guess it's niemeyer's call | 08:33 |
| wrtp | fwereade: i'm not sure i'll ever show him this branch | 08:34 |
| fwereade | wrtp, but I would like it most if we were able to drop the conecpt of a PA entirely, I really don't see why the MA shouldn't run those tasks if so configured | 08:34 |
| wrtp | fwereade: i agree. it seems fine for a machine-level task. | 08:35 |
| wrtp | fwereade: ah, there is one issue | 08:35 |
| fwereade | wrtp, ah bother, there's usually something ;p | 08:35 |
| wrtp | fwereade: we won't give PA-like authority to all MAs | 08:36 |
| wrtp | fwereade: but given that everything has all authority currently, i'm not sure it's a problem for the time being | 08:36 |
| fwereade | wrtp, I don't *think* that's a serious problem... I presume the magic secure API layer will be able to grant/revoke appropriately | 08:37 |
| fwereade | wrtp, yeah, exactly | 08:37 |
| wrtp | fwereade: yeah | 08:37 |
| fwereade | wrtp, ok, well, I think we know what we're doing, I wish you luck :) | 08:38 |
| wrtp | fwereade: thanks! toi aussi | 08:38 |
| wrtp | fwereade: i'm thinking along these kinds of lines for the container API: http://paste.ubuntu.com/1196267/ | 08:45 |
| fwereade | wrtp, +-0, I'm not sure I know enough to judge yet | 08:46 |
| fwereade | wrtp, I think I'll need to see some actual use :) | 08:47 |
| wrtp | fwereade: ok. i think that gives enough for use by both cloudinit and agents, but we'll see. | 08:47 |
| fwereade | wrtp, fwiw, it crosses my mind that agent commands should probably start their Run methods with `a.Conf.JujuDir = ctx.AbsPath(a.Conf.JujuDir)`, even if that's frequently a no-op | 09:47 |
| wrtp | fwereade: interesting point. | 09:48 |
| wrtp | fwereade: or should that happen where something passes the jujudir to something that might change directory? | 09:48 |
| fwereade | wrtp, I *think* that we should never be depending on working directory internally anyway | 09:49 |
| fwereade | wrtp, and I also think it's largely moot because we'll basically always be passing absolute paths anyway, but I think it will be more technically correct and mildly convenient for testing | 09:50 |
| fwereade | wrtp, (it's feeling very hard to write this code because I forsee it changing significantly, but I'm still not quite sure in what direction :)) | 09:51 |
| wrtp | fwereade: yeah, maybe. i don't really have a feel for how --juju-dir might be used in practice | 09:51 |
| fwereade | wrtp, IMO it should just always be passed explicitly | 09:52 |
| wrtp | fwereade: that sounds reasonable | 09:52 |
| wrtp | fwereade: so we should make it a required flag? | 09:52 |
| wrtp | fwereade: well, tbh defaulting to /var/lib/juju seems ok too | 09:52 |
| wrtp | fwereade: we could just give an error if the path was *not* absolute. | 09:53 |
| fwereade | wrtp, I'm inclined to just drop the defaults -- if anyone's ever running itby hand i think we want it to be explicit, and it doesn't cost us much to explicitly set it when generating upstart scripts | 09:53 |
| wrtp | fwereade: but AbsPath seems like a reasonable thing to do too | 09:53 |
| wrtp | fwereade: indeed | 09:53 |
| fwereade | wrtp, all it means is that you *can* run it with a relative path and have it do the right thing in all situations | 09:54 |
| fwereade | wrtp, although, maybe it won't be able to find the tools necessarily | 09:55 |
| wrtp | fwereade: true. i'm just wondering when we'd ever actually want to run an agent from the command line | 09:55 |
| wrtp | fwereade: it doesn't need to find the tools | 09:55 |
| fwereade | wrtp, when things are weird and we're trying to figure out what is going on :) | 09:55 |
| fwereade | wrtp, unit agent does | 09:55 |
| wrtp | fwereade: good point. | 09:56 |
| wrtp | fwereade: i think if things are weird, it's easy for us to pass an absolute pathname :-) | 09:56 |
| fwereade | wrtp, yeah, maybe just requiring abs is the right way to go | 09:57 |
| fwereade | wrtp, cheers | 09:58 |
| wrtp | fwereade: np | 09:58 |
| wrtp | Aram: morning | 10:32 |
| Aram | hello there. | 10:32 |
| wrtp | fwereade: i'm looking at the upstart package and wondering whether it might be best folded into container | 11:30 |
| wrtp | fwereade: it's juju specific, and the actual code is fairly trivial. | 11:30 |
| wrtp | fwereade: and in doing so, i wondered: is there any time we actually care about Service.{Start,Remove} idempotency? | 11:31 |
| fwereade | wrtp, hmmm, +0.9 to folding it into container | 11:49 |
| fwereade | wrtp, not sure about start/remove but it's not like it's heavily used, so probably not | 11:50 |
| wrtp | fwereade: thanks | 11:50 |
| fwereade | wrtp, is there anything I should have done to induce a charm in a dummy env to be downloadable? | 12:17 |
| wrtp | fwereade: i don't *think* so. it should just work, assuming it's pushing to storage. | 12:17 |
| fwereade | wrtp, ah, why might it not push to storage? do I have to tell it to expect puts/gets? | 12:18 |
| wrtp | fwereade: i don't think so | 12:18 |
| wrtp | fwereade: without seeing what you're doing, i'm not sure i can help much | 12:19 |
| fwereade | wrtp, I'm doing http://paste.ubuntu.com/1196552/ and expecting that a uniter will be able to download the result | 12:20 |
| fwereade | wrtp, Get http://127.0.0.1:38234/dummyenv/private/local_3a_series_2f_dummy-1: dial tcp 127.0.0.1:38234: connection refused | 12:20 |
| fwereade | wrtp, I can investigate myself, I'm just hoping for a shortcut to enlightenment, don't spend time on it :) | 12:21 |
| wrtp | fwereade: i think it *should* work. | 12:22 |
| fwereade | wrtp, cool, that is useful data :) | 12:22 |
| fwereade | wrtp, cheers | 12:22 |
| wrtp | fwereade: i *think* the juju deploy tests are testing this case | 12:23 |
| wrtp | fwereade: perhaps you're doing a Reset by accident? | 12:23 |
| fwereade | wrtp, hmm, I will poke around, that sounds quite plausible | 12:24 |
| fwereade | wrtp, cheers | 12:24 |
| fwereade | wrtp, bah, I trashed pkg and now it works | 12:26 |
| fwereade | wrtp, well it fails differently actually but in a much more scrutable way :) | 12:26 |
| wrtp | fwereade: good. i wonder what went on there | 12:26 |
| fwereade | wrtp, no idea, but I have found "trash pkg" to be a useful troubleshooting step every so often | 12:27 |
| fwereade | wrtp, couldn't remotely say what it's correlated with | 12:27 |
| wrtp | fwereade: i almost never do that. | 12:27 |
| wrtp | fwereade: i wonder how your setup differs | 12:27 |
| fwereade | wrtp, it's probably related to my bloody-minded insistence on keeping separate source dirs and swapping them around | 12:27 |
| wrtp | fwereade: lol | 12:27 |
| wrtp | fwereade: i'm impressed you manage to do that | 12:28 |
| fwereade | wrtp, meh, it fits my brain better and it costs my fingers little | 12:28 |
| wrtp | fwereade: ah, yes, i understand why you have the problems now | 12:28 |
| fwereade | wrtp, I presume something is checking for newer-than | 12:29 |
| wrtp | fwereade: you're moving source directories, but none of the source files change mtime | 12:29 |
| fwereade | wrtp, indeed | 12:29 |
| wrtp | fwereade: yeah. | 12:29 |
| fwereade | wrtp, the amazing thing honestly is that it works so well so much of the time ;p | 12:29 |
| wrtp | fwereade: you should trash the pkg directory each time you change source dirs | 12:29 |
| wrtp | fwereade: or touch all the .go files :-) | 12:29 |
| fwereade | wrtp, yeah, it's really just that actual adverse consequences from failing to do so are surprisingly rare, and so I sometimes forget :/ | 12:30 |
| wrtp | fwereade: fair enough. | 12:31 |
| fwereade | wrtp, separate question: is a JujuConnSuite meant to be already bootstrapped? I thought it was but can't see where it's done | 12:34 |
| wrtp | fwereade: it is, i believe | 12:34 |
| wrtp | fwereade: search for Bootstrap on juju/testing/conn.go | 12:35 |
| wrtp | s/on/in/ | 12:35 |
| * fwereade suspects he searched for Bots instead of Boots :/ | 12:36 | |
| fwereade | wrtp, thanks | 12:36 |
| wrtp | fwereade: we don't have bots yet :-) | 12:36 |
| fwereade | wrtp, for some reason "botstrap" seems to be one of my muscle-memory typos | 12:36 |
| Aram | wrtp: do you understand the purpose of this function? https://codereview.appspot.com/6503086/diff/7002/mstate/watcher/watcher.go#newcode168 | 12:48 |
| wrtp | Aram: yeah | 12:48 |
| wrtp | Aram: it's for using in tests | 12:48 |
| Aram | hmm. | 12:48 |
| wrtp | Aram: so that we can have shorter timeouts when waiting for nothing to happen | 12:49 |
| Aram | ok, but then we don't need to export it publicly? | 12:49 |
| wrtp | Aram: no, it's for any tests that use watchers | 12:49 |
| Aram | it can be in export_test.go | 12:49 |
| wrtp | Aram: the idea is that you can change something in the state, call Sync, then you know that the watcher will have triggered any sends that might happen | 12:50 |
| wrtp | Aram: no it can't | 12:50 |
| wrtp | Aram: that would be ok if we only wanted to use in tests of the watcher package itself | 12:50 |
| wrtp | Aram: i can't say i'm enormously keen on the idea, tbh. i'm not sure what particular advantage you get from having the done channels. | 12:58 |
| wrtp | niemeyer: yo! | 12:59 |
| Aram | wrtp: interesting, I thought the trick of exporting something for a pkg_test package from a pkg package _test file worked for every package used in a test, not only for pkg_test. | 12:59 |
| wrtp | Aram: no indeed not | 12:59 |
| niemeyer | Hello! | 12:59 |
| fwereade | niemeyer, heyhey | 12:59 |
| Aram | hi. | 12:59 |
| * wrtp gets a bite of lunch | 13:01 | |
| niemeyer | Aram: Just reproposed the branch | 13:04 |
| niemeyer | Aram: I think I've fixed the spurious error with Sync | 13:04 |
| niemeyer | Okay, I'm starting my morning by implementing a watcher, probably the EnvironConfig one, to get an idea if the infrastructure is working well for real in an end-to-end case, and will push that for review | 13:07 |
| niemeyer | After that I'm back in review mode, perhaps for the rest of the week | 13:07 |
| Aram | niemeyer: I'm running the test in a loop. | 13:08 |
| niemeyer | Aram: Cool | 13:08 |
| niemeyer | Aram: Any more errors? | 13:08 |
| Aram | nothing yet | 13:08 |
| niemeyer | Aram: Superb | 13:08 |
| niemeyer | Aram: It was a race.. the test is asserting very defined behavior, and with StartSync() we can actually move on without the watcher having done anything | 13:09 |
| === wrtp is now known as rogpeppe | ||
| fwereade | niemeyer, I am becoming fretful about how service configs change when charms are upgraded... is this something we've already thought about in detail, that I've missed? | 14:52 |
| niemeyer | fwereade: Hmm | 14:54 |
| niemeyer | fwereade: I'm afraid to not know the context | 14:54 |
| fwereade | niemeyer, well, just that units running an old version will not necessarily be able to properly understand a new config, and vice versa | 14:55 |
| fwereade | niemeyer, this may just be a matter of "write your service configs carefully" | 14:56 |
| niemeyer | fwereade: No, you're right, I don't think we have given the problem proper consideration yet | 14:56 |
| fwereade | niemeyer, ok -- well, I kinda need to stop for a while now, but I will try to think it through a little | 14:57 |
| fwereade | niemeyer, just wanted to check there wasn't anything that sprang to mind :) | 14:57 |
| niemeyer | fwereade: It's probably easy to do something sane | 14:58 |
| niemeyer | fwereade: E.g. do not run the hook while service config doesnt | 14:58 |
| niemeyer | 't validate properly | 14:58 |
| niemeyer | fwereade: with the current charm | 14:58 |
| niemeyer | fwereade: I'd be happy for us to discuss this, yet postpone the solution until a second point, though | 14:59 |
| niemeyer | fwereade: Just so you don't get blocked on this for too long | 14:59 |
| niemeyer | fwereade: UNless the solution is trivial, of course | 14:59 |
| niemeyer | fwereade: (which it might be, given the above) | 15:00 |
| hazmat | fwereade, you mean incompatible stored value with new schema? | 15:14 |
| hazmat | fwereade, unset values with defaults should switch out to new defaults/types cleanly | 15:15 |
| niemeyer | hazmat: Configuration options may also have disappeared, and the removal is only handled properly on the new hook, for example | 15:21 |
| niemeyer | hazmat: It's worth pondering about the edge cases more carefully at some point | 15:21 |
| hazmat | definitely | 15:22 |
| niemeyer | MachineWatcher tests pass! | 15:22 |
| hazmat | niemeyer, fwiw i found out that the whole yaml speed thing, was because pyyaml needs a different calling convention to actually use the c ext. | 15:23 |
| niemeyer | hazmat: I think there's a distinction between the loader and the parser | 15:23 |
| niemeyer | hazmat: The C extension is used at all times for certain tasks, IIRC | 15:24 |
| hazmat | niemeyer, there is its a two part combo.. with callbacks | 15:24 |
| niemeyer | hazmat: Go and Py were doing the same things in C and the same things in native lang | 15:24 |
| niemeyer | hazmat: Or the same layer, anyway | 15:24 |
| hazmat | but dump/load wouldn't use the c ext opportunistically without changing the parameters | 15:24 |
| niemeyer | hazmat: Sure, as I understand it you can remove the higher level so it's all in C too | 15:25 |
| niemeyer | hazmat: So the stuff goyaml does in Go, can be done in C | 15:25 |
| hazmat | its messier to do but sure.. with the change it basically halves the test time, and triples the speed of status on large envs. | 15:26 |
| niemeyer | hazmat: Yeah, C is fast :-) | 15:26 |
| hazmat | :-) | 15:26 |
| niemeyer | hazmat: Wonder how things would look like in that old scale check | 15:26 |
| hazmat | we're going to have simulatenous juju sprints on different continents | 15:26 |
| niemeyer | hazmat: Hah, nice :) | 15:27 |
| hazmat | niemeyer, i've got a simulator now for scale testing large envs.. specifically for the other proj | 15:27 |
| hazmat | and everyone does dev with it | 15:27 |
| hazmat | on the principal that the best way to ensure scaling is to incorporate it into dev pratice | 15:28 |
| niemeyer | hazmat: What does "everyone does dev with it" mean? | 15:29 |
| niemeyer | Aram, rogpeppe: A real watcher now: https://codereview.appspot.com/6497110 | 15:33 |
| Aram | I'll take a look in a moment. | 15:33 |
| niemeyer | Aram: I'll apply rogpeppe's comments to the foundation, and then I think I'll have to step out from impl for a while to clean up reviews | 15:34 |
| niemeyer | Aram: Actually, I'll do one more cleanup on presence to bring it in line with watcher before I do that, but then it's back to you | 15:35 |
| niemeyer | So two more branches on my plate.. will handle those right away | 15:35 |
| Aram | niemeyer: cheers. | 15:35 |
| niemeyer | rogpeppe: s/Non-existing/Non-existing/ !? :-) | 15:37 |
| niemeyer | rogpeppe: Non-existent? | 15:37 |
| rogpeppe | niemeyer: yeah! | 15:37 |
| rogpeppe | niemeyer: oops, sorry | 15:37 |
| niemeyer | rogpeppe: Cheers :) | 15:37 |
| niemeyer | rogpeppe: np | 15:37 |
| niemeyer | Watch helpers is up for review | 16:37 |
| niemeyer | I'll resend machine watchers again after lunch | 16:37 |
| niemeyer | biab | 16:37 |
| rogpeppe | fwereade: ping | 16:48 |
| rogpeppe | niemeyer: ping | 16:57 |
| niemeyer | rogpeppe: Yo | 16:57 |
| rogpeppe | niemeyer: i'm looking at fixing container, and i'm going around in circles a little bit | 16:58 |
| rogpeppe | niemeyer: i wonder if i could run some ideas past you | 16:58 |
| niemeyer | rogpeppe: Hmm, ok | 16:58 |
| niemeyer | rogpeppe: Sure | 16:58 |
| niemeyer | rogpeppe: What's broken there? | 16:58 |
| rogpeppe | niemeyer: so... container doesn't work at all currently | 16:58 |
| rogpeppe | niemeyer: it doesn't give the right flags to jujud etc | 16:59 |
| niemeyer | rogpeppe: Aren't we using it in the real-world tests that are run? | 16:59 |
| rogpeppe | niemeyer: i'm hoping it might be possible to make container use the same mechanism for installation as cloudinit | 17:00 |
| rogpeppe | niemeyer: no | 17:00 |
| rogpeppe | niemeyer: not yet | 17:00 |
| niemeyer | rogpeppe: Hmm, ok | 17:00 |
| rogpeppe | niemeyer: everything that runs currently is started by cloudinit | 17:00 |
| rogpeppe | niemeyer: here's an idea i've had: http://paste.ubuntu.com/1197028/ | 17:00 |
| rogpeppe | niemeyer: oops, one crucial line missing: http://paste.ubuntu.com/1197030/ | 17:01 |
| niemeyer | rogpeppe: What is changing and why? | 17:02 |
| rogpeppe | niemeyer: the idea is to replace the container package with the agent package. | 17:02 |
| rogpeppe | niemeyer: then environs/cloudinit can use that package to generate its cloudinit scripts | 17:03 |
| rogpeppe | niemeyer: and agents can use that package to start other agents | 17:03 |
| rogpeppe | niemeyer: it would start agents in new containers if required | 17:03 |
| niemeyer | rogpeppe: In the last couple of weeks we've had three different versions of what an Agent is | 17:03 |
| rogpeppe | niemeyer: yes, i think we're trying to find out :-) | 17:04 |
| rogpeppe | niemeyer: Agent may not be a good name here | 17:04 |
| niemeyer | rogpeppe: Yeah, but we have Agent today | 17:04 |
| rogpeppe | niemeyer: after all, it's just some information about an agent | 17:04 |
| niemeyer | rogpeppe: They exist already.. we can't give the same name to two different things | 17:04 |
| rogpeppe | niemeyer: i'm not sure why not. this is just one package's idea of an agent. different namespace. | 17:05 |
| niemeyer | rogpeppe: Our brains have a single namespace.. | 17:05 |
| niemeyer | rogpeppe: It sucks to say "an agent" and having no idea about what it is | 17:05 |
| rogpeppe | niemeyer: agent.Info ? | 17:05 |
| niemeyer | rogpeppe: container? | 17:06 |
| niemeyer | :) | 17:06 |
| niemeyer | rogpeppe: What are we trying to fix? | 17:06 |
| niemeyer | rogpeppe: There's no problem statement yet that I can correlate to | 17:06 |
| rogpeppe | niemeyer: we're trying to put the upstart generation stuff in one place | 17:06 |
| niemeyer | rogpeppe: We have that.. that's container | 17:06 |
| rogpeppe | niemeyer: ok, so let's call this package "container". and give it a similar API. | 17:07 |
| rogpeppe | niemeyer: (to the one i've proposed) | 17:07 |
| niemeyer | rogpeppe: I'm not arguing for that even.. I'm asking you to tell me what I'm trying to fix :) | 17:07 |
| rogpeppe | niemeyer: currently the container package can't deploy a machine agent | 17:07 |
| niemeyer | rogpeppe: Sounds sane.. it's used by the machine agent | 17:08 |
| rogpeppe | niemeyer: i'd like to be able to use it from environs/cloudinit | 17:08 |
| niemeyer | rogpeppe: Hmm | 17:10 |
| rogpeppe | niemeyer: we've got these two pieces of code that are similar but different: http://paste.ubuntu.com/1197050/ | 17:10 |
| rogpeppe | niemeyer: rather than copying all the logic from the latter to the former, i'd like to make both places use the same mechanism. | 17:11 |
| niemeyer | rogpeppe: What is similar among them? | 17:11 |
| rogpeppe | niemeyer: they should both be almost identical. | 17:11 |
| niemeyer | rogpeppe: Not really, they are managing independent commands, that need independent info, in very different circumstances | 17:12 |
| rogpeppe | niemeyer: except one runs the action there and then; the other generates a shell script to do the same on the remote machine | 17:12 |
| niemeyer | rogpeppe: So what is actually similar? | 17:12 |
| rogpeppe | niemeyer: everything up to InstallCommands vs Install | 17:13 |
| niemeyer | rogpeppe: Do we need a MachineConfig to deploy a unit? | 17:13 |
| rogpeppe | niemeyer: should be the same except that jujud agent arguments are different, but i think that need not be the case. | 17:13 |
| rogpeppe | niemeyer: we need a state info. and we need a VarDir. that's all it's used for. | 17:14 |
| niemeyer | rogpeppe: Do we need a MachineConfig to deploy a unit? | 17:14 |
| rogpeppe | niemeyer: i wasn't suggesting that we did. | 17:14 |
| niemeyer | rogpeppe: it is a question | 17:14 |
| niemeyer | rogpeppe: "No" is a fine answer | 17:14 |
| niemeyer | rogpeppe: I'm arguing that they are doing different things, and asking for the similarities, and you're saying that they are pretty much exactly the same | 17:15 |
| rogpeppe | niemeyer: no. a MachineConfig is a concept unique to environs/cloudinit. | 17:15 |
| niemeyer | rogpeppe: I don't see that | 17:15 |
| niemeyer | rogpeppe: and I'm showing you why that doesn't seem to be the case | 17:15 |
| rogpeppe | niemeyer: addAgentScript doesn't need a MachineConfig either | 17:15 |
| niemeyer | rogpeppe: We already have packages: container, upstart, cloudinit, environs/cloudinit, ... | 17:16 |
| rogpeppe | niemeyer: i was thinking of merging the upstart package into container - it's pretty trivial and not actually that helpful. | 17:16 |
| niemeyer | rogpeppe: If we're adding another layer, it must be clear what that layer is | 17:16 |
| niemeyer | rogpeppe: I'm not feeling we know that, given the line of thinking so far | 17:16 |
| rogpeppe | niemeyer: i was not proposing adding a layer, but changing an existing layer | 17:16 |
| niemeyer | rogpeppe: The "agent" package is a new layer, apparently | 17:17 |
| niemeyer | rogpeppe: It doesn't address the needs of container | 17:17 |
| rogpeppe | niemeyer: doesn't it? | 17:17 |
| rogpeppe | niemeyer: i was proposing it to replace container | 17:17 |
| niemeyer | rogpeppe: I don't see the word "LXC" there | 17:17 |
| niemeyer | rogpeppe: Nor the word unit | 17:18 |
| rogpeppe | niemeyer: should there be the word LXC there? or might it actually be ok to have that be an implementation detail of container? | 17:18 |
| rogpeppe | (or agent) | 17:18 |
| niemeyer | rogpeppe: It can be anything, but we need to know about what it is | 17:19 |
| niemeyer | rogpeppe: The proposal has to consider it, because that's exactly the reason why the container package exists | 17:19 |
| rogpeppe | niemeyer: likewise, does the mechanism for starting a unit agent need to know the *state.Unit? or might it be ok just to give it the info it actually needs? | 17:19 |
| niemeyer | rogpeppe: We can't obsolete the package without telling how it's going to work | 17:19 |
| rogpeppe | niemeyer: currently all we need to start a new unit in a container is the info provided in the proposal above. | 17:20 |
| rogpeppe | niemeyer: it's easy for the machine agent to derive that info from the *state.Unit and use that to call agent.Deploy (or container.Deploy) | 17:21 |
| niemeyer | rogpeppe: Again, it's not about "currently", it's about how we handle the problem being addressed by "container" | 17:21 |
| rogpeppe | niemeyer: ok. so let's see. what *is* the problem being addressed by "container"? | 17:21 |
| niemeyer | rogpeppe: We spent a lot of time thinking why we need that interface, I'd appreciate hearing your thoughts about how these problems we talked about will be handled | 17:22 |
| * rogpeppe goes back to look at those discussions. | 17:23 | |
| niemeyer | rogpeppe: Jun 14th | 17:25 |
| rogpeppe | niemeyer: i'm there | 17:25 |
| rogpeppe | niemeyer: i'm looking at this (http://paste.ubuntu.com/1040898/) and wondering what the container package actually wants from the *state.Unit value | 17:29 |
| niemeyer | rogpeppe: It wants to know what to deploy | 17:30 |
| rogpeppe | niemeyer: does it actually need to know any more than the args that need to be passed to jujud? | 17:31 |
| niemeyer | rogpeppe: How do we destroy a container given a list of arbitrary arguments to jujud? | 17:32 |
| niemeyer | rogpeppe: It feels like the thinking is very incipient | 17:32 |
| niemeyer | rogpeppe: I see 14 lines in addAgentScript, where most of those lines are already based on abstractions | 17:33 |
| rogpeppe | niemeyer: i'm not suggesting that the arbitrary args be a parameter to Deploy. but actually, we could easily make the jujud arguments uniform for all agents. | 17:33 |
| niemeyer | rogpeppe: The differences in the abstractions are exactly the things you're referring to | 17:33 |
| niemeyer | rogpeppe: Like a description for the agent, the information used to build the command line, etc | 17:33 |
| niemeyer | rogpeppe: I'm concerned that we're reinventing another wheel at this stage without even having the current wheels working | 17:34 |
| rogpeppe | niemeyer: ok, i'll make it work, then we'll see if it's worth abstracting | 17:35 |
| rogpeppe | niemeyer: i feel that it might be, but i agree that perhaps it's hard to tell at this stage. | 17:35 |
| niemeyer | rogpeppe: The right abstraction will likely take code out, rather than adding new layers such as Action, etc | 17:35 |
| rogpeppe | niemeyer: yeah. the difficulty is we've got these actions that we can either perform here and now, or remotely. Action was a way of trying to make both work uniformly. | 17:36 |
| rogpeppe | niemeyer: one other thing | 17:37 |
| rogpeppe | niemeyer: i'm been thinking about what new stuff we need to create to make the PA work in state. | 17:37 |
| rogpeppe | niemeyer: fwereade suggested earlier that we could just add a bool param to *state.Machine to say "run provisioning worker". so the MA would also be the PA when that's set. | 17:38 |
| rogpeppe | niemeyer: that would make lots of things easier (we'd get everything for free) but perhaps it's a bad idea. what do you think? | 17:38 |
| niemeyer | rogpeppe: Hmm | 17:39 |
| niemeyer | rogpeppe: I can't see any bad sides either | 17:40 |
| rogpeppe | niemeyer: great! | 17:40 |
| rogpeppe | niemeyer: that brings full upgrades about a week forward. | 17:40 |
| niemeyer | rogpeppe: Well, and that's a huge good side :) | 17:41 |
| rogpeppe | niemeyer: definitely. | 17:41 |
| niemeyer | received document: bson.M{"ok":0, "errmsg":"collection already exists"} | 17:46 |
| niemeyer | How unfortunate.. no error codes whatsoever | 17:46 |
| rogpeppe | niemeyer: guess you'll just have to string match | 17:47 |
| niemeyer | Yeah, sucks | 17:47 |
| niemeyer | Will file a bug upstream | 17:47 |
| niemeyer | Lovely missed pre-reqs.. | 18:13 |
| niemeyer | Alright, mstate/presence is polished | 19:29 |
| niemeyer | I'm done on the coding side for the moment, I think | 19:29 |
| niemeyer | I need to visit a friend at the hospital now.. back later | 19:30 |
| mramm | niemeyer: I hope your hospital trip goes well. Good luck. | 19:36 |
| mramm | niemeyer: if there is anything I can do to help, let me know. | 19:36 |
| wrtp | mramm: hiya | 19:39 |
| mramm | wrtp: hey! | 19:40 |
| niemeyer | mramm: Thanks, all good there.. his dad was making a delicate heart procedure | 23:37 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!