/srv/irclogs.ubuntu.com/2017/01/18/#juju-dev.txt

* thumper has a meeting with a bank person...00:40
thumperafk for a bit00:40
* perrito666 afk for the night, c u all00:47
babbageclunkmenn0: review plz? https://github.com/juju/juju/pull/682700:53
* redir eods01:47
axwwallyworld: can you please see my comments on https://github.com/juju/juju/pull/679502:07
wallyworldsure02:08
wallyworldaxw: commented02:16
wallyworldsee if you think extracting assertDestroy is worthwhile02:16
axwwallyworld: ta02:18
wallyworldthumper: back?02:43
thumperyeah02:43
thumpertrying to work out why this fucking test is failing02:43
wallyworldcan you PTAL at that PR?02:43
wallyworldi've fixed the things02:43
thumperyeah02:44
wallyworldta. sorry02:44
thumperwallyworld: lgtm02:46
wallyworldgreat tyvm02:46
wallyworldaxw: if you get a chance at some stage, would love a review on https://github.com/juju/juju/pull/6819. Once that's in, cmr is good enough (behind the flag) to share out a bit for feedback02:48
axwwallyworld: okey dokey, will look shortly02:48
wallyworldno rush02:48
thumperugh...02:50
thumperpretty sure this timing issue is due to clock.After(0) and the testing clock02:51
thumperhmm... no?02:59
babbageclunkthumper or someone else? I think menn0's busy. Could you take a look at https://github.com/juju/juju/pull/6827 plzthx?03:57
babbageclunkaxw, anastasiamac? ^03:59
axwbabbageclunk: will do, just reviewing wallyworld's one atm03:59
babbageclunkaxw: ok thanks!04:00
anastasiamacbabbageclunk: since axw will b looking, I'll skip :D04:10
axwbabbageclunk: reviewed04:11
babbageclunkanastasiamac: eminently sensible :)04:11
babbageclunkaxw: thanks!04:11
anastasiamacbabbageclunk: i'd question both :0 but nice that u think so04:11
anastasiamacaxw: babbageclunk: i like "adopt"... makes it sound almost humanitarian and benevolent04:12
axwanastasiamac: :p04:13
axwcan't have those instances roaming the streets04:13
anastasiamacunfed, unclothed...04:13
axwpick a pocket or two04:13
babbageclunkaxw, anastasiamac: I think I'm ok with AdoptInstances (although in this case it's a bit more like "here, you take them!")04:14
babbageclunkwhich, who hasn't felt like that sometimes?04:14
anastasiamacfostering does not have the same 'share the love" feel04:14
anastasiamacbabbageclunk: i didnt, until recently04:15
axwfoistering? :)04:17
axwI need to take a break from punning, working up a sweat here. lunch time04:18
babbageclunkaxw: That's some solid work!04:19
menn0babbageclunk: did you still need a review?04:41
babbageclunkmenn0: no thanks, axw gave me a better review anyway!04:45
babbageclunk;)04:45
menn0babbageclunk: ouch!04:45
* menn0 will remember to be nasty on his next review for babbageclunk 04:45
* babbageclunk d'ohs04:45
babbageclunkanastasiamac: Want to look at this one? https://github.com/juju/juju/pull/6829 (or axw)05:27
axwbabbageclunk: reviewed05:33
babbageclunkaxw: thanks! Yes, SetOwnerData will preserve keys that aren't set - I've updated the comment.05:40
axwbabbageclunk: thanks05:40
wallyworldaxw: can i get comment access to that storage doc?07:38
axwwallyworld: sorry, one sec07:39
axwwallyworld: try now07:39
wallyworldta07:39
axwwallyworld: there's a question from james beedy on your PR if you haven't seen07:39
wallyworldaxw: not yet, been out talking to anastasia and looking at specs07:40
=== frankban|afk is now known as frankban
axwwallyworld: I think I've addressed all of your comments in the doc, thanks for the review/comments. Would appreciate another look - I'm heading out shortly, so it can wait till tomorrow09:05
rogpeppeaxw: hiya09:21
rogpeppeaxw, wallyworld: do you know if the juju master branch is used any more?09:22
anastasiamacrogpeppe: it's meant to b releasable (for next release) but is not at this stage09:28
anastasiamacrogpeppe: why?09:28
anastasiamacrogpeppe: if u have smth for 2.2, it goes into develop, otherwise each release has it's branch 2.0, 2.1, etc09:29
rogpeppeanastasiamac: the last commit was in october09:29
rogpeppeanastasiamac: i thought it would be more up to date than the version branches09:29
anastasiamacrogpeppe: yes, there is a new dev process in place; we/ve benn releasing from staging09:29
anastasiamacrogpeppe: master is kind of "reserved"; we need to promote to it from develop>staging>master once we have a bless and passing tests09:30
jamperrito666: if you're around I have another 'juju show-status-log' question. Specifically, the last step is putting the machine into an error status, and that doesn't seem to be shown in the status log09:38
jamonly the 'pending' entries are09:38
jamperrito666: also wide entries in 'message' cause all lines to wrap09:41
jamperrito666: https://bugs.launchpad.net/juju/+bug/165738309:46
mupBug #1657383: juju show-status-log wraps all entries if one is too wide <show-status-log> <ui> <juju:Triaged> <https://launchpad.net/bugs/1657383>09:46
jamperrito666: maybe its a throttling thing, but I see 4 messages in 'juju debug-log' but only 2 in 'juju show-status-log' https://pastebin.canonical.com/176285/09:58
jamwallyworld: ^^09:58
=== jamespag` is now known as jamespage
rogpeppethis PR fixes the logging on API connect so that it doesn't print misleading errors: https://github.com/juju/juju/pull/683010:36
rogpeppejam: I think you reviewed the change that started printing the errors - you might want to take a look at this ^10:37
rogpeppemacgreagoir: likewise ^10:38
jamrogpeppe: your !a.HasNext() means that certError may be a nil object in the next print statement10:38
jamin the Debugf10:38
jamah, its a bool not an actual error10:39
rogpeppejam: perhaps i should rename it isCertErr10:39
jamrogpeppe: that might help, but I'm also wondering about the format string.10:39
jamerror dialing (certificate error: true): blah10:40
jamvs10:40
jamerror dialing (certificate error: false): blah10:40
jamI'm not sure if there is actual value in knowing about the certificate error status10:40
rogpeppejam: yeah, probably not. i just left it there because that PR seemed to find it useful10:40
rogpeppejam: it should be fairly obvious from the error message tbh10:41
jamrogpeppe: right. I'd like us to actually look at that instead10:41
jamrogpeppe: and then the comment needs updating10:41
rogpeppejam: i'll remove that, which makes the code exactly what it was before, i think10:41
jameg: // we won't retry anymore. Either this is a certificate error, or we're out of attempts10:41
rogpeppejam: before PR #662010:41
jamrogpeppe: that would indicate that the errors *aren't* clear.10:43
rogpeppejam: sorry, what would indicate that?10:43
rogpeppejam: AFAIK all those x509 errors include the string "x509" in them10:43
jamrogpeppe: PR 6620 would indicate that it was added because the error messages weren't clear10:43
rogpeppejam: the thing is that nothing outside of that code really cares whether it's a cert error or not10:44
rogpeppejam: the diagnosis is only used so we can abort early10:44
rogpeppejam: so unless you're specifically debugging that code, it's probably not worth making a deal of it10:45
rogpeppejam: the new "will retry" message should make it more obvious when a certificate-like error is being inappropriately retried10:46
jamrogpeppe: so if it was helpful for context of someone working around there, it is likely to be helpful to someone trying to help someone else who's having a problem.10:46
rogpeppejam: so you you now think that showing whether it's a cert error is worth doing?10:46
jamrogpeppe: I don't think the Annotation difference is necessary, but I think we want to keep something about certificate in the logged message, yes10:47
rogpeppejam: so what would you suggest to change in the current PR?10:47
jamrogpeppe: would "bad certificate" be clearer than "certificate error: true" ?10:48
rogpeppejam: i think that could be misleading10:48
rogpeppejam: the error string should say what the actual issue is - the important thing from the p.o.v. of that message is whether it was classified as a cert error, i think10:49
jamrogpeppe: btw, you missed cfg.Location on the new Debugf10:49
rogpeppejam: good catch, fixed10:50
jamrogpeppe: I take it you didn't actually run it to see the messages were helpful.10:50
jamexample output helps in this situation10:50
jamI'm happy with the change to Debugf10:50
jamI'm not 100% sure how to succinctly convey whether it was a certificate error.10:51
jamthe rest LGTM10:51
rogpeppejam: tbh i think that if you're trying to debug it, it'll be obvious in almost all cases whether it was a cert error - if it wasn't a cert error, it'll retry10:51
rogpeppejam: unless it's timed out, which is unlikely to happen immediately10:52
jamrogpeppe: so, as *I* haven't been trying to debug it, but macgreagoir did, I'd be interested to hear his thoughts10:52
rogpeppejam: here's some sample output: http://paste.ubuntu.com/23821416/10:53
jamrogpeppe: I have to say "certificate error true" doesn't help much when the error is ... cannot validate certificate...10:54
jamrogpeppe: its also redundant to put the cfg.Location when that's also in the error10:55
rogpeppejam: it might when the error is "x509: failed to load system roots and no roots provided"10:55
jamI wonder if that is normally true10:55
rogpeppejam: looks like it is10:56
jamrogpeppe: as you say, x509 probably gives enough context, and I *am* concerned about having errors that don't feel like garbage to read. Always embedding the Location makes it just look like nobody has ever read any of our debug output10:56
jamrogpeppe: is that always true for x509, or true always? Like will plain Dial also have the URL?10:57
rogpeppejam: looks like it's true for all websocket dials10:58
macgreagoirrogpeppe jam: I can't remember the bug I was debugging when I separated the x509 errors. It seemed useful at the time, so I left it in. If it happens not to be useful, please roll it out.10:58
jamhi macgreagoir. thanks. all the ones that I've seen so far also have that information in the error, so it feels redundant.10:58
jammacgreagoir: though the problem with errors is that its hard to know if you've seen 'all' of them10:59
jamrogpeppe: I'm tempted to take out the cfg.Location, though because we're dialing multiple locations, if you ever get one without that context you're lost10:59
jam(what dial failed)10:59
rogpeppejam: one possibility would be to remove the websocket.DialError wrapper, then always add the context11:00
rogpeppejam: but that's probably more logic than i'd ideally want to add for a fairly superficial thing11:01
jamrogpeppe: so on one hand its 'not a big deal', on the other it is potentially one of your first interactions, which is why you're dropping it to debug anyway11:02
rogpeppejam: websocket.DialConfig always returns a *websocket.DialError11:02
rogpeppejam: output now looks like this: http://paste.ubuntu.com/23821568/11:10
rogpeppejam: i've updated the PR11:10
wallyworldjam: sorry, missed your msg, was out to dinner. my recollection is that throttling could account for the observed behaviour, if it is not smart enough to account for message content as well as status value itself. without looking into it, i'd have to ask horatio12:11
jamwallyworld: I'm wondering if we should be throttling at all, given we're starting to make active use of it ourselves.12:12
jamwallyworld: anyawy, I'm happy to talk to perrito666 instead, he just needs to wake up. :)12:13
jamwallyworld: I hope you enjoyed your dinner.12:13
wallyworldyeah, throttling was only ever meant to kill reptitive status-update hook messages. it was a quick implementation that could use some rework AFAIR12:13
wallyworldi did, we went to a Jamie Olivier restaurant12:14
wallyworldi think there's been a few issues with the throttling, there's a bug or three still12:14
wallyworldjam: btw, thanks for comments on cmr spec. i'll garden a bit and put out for comment once i land one more pr12:15
jamwallyworld: happy to.12:16
jamping me after you've done it, hopefully I can look with fresh eyes12:16
wallyworldwill do, i'll update overnight or first thing tomorrow more likely. i have a little cleanup on the pr to auto expose and i want to land that12:17
wallyworldi don't mind if the spec has questions still to resolve, just want to get it ready for consumption so to speak12:18
jamsure12:18
perrito666jam: sorry I am here now12:26
perrito666jam: you have a few mins until I decide to leave for a coffee and some sort of bakery product which will take about 10 mins then12:33
perrito666jam: ill leave you with an answer at least, throtling was, as wallyworld said, something implemented in a hurry and could use some re-work12:37
rogpeppejam: where's the accepted place to report juju bugs these days? still launchpad?12:47
rogpeppeanyone?12:47
rogpeppeha, given there are only two issues on github.com/juju/juju, i guess so12:48
rogpeppeunless All The Bugs Have Been Fixed12:48
rick_hrogpeppe: yes, launchpad.net/juju12:54
rogpepperick_h: thanks12:54
rogpepperick_h: just filed https://bugs.launchpad.net/juju/+bug/165744812:54
mupBug #1657448: provider/azure: adding credential produces error when service principal exists <juju:New> <https://launchpad.net/bugs/1657448>12:54
wallyworldrogpeppe: andrew will be estatic :-) i'll let him know13:25
* perrito666 imagines wallyworld compiling a static version of andrew13:26
wallyworldone can never have too many of those13:26
perrito666wallyworld: arent you pass your bed time?13:27
wallyworldyeah13:27
wallyworldstuff to do13:28
perrito666wallyworld: do you need any of us to tuck you in ?13:28
wallyworldyes, and a goodnight story13:28
perrito666ill read you a user story for bed13:29
rogpeppewallyworld: thanks13:32
wallyworldrogpeppe: it's probably not even his bug, but mentally I s/azure/andrew :-)13:33
rogpeppewallyworld: it looks like it's a bug in the azure client lib13:33
rogpeppewallyworld: i filed a bug there too13:33
wallyworldsgtm, ty13:33
rogpeppewallyworld: MS loves byte-order marks13:34
wallyworldi bet13:34
perrito666uhhh, that is a cool internal idea, lets rename azure provider andrew provide internally13:34
rogpeppewallyworld: have you used the azure provider much?13:34
wallyworldnot much, just a smoke test or two13:34
natefinchvoidspace, katco, rick_h, frobware, macgreagoir: if there's a standup, I need to miss it to clear the snow in our driveway so we can get the kids to school.14:36
katconatefinch: k14:36
katconatefinch: happy exercize14:36
natefinch:)14:36
frobwaremerge jobs seem to be failing with either 'go' not found or 'lxd' not found. any ideas?15:50
frobwarehttp://juju-ci.vapour.ws:8080/job/github-merge-juju/10057/15:50
perrito666wow sounds like corrupt server15:51
perrito666bbl, have people doing some fixes at the house and need to shut down the power for a moment18:21
redirwoohoo nested arm64 booted18:22
jamperrito666: ping18:30
jamredir: nested?18:31
redirjam18:31
rediryes18:31
jamwhat do you mean by nested arm64?18:31
jamkvm in kvm ?18:31
redirbut just by fiddling with the domainXML18:31
redirjam yes18:32
jamah, not somethnig like running virtualized arm board on an arm board.18:32
rediryes with kvm acceleration instead of emulation18:32
redirlike we do for amd6418:32
redirit's apparently a bit different18:33
redirand it appears it will only work on xenial18:33
redirnot trusty18:33
redir:(18:34
jamredir: is it a kernel thing, such that HWE kernel would work on trusty, or is it userspace thing and the tools aren't backported (or HWE doesn't have it, either)18:36
=== dames is now known as thedac
redirjam AFAIU the qemu-efi package requires 15.04 minimum18:37
jamperrito666: when you have power again, can you have a look at https://github.com/juju/juju/pull/6828 I ended up needing to tweak some of the InstanceStatus stuff and realized we were missing some test coverage.18:38
redirI have some notes I'll add to the wiki.18:38
jamperrito666: Also, the different states for InstanceStatus make me wonder if I should be setting something different in the worker18:39
jamnamely Running vs Started and Provisioning vs Pending.18:39
jamperrito666: also, the SetInstanceStatus API accepts all the other values (Pending, etc). Should it be validating the values being set and rejecting ones that aren't in the right set?18:44
=== frankban is now known as frankban|afk
redirjam you got a sec?19:02
jamredir: sort of, I'm not officially working right now, but if you have a Q19:03
redirI have a q about the bridge used on arm19:03
redirshould be real quick19:03
jamsure19:04
redirhttps://hangouts.google.com/hangouts/_/canonical.com/john-reed19:04
redirthanks jam19:27
mupBug #1550821 opened: TestSetsAndUpdatesMembers timed out <ci> <go1.5> <regression> <trusty> <wily> <juju-core:Triaged> <juju-core 1.25:Fix Released> <https://launchpad.net/bugs/1550821>19:47
mupBug #1550821 changed: TestSetsAndUpdatesMembers timed out <ci> <go1.5> <regression> <trusty> <wily> <juju-core:Triaged> <juju-core 1.25:Fix Released> <https://launchpad.net/bugs/1550821>19:53
mupBug #1550821 opened: TestSetsAndUpdatesMembers timed out <ci> <go1.5> <regression> <trusty> <wily> <juju:Triaged by thumper> <juju-core:Fix Released> <juju-core 1.25:Fix Released> <https://launchpad.net/bugs/1550821>20:02
* redir lunches20:08
mupBug #1550821 changed: TestSetsAndUpdatesMembers timed out <ci> <go1.5> <regression> <trusty> <wily> <juju:Triaged by thumper> <juju-core:Fix Released> <juju-core 1.25:Fix Released> <https://launchpad.net/bugs/1550821>20:11
perrito666jam: pong, why do you always ping me when I am away?20:15
perrito666jam: ill check your PR20:17
perrito666jam: LGTM your changes seem sane enough20:27
anastasiamacthumper: since u were in cert's area, would u have a sec to look at https://github.com/juju/juju/pull/683121:28
thumperanastasiamac: later... in a deep dive now21:29
anastasiamacthumper: sure \o/ just flagging to for ur perusal21:30
anastasiamacthumper: this is a companion PR:  https://github.com/juju/utils/pull/26121:31
thumperack21:31
sinzuiabentley: can you review https://code.launchpad.net/~sinzui/juju-reports/ignore-missing-tasks/+merge/31507621:47
abentleysinzui: r=me with some comments.21:57
sinzuiabentley: thank you.21:59
perrito666brb22:00
menn0veebers: so no perfscale meeting22:01
menn0?22:01
veebersmenn0: hey, sorry I sent an email about re-scheduling it for next week22:02
menn0veebers: I vaguely remember that. just thought I'd still seen the meeting on the calendar this morning.22:03
menn0ignore me :)22:03
veebersmenn0: sorry about that, I only just moved the cal event :-P22:04
menn0all good... I was away at a meeting with an accountant and am just catching up.22:04
babbageclunkaxw: ping?23:07
anastasiamacbabbageclunk: it's 7am for axw... just saying...23:07
babbageclunkanastasiamac: yeah, I keep forgetting. I'm out this afternoon though.23:08
perrito666anastasiamac: I get up at 6:00 :p23:08
anastasiamacbabbageclunk: u could leave a msg here, m sure he'll read \o/23:08
anastasiamacperrito666: ah... we all are up early (and stay up late) but r u functioing and working at that time?23:09
perrito666anastasiamac: I never function :p23:11
perrito666btw, wasnt this like our previous standup time?23:14
perrito666we could go back there23:14
anastasiamacperrito666: it was :) and we could :) let's discuss today23:15
anastasiamacperrito666: alto nz'ers might b at or near their lunch ...23:16
axwbabbageclunk: pong23:28
babbageclunkoh hey axw, sorry for timezone confusion23:28
babbageclunkaxw: I'm doing AdoptInstances for ec2, and I think I need to update tags for volumes as well as for instances?23:29
babbageclunkaxw: thumper suggested you'd know about that?23:30
axwbabbageclunk: yes, that is correct23:31
axwbabbageclunk: several of the providers have support for storage, and use tagging (where possible) similarly to how it's used for tracking instances23:32
axwbabbageclunk: there's a separate interface for dealing with volumes/filesystems23:32
axwbabbageclunk: theoretically filesystems have the same issue, though there are no cloud filesystem implementations yet23:32
babbageclunkaxw: I can see that ec2 CreateTags will work on volume IDs as well - what's the best way to get all the volume IDs for a set of instance IDs?23:33
axwbabbageclunk: volumes aren't necessarily attached to an instance. they can be floating23:33
babbageclunkaxw: Oh, right - so it would need to happen higher up? The migration master would call AdoptVolumes as well as calling AdoptInstances?23:34
axwbabbageclunk: yes. and AdoptFilesystems23:34
babbageclunkaxw: ok - and what interface should those live on? environs.Environ or somewhere else?23:35
axwbabbageclunk: storage/{Filesystem,Volume}Source I think would be most appropriate23:35
babbageclunkaxw: ok, thanks! that probably gives me enough to start with.23:36
axwbabbageclunk: np, let me know if you get stuck23:36
babbageclunkaxw: I almost certainly will!23:36
babbageclunk(get stuck I mean)23:37
axw:p23:37

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