cluster-api icon indicating copy to clipboard operation
cluster-api copied to clipboard

Secret ownerReferences not restored

Open killianmuldoon opened this issue 2 years ago • 3 comments

When Cluster API is backed up and restored the ownerReferences on the Kubernetes Secrets relating to the cluster are not restored. This doesn't seem to cause issues in the functioning of the cluster, but it causes these Secrets to not be deleted when the Cluster is deleted.

The following flow then results in an error as some of the secrets continue to exist unexpectedly.

  1. Restore cluster 'my-cluster' from backup
  2. Delete cluster 'my-cluster'
  3. Create a new cluster called 'my-cluster'

There is no reconciler that watches Secrets in CAPI, so there's no obvious place to restore the references. Some options for improving this situation:

  1. Document that kubectl delete secrets -l cluster.x-k8s.io/cluster-name=cloister (or similar) should be run as part of cleaning up a Cluster API cluster.
  2. Add a cleanup step in CAPI to add ownerReferences to a Cluster, where it exists, based on the cluster name and namespace.
  3. Add a cleanup step, external or in the reconciler, to delete secrets that are not being used by any existing CAPI objects.

/kind cleanup /area ux

May be related to other orphaned object cleanups e.g. https://github.com/kubernetes-sigs/cluster-api/issues/6863

killianmuldoon avatar Jul 26 '22 12:07 killianmuldoon

Is this related (only) to the KCP secrets?

Naively I would have assumed that when we create Secrets with ownerRefs we should also adopt them after restore (~ at the same place in the code).

sbueringer avatar Aug 16 '22 10:08 sbueringer

I think this is the relevant code for kubeadmconfig: https://github.com/kubernetes-sigs/cluster-api/blob/1a286fdf7da314bdfbb76b042f8f25f1828734a0/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go#L430

In this function we only set the owner references on generating the secrets, if the secrets exist and has a correctly formatted KeyPair we don't re-add the owner references.

killianmuldoon avatar Aug 16 '22 13:08 killianmuldoon

Could be a good place to "adopt" if the ownerRefs of the current secrets are missing (not sure what is executed after the initial create)

sbueringer avatar Aug 16 '22 13:08 sbueringer

/assign

killianmuldoon avatar Nov 03 '22 17:11 killianmuldoon

/close

Fixed by #7587 and #7592

killianmuldoon avatar Nov 29 '22 16:11 killianmuldoon

@killianmuldoon: Closing this issue.

In response to this:

/close

Fixed by #7587 and #7592

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot avatar Nov 29 '22 16:11 k8s-ci-robot