[fix] block credentials deletion until all dependent resources are cl…

…eaned up (#498) * [fix] block credentials deletion until all dependent resources are cleaned up * add creds finalizers for linodemachine * update if confitional * add tests
linode · Sep 5, 2024 · 77f7986 · 77f7986
1 parent f5e267e
commit 77f7986
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 5 deletions.
diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go
@@ -18,6 +18,7 @@ package controller
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"time"
@@ -219,6 +220,10 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo
 	if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil && clusterScope.LinodeCluster.Spec.Network.LoadBalancerType != lbTypeDNS {
 		logger.Info("NodeBalancer ID is missing, nothing to do")
 
+		if len(clusterScope.LinodeMachines.Items) > 0 {
+			return errors.New("Waiting for associated LinodeMachine objects to be deleted")
+		}
+
 		if err := clusterScope.RemoveCredentialsRefFinalizer(ctx); err != nil {
 			logger.Error(err, "failed to remove credentials finalizer")
 			setFailureReason(clusterScope, cerrs.DeleteClusterError, err, r)
@@ -250,6 +255,10 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo
 	clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID = nil
 	clusterScope.LinodeCluster.Spec.Network.AdditionalPorts = []infrav1alpha2.LinodeNBPortConfig{}
 
+	if len(clusterScope.LinodeMachines.Items) > 0 {
+		return errors.New("Waiting for associated LinodeMachine objects to be deleted")
+	}
+
 	if err := clusterScope.RemoveCredentialsRefFinalizer(ctx); err != nil {
 		logger.Error(err, "failed to remove credentials finalizer")
 		setFailureReason(clusterScope, cerrs.DeleteClusterError, err, r)

diff --git a/controller/linodecluster_controller_test.go b/controller/linodecluster_controller_test.go
@@ -360,6 +360,19 @@ var _ = Describe("cluster-delete", Ordered, Label("cluster", "cluster-delete"),
 		},
 	}
 
+	linodeMachine := infrav1alpha2.LinodeMachine{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-machine",
+			Namespace: defaultNamespace,
+		},
+		Spec: infrav1alpha2.LinodeMachineSpec{
+			ProviderID: ptr.To("linode://123"),
+		},
+		Status: infrav1alpha2.LinodeMachineStatus{
+			Addresses: []clusterv1.MachineAddress{},
+		},
+	}
+
 	ctlrSuite := NewControllerSuite(
 		GinkgoT(),
 		mock.MockLinodeClient{},
@@ -383,6 +396,11 @@ var _ = Describe("cluster-delete", Ordered, Label("cluster", "cluster-delete"),
 					cScope.Client = mck.K8sClient
 					mck.LinodeClient.EXPECT().DeleteNodeBalancer(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
 				}),
+				Result("cluster deleted", func(ctx context.Context, mck Mock) {
+					reconciler.Client = mck.K8sClient
+					err := reconciler.reconcileDelete(ctx, logr.Logger{}, cScope)
+					Expect(err).NotTo(HaveOccurred())
+				}),
 			),
 			Path(
 				Call("nothing to do because NB ID is nil", func(ctx context.Context, mck Mock) {
@@ -411,12 +429,40 @@ var _ = Describe("cluster-delete", Ordered, Label("cluster", "cluster-delete"),
 					Expect(err.Error()).To(ContainSubstring("delete NB error"))
 				}),
 			),
+			Path(
+				Call("cluster not deleted because some LinodeMachines are yet to be deleted and NB present", func(ctx context.Context, mck Mock) {
+					cScope.LinodeClient = mck.LinodeClient
+					cScope.Client = mck.K8sClient
+					cScope.LinodeCluster.Spec.Network.NodeBalancerID = &nodebalancerID
+					mck.LinodeClient.EXPECT().DeleteNodeBalancer(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
+					cScope.LinodeMachines = infrav1alpha2.LinodeMachineList{
+						Items: []infrav1alpha2.LinodeMachine{linodeMachine},
+					}
+				}),
+				Result("cluster not deleted because some LinodeMachines are yet to be deleted and NB present", func(ctx context.Context, mck Mock) {
+					reconciler.Client = mck.K8sClient
+					err := reconciler.reconcileDelete(ctx, logr.Logger{}, cScope)
+					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("Waiting for associated LinodeMachine objects to be deleted"))
+				}),
+			),
+			Path(
+				Call("cluster not deleted because some LinodeMachines are yet to be deleted and NB nil", func(ctx context.Context, mck Mock) {
+					cScope.LinodeClient = mck.LinodeClient
+					cScope.Client = mck.K8sClient
+					cScope.LinodeCluster.Spec.Network.NodeBalancerID = nil
+					cScope.LinodeMachines = infrav1alpha2.LinodeMachineList{
+						Items: []infrav1alpha2.LinodeMachine{linodeMachine},
+					}
+				}),
+				Result("cluster not deleted because some LinodeMachines are yet to be deleted and NB nil", func(ctx context.Context, mck Mock) {
+					reconciler.Client = mck.K8sClient
+					err := reconciler.reconcileDelete(ctx, logr.Logger{}, cScope)
+					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("Waiting for associated LinodeMachine objects to be deleted"))
+				}),
+			),
 		),
-		Result("cluster deleted", func(ctx context.Context, mck Mock) {
-			reconciler.Client = mck.K8sClient
-			err := reconciler.reconcileDelete(ctx, logr.Logger{}, cScope)
-			Expect(err).NotTo(HaveOccurred())
-		}),
 	)
 })
 

diff --git a/templates/infra/linodeMachineTemplate.yaml b/templates/infra/linodeMachineTemplate.yaml
@@ -6,6 +6,8 @@ metadata:
 spec:
   template:
     spec:
+      credentialsRef:
+        name: ${CLUSTER_NAME}-credentials
       image: ${LINODE_OS:="linode/ubuntu22.04"}
       type: ${LINODE_CONTROL_PLANE_MACHINE_TYPE}
       region: ${LINODE_REGION}
@@ -27,6 +29,8 @@ metadata:
 spec:
   template:
     spec:
+      credentialsRef:
+        name: ${CLUSTER_NAME}-credentials
       image: ${LINODE_OS:="linode/ubuntu22.04"}
       type: ${LINODE_MACHINE_TYPE}
       region: ${LINODE_REGION}