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

remove finalizers before deleting migrated old MRs #272

Conversation

erhancagirici
Copy link
Collaborator

Description of your changes

In MR API migration process, old API MRs have their reconciliation paused, new MRs with the target API is created, then the old API MRs are deleted. However, deletion gets stuck because they have a finalizer and their reconciliation operations are paused.

The MRs have single finalizer named finalizer.managedresource.crossplane.io, which ensures the deletion of native cloud provider resource, however in this case it is a no-op, since the deletion policy is Orphan and we already would like to keep the cloud provider resource since we are just migrating it.

This PR adds a new migration step for old MRs, that removes the finalizer.
Also, it fixes a bug in the sorting of the steps, when step count to be executed exceeds 10.

I have:

  • Read and followed Crossplane's [contribution process].
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

  • Run unit tests
  • Manually tested with VPC resource

@Upbound-CLA
Copy link

Upbound-CLA commented Sep 6, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@sergenyalcin sergenyalcin left a comment

Choose a reason for hiding this comment

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

Thanks @erhancagirici LGTM!

@erhancagirici erhancagirici merged commit f73a662 into crossplane:main Sep 8, 2023
4 checks passed
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.

3 participants