| 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:11 |
|---|---|---|
| babbageclunk | wallyworld: just checking to see whether I can reproduce it in straight 2.2.3->2.2.3 migration | 02:13 |
| babbageclunk | wallyworld: yup | 02:31 |
| babbageclunk | thumper: ^ | 02:42 |
| thumper | fuck | 02:42 |
| babbageclunk | :( | 02:42 |
| * 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:43 |
| 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:44 |
| 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:45 |
| babbageclunk | thumper: of course. :) | 02:51 |
| * babbageclunk pops out briefly to pick up Ada | 02:52 | |
| 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:34 |
| axw | wallyworld: ok, a bit later | 03:35 |
| wallyworld | sure, np | 03:35 |
| 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:39 |
| 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:43 |
| babbageclunk | veebers: typing up the steps in the bug now - I'll paste a link here in a mo | 03:44 |
| veebers | babbageclunk: awesome, thanks | 03:45 |
| 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:49 |
| wgrant | ie. can I safely upgrade my smaller controllers to 2.2.3 tonight to test things out before bigger controllers get upgraded? | 03:50 |
| 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:53 |
| 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:54 |
| 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:55 |
| babbageclunk | wgrant: right - this is another lurking case of that same problem unfortunately. | 03:56 |
| 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> | 03:57 |
| veebers | babbageclunk: sweetbix, I'll check out the repro | 04:00 |
| veebers | babbageclunk: ah, nice catch. yeah the current model migration functional test doesn't do any relation removals | 04:04 |
| 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:05 |
| 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:08 |
| babbageclunk | oops - changed it not to look them up | 04:09 |
| babbageclunk | wallyworld: PR for the fix? https://github.com/juju/juju/pull/7840 | 05:18 |
| wallyworld | ok | 05:18 |
| babbageclunk | thanks | 05:18 |
| 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:25 |
| babbageclunk | I mean, the import data is good. | 05:26 |
| wallyworld | hmmm ok, i'm a bit confused then | 05:26 |
| 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:27 |
| 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:28 |
| 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:29 |
| 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:30 |
| wallyworld | if we are then either the export data is flawed, or the import is broken | 05:31 |
| 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:32 |
| 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 | 05:34 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!