=== kadams54 is now known as kadams54-away === natefinch-afk is now known as natefinch === brandon is now known as Guest42275 [02:48] niemeyer: you around? [02:55] wallyworld: ping [02:55] yo [02:55] chat? [02:55] sure [02:56] 1:1 [02:57] natefinch: its nearly midnight where he lives [02:58] perrito666: yeah, figured [03:03] perrito666: same where you live though, right? [03:04] natefinch: yup === kadams54 is now known as kadams54-away [03:16] Bug #1463641 opened: lease: data race in tests (again) [03:16] Bug #1463643 opened: worker/provisioner: 9 data races in tests [03:23] % go test [03:23] OOPS: 91 passed, 7 FAILED [03:23] --- FAIL: Test (79.72s) [03:23] FAIL [03:23] exit status 1 [03:23] FAIL github.com/juju/ju [03:23] well, gocheck, don't keep it to yourself ... [03:26] davechen1y: just reviewing your PR [03:26] davechen1y: how did races creep in again? [03:28] menn0: i don't know [03:29] davechen1y: someone messed up conflict resolution during a merge perhaps [03:29] no idea, but it got unfixed === kadams54-away is now known as kadams54 [03:32] davechen1y: ship it although the test still sucks [03:32] davechen1y: but that's not your fault [03:38] davechen1y: am I right in assuming that if I have an RPC method that doesn't have any return data, I still have to give it an argument, something like this: https://github.com/natefinch/juju/blob/wpt-plugins/procmanager/plugins/api/err.go#L13 [03:41] natefinch: sorry [03:41] no idea off hand [03:41] davechen1y: np [03:41] i think you can all rpc methods that don't have a return value [03:41] but i cannot think of any off hand [03:44] davechen1y: the goroutine that's defined in-line in the test seems unnecessary [03:45] davechen1y: AFAICS the select at the bottom should be reading directly from the subscription channel [03:45] davechen1y: and the way things are now if the lease is never released that goroutine will hang around forever [03:48] oh no [03:48] gocheck isn't thread safe [03:48] i think i need to go and have a lie down [03:48] menn0: i'm just applying the old fix from PR 2422 [03:48] davechen1y: yeah I know. i don't expect you to fix it. [03:48] i don't want to marry the test [03:49] davechen1y: i just noticed that it could be better while reviewing your change [03:49] patches accepted :thumbsup: [03:50] in david's britton, any test importing "time" would be shot on sight [04:04] davechen1y: http://paste.ubuntu.com/11686336/ [04:04] davechen1y: wondering about the best way to test this [04:04] davechen1y: fails on windows due to OS error message being different [04:04] davechen1y: is there a nice OS agnostic way to do this better? === kadams54 is now known as kadams54-away [04:26] thumper: does that error fit though os.IsNotExist ? http://golang.org/pkg/os/#IsNotExist [04:27] if it's been gift wrapped, you might have to write a helper to unwrap it [04:27] kk [04:27] I'll check [05:00] does anyone remember off the top of their head if it is safe to call c.Fail inside a goroutine ? [05:00] this is the same rule as t.Fatal [05:00] oh no [05:00] actually this is far worse [05:01] c.Assert in a goroutine triggers a race [05:07] wallyworld: I've put up a PR that addresses all the storage relationships I can. there's a bunch more we'll need to do when we support shared storage [05:07] Bug #1463661 opened: worker/provisioner: kvm broker test failure [05:07] wallyworld: and detaching/attaching volumes and filesystems from machines [05:08] fair enough, ty, will look [05:26] alrighty then, I'm done [05:27] laters [05:30] axw: given in this new pr SetFilesystemInfo ensures any required volume is provisioned, that means that 1.24 could be broken? [05:32] wallyworld: in theory. it's just a safe guard, the worker should be doing the right thing anyway [05:33] wallyworld: storageprovisioner waits until the volume is provisioned before trying to provision the filesystem [05:34] ok [08:50] Bug #1463643 changed: worker/provisioner: 9 data races in tests [09:00] TheMue, standup? [09:11] Bug #1463643 opened: worker/provisioner: 9 data races in tests [09:14] Bug #1463643 changed: worker/provisioner: 9 data races in tests [11:54] * TheMue discovered params.ErrorResults.Combine(), nice [11:59] katco: ping [12:20] fwereade: probably a public issue. I'm trying to run "juju run relation-get" but I'm running into a problem because it is a peer relation [12:20] specifically, trying to do "relation-ids" returns an empty list [12:21] and relation-list et al all tell me I'm using an "unknown relation id" [12:23] though if you pass the name of a known-incorrect relation name, it doesn't give an error, so I'm entirely possibly doing it wrong [12:27] ok it was pebkac. I wasn't specifying the endpoint name correctly [12:40] jam, that was not a deliberate http://dilbert.com/strip/1997-11-06 but glad to see it's resolved :) [12:41] fwereade: heh. So one problem is that finding the relation-id means it is a 2 round trip process [12:41] jamif it's inconvenient to jam the whole script into juju-run, you could just make it part of the charm and juju-run that script [12:44] fwereade: I'm trying out "relation-get -r $(relation-ids endpoint)" if you assume there is only one, which seems to work [12:44] jam, if it's a peer relation you can be sure [12:44] niemeyer: note the difference between https://godoc.org/github.com/natefinch/juju and http://godoc.org/gopkg.in/natefinch/juju.v0 [12:45] niedbalski: the latter doesn't show the directories (except the one I manually typed into the url at one point) [12:45] niemeyer: ^ (sorry niedbalski) [12:46] morning [12:47] morning perrito666 [12:47] * natefinch found a neat hack to show godoc of his WIP branches... just git tag your branch as v0 and then you can serve it up via godoc: https://godoc.org/gopkg.in/natefinch/juju.v0/procmanager/plugins/api [13:08] dimitern: this looked like it might be a networking bug (bug 1463480). it's a blocker; has anyone in our half of the world looked at this yet? [13:08] Bug #1463480: Failed upgrade, mixed up HA addresses [13:09] katco, I'll have a look [13:10] dimitern: ty sir [13:12] katco, it doesn't quite look like a networking issue, more likely something around sorting / picking a private address for the unit [13:15] katco, I'll add comments [13:15] * fwereade off collecting laura again, cath's back this evening, normal service will resume shortly [13:15] dimitern: katco : We need an engineer to comment on bug 1463608. Does modern Juju us pv (paravirtual) types in ec2? That type is going away alone with i386? [13:15] Bug #1463608: Deprecate support for 32-bit and PV AMI's for AWS Images [13:17] sinzui: we have instance types with pv, yeah [13:19] and have had user requests to keep t1.micro working, even though not all aws regions provide it [13:20] mgz_: Yeah. I recalled the request to keep that running, but AWS doesn’t agree. I think this raised the priority of my concern that we are loosing our ability to retest old juju [13:21] * sinzui want to stop making i386 agents. [13:21] sinzui: I think in all current supported jujus we can always force the instance type specifically with a constraint [13:22] provided there's at least one instance type that juju knows about that's still available [13:23] Bug #1463826 opened: TestProvisioningDoesNotOccurWithAnInvalidEnvironment fails [13:27] mgz_: http://cloud-images.ubuntu.com/releases/streams/v1/com.ubuntu.cloud:released:aws.json is still dominated by pv. I assume hvm will become the peferred typ [13:28] sinzui: yes, I think we should have a hvm one for each release though? [13:29] mgz_: I think so. Our get_ami prefers pv. I think all ami we provision are pv [13:44] Bug #1458721 changed: lease: data races in tests [13:44] Bug #1459085 changed: worker/logger: data race in tests [13:44] Bug #1461385 changed: apiserver/instancepoller: data race in test [13:50] Bug #1458721 opened: lease: data races in tests [13:50] Bug #1459085 opened: worker/logger: data race in tests [13:50] Bug #1461385 opened: apiserver/instancepoller: data race in test [13:53] Bug # changed: 1456315, 1458721, 1459085, 1461385, 1462412 [13:55] mgz_: can you review http://reviews.vapour.ws/r/1909/ [13:57] sinzui: lgtm [13:57] thank you mgz_ [14:51] Bug #1463870 opened: unitSuite teardown fails [15:19] dimitern: it's simple [15:19] dimitern: we're not populating the NetworkInfo of StartInstanceResult inside the StartInstance method of the brokers [15:19] dimitern: easy fix [15:20] voidspace, really? I though we did.. [15:20] * dimitern is looking at commit history of lxc-broker.go [15:20] dimitern: take a look at the last line of StartInstance in lxc-broker.go [15:21] voidspace, aw ffs.. [15:22] voidspace, of course :) good catch! I was thinking of args.NetworkInfo being updated after PCII() [15:23] dimitern: at least it's easy to fix :-) [15:23] dimitern: from my debugging, it *will* be propagated all the way up [15:24] voidspace, yeah, easy fix indeed, but please make a quick live test to make sure populating it won't break anything [15:27] dimitern: sure [15:27] dimitern: it *really* shouldn't [15:28] dimitern: but you never know I guess... [15:29] voidspace, yeah, it might break some tests around juju status, if it expects no networks to be listed [15:30] voidspace, as the networks added during SetInstanceInfo will show up there [15:30] dimitern: sure, I'll have to fix test failures [15:30] dimitern: and I'll check any failures to see if they're significant [15:33] voidspace, cheers! [15:41] this state.IPAddress <-> network.Address <-> params.Address sometimes drives me crazy. [15:41] would have liked a dumb type model package with pure structs for our model and and pure function interface to State (or however the persistency layer is called). [15:42] and the api as well as the persistency layer are accepting the dumb model [15:48] * TheMue takes a relaxing bike ride home and continues later [15:48] dimitern: ooh, now I have a provisioner error - invalid network name, so it looks like the parameters need a bit of massaging [15:48] digging in [15:48] empty network name: "" [15:52] TheMue: Could you do me a review of http://reviews.vapour.ws/r/1910/ ? [15:53] voidspace, right [15:54] voidspace, that's because NetworkName should be set on the InterfaceInfo [15:56] dimitern: sure, but I just passed it through from the BridgeNetworkConfig [15:56] voidspace, for now I'd suggest just turning the empty name into a warning log instead of an error in state.AddNetwork [15:57] or maybe just not add the network [15:57] or does every interface need to be associated with a network? [15:57] I'll look [15:57] voidspace, technically - yes, but it's not used anywhere yet [15:58] dimitern: I can still ssh into the container with "juju ssh" [15:58] dimitern: so it's still created ok [16:01] voidspace, that's good :) [16:04] ericsnow: hey reviewing your patch http://reviews.vapour.ws/r/1905 [16:04] katco: thanks [16:04] ericsnow: you lead in saying it's a refactor, but it looks like there's some new stuff? is that right? [16:05] katco: depends on how you look at it :) [16:05] ericsnow: well for instance, i'm not seeing where the leadership stuff came from? [16:06] katco: what leadership stuff? [16:06] katco: ah [16:06] katco: the old test double didn't have those method explicitly but I added them [16:07] ok, ty, that helps frame things [16:27] dooferlad, you have a review btw [16:27] dooferlad, good work! [16:28] dooferlad, don't forget to move your card ;) [16:30] Bug #1461871 changed: worker/diskmanager sometimes goes into a restart loop due to failing to update state [16:36] Bug #1461871 opened: worker/diskmanager sometimes goes into a restart loop due to failing to update state [16:42] Bug #1461871 changed: worker/diskmanager sometimes goes into a restart loop due to failing to update state [16:42] Bug #1463904 opened: TestReadLeadershipSettings fails [16:47] ericsnow, wwitzel3, katco: I need input on whether my plugin architecture is overkill or not: https://godoc.org/gopkg.in/natefinch/juju.v0/procmanager/plugins/api [16:48] natefinch: k [16:48] I think that if we never add anything to it, then it is. But I think the chances that we never add anything to it are small. [16:49] natefinch: tal [16:49] right now it can all be accomplished with simple CLI input and stdout output with return codes..... [16:55] katco: thanks for the review [16:55] ericsnow: ty for the refactor [16:57] natefinch: it doesn't seem like overkill to me [16:57] natefinch: one suggestion: for states: maybe just have 1 "errored" state, and allow juju to query the plugin for more info? [16:58] natefinch: it could allow for plugin-specific detail w/o having to enumerate all possible states [16:59] katco: my concern is that I'm making plugin authors write a whole RPC client thingy, when, right now, the requirements are simple enough to allow just printing to stdout and signifying an error via a non-zero exit code. [16:59] natefinch: that is a good point [17:00] natefinch: let's look at it practically... what languages will people most likely be writing plugins in? [17:02] katco: no idea. It would be trivial for us to make a helper packages for Go and python.... but as I state in my doc, it pretty much precludes bash. [17:03] natefinch: well not completely, right? bash can just as easily fork out to something that could create the json [17:03] katco: well, yes. but it's a lot harder than it is in a real programming language where someone's already written all the json-rpc code. [17:04] natefinch: well, what i mean to say is: echo $(python my-rpc-generator --id=1 --err="Couldn't start container") or some such [17:05] natefinch: plugins can look for assistance for the heavy lifting [17:06] I guess the question is - how likely is it that the requirements for these plugins is going to get significantly more complicated? If it's pretty likely, then I think the current architecture is fine. If it's not likely to change, then we might want to lower the barrier of entry for third parties writing plugins (that being said, I kinda wonder how many third parties we'll really have writing plugins for this). [17:07] natefinch: i think you're right to worry about the way juju interacts with containers as something that will evolve over time. [17:07] natefinch: that's my take. i'd love ericsnow and wwitzel3's [17:08] natefinch: i think the p(the way juju interacts w/ containers changes) > p(need to support more containers) [17:12] Bug #1463910 opened: Upgrade tests timeout on ppc64 [17:18] natefinch: it may be too much [17:18] natefinch: we need 3 commands (launch/status/stop) [17:19] natefinch: I don't think we need an Info; launch should return the LaunchDetails [17:19] ericsnow: info is status [17:19] natefinch: the status command should return just the status string that makes sense for the plugin [17:20] natefinch: I could see the status command returning extra volatile status-like data [17:21] ericsnow: this is what I have for how juju calls the plugins: https://godoc.org/gopkg.in/natefinch/juju.v2/procmanager/plugins [17:21] natefinch: having the status command return ProcessInfo means the plugin would have to store all that information (information Juju already has and would ignore) [17:21] ericsnow: I was assuming that you're getting information from the plugin, and storing that in state... otherwise, where are you getting that information from [17:22] natefinch: that is correct [17:22] natefinch: the plugin is not expected to store that information [17:23] ericsnow: I didn't think it would. Info just returns information about the process... whatever information it thinks is appropriate [17:23] natefinch: the only thing we need right now would be status [17:25] natefinch: I'm not sure that extra "Details" field is worth it [17:26] natefinch: is your branch based on our feature branch? [17:27] ericsnow: no... I could make it so, though [17:27] natefinch: that may help you [17:28] ericsnow: I was referencing it, and the doc... under "info" it has "technology specific details" which I figured must come from the plugin [17:28] natefinch: those details are a one-time thing at launch time [17:29] ericsnow: ok, I didn't realize that [17:29] natefinch: ah, sorry that wasn't more clear === kadams54_ is now known as kadams54-away [17:54] Bug #1463922 opened: Text file busy [17:55] katco, ericsnow: ok, so, I think what I'll do is rework the plugins to just be CLI input writing to stdout, and if we later need to make them complicated, we can just create new-style plugins or something. [17:55] natefinch: k [18:04] are there certain things that are not available in the leader context? when leader-elected is run I tried to gather some context info and nothing worked [18:08] cholcombe: should be the same as any other context [18:08] ok then i'm seeing some odd behavior [18:08] in the leader-elected hook JUJU_RELATION_ID and JUJU_RELATION are coming up blank [18:10] I'm not 100% up to speed on the leader election stuff.... but those are relation-specific variables that generally only get set during relation hooks (relation changed, relation joined etc). [18:11] ok [18:11] so what should i be doing in the leader-elected hook? [18:11] i'm not sure i have enough info in that hook to do anything [18:13] cholcombe: that's possible. you can always run is-leader or leader-get from other hooks to get info about the leader [18:13] yeah that's what is also strange [18:13] when i run is-leader in the other hooks they always return False [18:13] i'm on juju 1.23.3 [18:18] Bug #1461578 opened: TestKVMProvisionerObservesConfigChange fails [18:19] cholcombe: The guy who knows most about the leadership stuff is not here right now, though I think he said he'll be on later. Look for fwereade. Unfortunately, I don't know very much about it, since it's a new feature and still in the experimental phase. It should *function* however... [18:19] ok thanks :) [18:23] katco: PTAL: https://github.com/juju/charm/pull/137 [18:28] * natefinch just got smacked with a "Pull Requests welcome" when filing a bug against godoc.org :/ [19:01] ericsnow: this could use some comments: https://github.com/juju/charm/pull/137/files#diff-c0e084d1eb96781ebe2d307ae3dcebcbR265 [19:01] ericsnow: i'm having a little trouble understanding what that code is trying to do [19:02] ericsnow: the public method doesn't have a comment either (drive-by fix) [19:02] katco: k [19:03] katco: feel free to leave comments on the PR [19:04] ericsnow: sorry, doing so [19:07] ericsnow: so you said launch is expected to return an id and some details? === kadams54-away is now known as kadams54_ [19:10] * perrito666 read lunch [19:10] haha [19:11] natefinch: yep [19:11] ericsnow: where details is... like a map or something? [19:12] natefinch: LaunchDetails in package/plugin.go [19:15] ericsnow: did you push? I don't see it: https://github.com/juju/juju/blob/feature-proc-mgmt/process/plugin.go [19:16] natefinch: oh, it's ProcessDetails [19:17] ericsnow: I figured status was what was returned by the Status call [19:18] natefinch: yep [19:18] natefinch: it's in ProcessDetails for convenience [19:18] ok, so... launch should return the ID of the process and also whatever status on that id would report? [19:21] i've been neglectful of my duties. someone needs to be looking at this: bug 1463480 [19:21] Bug #1463480: Failed upgrade, mixed up HA addresses [19:23] (let ((victims '(wwitzel3 ericsnow perrito666 natefinch cherylj))) [19:23] (elt victims (random (length victims)))) [19:23] cherylj you have been selected by my computer's pseudorandomness [19:24] katco: you need a bot to do that [19:24] natefinch: since i use emacs, my irc client is the bot :) [19:25] katco: yes, but if there was a bot then other people with lesser editors could use it, too :) [19:25] mup: why can't you do this, ya mup. [19:25] katco: I apologize, but I'm pretty strict about only responding to known commands. [19:26] mup: help [19:26] natefinch: Run "help " for details on: bug, contrib, echo, help, infer, login, poke, register, run, sendraw, sms [19:26] heh [19:26] mup: help infer [19:26] katco: infer [-all] — Queries the WolframAlpha engine. [19:26] katco: If -all is provided, all known information about the query is displayed rather than just the primary results. [19:26] katco: yeah, I can take a look... [19:26] mup: infer randomly select between 1 and 4 [19:26] katco: 1. [19:27] mup: infer randomly select between these options: natefinch wwitzel3 ericsnow and cherylj [19:27] katco: Cannot infer much out of this. :-( [19:27] mup: infer randomly select natefinch wwitzel3 ericsnow cherylj [19:27] katco: Cannot infer much out of this. :-( [19:31] mup: infer RandomChoice{"natefinch"|"wwitzel3"|"ericsnow"|"cherylj"} [19:31] katco: Cannot infer much out of this. :-( [19:31] mup: infer RandomChoice{"natefinch" | "wwitzel3" | "ericsnow" | "cherylj"} [19:31] katco: Cannot infer much out of this. :-( [19:31] gah [19:32] i thought gustavo was using WA for this [19:32] mup: infer how good is juju? [19:32] katco: Cannot infer much out of this. :-( [19:34] cherylj: sorry, thank you for tal. [19:35] katco: it will be a lot easier to go look for the code [19:35] cherylj: i was having too much fun with mup [19:35] perrito666: RandomChoice works on WA's web interface [19:35] katco: the code of mup [19:45] alright all, I am shutting down to head into town to meet up with canonical pdxers, if you need me while I am not online call my cell [19:46] alexlist: tc [19:49] mup: infer RandomChoice[{'wwitzel', 'ericsnow', 'natefinch', 'cherylj'}] [19:50] lol [19:50] natefinch: mup straight up just doesn't like you [19:50] mup: infer RandomChoice[{'wwitzel', 'ericsnow', 'natefinch', 'cherylj'}] [19:50] natefinch: natefinch. [19:50] LOL [19:50] lol [19:50] mup: infer RandomChoice[{'wwitzel', 'ericsnow', 'natefinch', 'cherylj'}] [19:50] natefinch: cherylj. [19:50] there ya go [19:50] confirmed. doesn't like you. [19:56] ericsnow: so... when I launch a process, should launch return both the id and whatever status(id) would return? Or is there special data that is expected to be returned only at launch time? [19:57] natefinch: yeah, there may be extra data [19:58] ericsnow: ok, so I'm going to return a string id and a map[string]interface{} that'll be whatever yaml garbage launch wants to spit out [19:58] natefinch: yep [19:58] * natefinch assumes we'll want it to be yaml because that's the Juju way, even though yaml is terrible ;) [21:01] katco, ericsnow: new plugins - https://godoc.org/gopkg.in/natefinch/juju.v3/process/plugin [21:01] out for dinner, back in a few hours === natefinch is now known as natefinch-afk [21:24] mramm: I'm in our hangout a bit early if you are free === kadams54_ is now known as kadams54-away [22:45] davecheney: would https://github.com/go-check/check/pull/35/files fix our problem? [23:15] trivial review for someone: http://reviews.vapour.ws/r/1911/diff/# [23:16] if we had a trivial tag, I'd just land it :-)