[00:59] hpidcock: for whenever, here's that PR with the action data mutex added https://github.com/juju/juju/pull/11416. protects acess from the hook commands and the uniter itself during and after running an action [01:00] wallyworld: approved [01:00] ta [01:04] do we have any focal deployment tests in CI yet? [01:33] thumper: i know we added focal support to all the test charms back in late feb, not sure about what else [02:03] wallyworld: we don't use any cgo specifics from the github.com/coreos/go-systemd package that I can see [02:04] good [02:05] I take that back [02:06] no backsies [02:06] github.com/juju/juju/service/systemd uses github.com/coreos/go-systemd/util.IsRunningSystemd [02:07] oh phew [02:07] that uses a file, ALL good! [02:07] 10/10 no problems [02:08] awesome [02:11] https://www.irccloud.com/pastebin/IhJMfRZl/ [02:13] the snap is 125Mi [02:13] so I think this is a winner [02:13] bootstraps to lxd fine [02:14] great, you checked the jujud binary on controller machine just to be sure? [02:21] yep [02:21] have now :) [02:29] wallyworld: https://github.com/juju/juju/pull/11417 [02:29] looking [02:29] or anyone else [02:30] hpidcock: we can get rid of the x-dep plugin when we get to go mod :-) [02:31] wallyworld: yeah I think its probably best we will have our fork and see if we can get some features merged back into the official plugin [02:32] I've tried to keep the parameters the same where I can, but there are features they don't expose we might want to update in the upstream plugin [02:32] true, although i'm not sure people care about that plugin anymore [02:32] no I mean the go plugin not the dep one [02:33] ah right [02:52] https://discourse.juju.is/t/juju-progress-report-2020-w14/2882 [03:11] kelvinliu: small fixes for beta1 https://github.com/juju/juju/pull/11418 [03:11] looking [03:17] lgtm ty [03:17] hpidcock: those jenkins qa changes merged,i guess you are going to run jjb? [03:17] I've been iterating, just wanted to get my fixes landed [03:17] was banging my head for more than the minutes it should have taken [03:18] no worries, just duble checking to ensure nothing slips by [09:12] manadart, you got a sec? [09:14] stickupkid: Yep. [09:16] in daily [09:35] achilleasa: Got a sec to look at https://github.com/juju/juju/pull/11421 ? [09:36] manadart: looking [09:53] manadart: approved [09:53] achilleasa: Thanks. [10:20] manadart: can you take a look at https://github.com/juju/juju/pull/11422? [10:21] achilleasa: See how I go. I have to hit the road soon, but I'll look while my patch is landing. [10:22] manadart: not in a hurry; got one more following up. If busy I can ask Heather to take a look when she gets online [10:48] achilleasa: Approved it. [12:33] stickupkid: https://bugs.launchpad.net/juju/+bug/1730747 might be of interest to that reload-spaces work. I think we should be able to knock a few off the list [12:33] Bug #1730747: incomplete support for spaces in juju and maas [12:39] anyone up for a small review? https://github.com/juju/juju/pull/11423 [12:49] rick_h_, about 5-6 bugs in one :) [12:50] stickupkid: yea, exactly. Let's see if we can knock a couple out and maybe do individuals from there with tags for spaces and such for the future? [13:21] achilleasa, can look into testing non-modified part. [13:36] stickupkid: got a question about your PR comment [13:37] achilleasa, the former. I want see that we don't call SetState [13:38] stickupkid: but that's essentially testing the Modified method, right? [13:38] achilleasa, well, [13:38] no objection to adding another test; just trying to understand what value we get out of it [13:39] achilleasa, well I find it ensures that we're changing functionality based on a state [13:39] achilleasa, generally test that [13:40] achilleasa, as apposed to testing every error case [14:15] stickupkid: pushed a commit with the extra test; can you take a look? [14:22] achilleasa, already ticked [14:32] rick_h_, achilleasa can you delete the alpha space? [14:32] * stickupkid hopes the answer is no [14:32] stickupkid: my assumption is that it is read-only [14:33] I mean, the instancepoller puts virtual IPs into alpha; if you deleted that were would they go if everything must belong to a space? [14:33] (because VIPs don't match any of the CIDRs) [14:33] achilleasa, yeah chaos indeed [14:56] stickupkid: nope [14:56] wicked, this is great news [14:56] stickupkid: alpha space is the default, always there, home for things not assigned [23:16] babbageclunk: are you still after a review for PR 11426? [23:16] thumper: yes please [23:16] ok, I'll look now [23:20] ta [23:29] babbageclunk: code generally looks good, just one request around behaviour when things don't go well [23:29] ok thanks [23:35] thumper: I don't know about that - *in theory* there could be another change to the uniter state file later that would need a different upgrade step to migrate - if this one goes nuclear and trashes the file for changes it doesn't know about that might be a bad thing? [23:36] thumper: definitely a theoretical problem though [23:37] perhaps instead of using the validity function, perhaps just read the yaml, look for the remote-unit key, and if it is there add the remote-application one? [23:37] that way we avoid the validity issue [23:37] I think you are right on that aspect [23:38] ok, I'll tweak it to do that. Actually what's there now won't work in the scenario I'm describing, the Write call will fail if the state's invalid after a change. [23:38] So it needs to be done anyway [23:40] I'll leave the validity check on the way in, but make sure we can still write the change even if the resulting state isn't (yet) valid. [23:41] yeah, I guess the initial validity check is fine, because if it is valid, there is nothing to do [23:41] cool [23:41] I was just wondering how best to deal with the issues... [23:42] but I think you are right, there is potential that a later release would cause an invalid file in a different way [23:42] and that this upgrade step isn't there to fix that [23:42] but it would be called first if they are upgrading through multiple versions [23:42] Maybe a post-upgrade-steps sanity check? Just reads the state, and if it's bad then throws it away? [23:42] so I agree with you now that deleting it isn't the right thing to do [23:42] no... I don't think so [23:43] consider being invalid for two reasons? [23:43] 2.7 added remote-application [23:43] I mean, after all of the steps are run [23:43] but say 2.8.1 adds another key that is needed [23:43] so I don't think we should throw away the change just because the result isn't yet valid [23:43] because we can't guarantee that a future step wouldn't fix it [23:44] right, but there's a point where all the available upgrade steps have run, if the state's invalid at that point then maybe we need to throw it away [23:44] but instead of reading and writing using a structure, I'd recommend just using a map[string]interface [23:44] ah... I see what I think you are getting at [23:45] we don't have anything like that yet... [23:46] yeah, thinking about it I should be reading into a map, just in case the current defn of State's changed so much that I can't load the file as is. [23:46] will change that too [23:46] is anyone with experience with reactive charms able to provide some advice here? https://discourse.juju.is/t/workload-stuck-in-maintenance-status/2890