[00:00] thumper: TestSimple also does it's own cleanup. i think it's just to check that the EnsureDead/Stop etc methods work [00:01] ok [01:33] wallyworld_: is the openstack provider configuring a cloud-drive ? [01:33] or config drive [01:33] no [01:33] the instance just uses the storage attached to the instance [01:34] what format is that storage? [01:34] it depends on the instance [01:34] I got pointed at this: http://bazaar.launchpad.net/~cloud-init-dev/cloud-init/trunk/view/head:/doc/sources/configdrive/README.rst [01:34] larger instances use emphmeral disks [01:35] perhaps that's a feature of cloud-init the provider doesn't yet support [01:36] not that i am aware of anyway [01:36] how do you get your user data into openstack? [01:36] magic [01:36] can you wave your wand on my azure stuff [01:36] i think it is passed via cloud-init somehow, but need to check that [01:36] yes, no shit :) [01:37] * wallyworld_ waves his stick [01:37] sorry, not sure of the exact mechanism [01:37] it's been ages since i looked at that code [01:37] my question is how does cloud-init get it. normally you set a url for the kernel and the userdata is in provider storage at that url [01:37] but we can't set kernel params in azure [01:38] hence the cloud-drive thing - which I thought openstack used as well [01:39] bigjools: justed checked - openstack uses params passed when starting an instance to hold the user data [01:39] there's a userdata field in the struct [01:39] ah so it's direct [01:39] so it's not via cloud-init [01:39] cloud-init picks them up eventually [01:40] server, err = e.nova().RunServer(nova.RunServerOpts{ [01:40] Name: e.machineFullName(scfg.machineId), [01:40] FlavorId: spec.InstanceType.Id, [01:40] ImageId: spec.Image.Id, [01:40] UserData: userData, [01:40] SecurityGroupNames: groupNames, [01:40] }) [01:40] i'm not sure of the internals [01:40] you have a userData() func [01:41] yes, but i'm not sure how the instance works internally [01:41] which pretty much answers my question now [01:41] it renders to []byte [01:42] I need to render to a "clouddrive" [01:42] configdrive, even [01:43] wave your wand and it will happen [02:09] oh FFS!!!! [02:10] why do we insist on making testing things so fucking hard? [02:12] * bigjools wonders why there's two cloudinit packages [02:17] thumper: if you want to take a break from fucking hard, can I have a pre-imp for something that is only moderately hard [02:25] bigjools sure [02:26] thumper: gimme 2 mins and I'll call you [02:29] * bigjools prods thumper to answer === tasdomas_afk is now known as tasdomas [05:02] [05:02] Google Drive [05:02] The app is currently unreachable [05:02] shitter [05:02] thumper: for your todo list (tomorrow maybe) https://codereview.appspot.com/10534043/ and https://codereview.appspot.com/10447045/ [05:03] davecheney: they must have used some microsoft code in there somewhere [05:04] wallyworld_: http://instantrimshot.com/classic/?sound=rimshot [05:05] * wallyworld_ laughs [05:30] thumper: poke [05:31] I'm trying to sort out what an Envorin is, and why he knows a guy named 'eric' very intimately. (testing/environ_test.go) [05:32] I guess the goal is to just have a known home to compare against. Though we should probably s/Envorin/Environ/ [06:23] rogpeppe: I just found that "go1.1" is a build constraint. If you wanted to write your Sliceof code in 2 modules and have a '// +build go1.1' and a '// +build !go1.1' [06:23] I don't think it is worth divergence just yet [06:24] but we do have a mechanism to take advantage of features when available === tasdomas is now known as tasdomas_afk === tasdomas_afk is now known as tasdomas [07:24] jam: that doesn't really solve my problem - my workaround for the SliceOf code is to have every module register the slice types - the go1.1 constraint would poison lots of packages. [07:25] sure [07:25] jam: mornin' BTW! [07:25] I just hadn't seen it before [07:25] morning [07:28] i've had this out for review all week and i've still only had one review of it. if i'm to get the API stuff in, i'd really like another review, please! https://codereview.appspot.com/10259049/ [07:28] to whomsoever it may concern :-) [07:29] rogpeppe: to be honest, I've gone to look at it at least 3 times, it is just big enough and involves enough stuff I have trouble consuming it. I will certainly try again. [07:31] jam: sorry about that - there are probably a couple of trivial things that could be split off (the params.Life definitions for example) but i couldn't think of a way of splitting up the main change to agent.go and machine.go [07:32] rogpeppe: probably a lot of it is my unfamiliarity with parts of the internals, so I can't always grasp *why* a change is being made until I dig into it for a bit. [07:33] jam: well, it would be nice for the future if you acquire some familiarity with this part of the code, so please feel free to ask anything about what's going on - we could have chat about it if you want. [07:33] rogpeppe: so why did you need to add the worker.Worker.Kill implementation for the api server? It wasn't a worker before but should be? It was just useful for testing? [07:34] jam: it wasn't a worker before but now it is - rather than having explicit run loops in three places, the CL changes things to use worker.Runner in more places [07:35] jam: so in the machiner we've got the top level Runner that runs two workers; one of them runs any tasks that connect to the state; the other runs any tasks that connect to the API. each of them creates a runner themselves to run the tasks (so the runners are nested) [07:36] rogpeppe: and the presumably we iterate until one of those isn't necessary. [07:36] However, I would have thought that we would have tasks that couldn't quite be all API while we transition [07:36] so we could implement *an* API call and start using it [07:36] rather than having to finish off "everything a task might possibly need" before it can start using API. [07:37] Too much chance of split brain? [07:37] jam: i thought that too originally, but i think the idea is to transition an entire agent at a time [07:37] s/agent/worker [07:37] rogpeppe: Workers returns a worker that runs the unit agent workers. ??? [07:37] why is the method plural, but the result singular [07:38] And the doc string doesn't seem to explain much. [07:38] jam: which method? [07:38] rogpeppe: https://codereview.appspot.com/10259049/patch/5001/6006 [07:38] jam: ah, in unit.go [07:38] If it 'runs' it, I would have thought it would be a Runner [07:39] but a Runner is also a Worker? [07:39] jam: the unit agent has two levels of runner too [07:39] jam: yes [07:39] jam: that's an important point [07:40] For the docstring, I think it is trivial to see that Workers returns a worker, so focusing more about what the thing it returns will be used for might be helpful [07:40] jam: the Workers method is probably badly named [07:40] I also wouldn't make the method plural when it returns a single object, even if that object wraps multiple others. [07:41] jam: yeah, it's kind of a single object that encapsulates all the workers, but i agree [07:41] rogpeppe: MetaWorker ? [07:42] jam: TopLevelWorker? [07:42] jam: StateConnectingWorkerRunningOtherWorkers ? [07:42] // MetaWorker returns the Worker responsible for managing all individial workers that this Agent needs. [07:42] Just calling it Worker would eliminate the confusion on the returned type not being a slice [07:43] rogpeppe: so the thing you return isn't actually a runner, it is a newCloseWorker() which wraps a runner. [07:43] jam: StateWorker might be good enough [07:43] jam: yeah [07:44] rogpeppe: so do Runners actually implement the Worker api? [07:44] jam: that's so we close the state connection eventually [07:44] jam: yes [07:44] jam: (all it requires is Kill and Wait) [07:47] rogpeppe: why do you start the runner in cmd/jujud/machine/Init() rather than in Run [07:47] jam: that's so the tests can kill it [07:48] rogpeppe: I can see that as a reason to have it as an attribute, but Init seems more like setup stuff-without-starting anything, no? [07:48] jam: because we're not using our own tomb now - we're leaching off the Runner's liveness [07:49] jam: yeah, i take your point. [07:49] jam: even though the runner is empty, calling Init without Run will leak [07:51] rogpeppe: so you already have a method called StateWorker in MachineAgent code. Though perhaps the functionality is similiar enough that name sharing is a good thing? [07:52] jam: yes, that's what i was thinking [07:53] jam: and when we transition the unit agent, we'll probably have an APIWorker *and* a StateWorker in there until we can finally delete the StateWorker [07:53] rogpeppe: I would *really* like to call this thing the StateRunner instead, though. [07:53] That makes it clear the thing isn't actually doing anything on its own. [07:53] jam: yeah, seems like a good idea [07:54] But it is managing all the tasks that want access to State. [07:55] rogpeppe: It also seems odd that APIWorker (maybe called APIRunner) has a side effect of starting the StateRunner. [07:55] Rather than a sepacate [07:55] MachineAgent.StartAPIRunner() and MachineAgent.MaybeStartStateRunner ? [07:56] rogpeppe: naming aside, calling agent.APIFoo() and getting a runner, but having a side effect of mutating agent.runner seems a bad idea. [07:56] jam: unfortunately it needs to happen like that, because in general we can't start the state worker until we've connected to the API [07:57] certainly when reading the code it wasn't immediately obvious that "a.runner" isn't the same thing as the "runner" you are building up. [07:57] rogpeppe: you can check and refuse if the API side isn't running yet [07:57] and callers need to call them sequentially. [07:57] or we have a StartRunners call that does both [07:57] morning all! [07:57] welcome back dimitern [07:57] dimitern: yo! [07:58] thanks! :) [07:58] jam: we can't even know if we *need* to run a state server until we've connected to the API [07:58] s/state server/state worker/ [07:58] jam: so there has to be some communication between the two [07:59] jam: i know it might seem awkward, but i haven't been able to think of a more elegant way of solving the problem [07:59] rogpeppe: again, you can always start the state worker after the API. I'm objecting to having a side effect of "create this runner for me", also starting and forcibly instantiating another runner that isn't actually the one I asked for. [08:00] rogpeppe: I'm just asking to move the code into another function that more clearly signals that it is potentially starting 2 things. [08:01] rogpeppe: If I'm understanding the code, the block from "m := entity.(*machineagent.Machine)" until the end of if needsStateWorker could be done after you return the newCloseWorker [08:01] which means it could be in a different function. [08:01] you need the 'entity', but you could expose that. [08:02] jam: i'm not sure it can be [08:02] jam: in the future, the information on how to connect to the state will be held in the API [08:03] jam: so we'll have to pass some information that we've obtained from the API into StateWorker [08:03] rogpeppe: so after we have the API runner up, we ask the API if we also need to start the StateWorker. [08:03] jam: yes [08:03] jam: that's what we do [08:03] rogpeppe: The point is you have a function labeled "Give me an APIWorker", that should be straightforward. But it has this side effect of maybe starting a StateWorker [08:03] which is nowhere in the contract we've defined [08:04] it doesn't even activate the APIWorker (doesn't add it to a.runner) [08:04] so it seems doubly strange that it would add a different worker to a.runner. [08:04] jam: unfortunately one of the things we have to do after connecting to the API is maybe start a new state worker [08:04] rogpeppe: so I'm just asking you to have a function called "StartWorkers" which connects to the API, creates an APIWorker, and then checks to see if it needs to start a state worker (and does so) [08:05] jam: how can it do that? - it won't have an API connection [08:05] rogpeppe: you just created the API connection [08:05] how does it not have one? [08:06] jam: creating an API worker doesn't create the API connection until some time later [08:06] rogpeppe: then how is your code doing it today? [08:06] jam: the only time we know we have an API connection is inside APIWorker itself [08:06] if it doesn't have the connection? [08:07] jam: so you'd be happy if StartWorkers was called from within APIWorker? [08:07] rogpeppe: wrong way around [08:08] jam: but if StartWorkers adds APIWorker to the top level runner, how can it get access to the API connection that only APIWorker has access to? [08:10] jam: perhaps i might understand better what you're suggesting if you paste some pseudocode [08:10] rogpeppe: rough sketch: http://paste.ubuntu.com/5800860/ [08:11] rogpeppe: yeah, I was working on that [08:11] jam: what calls StartWorkers? [08:12] rogpeppe: right now it is an immediate replacement for whatever called APIWorker [08:14] jam: currently APIWorker is called by the top level Runner [08:14] jam: which is important because it gives us our top level run loop (retrying on failure) [08:15] rogpeppe: so MachineAgent is also a Worker? [08:16] jam: no [08:16] jam: machine.go:87,90 [08:17] rogpeppe: is there a reason we couldn't just have Workers() here, which returns a slice of Workers, which might be 1 for just the API or 2 for both the API and State worker if we find we need it. [08:17] jam: that's how APIWorker gets called [08:17] Then we don't have to touch a.runner [08:18] jam: shall we G+ this - it seems like a bit of higher bandwidth might be useful [08:20] jam: i'm not sure what you mean with your slice suggestion. are you suggesting that MachineAgent.Run calls Workers directly? [08:21] rogpeppe: just a sec, digesting a bit. [08:24] jam: the difficulty is that eventually we *do* need two independent workers in the top level a.runner, but we only know whether to run the state worker *after* the first API connection has been made, which happens to be exactly what the other runner has to do as a first step. i suppose we could have another worker function which connects to the API and whose sole purpose is to add the other runners to the top-level runner [08:24] jam: but that's more code and not necessarily any clearer [08:26] jam: and there's also the point that at some point we might have to *remove* the state worker (if the machine jobs change), which will probably require the API worker to mess with the top-level runner in a similar way [08:28] jam: so now with the go-bot do I need to pull from a different location? bzr pull --overwrite --remember lp:juju-core ? [08:29] jam: would something like this help matters? http://paste.ubuntu.com/5800883/ [08:30] dimitern: if you have any existing proposals, they need to be reproposed [08:30] dimitern: but the pull location is still the same [08:30] rogpeppe: no, save for the one mgz_ took over [08:30] dimitern: did you have a good holiday, BTW? [08:31] rogpeppe: oh yeah, relaxed - just what I needed [08:31] dimitern: are you officially back now? (i thought you were back tomorrow) [08:31] dimitern: it depends how you got your branch. If you got it from 'bzr branch lp:juju-core' that still points at the right branch, if you use 'go get launchpad.net/juju-core' that points to the wrong one (because it pre-resolves the full branch URL). you can always just do it and not worry, because it won't be the wrong thing to do (bzr pull --remember lp:juju-core) [08:31] rogpeppe: i'm back today [08:31] dimitern: cool! [08:32] dimitern: we get a day and a half of overlap before i'm away... [08:32] rogpeppe: thinking a different way, what do we gain by not starting the StateWorker ? [08:32] rogpeppe: yeah [08:32] jam: will bzr info tell me that? [08:32] jam: if we do start the StateWorker, how does it know how to connect to the state? [08:33] rogpeppe: the fundamental bit for me is that it feels ugly to have APIWorker (create an api worker for me to start tasks on) have a side effect of touching the top level runner. We can manage the distrust by having 'Here be dragons' comments, but I'm trying to sort out if we can decouple it. [08:33] dimitern: it can [08:33] jam: i definitely see your point, but i *think* this is a fundamental causality issue [08:34] jam: only when we connect to the API can we work out if (and how) to start the state worker [08:34] jam: and the APIWorker is the only place that connects to the API [08:34] rogpeppe: well today we can just use a.Conf.Conf as you are already doing, no? [08:34] jam: currently we can, but in the future we won't be able to [08:34] And when we are at the point where API knows better how to connect to State, won't we be at the point where we refuse to let anything but the API server itself connect to state? [08:35] and the API server won't be connecting to itself. [08:35] jam: i have this: http://paste.ubuntu.com/5800889/ [08:35] dimitern: parent branch: bzr+ssh://bazaar.launchpad.net/+branch/juju-core/ [08:35] that is always "lp:juju-core" even when we point that at a different branch [08:35] jam: in the future, we will only know whether to run an API server by connecting to an API server [08:35] jam: so it's fine then? [08:36] dimitern: 'go get' would end with: parent branch: http://launchpad.net/~juju/juju-core/trunk/ (which would be the old wrong branch) [08:36] dimitern: yes, you are fine [08:36] jam: i do indeed hope that we get to the stage where the API server is the only thing that needs a state connection [08:36] jam: cheers [08:36] dimitern: note though that juju-core now has a bunch more external dependencies [08:36] lp:gwacl and lp:golxc come to mind [08:36] which also needs 'apt-get install libcurl4-openssl-dev' [08:37] and a branch from github [08:37] [09:35:02] and the API server won't be connecting to itself. [08:37] though 'go get launchpad.net/juju-core' will sort some of that out for you [08:37] jam: it will be connecting to other instances of itself [08:37] jam: yeah, just discovered these and i'm go getting them [08:37] rogpeppe: how does the first one start? [08:37] jam: bootstrap [08:37] jam: that's the "0" case at the top of the MachineAgent.Run [08:38] jam: that's the only time we can't first connect to an API server. [08:38] rogpeppe: in which case, I would offer that the task which actually runs an API server should be the thing starting the StateWorker and not APIWorker [08:39] jam: the StateWorker *is* the thing that runs an API server [08:39] anyway, I don't really want to argue it for too long, but having a "Create me one of these" have a side effect of creating a second one and mutating the state of the caller is hard to keep track off and should be well guarged. [08:40] jam: i agree with your discomfort, but i haven't seen a decent alternative yet [08:40] Hi jam, I replied to your comment on lp https://code.launchpad.net/~rvb/juju-core/az-public-storage/+merge/171251/comments/382236 [08:40] rogpeppe: the ordering *could* be, "connect to the API, poll it for the jobs to run, one of those jobs will start up, and ask the agent to start another worker, which then gets a task to run the API server" [08:42] rvba: I didn't see anything from jtv on the MP, sorry if I missed it. [08:42] jam: so rather than adding to the MachineAgent runner directly, we start another worker that does the same thing? [08:42] rvba: to be fair, I still don't see anything looking back again. [08:42] jam: https://codereview.appspot.com/10541044/ [08:43] rvba: ugh, split brain between LP and Reitveld. Reitveld mirrors most comments back into Launchpad, except when the Reitveld identity isn't known to LP, then those messages just get dropped by LP [08:43] rvba: if you look here: https://code.launchpad.net/~rvb/juju-core/az-public-storage/+merge/171251 [08:43] it *also* means that JTV's message isn't in my email folder [08:43] because Rietveld only sends messages for things which you've already commented on. [08:43] sorry about that. [08:44] jam: in the end, starting the state worker *has* to be a consequence of connecting to the API. hmm, one possibility to make things easier to understand: [08:44] is it possible for jtv to register his alternative email with LP? [08:44] jam: we could explicitly pass the top level runner into the APIWorker [08:44] jam: so that it's more obvious that APIWorker can control it [08:45] jam: would that be better for you? [08:47] Oh hi guys... I'll try to register that email then. [08:50] jam: something like this: http://paste.ubuntu.com/5800919/ [08:50] rogpeppe: I would be happy if the api was "add yourself to my runner", and then we just end up adding 2 things to the runner. rather than "create something and I'll take care of registering it" which then has a side effect of creating and registering an "unrelated" thing. [08:51] mgz_: ping [08:51] jam: the second thing has to be added to the runner as a side-effect of running the first thing. unless we have some entirely different code which connects to the API for the first time, starts *another* worker that connects to the API and a state worker if appropriate. [08:52] fwereade: ping [08:52] rvba: I just sent an email to juju-dev about the config naming question. I actually prefer your method, but we were explicitly asked to do the common-names thing. Which is why I proxied it to you. [08:53] dimitern, pong [08:53] rogpeppe: the issue is that APIWorker isn't *running* the worker yet, right? [08:53] dimitern, welcome back :) [08:53] fwereade: thanks :) [08:53] jam: which worker? [08:53] fwereade: i was thinking to pick up the deployer API stuff [08:53] it seems odd to have the StateWorker registered with the top level runner before the API Worker has been registered [08:53] calling APIWorker() creates a worker object [08:53] but hasn't added it to the topLevelRunner yet [08:53] jam: okay, thanks for starting the discussion. [08:53] fwereade: as agreed before, if mgz_ haven't started on it [08:54] dimitern, +1but speak to danilos -- he's about to go away but hasn't yet and has, AIUI, been looking into it [08:54] dimitern, I'm not up to date on where he is with it though [08:54] dimitern: right, danilos has started to do some of the infrastructure. [08:54] jam: erm, i don't quite understand. you don't actually add workers to runners, you add a function that starts a worker. [08:54] and has 3 days of overlap with you to hand it off. [08:54] fwereade, jam: ok [08:54] danilos__: hey [08:55] jam: the function that calls APIWorker is added to the top level runner immediately [08:56] jam: machine.go:87 [08:57] jam: i saw the mail about the objectives - what's the deadline for that? [08:57] rogpeppe: so we have some more naming confusion. [08:58] StartWorker doesn't start anything [08:58] it registers something that you'll want to start later, right? [08:58] jam: no, it will call the function immediately [08:58] dimitern: officially the end of this week. With official recognition that it is likely we'll miss it by a bit. [08:59] dimitern: though it doesn't have to take super long. You can do it in an hour or so. You have a lot of other people's objectives you can crib from. [08:59] jam: which ones should I take as a template - your or tim's ? [08:59] jam: the slight tension is that the name StartWorker implies only a single worker, but actually there's a sequential succession of workers [09:00] jam: each one started some time after the last one has quit, assuming it didn't quit with a fatal error [09:00] dimitern: actually, you're officially under Tim now [09:00] according to directory.canonical.com [09:00] I wash my hands of you :) [09:00] jam: ok :) \o/ [09:01] rvba: sorry if my comments came across as a bit attacking. It certainly wasn't meant that way. I only commented because of the naming thing, and then I was surprised about the 1 review bit. [09:02] jam: no worries ;) [09:03] rogpeppe: so it feels like the 'right' fix is to change the StartWorkers api so that the function you call can optionally return more workers that you might want to start. However, your last paste seems reasonable enough for now, and has sufficient Here Be Dragons to avoid people getting lost as to why it is happening. [09:08] jam: thanks. i'm not sure how changing the StartWorker API would help really. it seems like it would make the Runner API more complex for no particular gain. if you want another worker, just adding it to the runner directly seems like a reasonable way to do it. [09:08] TheMue, config-get --all LGTM [09:08] rogpeppe: runner.StartWorkers(..., func()) it doesn't feel like that func() should know what runner it is being attached to, so that it can be arbitrarily called by some other runner. [09:09] rogpeppe: it is a 'do we have a singleton per process', imagine writing tests cases for this [09:09] that want to start their own runner, and add this thing to them. [09:09] that should get added to whatever runner they created [09:09] rogpeppe: anyway, the bit you wrote is 'ok', it doesn't feel 'right', but it is acceptable [09:09] your pastebin does at least let you set which one you want it to add any future work to. [09:10] jam: yeah, it's pragmatic code - it's not beautifully regular or modular, but it encapsulates the task in hand and the scope is limited [09:11] night all [09:12] rogpeppe: yeah, I think 1 exception is ok when well documented, if we end up with 2 we should wonder, and if we have 5 exceptions, then we probably have the design wrong. [09:14] jam: when we do multi-tenant, i think we will probably be doing a lot of runner-manipulating - there will probably be a worker who's sole responsibility is to add and remove other workers from the API runner. [09:14] jam: at least, that's my wand-wavy plan [09:19] fwereade: thx [09:20] fwereade: and also thx for the better doc ;) [09:23] jam: this is, i think, something more like you were suggesting. i don't think it's a great improvement: http://paste.ubuntu.com/5800976/ [09:24] rogpeppe: I'm pretty sure line 32 is a.APIWorker [09:25] jam: indeed it is [09:26] rogpeppe: so the idea is you don't need the for{} in your original code because topLevelRunner handles that? [09:26] jam: yes [09:30] rogpeppe: I'll also note in your current proposal of "how do we know what to connect to", there is no actual connection of a.StateWorker() to anything the API returns [09:30] jam: not currently [09:30] though I realize you want to put something there. [09:31] jam: eventually the state server addresses will be accessible through the API [09:32] rogpeppe: I think changing the signature of StartWorker such that the function you pass in can take the runner as an argument, allowing it to add units would be a good idea. Especially under your proposal that there will be a worker which does a lot of start/stopping of stuff in its parent runner (rather than as children of itself). [09:32] However, your old pastebin is my current favorite for the time being. [09:32] ( [09:33] http://paste.ubuntu.com/5800919/ [09:34] jam: we *could* do that, but i think i'm happy having a runner as a closure variable too - there's a certain purity in having a function with zero args [09:34] rogpeppe: as long as we never have to do something like migrate the workers to another runner, etc [09:34] it *seems* like workers shouldn't know what runner they are running inside [09:34] which the closure breaks that [09:34] jam: if we do, the code is small and easy to change [09:36] mgz_: did your patch finally land? [09:37] jam: how about this? http://paste.ubuntu.com/5800999/ [09:38] rogpeppe: it is pretty much equivalent to http://paste.ubuntu.com/5800919/ for me. [09:38] rogpeppe: you still have to wrap that in a closure [09:38] jam: ok [09:38] that closure still has to save a.runner as a const in its closure [09:38] etc. [09:39] jam: i was trying to make it so that the worker doesn't know what runner it's running in [09:39] It is probably slightly better at having APIWorker not know the internals of what a StateWorker is [09:39] rogpeppe: sure, but passing that in is the same thing. [09:39] jam: that knowledge is held outside (in MachineAgent.Run) [09:40] rogpeppe: so if a runner passes itself into the thing it is calling, then the thing it is calling doesn't "know" where it is running, it is "told" where it is running. [09:40] which means if another runner got that task, it would just run on the other runner. [09:40] in this case, the closure has to *know*, which is true of either form of your code. [09:41] jam: that assumes that we always want to add the state worker to the same runner that's running the API worker. [09:41] rogpeppe: if given both, I probably like http://paste.ubuntu.com/5800999/ more, it just doesn't solve the specific issue I was having troubles with. [09:42] Having nested runners that can start other runners that *aren't* nested underneath them is also a little bit confusing [09:44] jam: the startStateAPI func thing works out quite nicely actually: http://paste.ubuntu.com/5801013/ [09:46] jam: i agree it's a bit confusing, but the whole look-at-api-then-connect-to-state-but-not-if-we-are-bootstrapping thing is a little inherently confusing, i think [09:46] jam: and i *think* that the code is just a reflection of that fundamental awkwardness [09:50] jam: yup [09:57] fwereade, rogpeppe: so what was decided on package naming vs tasks. Specifically, what package should I put upgrader in? Its own as state/apiserver/upgrader/upgrader.go, or sharing machine as state/apiserver/machine/upgrader.go? (I get the impression it may be run by machine or unit, so it should be its own thing) [09:57] jam: it should be in its own package [09:58] jam, rogpeppe: this is kinda the problem with the "let's segment-by-agent" scheme [09:58] jam: this CL shows where i'm aiming https://codereview.appspot.com/10494043/ [09:58] rogpeppe, would you remind me what your objection was to putting common test infrastructure in its own package? [09:59] rogpeppe, (iirc that was the main factor in your decision -- I hope I'm not misrepresenting?) [10:00] rogpeppe: does this correctly reflect your notes on concurrency hazards in provider implementations? https://codereview.appspot.com/10602043 [10:00] fwereade: that was part of it. the main thing was to try to keep packages from proliferating wildly. [10:01] jtv: will look in a mo [10:01] rogpeppe, small packages with clear purposes are usually considered a good thing [10:01] thx [10:01] fwereade: the way we're going, we'll have apiserver/machine, apiserver/machiner and apiserver/machineagent [10:02] rogpeppe, all the more so in go, surely, considering that the only encapsulation boundaries are at packages edges [10:02] fwereade: i think it's reasonable to gather the things that will only ever run in one agent inside a package for that agent [10:02] * jam goes and hides in a hole to get actual coding done [10:02] will emerge around standup time [10:03] fwereade: i'm happy to encapsulate by type as well as by package [10:03] rogpeppe, that's all very well in theory but when not enforced by language or ultra-string convention it tends to degrade ;) [10:04] rogpeppe, I think that the details of exactly what runs where will be more fluid than you anticipate [10:04] rogpeppe, and I would prefer not to impede our flexibility by signalling that the most important feature of, say, upgrader, is which agent runs it [10:05] rogpeppe, we have a distributed system with a bunch of responsibilities [10:05] fwereade: i would definitely put reusable components inside their own packages [10:05] fwereade: so the upgrader would get its own package [10:06] fwereade: one mo, i'll paste a couple of sketches [10:06] rogpeppe, and uniter? [10:06] rogpeppe, I'll probably want to run some of those in machine agents at some point [10:06] fwereade: it would go into apiserver/unit [10:07] fwereade: really? [10:07] rogpeppe, remove-unit --force [10:07] rogpeppe, I'm not going to try a transaction to clean up the whole unit state [10:07] rogpeppe, revoking the original unit's access and running a sandboxed uniter with a fake charm would work just fine though [10:07] fwereade: woah [10:08] fwereade: that seems a bit... heavyweight [10:08] fwereade: interesting idea though [10:09] rogpeppe, I only thought of it relatively recently, but it seemed like a possible end-run around a lot of the diffculty [10:11] rogpeppe, it would definitely require that the uniter be decomposed a little but that's definitely not a bad thing [10:12] fwereade: it's already decomposing, arf arf :-) [10:12] fwereade: it sounds like an interesting approach [10:12] rogpeppe, I'd love to be able to drop most of the giant integrationy tests there and be able to run detailed unit tests on all the modes, for example [10:13] fwereade: using the unit agent for that purpose would presumably only require a small subset of the full uniter API? [10:13] rogpeppe, yeah, that was the thought [10:13] fwereade: so it might have its own API facade anyway? [10:15] fwereade: ok, so if we go with the "packages for everything" approach, this is how i see the machine package looking: http://paste.ubuntu.com/5801079/ [10:15] rogpeppe, that hadn't been my thought in particular -- allowing access to that facade for force-dying units on ManageState machines connections seems maybe plausible [10:16] rogpeppe, sorry, the machine agent implementation? [10:16] fwereade: what does cleaning up the unit state actually involve? is it that complex that it's a great help to have whole uniter around for it? [10:17] rogpeppe, basically a load of scope-leaving [10:18] rogpeppe, it may indeed not involve the whole thing, that's why I mention decomposing it [10:18] fwereade: so, the machine package would just integrate together all the APIs that we want to present to the machine agent. [10:20] danilos__: hey [10:20] dimitern, hey-hey [10:20] dimitern, welcome back, I hope it was nice two weeks off :) [10:20]