/srv/irclogs.ubuntu.com/2020/05/18/#juju.txt

babbageclunktlm: can you review this plz? https://github.com/juju/juju/pull/1159101:10
tlmsure01:10
kelvinliuwallyworld: free HO?01:11
wallyworldkelvinliu: ok01:11
tlmlgtm babbageclunk, will try again with that when it merges. Just go it again01:12
babbageclunktlm: thanks!01:29
babbageclunksorry about that01:29
tlmnot your fault01:34
thumperhpidcock: if you are going to update juju to use -N on ppc64el, did you want to look at the etcd-io/bbolt?02:27
hpidcockthumper: I can look at that too02:28
thumperlt looks like a drop in replacement with a mem fix02:29
thumperperhaps extra fixes too02:29
thumperat least that is the theory02:29
thumperhpidcock: how's the python fun going?02:29
hpidcockthumper: I think I'm done for pylibjuju for now, just need to cut a release, might have it coincide with rc202:31
thumpertlm: looks like a test failure in your model agent landing02:31
thumperhpidcock: ack02:31
thumpertlm: FAIL: machine_test.go:640: MachineSuite.TestManageModelRunsCleaner02:31
thumpertlm: I'm wondering how useful that test is, looking at the content, it is doing a hell of a lot that isn't what we care about02:37
thumpertlm: looking, I'm not sure you touched that one at all, and it is just a time sensitive test02:54
tlmthumper: sorry back from lunch02:56
tlmare this test is driving me nuts02:56
tlmthumper: what do you recommend ?03:00
babbageclunktlm: sorry - I've been playing whack-a-mole with those tests. I'm going to bump up the timeouts wholesale03:06
tlmok babbageclunk, no issues at all. Weird that mine is the one suffering. Making me wonder if I have missed something03:07
babbageclunktlm: it's possible I guess but I can't see how it would be something you've done - those tests won't be running your workers03:12
thumpertlm: just try to merge again03:29
thumperI have a branch that should fix that intermittent timeouit03:29
thumperjust pushing now03:29
thumperbabbageclunk: https://github.com/juju/juju/pull/1159203:32
hpidcockwallyworld: can you fork github.com/hashicorp/raft-boltdb into juju/raft-boltdb please03:33
wallyworldok03:34
thumperhpidcock: what level of changes do we need?03:34
hpidcockthumper: just a path rewrite on the bolt db03:34
hpidcockbecause etcd renamed the project03:34
thumperhpidcock: and go mod doesn't help there?03:34
thumperah...03:34
thumperpoo03:34
hpidcocknot when they renamed it03:34
wallyworldthumper: could you do the fork, i'm in the middle of some Z^%W%@! unit tests03:35
thumperwallyworld: ack03:35
thumperhpidcock: here you go: https://github.com/juju/raft-boltdb03:35
hpidcockthumper: thanks03:35
babbageclunkthumper: approved with gusto!03:40
hpidcockthumper: wallyworld: can you both review and merge please https://github.com/juju/raft-boltdb/pull/103:45
tlmthumper: thanks for the PR03:57
babbageclunkthumper: your pr hit a different intermittent failure, I kicked it off again04:15
babbageclunkduh, sorry, that was a check build not a merge one04:15
* babbageclunk is a dork04:15
* tlm offers babbageclunk a run04:16
* babbageclunk accepts04:17
thumperbabbageclunk: no worries04:21
thumperI've filed a bug for that04:21
thumperwe get a lot of intermittent failures in that package04:21
thumperI feel that they all have the same root cause04:21
thumperbut I've not looked yet04:21
thumpertlm: for the record, I kicked your PR merge again04:22
tlmthanks thumper04:24
hpidcockthumper: can you both review and merge please https://github.com/juju/raft-boltdb/pull/104:29
wallyworldbabbageclunk: it's bigger than it looks due to deleting a lot of code and moving some code. i still have a unit test to fix in worker/uniter but good apart from that https://github.com/juju/juju/pull/1159304:33
babbageclunkwallyworld: normally people say the other way?04:33
wallyworldhpidcock: looking now04:33
babbageclunkwallyworld: ok. looking04:33
babbageclunkoops meant a comma there04:33
babbageclunkfullstop sounds super terse!04:33
wallyworldall good04:33
babbageclunkwhoa, looks big!04:35
wallyworldlots of deleted code04:35
wallyworldand moved code04:35
wallyworldcore changes not too bad04:35
wallyworldhpidcock: done04:35
hpidcockwallyworld: many thanks04:35
wallyworldbabbageclunk: thre's 4 commits which natch the pr description if that helps. the raft and lease worker bits should be familiar hopefully04:36
babbageclunkok04:37
babbageclunkyeah, that definitely helps04:37
wallyworldit's all a bit of a rush sorry04:37
wallyworldotherwise i'd have done separate prs04:37
wallyworldjust got this fix this %W@$!%$ uniter test04:38
babbageclunkno worries!04:43
babbageclunkwallyworld: oh, you've done the autoexpire removal work, nice04:48
* babbageclunk gets rid of that part of his branch04:48
wallyworldbabbageclunk: yeah, sorry, i had to cause it was all mixed up in the work04:51
babbageclunkmakes sense04:53
wallyworldthere' sstill the dummy provider stuff though04:54
wallyworldi think there's a fair bit that can be deleted off that04:54
* tlm ducking out for a little bit to get some air04:59
wallyworldbabbageclunk: i added an implementation of RevokeLease() in the dummy store and that fixes the tests05:05
babbageclunknice05:06
wallyworldmaybe i can delete ExpireLease() now for the dummy store, i think we only use it to claim a lease for leadership tsting05:07
wallyworldyup, nothing uses it05:08
babbageclunkwallyworld: the only extra bit is that there needs to be a background goroutine for the dummy lease store so it can expire leases internally05:08
wallyworldbabbageclunk: i thought about it but from what i can see, we only ever claim a lease to set up a unit leader05:08
wallyworldi am pretty sure the testst will now all pass05:09
babbageclunkok, if you don't think there are any places that need expiry that's easier05:09
wallyworldyeah, i'll see if the current tests pass05:09
babbageclunksounds good05:09
wallyworldi'll add expiry if needed but i don't think so05:09
wallyworldkelvinliu: did moving the uniter struct initialisation help?05:11
kelvinliuHO?05:11
wallyworldsure05:11
hpidcockthumper: the deferreturn issue fix was landed https://go-review.googlesource.com/c/go/+/234105/05:26
hpidcockhasn't been picked up for a backport to 1.14 yet. Will need to keep an eye out for it.05:31
hpidcockthumper: https://github.com/juju/juju/pull/1159406:17
wallyworldkelvinliu: this solves most of it - leadership stable after removing wrench. it keeps logging that it wants to depose leadership so a small issue to solve still https://pastebin.ubuntu.com/p/Fx8Y8XsfSd/06:19
wallyworldkelvinliu: just afk for a bit, be back soon06:26
kelvinliuwallyworld:  looking now, ty06:28
wallyworldkelvinliu: did it work for you too?07:20
kelvinliuyes finishing the pr now07:21
wallyworldkelvinliu: did you see the repeated messages about running a leader deposed hook?07:21
wallyworldseems to be more log noise than anything since show-status-log is ok07:22
wallyworldbut something needs fixing07:22
wallyworldit might be the addition of the logger which now prints messages07:23
wallyworldso it's always been there07:23
kelvinliuI saw the warning message even before this branch07:27
wallyworldlots og them repeated?07:29
wallyworldi'll see if i can fix07:29
kelvinliudid u see lots of repeat? I only saw once07:29
wallyworldi saw lots of repeats07:29
wallyworldwe expect one but not repeated07:30
kelvinliuI can't re-produce the warning message now..07:41
wallyworldi'm trying again, we'll see07:44
wallyworldkelvinliu: it happens after adding and removing the wrench file07:46
wallyworldkelvinliu: and it happens because the unit agent local state struct gets Leader=true for non leaders for some reason07:47
wallyworldbecause looks like leader tracker is setting remotestate leader to true07:48
kelvinliuu mean the local leader state is out of sync07:49
wallyworldseems like it, need to do more debugging07:49
wallyworldkelvinliu: yeah, the new tracker still gives bd results for no leaders after the wrench file is removed :-(07:57
kelvinliuwallyworld:  did u build the latest code?07:59
kelvinliuit works fine for me07:59
wallyworldkelvinliu: i'll pull your latest code and try again08:00
wallyworldi was working with my initial diff08:00
kelvinliuwallyworld: I just removed debugging msg and fixed tests. no much change08:01
wallyworldok, i'll pull latest any and try08:02
kelvinliuyep08:10
stickupkidmanadart, https://github.com/juju/python-libjuju/pull/42310:27
stickupkidor hpidcock if you're around10:28
manadartstickupkid: Approved it.10:31
stickupkidta10:31
manadartstickupkid: Landed on develop instead of 2.8. Backport: https://github.com/juju/juju/pull/11597./14:06
manadartachilleasa, hml: Can you tick that patch? ^14:24
hmlmanadart: looking14:24
manadarthml, petevg: Test the shutdown service. It does indeed just fail on Bionic, and is pointless.14:25
hmlmanadart: i’m getting a compare changes screen, not a pr14:26
manadarthml: https://github.com/juju/juju/pull/1159714:26
stickupkidmanadart, done14:26
petevgmanadart: hah! I guess that's a strong argument for just queuing it up for now. And also for making a bug to actually fix it ...14:26
stickupkidmanadart, achilleasa do we still need this acceptance test, or does the new CI test cover this? https://github.com/juju/juju/blob/develop/acceptancetests/assess_network_spaces.py15:13
manadartstickupkid: Which new one do you mean?15:31
stickupkidhttps://github.com/juju/juju/tree/develop/tests/suites/spaces_ec215:31
manadartstickupkid: Thought so. It doesn't really test the same things. That one tests bindings, including the upgrade-charm path.15:33
stickupkidshame, wanted to get rid of another test tbh15:33
manadartstickupkid: Python one tests space constraints, including container-in-machine.15:33
stickupkidmanadart, fiiiiiiiiiiiiiiiiiiine, will add it to the lst of things to move rather than delete15:34
manadartstickupkid: I can re-write in shell style when I do that bindings card in the "Doing" lane./15:34
stickupkidmanadart, let's do that because the python one doesn't do a good job at cleaning up15:35
manadartstickupkid: Ack.15:35
stickupkidalso I'm pretty sure we can reuse the VPC... in the python test, but let's ignore that for now15:35
stickupkidhml, you could fix that as a tempary measure I guess for running out of VPCs in eu-west-115:36
manadarthml, petevg. Did a quick smoke test on MAAS. Bionic containers appear to release IPs upon both remove-machine and kill-controller, so we don't need a network shutdown service.15:37
* manadart heads home.15:37
thumperpetevg: https://github.com/juju/juju/pull/11598 plz20:57
petevgthumper: taking a look20:58
thumperpetevg: it is just forward porting the fix I did on friday20:58
thumperas wallyworld mentioned, should get it into the 2.8 branch20:58
thumperI should have done it friday, or yesterday, but it slipped my mind20:58
petevgthumper: Got it. I marked it as approved.20:59
thumperpetevg: ta21:00
petevgnp21:00
thumperpetevg: bug 187684921:25
mupBug #1876849: [bionic-stein] openvswitch kernel module was not loaded prior to a container startup which lead to an error <cdo-qa> <OpenStack neutron-openvswitch charm:Incomplete> <juju:New> <https://launchpad.net/bugs/1876849>21:25
tlmwallyworld: https://github.com/juju/juju/pull/1159923:42
wallyworldlooking23:43
wallyworldtlm: ta, lgtm23:43

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!