[02:11] wallyworld: bugger - I think I've worked it out and it's actually a bug in the migration import (so in 2.2.3) for subordinates. [02:13] wallyworld: just checking to see whether I can reproduce it in straight 2.2.3->2.2.3 migration [02:31] wallyworld: yup [02:42] thumper: ^ [02:42] fuck [02:42] :( [02:43] * thumper sads [02:43] what don' [02:43] Sorry, I wish I'd worked it out sooner [02:43] what don't we do right? [02:44] It's basically the reverse of the problem you fixed in the export - needs to skip creating relation scopes when the relation unit isn't valid. [02:44] bugger... [02:45] thumper: https://github.com/juju/juju/blob/2.2/state/migration_import.go#L1090 [02:45] axw: i've pushed some changes to the status watcher [02:45] bollocks [02:45] wallyworld: ok, looking [02:45] babbageclunk: can you please create a patch against 2.2 [02:45] yup yup [02:45] we may roll a fast 2.2.4 [02:45] but I'd like confirmation that it works first :) [02:51] thumper: of course. :) [02:52] * babbageclunk pops out briefly to pick up Ada [03:34] axw: if you need a break from vsphere, here's PR which adds firewall checking to cmr ingress. the worker business logic is all of 3 lines, but it needs some apiserver boilerplate to add the SetRelationStatus API https://github.com/juju/juju/pull/7839 [03:35] wallyworld: ok, a bit later [03:35] sure, np [03:39] babbageclunk: please file a bug so we can track this issue [03:39] * thumper creates a 2.2.4 milestone [03:39] ffs [03:39] thumper: ok [03:43] babbageclunk: when you have a moment could you describe the repro steps? It seems there is a gap in the functional testing that this slipped through [03:44] veebers: typing up the steps in the bug now - I'll paste a link here in a mo [03:45] babbageclunk: awesome, thanks [03:49] Is this likely to be a regression relating to the subordinate relation issues that were fully fixed in 2.2.2, or is it just a migration bug? [03:50] ie. can I safely upgrade my smaller controllers to 2.2.3 tonight to test things out before bigger controllers get upgraded? [03:53] wgrant: it's related to the subordinate problem in 2.2, but it only happens on migration - upgrading won't cause it. [03:53] babbageclunk: Cool, thanks. [03:54] wgrant: if the subordinates are ok in your 2.2.2 model then they'll be ok in 2.2.3 (at least in the context of this bug). [03:54] Great. [03:55] wgrant: sorry, my mention of upgrading earlier probably muddied the water a bit, but I'm working on 1.25 -> 2 upgrade, which is migration under the hood. [03:55] babbageclunk: Oh, I ddn't see that, I was just worried since the subordinate relation code had been historically fragile. [03:56] wgrant: right - this is another lurking case of that same problem unfortunately. [03:57] veebers: https://bugs.launchpad.net/juju/+bug/1715794 [03:57] Bug #1715794: migration: subordinate with multiple principals imports incorrectly [04:00] babbageclunk: sweetbix, I'll check out the repro [04:04] babbageclunk: ah, nice catch. yeah the current model migration functional test doesn't do any relation removals [04:05] Would a the state dump from both ends of the migration reveal this, or would the original fix exclude the problematic relationscopes from the dump? [04:08] wgrant: do you mean dump-model? I think it wouldn't show the problematic relationscopes, because we've changed it not to look them (since they won't exist in a good model). [04:08] wgrant: but dump-db will show them. [04:09] oops - changed it not to look them up [05:18] wallyworld: PR for the fix? https://github.com/juju/juju/pull/7840 [05:18] ok [05:18] thanks [05:25] babbageclunk: so to make sure i understand, we modify import to ignore bad data [05:25] should the first line in the PR description be "incorrect" not "correct" [05:25] wallyworld: no, the import is good - we were fabricating relation scopes even when they weren't in the import data. [05:26] I mean, the import data is good. [05:26] hmmm ok, i'm a bit confused then [05:27] we change import to say if ru not valid then.... [05:27] how would ru not be valid if import data ws ok [05:27] The relationscopes aren't reflected directly in the export format, but where it used to have two entries for settings under the endpoint it now only has one. [05:28] right, so the import data *is* wrong [05:28] wallyworld: nope [05:28] but you just said above it is? [05:28] we export bad data [05:28] No, we create bad data when importing good data. [05:28] which we then attempt to import [05:29] but the data by definition is not good if it allows us to create bad data [05:29] The export is right, but the nested loop in the code creates relationscopes and settings records even when there's nothing corresponding to them in the import data. [05:29] sounds like the export format is fawed [05:30] wallyworld: I think you might be right - the import is smarter than it should be in this case. [05:30] i guess i don't understand why ru.Valid() is needed - that seems like too late in the process. we should not even be getting to the point where we have bad ry data [05:31] if we are then either the export data is flawed, or the import is broken [05:32] we should never create bad ry's [05:32] ru [05:32] which we then happen to filter out via Valid() [05:32] shouldn't the problem be solved further up? [05:34] The other option I toyed with was to continue if endpoint.Settings(unit.Name()) returned nil. [05:34] Shall we do a hangout? [05:34] sure [05:34] standup one? [05:34] yup