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