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 | 00:59 |
---|---|---|
hpidcock | wallyworld: approved | 01:00 |
wallyworld | ta | 01:00 |
thumper | do we have any focal deployment tests in CI yet? | 01:04 |
wallyworld | thumper: i know we added focal support to all the test charms back in late feb, not sure about what else | 01:33 |
hpidcock | wallyworld: we don't use any cgo specifics from the github.com/coreos/go-systemd package that I can see | 02:03 |
wallyworld | good | 02:04 |
hpidcock | I take that back | 02:05 |
wallyworld | no backsies | 02:06 |
hpidcock | github.com/juju/juju/service/systemd uses github.com/coreos/go-systemd/util.IsRunningSystemd | 02:06 |
hpidcock | oh phew | 02:07 |
hpidcock | that uses a file, ALL good! | 02:07 |
hpidcock | 10/10 no problems | 02:07 |
wallyworld | awesome | 02:08 |
hpidcock | https://www.irccloud.com/pastebin/IhJMfRZl/ | 02:11 |
hpidcock | the snap is 125Mi | 02:13 |
hpidcock | so I think this is a winner | 02:13 |
hpidcock | bootstraps to lxd fine | 02:13 |
wallyworld | great, you checked the jujud binary on controller machine just to be sure? | 02:14 |
hpidcock | yep | 02:21 |
hpidcock | have now :) | 02:21 |
hpidcock | wallyworld: https://github.com/juju/juju/pull/11417 | 02:29 |
wallyworld | looking | 02:29 |
hpidcock | or anyone else | 02:29 |
wallyworld | hpidcock: we can get rid of the x-dep plugin when we get to go mod :-) | 02:30 |
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:31 |
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:32 |
wallyworld | ah right | 02:33 |
timClicks[m] | https://discourse.juju.is/t/juju-progress-report-2020-w14/2882 | 02:52 |
wallyworld | kelvinliu: small fixes for beta1 https://github.com/juju/juju/pull/11418 | 03:11 |
kelvinliu | looking | 03:11 |
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:17 |
wallyworld | no worries, just duble checking to ensure nothing slips by | 03:18 |
stickupkid | manadart, you got a sec? | 09:12 |
manadart | stickupkid: Yep. | 09:14 |
stickupkid | in daily | 09:16 |
manadart | achilleasa: Got a sec to look at https://github.com/juju/juju/pull/11421 ? | 09:35 |
achilleasa | manadart: looking | 09:36 |
achilleasa | manadart: approved | 09:53 |
manadart | achilleasa: Thanks. | 09:53 |
achilleasa | manadart: can you take a look at https://github.com/juju/juju/pull/11422? | 10:20 |
manadart | achilleasa: See how I go. I have to hit the road soon, but I'll look while my patch is landing. | 10:21 |
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:22 |
manadart | achilleasa: Approved it. | 10:48 |
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:33 |
achilleasa | anyone up for a small review? https://github.com/juju/juju/pull/11423 | 12:39 |
stickupkid | rick_h_, about 5-6 bugs in one :) | 12:49 |
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? | 12:50 |
stickupkid | achilleasa, can look into testing non-modified part. | 13:21 |
achilleasa | stickupkid: got a question about your PR comment | 13:36 |
stickupkid | achilleasa, the former. I want see that we don't call SetState | 13:37 |
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:38 |
stickupkid | achilleasa, well I find it ensures that we're changing functionality based on a state | 13:39 |
stickupkid | achilleasa, generally test that | 13:39 |
stickupkid | achilleasa, as apposed to testing every error case | 13:40 |
achilleasa | stickupkid: pushed a commit with the extra test; can you take a look? | 14:15 |
stickupkid | achilleasa, already ticked | 14:22 |
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:32 |
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:33 |
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 | 14:56 |
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:16 |
babbageclunk | ta | 23:20 |
thumper | babbageclunk: code generally looks good, just one request around behaviour when things don't go well | 23:29 |
babbageclunk | ok thanks | 23:29 |
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:35 |
babbageclunk | thumper: definitely a theoretical problem though | 23:36 |
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:37 |
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:38 |
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:40 |
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:41 |
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:42 |
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:43 |
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:44 |
thumper | we don't have anything like that yet... | 23:45 |
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 | 23:46 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!