resolve bad migration operation causing drop of permissions (fixes #403)

[fmigneault]

resolve bad migration operation causing drop of permissions (fixes #403)

[tlvu]

Approve the spirit of it as I have no knowledge of DB migration.

Curious, how do you know this will fix the issue since I do not see any new test case reproducing the issue to confirm the fix works?

I assume once this PR is merged, you'll create a new Magpie docker and test a migration in a real PAVICS stack using our birdhouse-deploy recipe?

[tlvu]

I have no idea about DB migration but shouldn't the fix be in a new migration step? Fixing in an existing migration step means a server that already execute this step won't attempt it again and miss out on the fix?

Discard what I said if it makes no sense since I know nothing about DB migration.

[fmigneault]

It has to be applied there so that a server that did not run the migration will do it properly after rollback.
If added as new step, permissions will still be dropped at that step and later 'new' step will have no effect as there is nothing left to work with.
Migration steps have their own version dissociated from magpie versions.

[tlvu]

It has to be applied there so that a server that did not run the migration will do it properly after rollback.

Oh wait so there is a way to rollback a DB migration? I didn't have to rm -rf /data/magpie_persist when reverting from 3.5.1 to 1.7.3?

[fmigneault]

Curious, how do you know this will fix the issue since I do not see any new test case reproducing the issue to confirm the fix works?

I assume once this PR is merged, you'll create a new Magpie docker and test a migration in a real PAVICS stack using our birdhouse-deploy recipe?

@tlvu
I used birdhouse-deploy version 1.11.24 and applied all permissions seen in #401, then ran the migration operations via debug script to step through lines. I noticed that the permission resolution was incorrectly matching bad permissions together based on that the missing compare this PR adds.
After patch, the permissions seem to remain as expected. (still need to test on your side though, as I don't have the full database)

I will tag another version after the few recent bugfixes are merged.

[tlvu]

I used birdhouse-deploy version 1.11.24 and applied all permissions seen in #401,

You meant #403? Not #401 right?

[fmigneault]

You meant #403? Not #401 right?

@tlvu

Yes. My bad.
I tagged a new version 3.8.0 with all fixes.
You can let me know if you see any more errors.

[tlvu]

I tagged a new version 3.8.0 with all fixes.
You can let me know if you see any more errors.

@fmigneault

Woo a jump from 3.5.1 to 3.8.0? So during last Magpie upgrade, 3.7.0 was available but we didn't want to use it?

Before I spend, and potentially waste time on this 3.8.0 again, what tests did you perform on this 3.8.0? Is it integrated here https://github.com/bird-house/birdhouse-deploy/compare/restore-previous-broken-magpie-upgrade-so-we-can-work-on-a-fix, how has it been tested?

[dbyrns]

@tlvu, Francis tests the migration to 3.8.0 using this procedure : #407 (comment).
So this migration is tested and safe. However, no migration should be done to an earlier version as, like we discussed with @huard, we don't want to support old versions and the fix is in 3.8.0. The only thing we cannot do is testing with your own server data, so hopefully you will not waste your time this time. Also remember that Magpie has an exhaustive list of tests, but edge cases with production data are hard to predict and it's not because of ill will that errors happen.