[release-1.2] 🐛 Fix kubeadmconfig bootstrapsecret ownerRef reconciliation

[k8s-infra-cherrypick-robot]

This is an automated cherry-pick of #7587

/assign sbueringer

[sbueringer]

/lgtm
/approve
/retest

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [sbueringer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[killianmuldoon]

/lgtm

[aanoaa]

in my case, this operation occurs validation error such as..

E1125 01:26:05.678896      12 controller.go:326] "Reconciler error" err="could not add KubeadmConfig hshong-test-control-plane-d5snx as ownerReference to bootstrap Secret hshong-test-control-plane-8rdj6: Secret \"hshong-test-control-plane-8rdj6\" is invalid: metadata.ownerReferences: Invalid value: []v1.OwnerReference{v1.OwnerReference{APIVersion:\"bootstrap.cluster.x-k8s.io/v1beta1\", Kind:\"KubeadmConfig\", Name:\"hshong-test-control-plane-8rdj6\", UID:\"6d1904bf-26b3-4edb-b7ca-b17854bd9b28\", Controller:(*bool)(0xc004564be0), BlockOwnerDeletion:(*bool)(nil)}, v1.OwnerReference{APIVersion:\"cluster.x-k8s.io/v1beta1\", Kind:\"Machine\", Name:\"hshong-test-control-plane-d5snx\", UID:\"7e3f24e8-8fe7-4d11-83d5-99dfe748cc97\", Controller:(*bool)(0xc004564bee), BlockOwnerDeletion:(*bool)(nil)}}: Only one reference can have Controller set to true. Found \"true\" in references for hshong-test-control-plane-8rdj6 and hshong-test-control-plane-d5snx" controller="kubeadmconfig" controllerGroup="bootstrap.cluster.x-k8s.io" controllerKind="KubeadmConfig" kubeadmConfig="cloudtech-team-dev/hshong-test-control-plane-8rdj6" namespace="cloudtech-team-dev" name="hshong-test-control-plane-8rdj6" reconcileID=3bb42394-0216-449b-ae0e-7baa7c1ac0d0

cause the machine resource of contol plane already takes controller: true attr for same KubeadmControlPlane resource.

Multiple controllers can race to adopt a given object, but only one can win by being the first to add a ControllerRef to the object's OwnerReferences list. The losers will see their adoptions fail due to a validation error as explained above.

• https://github.com/kubernetes/design-proposals-archive/blob/main/api-machinery/controller-ref.md#adoption

[sbueringer]

Thx for the feedback @aanoaa !!

@killianmuldoon I"m a bit confused about the error message. We are adding the ConfigOwner as ownerRef, right? So we're essentially either adding a MachinePool or a Machine as owner. The error seems to suggest we add a KubeadmConfig as owner.

Independent of that. The error from @aanoaa seems to suggest that the KubeadmConfig is already owned by a KubeadmConfig.

Just for my understanding. We have e.g. the following ownerRefs:

1. Machine => (owns) KubeadmConfig
2. Machine => (owns) Secret

This code established the second ownerRef and uses the first as data for that?

I assume we can be sure that 1. is always correct, can we then just drop additional wrong controller ownerRefs that might be already on the Secret?

[sbueringer]

Update: what I described seems to be indeed what our new code does, but looking at the Secret creation we should add the KubeadmConfig as owner here and not the configOwner (and then the error message is correct)

So the correct chain is:

Machine => (owns) KubeadmConfig => (owns) Secret

P.S. Not a huge issue I should have also figured this out in review :/, but please also check what went wrong with testing (just in case we have a bug in the test or something). But maybe we just missed reconciler errors in the logs, the ownerRef wasn't overwritten as it couldn't be.

[killianmuldoon]

Thanks for catching this @aanoaa - I'm working on a fix now.

[sbueringer]

Just to re-iterate.

Thank you very much @aanoaa for testing, finding and reporting this issue!!

Bug fixes should be merged in a few minutes across branches.