[00:00] jw4: i will keep digging now [00:00] ericsnow: why the dial changes? [00:00] menn0: I'll be in and out for the next few hours... making supper for kids, and then school open house - I'll try to get on longer later [00:00] jw4: no worries [00:04] wallyworld: in 1.2 http.Transport doesn't have the KeepAlive field, so I had to manually create the same effect [00:05] ok [00:09] wallyworld: thanks for the review :) [00:09] np [00:19] Bug #1447853 was opened: Local charms are not added to storage on upgrade to 1.22.x [00:23] menn0: fixes pushed when you get a chance [00:23] wallyworld: kk [00:26] wallyworld: ship it [00:26] \o/ [00:26] ty [00:31] Bug #1447853 changed: Local charms are not added to storage on upgrade to 1.22.x [00:40] Bug #1447853 was opened: Local charms are not added to storage on upgrade to 1.22.x [00:52] wallyworld, I am exhausted. I haven't found much. I split bug 1447846 from the other upgrade issue and captured some unit log [00:52] Bug #1447846: Hooks don't fire after upgrade 1.23.0 [00:53] sinzui: ty, menn0 is working that bug at the moment [00:53] wallyworld, mgz and I also discovered bug 1447853, but I know have a cheep work around [00:53] Bug #1447853: Local charms are not added to storage on upgrade to 1.22.x [00:53] ah bollocks, saw that float past [00:54] will have to fix for 1.24 [00:54] wallyworld, the work around is to upgrade-charm --force. it works a minute later :) [00:54] at least there is that === kadams54-away is now known as kadams54 [02:01] anyone have a minute to review a patch for a very mechanical change: https://github.com/juju/govmomi/pull/2 [02:30] fwereade: ping? [02:34] jam: ping? [03:00] wallyworld: i have a learned a lot more about the hook not firing bug [03:00] wallyworld: not sure I know how to fix it though [03:00] oh? [03:01] did you want to talk? [03:01] wallyworld: having jam or fwereade around would be handy. it's leader related. [03:01] oh joy [03:01] jam will be online soonish [03:01] wallyworld: quick chat would be good [03:01] ok, same hangout [03:02] wallyworld: i'm there now === kadams54 is now known as kadams54-away [03:25] menn0: if you get a chance, but doesn't matter if not http://reviews.vapour.ws/r/1481/ [03:26] wallyworld: got it [03:40] menn0: does that mean it's not related to my changes? [03:41] jw4: at this stage it's looking like the hooks not firing issue is related to the leadership work [03:41] jw4: i'm not any wiser about that mode error in the logs though [03:42] menn0: well I know about the error - that's not critical [03:42] menn0: cool, I'll try and dig into it too [03:42] for my edification [03:43] we just had an earthquake, 6.4 but very deep [03:44] whoa [03:44] because of the depth it wasn't that severe [03:44] menn0: any damage [03:44] but my chair was rolling around by itself [03:45] no damage i think but it was a strange rolling feeling and went on for a while [03:45] i've only experienced short "bangs" up until now [03:45] menn0: is south island sinking? [03:45] that's freaky... are earthquakes common in that area? Pacific Rim and all [03:45] yep, quite common in NZ [03:48] anastasiamac: he's ignoring the question... I think he's rescuing pets or something [03:49] anastasiamac: probably :) [03:50] quite a bit of activity recently: http://www.geonet.org.nz/quakes/felt [03:51] jw4: :D [03:51] :) [03:51] menn0: u should have apost-earthquake party, considering it's friday [03:56] anastasiamac: they're a little too common to be having parties for each one :) [03:57] menn0: on a serious note, it's gr8 that it was deep. 6.4 sounds really scary... :)) [03:59] anastasiamac: yes. this one was the same magnitude as the one that did this a few years ago: http://en.wikipedia.org/wiki/2011_Christchurch_earthquake [04:00] the city is still a long way from recovering from that one [04:09] menn0: the earth moved for you in cape town as well didn't it? [04:10] wallyworld: ssh not in public :) [04:11] Bug #1447895 was opened: Panic if jujud restarts while action is running [04:11] Bug #1447899 was opened: upgrade fails if no explicit version is specified [04:28] nice sleuthing menn0 [04:28] jw4: with the hook not firing bug? [04:28] yeah [04:29] finding the WatchLeadershipSettings missing document [04:50] wallyworld, jw4: bingo! [04:50] wallyworld, jw4: I have a fix for the hook not firing issue [04:51] wallyworld, jw4: adding an upgrade step that adds the leadership documents in the settings collection fixes the issue [04:51] wallyworld, jw4: I hacked it in without tests so let me sort that out before proposing [04:52] menn0: suh-weet! [04:59] wallyworld, jw4: initial, rough fix for hooks not firing issue [04:59] https://github.com/mjs/juju/tree/1447846-hooks-dont-fire-after-upgrade-1.23 [04:59] writing the tests now [05:04] other than missing docstring on https://github.com/juju/juju/compare/1.23...mjs:1447846-hooks-dont-fire-after-upgrade-1.23#diff-d7b2b2c8e8ce6dfc1b7f09c3cf9744d1R1203 [05:04] and forthcoming tests... [05:04] looks great menn0 [05:13] menn0: branch looks good, still want to understand why listener hangs if there's no record. i think that's poor behaviour [05:14] wallyworld: I bet it's because the watcher isn't returning an initial empty event, like it's supposed to [05:15] wallyworld, menn0 it was a little tricky getting that initial guaranteed event in the Action Watchers, so I wouldn't be surprised if the Leadership Watchers have a problem there too [05:15] jw4: I think the problem here is that the code assumes the leadership doc will be there b/c it is for services created under 1.23 [05:16] jw4: but someone forgot to write this upgrade step [05:16] the Watchers (afair) are supposed to guarantee an initial event, possibly empty, even if there are no records... [05:16] jw4: and without the doc there when the watcher is created the initial event isn't fired [05:16] jw4: are you sure about that? [05:16] menn0: yeah, I think you're right, but I remember fwereade stressing to me the importance of that... (but I could be remembering wrong) [05:17] (the importance of the initial, possibly empty, guaranteed event) [05:17] jw4: yeah, I know there's been talk about this but I'm not exactly sure what should happen [05:18] hopefully fwereade will be on soon and will chime in... I guess it's EOD for you though now (and EOW) [05:19] jw4: i've got a bit more time left [05:19] jw4: i'm going to try and get this fix done as much as I can [05:20] jw4: regardless of what the watchers are supposed to do I'm pretty sure we want this upgrade step [05:20] menn0: regardless, I think your fix is appropriate in this case [05:20] jw4: exactly, but to me that's not how i thought the watchers worked [05:21] +1 [05:21] wallyworld: yeah, I'm fairly confident that's how I was instructed to implement the actions related watchers, but we should verify [05:32] * jw4 off to bed === urulama__ is now known as urulama [05:51] wallyworld: ping [05:52] yo [05:52] wallyworld: i have to EOD very soon [05:52] one sec [05:54] back [05:54] wallyworld: just need to hand over the fix for this [05:54] wallyworld: pushing now... [05:54] wallyworld: give me a sec [05:54] sure, is it ready to land after review? [05:54] or tests needed? [05:55] wallyworld: the hard tests are done [05:55] wallyworld: just needs a test for upgrade step idempotency [05:56] ok [05:58] it's here: https://github.com/mjs/juju/tree/1447846-hooks-dont-fire-after-upgrade-1.23 [05:58] menn0: i'll either do it or hand over; it's my son's birthday today and i have to go to dinner soonish. either way will be done. thanks for fixing [05:58] it's ready to go apart from that one test [05:59] if no-one gets it done today it'll take me almost no time on Monday [05:59] wallyworld: and we need to forward port to master [05:59] ok, np. tbh, i don't think we are cutting a release on friday anyway [06:00] but it's now done so the risk is removed [06:02] wallyworld: ok good [06:02] wallyworld: well have a good night and a weekend [06:02] speak to you next week [06:54] fwereade: you online? [10:17] fwereade: hiya [10:28] TheMue: hi there, you ocr? would love a review of this so as it needs to land for 1.24 http://reviews.vapour.ws/r/1481/ it removes a feature flag [11:13] jam: ping [11:27] wwitzel3, ping? [11:30] mattyw: pong [11:40] wallyworld_, so do we still serve client facade v0? I'm not seeing where we do [11:40] wallyworld_, and that would STM to be a problem... [11:41] fwereade: ah, right, i wasn't sure about that. i wasn't sure what the exact mechanism was there [11:42] wallyworld_, the ideal, I think, is to *never touch* the client API, and carve off changing functionality into more service-specific facades [11:42] wallyworld_, otherwise you have to have an almost-perfectly-cloned Client facade [11:43] wallyworld_, then when (say) storage evolves, it shouldn't need to hit more than one or two small facades [11:43] wallyworld_, rather than dragging the Client mess into the equation every time [11:44] wallyworld_, what actually changed on Client? [11:44] fwereade: yeah sadly i think you are right, i'll have to do that, but will miss the branching of 1.24. bollocks. will have to forward port [11:44] wallyworld_, bad luck :( [11:44] fwereade: new params to 2 apis were added [11:45] fwereade: if a newer client adds additonal params to the call, they will be ignored, but we need a way to tell the user [11:45] hence the api bump [11:45] wallyworld_, yeah, indeed, it really is a different API [11:46] wallyworld_, so I *think* that the right thing to do [11:46] wallyworld_, is to pull out a Service service facade from client [11:46] yeah, i suspect so too [11:46] i'll revisit over the weekend [11:47] i really want to remove the flag for 1.24 [12:35] morning [12:44] perrito666: wala [12:45] perrito666, ¿good? morning === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 [14:02] wwitzel3: ericsnow: stand up === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 === liam_ is now known as Guest67926 [15:56] all, looking at this: https://bugs.launchpad.net/juju-core/+bug/1447846 [15:56] Bug #1447846: Hooks don't fire after upgrade 1.23.0 [15:57] ^^^ based on menno's last comment is this something someone can pick up and complete today? [15:57] alexisb: menn0 said it's almost done.. just one test for idempotency left [15:58] alexisb: yeah looks doable for someone to pick up [15:58] katco, can you please delegate [15:58] alexisb: sure [15:58] thanks [15:59] moonstone are all currently working on bugs. any volunteers? [16:03] cherylj: perrito666: do you two have capacity to land a simple bug? === urulama is now known as urulama__ [16:07] katco: sorry I dont feel really reliable right now. [17:04] perrito666: no worries. feel better soon [17:04] cherylj: sorry i lost connection. can you take that bug? [17:23] man I really hate that we require go 1.2 for juju, it totally screws up my development environment for everything else... and many projects already require 1.3+ [17:26] natefinch: here's my setup [17:26] natefinch: ~/.local/go-1.{2,3,4} [17:26] natefinch: ~/.local/go -> go-1.2 during day [17:26] natefinch: export GOPATH=~/.local/go [17:26] natefinch: there is some churn with having to rebuild utils [17:27] hmmm...virtualenv for go... [17:27] ericsnow: or there's that :) [17:29] ericsnow: there's a tool for that... forget which of the myriad tools out there it is. of course, as katco said, you can always just run them side by side. [17:32] natefinch: wwitzel3: planning time! [17:32] I'm there? [17:33] is it not in moonstone? .. (checks calendar) [17:34] wwitzel3: not moonstone === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 [17:50] cmars: http://reviews.vapour.ws/r/1484/ [17:55] wwitzel3, so i guess CI is going to set that feature flag to get that test to pass? what's the difference between not registering the provider vs. blocking bootstrap? [17:57] cmars: the code on the unit doesn't see the provider as registered since the flag is local [17:57] cmars: jam pointed this out no the ML and the QA ran in to it with CI tests [17:58] hey; could I please get a quick review on http://reviews.vapour.ws/r/1486/ [17:58] wwitzel3, ok, got it. so this moves the featureflag check to the client? in what context does environ.Bootstrap run? [17:58] (environ's not an area of the codebase I'm familiar with) [17:58] it's a simple module; ~100 lines of code that actually does something [18:03] cmars: just during juju bootstrap === kadams54 is now known as kadams54-away [18:34] cmars: quick review? http://reviews.vapour.ws/r/1487/diff/# [18:40] cmars: thanks for the review [19:53] /query alexisb [19:53] well, another bug in my irc client [19:53] I should stop using daily snapshots [20:24] sometimes I think Juju is the biggest example of what not to do with interfaces :/ [20:25] you exagerate [20:25] perhaps slightly [20:25] there might be a few worse examples in the world ;) [20:27] I like the way everything is a nice generic interface and then in the comments we just say "this is for precise0 [20:27] s/0/" [20:34] natefinch: I think that is an example on how not to used a typed language [21:10] anyone still around for trivial reviews? [21:16] Bug #1448308 was opened: Skipped TestUniterUpgradeConflicts on ppc64 [21:21] mgz: sorry I missed that format stuff :/ [21:22] anyone around that feels comfortable reviewing http://reviews.vapour.ws/r/1490/ [21:23] it fixes the "hooks not firing" bug [21:23] #1447846 [21:23] Bug #1447846: Hooks don't fire after upgrade 1.23.0 [21:23] ericsnow: I can look [21:23] mgz: oh, cool [21:24] ericsnow: I also have a trivial ppc64 test skip branch [21:24] mgz: note that I only added the idempotency test, menn0 did the rest [21:24] /1488 [21:24] mgz: I'll take a look [21:24] ericsnow: yeah, I already looked over menno's branch [21:24] mgz: nice [21:25] mgz: I'm pretty sure I got the idempotency check right [21:27] mgz: in the bug you make it sound like skipping the test might be masking a real bug [21:28] ericsnow: it might be, but we hav ehte bug, and know we need to do functional testing of the leadership stuff [21:28] mgz: k [21:29] mgz: so 1448308 will be closed only as soon as we resolve the issue (and not when the skip-the-test patch merges), right? [21:29] ericsnow: how-upgrades-work question [21:29] mgz: sure [21:30] ericsnow: indeed, I'm keeping the skipped-tests bugs open, because we keep filing bugs and linking them in the codebase, but counting them as fixed when the test is skipped [21:30] which is wrong... [21:31] mgz: cool, then LGTM [21:31] I'm on 1.23.0 which did not do this step, my state server notices there's a 1.23.2 with this fix and downloads it... how does it know to run this step? [21:32] we just always do steps on new versions and the steps must apply? [21:32] even minor ones, right? [21:33] mgz: I'm not sure, but...the idempotency invariant of upgrade steps ensures it's a noop if you're already okay [21:33] right [21:34] I expect it simply runs all the steps [21:37] ericsnow: test changes look good [21:37] mgz: k, cool [21:38] mgz: it made sense to factor out that stuff into helpers :) [21:38] I'd be somewhat interested in a test that ran the step on a as-if just bootstrapped system, no services [21:39] we don't try and do all error paths in these things, but that one does seem relevent [21:41] mgz: so basically the same thing as the existing test but without adding any services first? [21:42] (it should just return success without running any ops basically) [21:42] ericsnow: yup [21:42] mgz: I'll add such a test! [21:42] ericsnow: lgtm [21:43] hm, can I shipit and leave a comment in the same thing? not sure on the magic [21:43] I guess I jsut include that string in my text huh. [21:58] mgz: so is it correct that CI covers the upgrade scenarios now (i.e. the bug trigger will get exercised)? [21:59] ericsnow: see all the failures on upgrade jobs on 1.23 and trunk :) [22:00] mgz: that's what I figured :) [22:00] mgz: thanks for the review [22:00] http://reports.vapour.ws/releases/2557 [22:30] "panic: rescanned document misses transaction in queue" ...that one's new to me === kadams54-away is now known as kadams54 [22:43] mgz: yeah, me too :\ [22:47] mgz: hmmm...see #1318366 [22:47] Bug #1318366: jujud on state server panic misses transaction in queue [22:48] ericsnow, menn0, dammit, sorry, thanks [23:00] mgz: I'm going to ask menn0 to double check the upgrade step and tests when he gets a chance; that failure is a little suspicious. [23:06] ericsnow: seems wise, we're not in a rush now [23:07] mgz: yep :) === kadams54 is now known as kadams54-away