Merge pull request kubernetes-sigs#7623 from killianmuldoon/fix/ownerref-bug

🐛 Fix bug in kubeadmconfig adoption

Author: k8s-ci-robot

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go

@@ -1026,7 +1026,7 @@ func (r *KubeadmConfigReconciler) storeBootstrapData(ctx context.Context, scope
 	return nil
 }
 
-// Ensure the bootstrap secret has the configOwner as a controller OwnerReference.
+// Ensure the bootstrap secret has the KubeadmConfig as a controller OwnerReference.
 func (r *KubeadmConfigReconciler) ensureBootstrapSecretOwnersRef(ctx context.Context, scope *Scope) error {
 	secret := &corev1.Secret{}
 	err := r.Client.Get(ctx, client.ObjectKey{Namespace: scope.Config.Namespace, Name: scope.Config.Name}, secret)
@@ -1041,11 +1041,14 @@ func (r *KubeadmConfigReconciler) ensureBootstrapSecretOwnersRef(ctx context.Con
 	if err != nil {
 		return errors.Wrapf(err, "failed to add KubeadmConfig %s as ownerReference to bootstrap Secret %s", scope.ConfigOwner.GetName(), secret.GetName())
 	}
+	if c := metav1.GetControllerOf(secret); c != nil && c.Kind != "KubeadmConfig" {
+		secret.OwnerReferences = util.RemoveOwnerRef(secret.OwnerReferences, *c)
+	}
 	secret.OwnerReferences = util.EnsureOwnerRef(secret.OwnerReferences, metav1.OwnerReference{
-		APIVersion: scope.ConfigOwner.GetAPIVersion(),
-		Kind:       scope.ConfigOwner.GetKind(),
-		UID:        scope.ConfigOwner.GetUID(),
-		Name:       scope.ConfigOwner.GetName(),
+		APIVersion: bootstrapv1.GroupVersion.String(),
+		Kind:       "KubeadmConfig",
+		UID:        scope.Config.UID,
+		Name:       scope.Config.Name,
 		Controller: pointer.Bool(true),
 	})
 	err = patchHelper.Patch(ctx, secret)

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller_test.go

@@ -117,6 +117,105 @@ func TestKubeadmConfigReconciler_Reconcile_ReturnEarlyIfKubeadmConfigIsReady(t *
 	g.Expect(result.RequeueAfter).To(Equal(time.Duration(0)))
 }
 
+// Reconcile returns early if the kubeadm config is ready because it should never re-generate bootstrap data.
+func TestKubeadmConfigReconciler_TestSecretOwnerReferenceReconciliation(t *testing.T) {
+	g := NewWithT(t)
+
+	clusterName := "my-cluster"
+	cluster := builder.Cluster(metav1.NamespaceDefault, clusterName).Build()
+	machine := builder.Machine(metav1.NamespaceDefault, "machine").
+		WithVersion("v1.19.1").
+		WithClusterName(clusterName).
+		WithBootstrapTemplate(bootstrapbuilder.KubeadmConfig(metav1.NamespaceDefault, "cfg").Unstructured()).
+		Build()
+	machine.Spec.Bootstrap.DataSecretName = pointer.String("something")
+
+	config := newKubeadmConfig(metav1.NamespaceDefault, "cfg")
+	config.SetOwnerReferences(util.EnsureOwnerRef(config.GetOwnerReferences(), metav1.OwnerReference{
+		APIVersion: machine.APIVersion,
+		Kind:       machine.Kind,
+		Name:       machine.Name,
+		UID:        machine.UID,
+	}))
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      config.Name,
+			Namespace: config.Namespace,
+		},
+		Type: corev1.SecretTypeBootstrapToken,
+	}
+	config.Status.Ready = true
+
+	objects := []client.Object{
+		config,
+		machine,
+		secret,
+		cluster,
+	}
+	myclient := fake.NewClientBuilder().WithObjects(objects...).Build()
+
+	k := &KubeadmConfigReconciler{
+		Client: myclient,
+	}
+
+	request := ctrl.Request{
+		NamespacedName: client.ObjectKey{
+			Namespace: metav1.NamespaceDefault,
+			Name:      "cfg",
+		},
+	}
+	var err error
+	key := client.ObjectKeyFromObject(config)
+	actual := &corev1.Secret{}
+
+	t.Run("KubeadmConfig ownerReference is added on first reconcile", func(t *testing.T) {
+		_, err = k.Reconcile(ctx, request)
+		g.Expect(err).NotTo(HaveOccurred())
+
+		g.Expect(myclient.Get(ctx, key, actual)).To(Succeed())
+
+		controllerOwner := metav1.GetControllerOf(actual)
+		g.Expect(controllerOwner).To(Not(BeNil()))
+		g.Expect(controllerOwner.Kind).To(Equal(config.Kind))
+		g.Expect(controllerOwner.Name).To(Equal(config.Name))
+	})
+
+	t.Run("KubeadmConfig ownerReference re-reconciled without error", func(t *testing.T) {
+		_, err = k.Reconcile(ctx, request)
+		g.Expect(err).NotTo(HaveOccurred())
+
+		g.Expect(myclient.Get(ctx, key, actual)).To(Succeed())
+
+		controllerOwner := metav1.GetControllerOf(actual)
+		g.Expect(controllerOwner).To(Not(BeNil()))
+		g.Expect(controllerOwner.Kind).To(Equal(config.Kind))
+		g.Expect(controllerOwner.Name).To(Equal(config.Name))
+	})
+	t.Run("non-KubeadmConfig controller OwnerReference is replaced", func(t *testing.T) {
+		g.Expect(myclient.Get(ctx, key, actual)).To(Succeed())
+
+		actual.SetOwnerReferences([]metav1.OwnerReference{
+			{
+				APIVersion: machine.APIVersion,
+				Kind:       machine.Kind,
+				Name:       machine.Name,
+				UID:        machine.UID,
+				Controller: pointer.Bool(true),
+			}})
+		g.Expect(myclient.Update(ctx, actual)).To(Succeed())
+
+		_, err = k.Reconcile(ctx, request)
+		g.Expect(err).NotTo(HaveOccurred())
+
+		g.Expect(myclient.Get(ctx, key, actual)).To(Succeed())
+
+		controllerOwner := metav1.GetControllerOf(actual)
+		g.Expect(controllerOwner).To(Not(BeNil()))
+		g.Expect(controllerOwner.Kind).To(Equal(config.Kind))
+		g.Expect(controllerOwner.Name).To(Equal(config.Name))
+	})
+}
+
 // Reconcile returns nil if the referenced Machine cannot be found.
 func TestKubeadmConfigReconciler_Reconcile_ReturnNilIfReferencedMachineIsNotFound(t *testing.T) {
 	g := NewWithT(t)