diff --git a/controllers/machine_controller.go b/controllers/machine_controller.go index 44c001a1b2ce..aaed7812d2d4 100644 --- a/controllers/machine_controller.go +++ b/controllers/machine_controller.go @@ -376,38 +376,42 @@ func (r *MachineReconciler) isDeleteNodeAllowed(ctx context.Context, cluster *cl return errNilNodeRef } - // Get the control plane object associated with the cluster - controlPlane, err := getControlPlaneFromRef(ctx, r.Client, cluster.Spec.ControlPlaneRef) - if err != nil { - return err + var controlPlane *unstructured.Unstructured + + // controlPlaneRef is an optional field in the Cluster so return nil if it isn't set + if cluster.Spec.ControlPlaneRef != nil { + var err error + controlPlane, err = external.Get(ctx, r.Client, cluster.Spec.ControlPlaneRef, cluster.Spec.ControlPlaneRef.Namespace) + if err != nil { + return err + } } // Check if the ControlPlane is externally managed (AKS, EKS, GKE, etc) // and skip the following section if control plane is externally managed // because there will be no control plane nodes registered - if !util.IsExternalManagedControlPlane(controlPlane) { + if controlPlane != nil && util.IsExternalManagedControlPlane(controlPlane) { + return nil + } - // Get all of the machines that belong to this cluster. - machines, err := getActiveMachinesInCluster(ctx, r.Client, machine.Namespace, machine.Labels[clusterv1.ClusterLabelName]) - if err != nil { - return err - } + // Get all of the machines that belong to this cluster. + machines, err := getActiveMachinesInCluster(ctx, r.Client, machine.Namespace, machine.Labels[clusterv1.ClusterLabelName]) + if err != nil { + return err + } - // Whether or not it is okay to delete the NodeRef depends on the - // number of remaining control plane members and whether or not this - // machine is one of them. - switch numControlPlaneMachines := len(util.GetControlPlaneMachines(machines)); { - case numControlPlaneMachines == 0: - // Do not delete the NodeRef if there are no remaining members of - // the control plane. - return errNoControlPlaneNodes - default: - // Otherwise it is okay to delete the NodeRef. - return nil - } + // Whether or not it is okay to delete the NodeRef depends on the + // number of remaining control plane members and whether or not this + // machine is one of them. + switch numControlPlaneMachines := len(util.GetControlPlaneMachines(machines)); { + case numControlPlaneMachines == 0: + // Do not delete the NodeRef if there are no remaining members of + // the control plane. + return errNoControlPlaneNodes + default: + // Otherwise it is okay to delete the NodeRef. + return nil } - // In other control plane types, it is OK to delete the NodeRef - return nil } func (r *MachineReconciler) drainNode(ctx context.Context, cluster *clusterv1.Cluster, nodeName string, machineName string) error { diff --git a/controllers/machine_helpers.go b/controllers/machine_helpers.go index f1e8021f3776..4e70abc82158 100644 --- a/controllers/machine_helpers.go +++ b/controllers/machine_helpers.go @@ -20,12 +20,9 @@ import ( "context" "github.com/pkg/errors" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" - "sigs.k8s.io/cluster-api/controllers/external" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -69,16 +66,3 @@ func hasMatchingLabels(matchSelector metav1.LabelSelector, matchLabels map[strin } return true } - -// getControlPlaneFromRef returns the control plane object from the provided ObjectReference -func getControlPlaneFromRef(ctx context.Context, c client.Client, controlPlaneRef *corev1.ObjectReference) (*unstructured.Unstructured, error) { - // controlPlaneRef is an optional field in the Cluster so return nil if it isn't set - if controlPlaneRef == nil { - return nil, nil - } - controlPlane, err := external.Get(ctx, c, controlPlaneRef, controlPlaneRef.Namespace) - if err != nil { - return nil, err - } - return controlPlane, nil -} diff --git a/controllers/machine_helpers_test.go b/controllers/machine_helpers_test.go index eaf4b071fde3..4ec8e56ac356 100644 --- a/controllers/machine_helpers_test.go +++ b/controllers/machine_helpers_test.go @@ -22,14 +22,11 @@ import ( . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client/fake" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" - kcpv1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" ) func Test_getActiveMachinesInCluster(t *testing.T) { @@ -202,46 +199,3 @@ func TestMachineHealthCheckHasMatchingLabels(t *testing.T) { }) } } - -func TestGetControlPlaneFromRef(t *testing.T) { - g := NewWithT(t) - - kcp1 := kcpv1.KubeadmControlPlane{ - TypeMeta: metav1.TypeMeta{ - Kind: "KubeadmControlPlane", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "kcp1", - Namespace: "kcp1", - Labels: map[string]string{ - clusterv1.ClusterLabelName: "kcp1", - }, - }, - Status: kcpv1.KubeadmControlPlaneStatus{ - Ready: true, - Initialized: true, - }, - } - - controlPlaneRef := &corev1.ObjectReference{ - APIVersion: "controlplane.cluster.x-k8s.io/v1alpha3", - Kind: "KubeadmControlPlane", - Name: "kcp1", - Namespace: "kcp1", - } - - ctx := context.Background() - c := fake.NewFakeClientWithScheme(scheme.Scheme, &kcp1) - controlPlane, err := getControlPlaneFromRef(ctx, c, controlPlaneRef) - g.Expect(err).To(BeNil()) - - ready, found, err := unstructured.NestedBool(controlPlane.Object, "status", "ready") - g.Expect(ready).To(Equal(true)) - g.Expect(found).To(Equal(true)) - g.Expect(err).To(BeNil()) - - managed, found, err := unstructured.NestedBool(controlPlane.Object, "status", "externalManagedControlPlane") - g.Expect(managed).To(Equal(false)) - g.Expect(found).To(Equal(false)) - g.Expect(err).To(BeNil()) -} diff --git a/util/util.go b/util/util.go index 03a7ff867f24..2cf80ab10483 100644 --- a/util/util.go +++ b/util/util.go @@ -200,10 +200,6 @@ func GetControlPlaneMachinesFromList(machineList *clusterv1.MachineList) (res [] // IsExternalManagedControlPlane returns a bool indicating whether the control plane referenced // in the passed Unstructured resource is an externally managed control plane such as AKS, EKS, GKE, etc. func IsExternalManagedControlPlane(controlPlane *unstructured.Unstructured) bool { - // The controlPlaneRef field in the Cluster is optional, so return false if the control plane is not set - if controlPlane == nil { - return false - } managed, found, err := unstructured.NestedBool(controlPlane.Object, "status", "externalManagedControlPlane") if err != nil || !found { return false diff --git a/util/util_test.go b/util/util_test.go index ee48dca72d2c..4fb60453587c 100644 --- a/util/util_test.go +++ b/util/util_test.go @@ -602,12 +602,6 @@ func TestGetMachinesForCluster(t *testing.T) { func TestIsExternalManagedControlPlane(t *testing.T) { g := NewWithT(t) - t.Run("should return false if the controlPlane resource is nil", func(t *testing.T) { - var controlPlane *unstructured.Unstructured - result := IsExternalManagedControlPlane(controlPlane) - g.Expect(result).Should(Equal(false)) - }) - t.Run("should return true if control plane status externalManagedControlPlane is true", func(t *testing.T) { controlPlane := &unstructured.Unstructured{ Object: map[string]interface{}{