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

fix(migrations/policy): delete objects before crds #336

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ralgozino
Copy link
Member

@ralgozino ralgozino commented Jan 15, 2025

Summary 💡

When switching policy type from kyverno to none, delete first the objects using the APIs of the CRDs and then the CRDs. Otherwise we could sometimes end up in a race condition where the objects cannot be deleted because the APIs are not available anymore.

Closes #335

Description 📝

Instead of building the whole kustomize base for Kyverno and piping it to the kubectl delete, the command that deletes Kyverno resources has been split into 2 commands that are applied separately, one after the other: policies and core, respectively.

See the issue #335 for more details on the reasoning behind this PR.

Breaking Changes 💔

None

Tests performed 🧪

  • Tested the change with KFD version v1.31.0.
  • Tested migrating from kyverno to none several times, the race condition did not present again.
  • Tested migration with installDefaultPolicies set to false

Future work 🔧

None, notice that now that we switched to kapp, the migration could be dropped entirely and let kapp handle the deletion.

@ralgozino ralgozino self-assigned this Jan 15, 2025
When switching from kyverno to none, delete first the objects using the
APIs of the CRDs and then the CRDs. Otherwise we could sometimes end up
in a race condition where the objects cannot be deleted because the APIs
are not available anymore.

Fixes #335
@ralgozino ralgozino force-pushed the fix-kyverno-to-none-migraiton branch from f079196 to 2885475 Compare January 16, 2025 15:07
@ralgozino ralgozino marked this pull request as ready for review January 16, 2025 15:08
Copy link
Member

@stefanoghinelli stefanoghinelli left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

Migrating from policy.type kyverno to none sometimes (?) fails
2 participants