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 finalizers preventing namespace deletion #391

Merged
merged 3 commits into from
Jan 23, 2025
Merged

Conversation

djnewbould
Copy link
Contributor

@djnewbould djnewbould commented Jan 14, 2025

Finalizers on custom resource definitions cause both the factory-plus namespace to be stuck in a terminating state. The added job runs on the helm post delete hook and patches the Kerberos Key to remove the finalizers.

  • Fix factory-plus namespace deletion.

How to test

  1. Run Helm uninstall acs -n factory-plus.
  2. Wait for the job to complete.
  3. Run kubectl delete namesapce factory-plus to delete the factory-plus namespace.
  4. The namespace should delete and not be stuck in a terminating state.

@djnewbould djnewbould changed the title Fix Finalizers Preventing Namespace Deletion Fix finalizers preventing namespace deletion Jan 14, 2025
@amrc-benmorrow
Copy link
Contributor

This looks good. Getting an edge cluster to uninstall cleanly will be less straightforward as there are multiple charts involved, but making the edge-cluster chart uninstall on its own will be a good start.

I presume you've tested that Helm is willing to run a post-delete hook with the kerberos-keys still present? I wasn't sure it would.

When it comes to the LocalSecret resources you may run into an issue as this CRD is not yet installed onto the central cluster. Probably the best way forward is to make sure it is; copy the CRD from acs-krb-keys-operator/crd to deploy/crds.

@djnewbould
Copy link
Contributor Author

This looks good. Getting an edge cluster to uninstall cleanly will be less straightforward as there are multiple charts involved, but making the edge-cluster chart uninstall on its own will be a good start.

I presume you've tested that Helm is willing to run a post-delete hook with the kerberos-keys still present? I wasn't sure it would.

When it comes to the LocalSecret resources you may run into an issue as this CRD is not yet installed onto the central cluster. Probably the best way forward is to make sure it is; copy the CRD from acs-krb-keys-operator/crd to deploy/crds.

I didn't notice any issues with the local secret resource,. Do you think its worth copying across anyway?

@amrc-benmorrow
Copy link
Contributor

I've moved the edge cluster conversations into a new ticket on our internal Jira.

Ideally this would be rebased onto main to remove the reverted commits, but I'm not going to hold up approving it for that.

When deleting the factory-plus namespace, finalizers prevented its deletion. This post delete job removes the finalizers to fix this issue.
@djnewbould djnewbould force-pushed the dn/uninstall-clean-up branch from 4e5c73c to d0caac0 Compare January 23, 2025 11:59
@djnewbould djnewbould force-pushed the dn/uninstall-clean-up branch from d0caac0 to 7d8986a Compare January 23, 2025 13:26
@djnewbould djnewbould marked this pull request as ready for review January 23, 2025 13:27
@djnewbould djnewbould merged commit 00a83ad into main Jan 23, 2025
1 check passed
@djnewbould djnewbould deleted the dn/uninstall-clean-up branch January 23, 2025 13:29
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.

2 participants