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

feat(operator): add finalizer to package CRs to await removals properly #634

Closed
wants to merge 4 commits into from

Conversation

mjnagel
Copy link
Contributor

@mjnagel mjnagel commented Aug 2, 2024

Description

Adds a k8s finalizer to all Package CR resources during initial reconciliation. This will ensure that all packages end up with the finalizer, even on an upgrade.

Switching to a finalizer removes the usage/need for Pepr's .IsDeleted since we are actually watching for updates now (when a resource is deleted it will have an update with a deletionTimestamp). The deletion logic was moved to the packageReconciler to trigger when a deletionTimestamp is present. The first step in the deletion logic is patching the Package to a new status of "Deleting".

A few notes on the new behavior:

  • Packages with SSO clients will take a few seconds to full delete their client and remove the token from the store. During this time they will be in the Deleting phase.
  • Packages without SSO clients will delete nearly instantly in most cases.
  • Since this is now in the same queue as other events, deletions may be slightly delayed depending on other events in the queue. This is mostly a good thing since it ensures that deletions are processed before new creations/changes - but it does result in slightly odd behavior for the end user since status does not change while a package is waiting in the queue.

Related Issue

Fixes #523

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@mjnagel mjnagel self-assigned this Aug 2, 2024
@mjnagel mjnagel marked this pull request as draft August 2, 2024 20:41
@mjnagel
Copy link
Contributor Author

mjnagel commented Sep 16, 2024

Closing for now, pending upstream Pepr changes to natively support this (see issue).

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.

Leverage finalizer for Package deletions
1 participant