diff --git a/internal/controllers/machine/machine_controller_phases.go b/internal/controllers/machine/machine_controller_phases.go index 4fa3685e3c3b..126c7de7232c 100644 --- a/internal/controllers/machine/machine_controller_phases.go +++ b/internal/controllers/machine/machine_controller_phases.go @@ -116,18 +116,8 @@ func (r *Reconciler) reconcileExternal(ctx context.Context, cluster *clusterv1.C return external.ReconcileOutput{}, err } - // With the migration from v1alpha2 to v1alpha3, Machine controllers should be the owner for the - // infra Machines, hence remove any existing machineset controller owner reference - if controller := metav1.GetControllerOf(obj); controller != nil && controller.Kind == "MachineSet" { - gv, err := schema.ParseGroupVersion(controller.APIVersion) - if err != nil { - return external.ReconcileOutput{}, err - } - if gv.Group == clusterv1.GroupVersion.Group { - ownerRefs := util.RemoveOwnerRef(obj.GetOwnerReferences(), *controller) - obj.SetOwnerReferences(ownerRefs) - } - } + // removeOutdatedOwnerRefs removes MachineSet and ControlPlane owners from the objects referred to by a Machine. + removeOutdatedOwnerRefs(cluster, m, obj) // Set external object ControllerReference to the Machine. if err := controllerutil.SetControllerReference(m, obj, r.Client.Scheme()); err != nil { @@ -372,3 +362,31 @@ func (r *Reconciler) reconcileCertificateExpiry(ctx context.Context, _ *clusterv return ctrl.Result{}, nil } + +// removeOutdatedOwnerRefs will remove any MachineSet or ControlPlane ownerReferences from Infrastructure Machine and +// Bootstrap objects referred to by the machine. These ownerReferences are added on creation as the Machine does not exist +// when the objects are initially created. +func removeOutdatedOwnerRefs(cluster *clusterv1.Cluster, m *clusterv1.Machine, obj *unstructured.Unstructured) { + cpGVK := getControlPlaneGVKForMachine(cluster, m) + for _, owner := range obj.GetOwnerReferences() { + if owner.APIVersion == clusterv1.GroupVersion.String() && owner.Kind == "MachineSet" || + cpGVK != nil && owner.APIVersion == cpGVK.GroupVersion().String() && owner.Kind == cpGVK.Kind { + ownerRefs := util.RemoveOwnerRef(obj.GetOwnerReferences(), owner) + obj.SetOwnerReferences(ownerRefs) + + } + } +} + +// getControlPlaneGVKForMachine returns the Kind of the control plane in the Cluster associated with the Machine. +// This function checks that the machine is managed by a control plane, and then retrieves the kind from the Cluster's +// .spec.controlPlaneRef. +func getControlPlaneGVKForMachine(cluster *clusterv1.Cluster, machine *clusterv1.Machine) *schema.GroupVersionKind { + if _, ok := machine.GetLabels()[clusterv1.MachineControlPlaneNameLabel]; ok { + if cluster.Spec.ControlPlaneRef != nil { + gvk := cluster.Spec.ControlPlaneRef.GroupVersionKind() + return &gvk + } + } + return nil +} diff --git a/internal/controllers/machineset/machineset_controller_test.go b/internal/controllers/machineset/machineset_controller_test.go index 49db0fa23647..c2e11d696010 100644 --- a/internal/controllers/machineset/machineset_controller_test.go +++ b/internal/controllers/machineset/machineset_controller_test.go @@ -228,14 +228,20 @@ func TestMachineSetReconciler(t *testing.T) { g.Expect(im.GetAnnotations()).To(HaveKeyWithValue("annotation-1", "true"), "have annotations of MachineTemplate applied") g.Expect(im.GetAnnotations()).To(HaveKeyWithValue("precedence", "MachineSet"), "the annotations from the MachineSpec template to overwrite the infrastructure template ones") g.Expect(im.GetLabels()).To(HaveKeyWithValue("label-1", "true"), "have labels of MachineTemplate applied") + } + g.Eventually(func() bool { + g.Expect(env.List(ctx, infraMachines, client.InNamespace(namespace.Name))).To(Succeed()) + // The Machine reconciler should remove the ownerReference to the MachineSet on the InfrastructureMachine. hasMSOwnerRef := false - for _, o := range im.GetOwnerReferences() { - if o.Kind == machineSetKind.Kind { - hasMSOwnerRef = true + for _, im := range infraMachines.Items { + for _, o := range im.GetOwnerReferences() { + if o.Kind == machineSetKind.Kind { + hasMSOwnerRef = true + } } } - g.Expect(hasMSOwnerRef).To(BeTrue(), "have ownerRef to MachineSet") - } + return hasMSOwnerRef + }, timeout).Should(BeFalse(), "infraMachine should not have ownerRef to MachineSet") t.Log("Creating a BootstrapConfig for each Machine") bootstrapConfigs := &unstructured.UnstructuredList{} @@ -251,14 +257,20 @@ func TestMachineSetReconciler(t *testing.T) { g.Expect(im.GetAnnotations()).To(HaveKeyWithValue("annotation-1", "true"), "have annotations of MachineTemplate applied") g.Expect(im.GetAnnotations()).To(HaveKeyWithValue("precedence", "MachineSet"), "the annotations from the MachineSpec template to overwrite the bootstrap config template ones") g.Expect(im.GetLabels()).To(HaveKeyWithValue("label-1", "true"), "have labels of MachineTemplate applied") + } + g.Eventually(func() bool { + g.Expect(env.List(ctx, bootstrapConfigs, client.InNamespace(namespace.Name))).To(Succeed()) + // The Machine reconciler should remove the ownerReference to the MachineSet on the Bootstrap object. hasMSOwnerRef := false - for _, o := range im.GetOwnerReferences() { - if o.Kind == machineSetKind.Kind { - hasMSOwnerRef = true + for _, im := range bootstrapConfigs.Items { + for _, o := range im.GetOwnerReferences() { + if o.Kind == machineSetKind.Kind { + hasMSOwnerRef = true + } } } - g.Expect(hasMSOwnerRef).To(BeTrue(), "have ownerRef to MachineSet") - } + return hasMSOwnerRef + }, timeout).Should(BeFalse(), "bootstrap should not have ownerRef to MachineSet") // Set the infrastructure reference as ready. for _, m := range machines.Items {