[02:11] <babbageclunk> 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] <babbageclunk> wallyworld: just checking to see whether I can reproduce it in straight 2.2.3->2.2.3 migration
[02:31] <babbageclunk> wallyworld: yup
[02:42] <babbageclunk> thumper: ^
[02:42] <thumper> fuck
[02:42] <babbageclunk> :(
[02:43]  * thumper sads
[02:43] <thumper> what don'
[02:43] <babbageclunk> Sorry, I wish I'd worked it out sooner
[02:43] <thumper> what don't we do right?
[02:44] <babbageclunk> 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] <thumper> bugger...
[02:45] <babbageclunk> thumper: https://github.com/juju/juju/blob/2.2/state/migration_import.go#L1090
[02:45] <wallyworld> axw: i've pushed some changes to the status watcher
[02:45] <thumper> bollocks
[02:45] <axw> wallyworld: ok, looking
[02:45] <thumper> babbageclunk: can you please create a patch against 2.2
[02:45] <babbageclunk> yup yup
[02:45] <thumper> we may roll a fast 2.2.4
[02:45] <thumper> but I'd like confirmation that it works first :)
[02:51] <babbageclunk> thumper: of course. :)
[02:52]  * babbageclunk pops out briefly to pick up Ada 
[03:34] <wallyworld> 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] <axw> wallyworld: ok, a bit later
[03:35] <wallyworld> sure, np
[03:39] <thumper> babbageclunk: please file a bug so we can track this issue
[03:39]  * thumper creates a 2.2.4 milestone
[03:39] <thumper> ffs
[03:39] <babbageclunk> thumper: ok
[03:43] <veebers> 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] <babbageclunk> veebers: typing up the steps in the bug now - I'll paste a link here in a mo
[03:45] <veebers> babbageclunk: awesome, thanks
[03:49] <wgrant> 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] <wgrant> ie. can I safely upgrade my smaller controllers to 2.2.3 tonight to test things out before bigger controllers get upgraded?
[03:53] <babbageclunk> wgrant: it's related to the subordinate problem in 2.2, but it only happens on migration - upgrading won't cause it.
[03:53] <wgrant> babbageclunk: Cool, thanks.
[03:54] <babbageclunk> 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] <wgrant> Great.
[03:55] <babbageclunk> 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] <wgrant> babbageclunk: Oh, I ddn't see that, I was just worried since the subordinate relation code had been historically fragile.
[03:56] <babbageclunk> wgrant: right - this is another lurking case of that same problem unfortunately.
[03:57] <babbageclunk> veebers: https://bugs.launchpad.net/juju/+bug/1715794
[03:57] <mup> Bug #1715794: migration: subordinate with multiple principals imports incorrectly <juju:Triaged> <https://launchpad.net/bugs/1715794>
[04:00] <veebers> babbageclunk: sweetbix, I'll check out the repro
[04:04] <veebers> babbageclunk: ah, nice catch. yeah the current model migration functional test doesn't do any relation removals
[04:05] <wgrant> 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] <babbageclunk> 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] <babbageclunk> wgrant: but dump-db will show them.
[04:09] <babbageclunk> oops - changed it not to look them up
[05:18] <babbageclunk> wallyworld: PR for the fix? https://github.com/juju/juju/pull/7840
[05:18] <wallyworld> ok
[05:18] <babbageclunk> thanks
[05:25] <wallyworld> babbageclunk: so to make sure i understand, we modify import to ignore bad data
[05:25] <wallyworld> should the first line in the PR description be "incorrect" not "correct"
[05:25] <babbageclunk> wallyworld: no, the import is good - we were fabricating relation scopes even when they weren't in the import data.
[05:26] <babbageclunk> I mean, the import data is good.
[05:26] <wallyworld> hmmm ok, i'm a bit confused then
[05:27] <wallyworld> we change import to say if ru not valid then....
[05:27] <wallyworld> how would ru not be valid if import data ws ok
[05:27] <babbageclunk> 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] <wallyworld> right, so the import data *is* wrong
[05:28] <babbageclunk> wallyworld: nope
[05:28] <wallyworld> but you just said above it is?
[05:28] <wallyworld> we export bad data
[05:28] <babbageclunk> No, we create bad data when importing good data.
[05:28] <wallyworld> which we then attempt to import
[05:29] <wallyworld> but the data by definition is not good if it allows us to create bad data
[05:29] <babbageclunk> 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] <wallyworld> sounds like the export format is fawed
[05:30] <babbageclunk> wallyworld: I think you might be right - the import is smarter than it should be in this case.
[05:30] <wallyworld> 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] <wallyworld> if we are then either the export data is flawed, or the import is broken
[05:32] <wallyworld> we should never create bad ry's
[05:32] <wallyworld> ru
[05:32] <wallyworld> which we then happen to filter out via Valid()
[05:32] <wallyworld> shouldn't the problem be solved further up?
[05:34] <babbageclunk> The other option I toyed with was to continue if endpoint.Settings(unit.Name()) returned nil.
[05:34] <babbageclunk> Shall we do a hangout?
[05:34] <wallyworld> sure
[05:34] <wallyworld> standup one?
[05:34] <babbageclunk> yup