=== petevg is now known as petevg_afk [01:17] wallyworld: I have a few fixes for vsphere in https://github.com/juju/juju/pull/7256, I'd appreciate a review at some point so I can land it for the beta [02:19] anyone up for a fairly mechanical review? https://github.com/juju/juju/pull/7253 [02:20] pm me if you have any questions [02:31] Bug #1682827 changed: Bootstrap on OpenStack Cloud fails [03:00] wallyworld: so you ok with me landing that MAAS PR then? [03:01] yeah [03:01] we can followup over the next few days and try and get it validated [03:10] axw, thanks for the review [03:11] cmars: np [03:18] axw: the vsphere pr is reviewed [03:18] wallyworld: thanks [03:18] hopefully that will be the last change for a while... :-) [03:26] wallyworld: I'm going to leave it at 2 sec to match what the govc tool does, OK? [03:26] axw: sure, np. was just curious [03:26] wallyworld: no worries. thanks [06:30] axw: it doesn't seem right to disable the default cpu/cores stuff [06:31] axw: as you can supply '--constraints' if you really are constrained [06:31] the out-of-the-box result should be something that we feel is reasonable for a controller [06:31] I know we've had problems with <1GB mem in the past [06:31] jam: we already have controller-specific constraitns in the provider/common package [06:31] axw: wouldn't that also be expected for any charm? [06:32] default cpu cores = 2 might be a bit much [06:32] maybe 1 core 1GB ? [06:32] but removing it entirely seems like you'll just run into "all these charms just don't seem to work" [06:33] jam: AFAIK, no other provider overrides CPU/mem in the provider code [06:33] jam: hmm, maybe the 1GB limit is in the provider, mixed in with image selection. seeing it in the azure provider now. I can put that in. [06:34] axw: I can't say as to where it exists, but some sort of sane "minimum default" seems useful. [06:35] jam: it's different because it'll vary by site. but yes, we should - I thought it was handled outside the provider, but looks like I was wrong [06:36] axw: could we just handle the "this is already off" error instead of pre-checking? [06:36] not sure if they give clear errors [06:38] jam: maybe, not sure if the destroy task will still be actioned if that happens. I'll look at optimising it later [06:39] axw: not a big deal, pre-check is always a little bit racy [06:46] juju is once again not setting JUJU_AVAILABILITY_ZONE in all cases: https://bugs.launchpad.net/juju/+bug/1684325 [06:46] Bug #1684325: customize-failure-domain has no effect when ceph-mon is deployed in a container [07:35] jam: turns out there's a default of 1GB mem in the OVF anyway, and then vcenter may allocate more. since it's defined in the images and they could change, we should still check - but I'm going to defer until after my current work [07:36] axw: k [07:43] jam: axw: i checked the code - the controller contraints are processed at bootstrap in withDefaultControllerConstraints() in environs/bootstrap. We honor any constraints the user may have passed in though. why do we think vSphere should be special and get to set it's own constraints that sometimes fail? [07:43] wallyworld: juju should be defining default constraints for a controller, it doesn't have to come from vsphere [07:44] right, that's what we do [07:44] i was confused as to your pushback on the PR [07:44] all that was done was special vSphere constraints were removed and we rely on the controller contraints already in place [07:45] wallyworld: the one piece I don't think we do in one location, is set default constraints for non-controller machines [07:46] wallyworld: e.g. setting 1GB mem. I think we might do that when selecting instance types, but that doesn't apply to vSphere [07:46] right, there's aren't any [07:46] hmm, ok there could a mem constraint [07:46] not sure [07:47] wallyworld: anyway, as I said above the OVF says by default that it needs 1GB mem, 2 vCPUs [07:47] doesn't specify Hz [07:47] so we are ok then [07:48] wallyworld: yes, but I think we should check the minimum memory, in case the images change, or images for other series/OS come along [07:48] if that's what we do for other providers, sgtm [07:48] wallyworld: doesn't need to be done as a constraint, just check the ImportSpec that gets generated from the OVF [07:49] I'll do it later, working on higher priority things atm [07:49] yep [07:50] axw: no rush since you have stuff happening; if you get a chance, but can wait till tomorrow, i have soccer in a bit anyway https://github.com/juju/juju/pull/7258 [07:51] wallyworld: ok, probably a bit later - I keep context switching and losing where I'm at :) [07:52] yeah, no worries, leave it go, just wanted to let you know it was there for later [07:52] tomorrow is fine [07:52] the gui doen't even use it yet [08:07] axw, wallyworld, jam: any chance of a review of https://github.com/juju/juju/pull/7254? [08:07] rogpeppe: i'm just heading out to soccer but will be back later this evening [08:08] wallyworld: k [11:14] Bug #1680582 changed: Agents losing connection to leader tracker [11:23] Bug #1680582 opened: Agents losing connection to leader tracker [11:29] Bug #1680582 changed: Agents losing connection to leader tracker [11:40] here's a small addition to juju/utils, in case anyone cares to review it: https://github.com/juju/utils/pull/273 [12:14] rogpeppe: I made some comments on it as well [12:14] jam: thanks [12:14] jam: except you're too late :) [12:14] rogpeppe: its never too late :) [12:15] jam: well, it's merging already [12:15] rogpeppe: sure, but you can always land a follow up [12:15] jam: i'm not convinced by the "no panic ever" thing. [12:15] jam: lots of our code can panic [12:15] rogpeppe: we've been bit several times with panic() in production code, and adding more doesn't make it better [12:16] jam: this is something that's easy to verify at the call site - like a number of panics that are in the go stdlib [12:17] rogpeppe: well, I'd guess that not all callers are going to be passing in consts [12:17] jam: more-or-less anything that takes a pointer as an argument can panic [12:17] jam: in practice, all callers are passing in a const for the "var=" part [12:18] jam: in this case, i think i prefer the panic to making it return an error [12:20] jam: FWIW i just used the existing tests from juju/environs/tools/build.go [12:20] jam: the implementation there didn't panic when not passed a "=" - it just silently got it wrong [12:21] jam: i guess it could just silently ignore a badly formatted argument [12:23] rogpeppe: if it is only ever used with const values, then I'm ~ok with a panic() but it feels like the type of helper that ends up folded into code that gets called from iterating over an array of env vars someone was asked to set [12:25] jam: ok, i'll make it silently ignore the bad value. i'd prefer that to adding an error return, because it's very convenient to use it inline currently. [12:26] thk [12:26] thx [12:27] jam: these are the only calls for the moment, BTW: http://paste.ubuntu.com/24419916/ [12:28] jam: thanks for the review of https://github.com/juju/juju/pull/7254 [12:34] jam: here you go: https://github.com/juju/utils/pull/274 [12:35] rogpeppe: lgtm [12:36] jam: thanks [12:36] jam: wanna approve it on the PR? [12:36] I did [12:36] well, I hit approve, apparently not submit review [12:36] rogpeppe: done [12:37] jam: thanks [13:17] jam: what's the PR you wanted to get reviewed? [13:17] https://github.com/juju/juju/pull/7260 [13:22] Is there a way to use dynamic variables such as env variables when deploying bundles with the native juju deploy in 2.x? [13:32] is there a doc on building juju dpkg? [13:32] *deb [13:32] ? [13:34] anyone have a clue what failed here? http://juju-ci.vapour.ws:8080/job/github-merge-juju-utils/178/console [13:40] glitch in matrix [13:40] watch out for people in black coats [13:42] wpk: :) [13:43] here's a small change to make the cmd tests pass under Go tip: https://github.com/juju/juju/pull/7261 [16:47] i'm looking for a second review of this if poss, please: https://github.com/juju/juju/pull/7261 === frankban is now known as frankban|afk [21:01] morning === mup_ is now known as mup