/srv/irclogs.ubuntu.com/2017/04/20/#juju-dev.txt

=== petevg is now known as petevg_afk
axwwallyworld: 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 beta01:17
cmarsanyone up for a fairly mechanical review? https://github.com/juju/juju/pull/725302:19
cmarspm me if you have any questions02:20
mupBug #1682827 changed: Bootstrap on OpenStack Cloud fails <juju:New> <https://launchpad.net/bugs/1682827>02:31
axwwallyworld: so you ok with me landing that MAAS PR then?03:00
wallyworldyeah03:01
wallyworldwe can followup over the next few days and try and get it validated03:01
cmarsaxw, thanks for the review03:10
axwcmars: np03:11
wallyworldaxw: the vsphere pr is reviewed03:18
axwwallyworld: thanks03:18
wallyworldhopefully that will be the last change for a while... :-)03:18
axwwallyworld: I'm going to leave it at 2 sec to match what the govc tool does, OK?03:26
wallyworldaxw: sure, np. was just curious03:26
axwwallyworld: no worries. thanks03:26
jamaxw: it doesn't seem right to disable the default cpu/cores stuff06:30
jamaxw: as you can supply '--constraints' if you really are constrained06:31
jamthe out-of-the-box result should be something that we feel is reasonable for a controller06:31
jamI know we've had problems with <1GB mem in the past06:31
axwjam: we already have controller-specific constraitns in the provider/common package06:31
jamaxw: wouldn't that also be expected for any charm?06:31
jamdefault cpu cores = 2 might be a bit much06:32
jammaybe 1 core 1GB ?06:32
jambut removing it entirely seems like you'll just run into "all these charms just don't seem to work"06:32
axwjam: AFAIK, no other provider overrides CPU/mem in the provider code06:33
axwjam: 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
jamaxw: I can't say as to where it exists, but some sort of sane "minimum default" seems useful.06:34
axwjam: 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 wrong06:35
jamaxw: could we just handle the "this is already off" error instead of pre-checking?06:36
jamnot sure if they give clear errors06:36
axwjam: maybe, not sure if the destroy task will still be actioned if that happens. I'll look at optimising it later06:38
jamaxw: not a big deal, pre-check is always a little bit racy06:39
iceyjuju is once again not setting JUJU_AVAILABILITY_ZONE in all cases: https://bugs.launchpad.net/juju/+bug/168432506:46
mupBug #1684325: customize-failure-domain has no effect when ceph-mon is deployed in a container <juju:New> <https://launchpad.net/bugs/1684325>06:46
axwjam: 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 work07:35
jamaxw: k07:36
wallyworldjam: 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
jamwallyworld: juju should be defining default constraints for a controller, it doesn't have to come from vsphere07:43
wallyworldright, that's what we do07:44
wallyworldi was confused as to your pushback on the PR07:44
wallyworldall that was done was special vSphere constraints were removed and we rely on the controller contraints already in place07:44
axwwallyworld: the one piece I don't think we do in one location, is set default constraints for non-controller machines07:45
axwwallyworld: e.g. setting 1GB mem. I think we might do that when selecting instance types, but that doesn't apply to vSphere07:46
wallyworldright, there's aren't any07:46
wallyworldhmm, ok there could a mem constraint07:46
wallyworldnot sure07:46
axwwallyworld: anyway, as I said above the OVF says by default that it needs 1GB mem, 2 vCPUs07:47
axwdoesn't specify Hz07:47
wallyworldso we are ok then07:47
axwwallyworld: yes, but I think we should check the minimum memory, in case the images change, or images for other series/OS come along07:48
wallyworldif that's what we do for other providers, sgtm07:48
axwwallyworld: doesn't need to be done as a constraint, just check the ImportSpec that gets generated from the OVF07:48
axwI'll do it later, working on higher priority things atm07:49
wallyworldyep07:49
wallyworldaxw: 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/725807:50
axwwallyworld: ok, probably a bit later - I keep context switching and losing where I'm at :)07:51
wallyworldyeah, no worries, leave it go, just wanted to let you know it was there for later07:52
wallyworldtomorrow is fine07:52
wallyworldthe gui doen't even use it yet07:52
rogpeppeaxw, wallyworld, jam: any chance of a review of https://github.com/juju/juju/pull/7254?08:07
wallyworldrogpeppe: i'm just heading out to soccer but will be back later this evening08:07
rogpeppewallyworld: k08:08
mupBug #1680582 changed: Agents losing connection to leader tracker <canonical-bootstack> <juju-core:Won't Fix> <https://launchpad.net/bugs/1680582>11:14
mupBug #1680582 opened: Agents losing connection to leader tracker <canonical-bootstack> <juju-core:Won't Fix> <https://launchpad.net/bugs/1680582>11:23
mupBug #1680582 changed: Agents losing connection to leader tracker <canonical-bootstack> <juju-core:Won't Fix> <https://launchpad.net/bugs/1680582>11:29
rogpeppehere's a small addition to juju/utils, in case anyone cares to review it: https://github.com/juju/utils/pull/27311:40
jamrogpeppe: I made some comments on it as well12:14
rogpeppejam: thanks12:14
rogpeppejam: except you're too late :)12:14
jamrogpeppe: its never too late :)12:14
rogpeppejam: well, it's merging already12:15
jamrogpeppe: sure, but you can always land a follow up12:15
rogpeppejam: i'm not convinced by the "no panic ever" thing.12:15
rogpeppejam: lots of our code can panic12:15
jamrogpeppe: we've been bit several times with panic() in production code, and adding more doesn't make it better12:15
rogpeppejam: this is something that's easy to verify at the call site - like a number of panics that are in the go stdlib12:16
jamrogpeppe: well, I'd guess that not all callers are going to be passing in consts12:17
rogpeppejam: more-or-less anything that takes a pointer as an argument can panic12:17
rogpeppejam: in practice, all callers are passing in a const for the "var=" part12:17
rogpeppejam: in this case, i think i prefer the panic to making it return an error12:18
rogpeppejam: FWIW i just used the existing tests from juju/environs/tools/build.go12:20
rogpeppejam: the implementation there didn't panic when not passed a "=" - it just silently got it wrong12:20
rogpeppejam: i guess it could just silently ignore a badly formatted argument12:21
jamrogpeppe: 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 set12:23
rogpeppejam: 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
jamthk12:26
jamthx12:26
rogpeppejam: these are the only calls for the moment, BTW: http://paste.ubuntu.com/24419916/12:27
rogpeppejam: thanks for the review of https://github.com/juju/juju/pull/725412:28
rogpeppejam: here you go: https://github.com/juju/utils/pull/27412:34
jamrogpeppe: lgtm12:35
rogpeppejam: thanks12:36
rogpeppejam: wanna approve it on the PR?12:36
jamI did12:36
jamwell, I hit approve, apparently not submit review12:36
jamrogpeppe: done12:36
rogpeppejam: thanks12:37
wpkjam: what's the PR you wanted to get reviewed?13:17
jamhttps://github.com/juju/juju/pull/726013:17
zeestratIs there a way to use dynamic variables such as env variables when deploying bundles with the native juju deploy in 2.x?13:22
wpkis there a doc on building juju dpkg?13:32
wpk*deb13:32
wpk?13:32
rogpeppeanyone have a clue what failed here? http://juju-ci.vapour.ws:8080/job/github-merge-juju-utils/178/console13:34
wpkglitch in matrix13:40
wpkwatch out for people in black coats13:40
rogpeppewpk: :)13:42
rogpeppehere's a small change to make the cmd tests pass under Go tip: https://github.com/juju/juju/pull/726113:43
rogpeppe i'm looking for a second review of this if poss, please: https://github.com/juju/juju/pull/726116:47
=== frankban is now known as frankban|afk
thumpermorning21:01
=== mup_ is now known as mup

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!