Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1970338: Fix lookup for migmigration by UID so it returns true when exists #1191

Merged
merged 1 commit into from Sep 2, 2021

Conversation

djwhatle
Copy link
Contributor

@djwhatle djwhatle commented Sep 2, 2021

Background info on fix:

At the beginning of the migration process we try to clean up old Velero CRs to unblock the system.
We use various tests to determine if a backup is "stale" and worthy of deletion. One of those tests is to check if there is another running migration linked with an in-progress Backup. If we find this, we won't delete the Backup.

The function responsible for checking whether a migration is associated with the backup was always returning false because it was doing a lookup on migmigrations with an invalid labelSelector that would never return any results.
This means that when two migrations are running, one of them in the BackupCreated phase, one of them in the CleanStaleBackups phase, the second migration will clobber the first migrations active backup

To resolve this, I added a usable label to the migmigration so we can lookup by UID, then used that label in the faulty function. I am no longer seeing failures when running parallel migrations.

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Valid bug 1970338

Copy link
Contributor

@dymurray dymurray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch!!

@djwhatle djwhatle merged commit 7a4a476 into migtools:master Sep 2, 2021
djwhatle added a commit to djwhatle/mig-controller that referenced this pull request Sep 2, 2021
dymurray pushed a commit that referenced this pull request Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants