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