[00:40] * thumper has a meeting with a bank person... [00:40] afk for a bit [00:47] * perrito666 afk for the night, c u all [00:53] menn0: review plz? https://github.com/juju/juju/pull/6827 [01:47] * redir eods [02:07] wallyworld: can you please see my comments on https://github.com/juju/juju/pull/6795 [02:08] sure [02:16] axw: commented [02:16] see if you think extracting assertDestroy is worthwhile [02:18] wallyworld: ta [02:43] thumper: back? [02:43] yeah [02:43] trying to work out why this fucking test is failing [02:43] can you PTAL at that PR? [02:43] i've fixed the things [02:44] yeah [02:44] ta. sorry [02:46] wallyworld: lgtm [02:46] great tyvm [02:48] axw: 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 feedback [02:48] wallyworld: okey dokey, will look shortly [02:48] no rush [02:50] ugh... [02:51] pretty sure this timing issue is due to clock.After(0) and the testing clock [02:59] hmm... no? [03:57] thumper or someone else? I think menn0's busy. Could you take a look at https://github.com/juju/juju/pull/6827 plzthx? [03:59] axw, anastasiamac? ^ [03:59] babbageclunk: will do, just reviewing wallyworld's one atm [04:00] axw: ok thanks! [04:10] babbageclunk: since axw will b looking, I'll skip :D [04:11] babbageclunk: reviewed [04:11] anastasiamac: eminently sensible :) [04:11] axw: thanks! [04:11] babbageclunk: i'd question both :0 but nice that u think so [04:12] axw: babbageclunk: i like "adopt"... makes it sound almost humanitarian and benevolent [04:13] anastasiamac: :p [04:13] can't have those instances roaming the streets [04:13] unfed, unclothed... [04:13] pick a pocket or two [04:14] axw, anastasiamac: I think I'm ok with AdoptInstances (although in this case it's a bit more like "here, you take them!") [04:14] which, who hasn't felt like that sometimes? [04:14] fostering does not have the same 'share the love" feel [04:15] babbageclunk: i didnt, until recently [04:17] foistering? :) [04:18] I need to take a break from punning, working up a sweat here. lunch time [04:19] axw: That's some solid work! [04:41] babbageclunk: did you still need a review? [04:45] menn0: no thanks, axw gave me a better review anyway! [04:45] ;) [04:45] babbageclunk: ouch! [04:45] * menn0 will remember to be nasty on his next review for babbageclunk [04:45] * babbageclunk d'ohs [05:27] anastasiamac: Want to look at this one? https://github.com/juju/juju/pull/6829 (or axw) [05:33] babbageclunk: reviewed [05:40] axw: thanks! Yes, SetOwnerData will preserve keys that aren't set - I've updated the comment. [05:40] babbageclunk: thanks [07:38] axw: can i get comment access to that storage doc? [07:39] wallyworld: sorry, one sec [07:39] wallyworld: try now [07:39] ta [07:39] wallyworld: there's a question from james beedy on your PR if you haven't seen [07:40] axw: not yet, been out talking to anastasia and looking at specs === frankban|afk is now known as frankban [09:05] wallyworld: 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 tomorrow [09:21] axw: hiya [09:22] axw, wallyworld: do you know if the juju master branch is used any more? [09:28] rogpeppe: it's meant to b releasable (for next release) but is not at this stage [09:28] rogpeppe: why? [09:29] rogpeppe: if u have smth for 2.2, it goes into develop, otherwise each release has it's branch 2.0, 2.1, etc [09:29] anastasiamac: the last commit was in october [09:29] anastasiamac: i thought it would be more up to date than the version branches [09:29] rogpeppe: yes, there is a new dev process in place; we/ve benn releasing from staging [09:30] rogpeppe: master is kind of "reserved"; we need to promote to it from develop>staging>master once we have a bless and passing tests [09:38] perrito666: 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 log [09:38] only the 'pending' entries are [09:41] perrito666: also wide entries in 'message' cause all lines to wrap [09:46] perrito666: https://bugs.launchpad.net/juju/+bug/1657383 [09:46] Bug #1657383: juju show-status-log wraps all entries if one is too wide [09:58] perrito666: 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] wallyworld: ^^ === jamespag` is now known as jamespage [10:36] this PR fixes the logging on API connect so that it doesn't print misleading errors: https://github.com/juju/juju/pull/6830 [10:37] jam: I think you reviewed the change that started printing the errors - you might want to take a look at this ^ [10:38] macgreagoir: likewise ^ [10:38] rogpeppe: your !a.HasNext() means that certError may be a nil object in the next print statement [10:38] in the Debugf [10:39] ah, its a bool not an actual error [10:39] jam: perhaps i should rename it isCertErr [10:39] rogpeppe: that might help, but I'm also wondering about the format string. [10:40] error dialing (certificate error: true): blah [10:40] vs [10:40] error dialing (certificate error: false): blah [10:40] I'm not sure if there is actual value in knowing about the certificate error status [10:40] jam: yeah, probably not. i just left it there because that PR seemed to find it useful [10:41] jam: it should be fairly obvious from the error message tbh [10:41] rogpeppe: right. I'd like us to actually look at that instead [10:41] rogpeppe: and then the comment needs updating [10:41] jam: i'll remove that, which makes the code exactly what it was before, i think [10:41] eg: // we won't retry anymore. Either this is a certificate error, or we're out of attempts [10:41] jam: before PR #6620 [10:43] rogpeppe: that would indicate that the errors *aren't* clear. [10:43] jam: sorry, what would indicate that? [10:43] jam: AFAIK all those x509 errors include the string "x509" in them [10:43] rogpeppe: PR 6620 would indicate that it was added because the error messages weren't clear [10:44] jam: the thing is that nothing outside of that code really cares whether it's a cert error or not [10:44] jam: the diagnosis is only used so we can abort early [10:45] jam: so unless you're specifically debugging that code, it's probably not worth making a deal of it [10:46] jam: the new "will retry" message should make it more obvious when a certificate-like error is being inappropriately retried [10:46] rogpeppe: 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] jam: so you you now think that showing whether it's a cert error is worth doing? [10:47] rogpeppe: I don't think the Annotation difference is necessary, but I think we want to keep something about certificate in the logged message, yes [10:47] jam: so what would you suggest to change in the current PR? [10:48] rogpeppe: would "bad certificate" be clearer than "certificate error: true" ? [10:48] jam: i think that could be misleading [10:49] jam: 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 think [10:49] rogpeppe: btw, you missed cfg.Location on the new Debugf [10:50] jam: good catch, fixed [10:50] rogpeppe: I take it you didn't actually run it to see the messages were helpful. [10:50] example output helps in this situation [10:50] I'm happy with the change to Debugf [10:51] I'm not 100% sure how to succinctly convey whether it was a certificate error. [10:51] the rest LGTM [10:51] jam: 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 retry [10:52] jam: unless it's timed out, which is unlikely to happen immediately [10:52] rogpeppe: so, as *I* haven't been trying to debug it, but macgreagoir did, I'd be interested to hear his thoughts [10:53] jam: here's some sample output: http://paste.ubuntu.com/23821416/ [10:54] rogpeppe: I have to say "certificate error true" doesn't help much when the error is ... cannot validate certificate... [10:55] rogpeppe: its also redundant to put the cfg.Location when that's also in the error [10:55] jam: it might when the error is "x509: failed to load system roots and no roots provided" [10:55] I wonder if that is normally true [10:56] jam: looks like it is [10:56] rogpeppe: 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 output [10:57] rogpeppe: is that always true for x509, or true always? Like will plain Dial also have the URL? [10:58] jam: looks like it's true for all websocket dials [10:58] rogpeppe 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] hi macgreagoir. thanks. all the ones that I've seen so far also have that information in the error, so it feels redundant. [10:59] macgreagoir: though the problem with errors is that its hard to know if you've seen 'all' of them [10:59] rogpeppe: 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 lost [10:59] (what dial failed) [11:00] jam: one possibility would be to remove the websocket.DialError wrapper, then always add the context [11:01] jam: but that's probably more logic than i'd ideally want to add for a fairly superficial thing [11:02] rogpeppe: 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 anyway [11:02] jam: websocket.DialConfig always returns a *websocket.DialError [11:10] jam: output now looks like this: http://paste.ubuntu.com/23821568/ [11:10] jam: i've updated the PR [12:11] jam: 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 horatio [12:12] wallyworld: I'm wondering if we should be throttling at all, given we're starting to make active use of it ourselves. [12:13] wallyworld: anyawy, I'm happy to talk to perrito666 instead, he just needs to wake up. :) [12:13] wallyworld: I hope you enjoyed your dinner. [12:13] yeah, throttling was only ever meant to kill reptitive status-update hook messages. it was a quick implementation that could use some rework AFAIR [12:14] i did, we went to a Jamie Olivier restaurant [12:14] i think there's been a few issues with the throttling, there's a bug or three still [12:15] jam: btw, thanks for comments on cmr spec. i'll garden a bit and put out for comment once i land one more pr [12:16] wallyworld: happy to. [12:16] ping me after you've done it, hopefully I can look with fresh eyes [12:17] will 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 that [12:18] i don't mind if the spec has questions still to resolve, just want to get it ready for consumption so to speak [12:18] sure [12:26] jam: sorry I am here now [12:33] jam: 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 then [12:37] jam: ill leave you with an answer at least, throtling was, as wallyworld said, something implemented in a hurry and could use some re-work [12:47] jam: where's the accepted place to report juju bugs these days? still launchpad? [12:47] anyone? [12:48] ha, given there are only two issues on github.com/juju/juju, i guess so [12:48] unless All The Bugs Have Been Fixed [12:54] rogpeppe: yes, launchpad.net/juju [12:54] rick_h: thanks [12:54] rick_h: just filed https://bugs.launchpad.net/juju/+bug/1657448 [12:54] Bug #1657448: provider/azure: adding credential produces error when service principal exists [13:25] rogpeppe: andrew will be estatic :-) i'll let him know [13:26] * perrito666 imagines wallyworld compiling a static version of andrew [13:26] one can never have too many of those [13:27] wallyworld: arent you pass your bed time? [13:27] yeah [13:28] stuff to do [13:28] wallyworld: do you need any of us to tuck you in ? [13:28] yes, and a goodnight story [13:29] ill read you a user story for bed [13:32] wallyworld: thanks [13:33] rogpeppe: it's probably not even his bug, but mentally I s/azure/andrew :-) [13:33] wallyworld: it looks like it's a bug in the azure client lib [13:33] wallyworld: i filed a bug there too [13:33] sgtm, ty [13:34] wallyworld: MS loves byte-order marks [13:34] i bet [13:34] uhhh, that is a cool internal idea, lets rename azure provider andrew provide internally [13:34] wallyworld: have you used the azure provider much? [13:34] not much, just a smoke test or two [14:36] voidspace, 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] natefinch: k [14:36] natefinch: happy exercize [14:36] :) [15:50] merge jobs seem to be failing with either 'go' not found or 'lxd' not found. any ideas? [15:50] http://juju-ci.vapour.ws:8080/job/github-merge-juju/10057/ [15:51] wow sounds like corrupt server [18:21] bbl, have people doing some fixes at the house and need to shut down the power for a moment [18:22] woohoo nested arm64 booted [18:30] perrito666: ping [18:31] redir: nested? [18:31] jam [18:31] yes [18:31] what do you mean by nested arm64? [18:31] kvm in kvm ? [18:31] but just by fiddling with the domainXML [18:32] jam yes [18:32] ah, not somethnig like running virtualized arm board on an arm board. [18:32] yes with kvm acceleration instead of emulation [18:32] like we do for amd64 [18:33] it's apparently a bit different [18:33] and it appears it will only work on xenial [18:33] not trusty [18:34] :( [18:36] redir: 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) === dames is now known as thedac [18:37] jam AFAIU the qemu-efi package requires 15.04 minimum [18:38] perrito666: 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] I have some notes I'll add to the wiki. [18:39] perrito666: Also, the different states for InstanceStatus make me wonder if I should be setting something different in the worker [18:39] namely Running vs Started and Provisioning vs Pending. [18:44] perrito666: 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? === frankban is now known as frankban|afk [19:02] jam you got a sec? [19:03] redir: sort of, I'm not officially working right now, but if you have a Q [19:03] I have a q about the bridge used on arm [19:03] should be real quick [19:04] sure [19:04] https://hangouts.google.com/hangouts/_/canonical.com/john-reed [19:27] thanks jam [19:47] Bug #1550821 opened: TestSetsAndUpdatesMembers timed out [19:53] Bug #1550821 changed: TestSetsAndUpdatesMembers timed out [20:02] Bug #1550821 opened: TestSetsAndUpdatesMembers timed out [20:08] * redir lunches [20:11] Bug #1550821 changed: TestSetsAndUpdatesMembers timed out [20:15] jam: pong, why do you always ping me when I am away? [20:17] jam: ill check your PR [20:27] jam: LGTM your changes seem sane enough [21:28] thumper: since u were in cert's area, would u have a sec to look at https://github.com/juju/juju/pull/6831 [21:29] anastasiamac: later... in a deep dive now [21:30] thumper: sure \o/ just flagging to for ur perusal [21:31] thumper: this is a companion PR: https://github.com/juju/utils/pull/261 [21:31] ack [21:47] abentley: can you review https://code.launchpad.net/~sinzui/juju-reports/ignore-missing-tasks/+merge/315076 [21:57] sinzui: r=me with some comments. [21:59] abentley: thank you. [22:00] brb [22:01] veebers: so no perfscale meeting [22:01] ? [22:02] menn0: hey, sorry I sent an email about re-scheduling it for next week [22:03] veebers: I vaguely remember that. just thought I'd still seen the meeting on the calendar this morning. [22:03] ignore me :) [22:04] menn0: sorry about that, I only just moved the cal event :-P [22:04] all good... I was away at a meeting with an accountant and am just catching up. [23:07] axw: ping? [23:07] babbageclunk: it's 7am for axw... just saying... [23:08] anastasiamac: yeah, I keep forgetting. I'm out this afternoon though. [23:08] anastasiamac: I get up at 6:00 :p [23:08] babbageclunk: u could leave a msg here, m sure he'll read \o/ [23:09] perrito666: ah... we all are up early (and stay up late) but r u functioing and working at that time? [23:11] anastasiamac: I never function :p [23:14] btw, wasnt this like our previous standup time? [23:14] we could go back there [23:15] perrito666: it was :) and we could :) let's discuss today [23:16] perrito666: alto nz'ers might b at or near their lunch ... [23:28] babbageclunk: pong [23:28] oh hey axw, sorry for timezone confusion [23:29] axw: I'm doing AdoptInstances for ec2, and I think I need to update tags for volumes as well as for instances? [23:30] axw: thumper suggested you'd know about that? [23:31] babbageclunk: yes, that is correct [23:32] babbageclunk: several of the providers have support for storage, and use tagging (where possible) similarly to how it's used for tracking instances [23:32] babbageclunk: there's a separate interface for dealing with volumes/filesystems [23:32] babbageclunk: theoretically filesystems have the same issue, though there are no cloud filesystem implementations yet [23:33] axw: 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] babbageclunk: volumes aren't necessarily attached to an instance. they can be floating [23:34] axw: Oh, right - so it would need to happen higher up? The migration master would call AdoptVolumes as well as calling AdoptInstances? [23:34] babbageclunk: yes. and AdoptFilesystems [23:35] axw: ok - and what interface should those live on? environs.Environ or somewhere else? [23:35] babbageclunk: storage/{Filesystem,Volume}Source I think would be most appropriate [23:36] axw: ok, thanks! that probably gives me enough to start with. [23:36] babbageclunk: np, let me know if you get stuck [23:36] axw: I almost certainly will! [23:37] (get stuck I mean) [23:37] :p