[improvement] : add conditions to check if linodevpc and linodefirewall are ready (#535)

* add conditions to check if linodevpc and linodefirewall are ready

* fix lint errors

* fix unittests

* add unittests for new conditions added

Author: rahulait

controller/linodecluster_controller.go

@@ -26,6 +26,7 @@ import (
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/client-go/tools/record"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -49,7 +50,11 @@ import (
 	"github.com/linode/cluster-api-provider-linode/util/reconciler"
 )
 
-const lbTypeDNS string = "dns"
+const (
+	lbTypeDNS string = "dns"
+
+	ConditionPreflightLinodeVPCReady clusterv1.ConditionType = "PreflightLinodeVPCReady"
+)
 
 // LinodeClusterReconciler reconciles a LinodeCluster object
 type LinodeClusterReconciler struct {
@@ -112,6 +117,7 @@ func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	return r.reconcile(ctx, clusterScope, logger)
 }
 
+//nolint:cyclop // can't make it simpler with existing API
 func (r *LinodeClusterReconciler) reconcile(
 	ctx context.Context,
 	clusterScope *scope.ClusterScope,
@@ -163,6 +169,17 @@ func (r *LinodeClusterReconciler) reconcile(
 	}
 
 	// Create
+	if clusterScope.LinodeCluster.Spec.VPCRef != nil {
+		if !reconciler.ConditionTrue(clusterScope.LinodeCluster, ConditionPreflightLinodeVPCReady) {
+			res, err := r.reconcilePreflightLinodeVPCCheck(ctx, logger, clusterScope)
+			if err != nil || !res.IsZero() {
+				conditions.MarkFalse(clusterScope.LinodeCluster, ConditionPreflightLinodeVPCReady, string("linode vpc not yet available"), clusterv1.ConditionSeverityError, "")
+				return res, err
+			}
+		}
+		conditions.MarkTrue(clusterScope.LinodeCluster, ConditionPreflightLinodeVPCReady)
+	}
+
 	if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" {
 		if err := r.reconcileCreate(ctx, logger, clusterScope); err != nil {
 			if !reconciler.HasConditionSeverity(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.ConditionSeverityError) {
@@ -192,6 +209,34 @@ func (r *LinodeClusterReconciler) reconcile(
 	return res, nil
 }
 
+func (r *LinodeClusterReconciler) reconcilePreflightLinodeVPCCheck(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) (ctrl.Result, error) {
+	name := clusterScope.LinodeCluster.Spec.VPCRef.Name
+	namespace := clusterScope.LinodeCluster.Spec.VPCRef.Namespace
+	if namespace == "" {
+		namespace = clusterScope.LinodeCluster.Namespace
+	}
+	linodeVPC := infrav1alpha2.LinodeVPC{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+			Name:      name,
+		},
+	}
+	if err := clusterScope.Client.Get(ctx, client.ObjectKeyFromObject(&linodeVPC), &linodeVPC); err != nil {
+		logger.Error(err, "Failed to fetch LinodeVPC")
+		if reconciler.RecordDecayingCondition(clusterScope.LinodeCluster,
+			ConditionPreflightLinodeVPCReady, string(cerrs.CreateClusterError), err.Error(),
+			reconciler.DefaultTimeout(r.ReconcileTimeout, reconciler.DefaultClusterControllerReconcileTimeout)) {
+			return ctrl.Result{}, err
+		}
+		return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
+	} else if !linodeVPC.Status.Ready {
+		logger.Info("LinodeVPC is not yet available")
+		return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
+	}
+	r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "LinodeVPC is now available")
+	return ctrl.Result{}, nil
+}
+
 func setFailureReason(clusterScope *scope.ClusterScope, failureReason cerrs.ClusterStatusError, err error, lcr *LinodeClusterReconciler) {
 	clusterScope.LinodeCluster.Status.FailureReason = util.Pointer(failureReason)
 	clusterScope.LinodeCluster.Status.FailureMessage = util.Pointer(err.Error())

controller/linodecluster_controller_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/linode/linodego"
 	"go.uber.org/mock/gomock"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -63,6 +64,19 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc
 		ObjectMeta: metadata,
 		Spec: infrav1alpha2.LinodeClusterSpec{
 			Region: "us-ord",
+			VPCRef: &corev1.ObjectReference{Name: "vpctest"},
+		},
+	}
+	linodeVPC := infrav1alpha2.LinodeVPC{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "vpctest",
+			Namespace: defaultNamespace,
+		},
+		Spec: infrav1alpha2.LinodeVPCSpec{
+			Region: "us-ord",
+			Subnets: []infrav1alpha2.VPCSubnetCreateOptions{
+				{Label: "subnet1", IPv4: "10.0.0.0/8"},
+			},
 		},
 	}
 
@@ -91,9 +105,38 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc
 
 	ctlrSuite.Run(
 		OneOf(
+			Path(
+				Call("vpc doesn't exist", func(ctx context.Context, mck Mock) {
+				}),
+				OneOf(
+					Path(Result("", func(ctx context.Context, mck Mock) {
+						reconciler.Client = k8sClient
+						_, err := reconciler.reconcile(ctx, cScope, mck.Logger())
+						Expect(err).NotTo(HaveOccurred())
+						Expect(rec.ConditionTrue(&linodeCluster, ConditionPreflightLinodeVPCReady)).To(BeFalse())
+					})),
+				),
+			),
+			Path(
+				Call("vpc present but not ready", func(ctx context.Context, mck Mock) {
+					Expect(k8sClient.Create(ctx, &linodeVPC)).To(Succeed())
+					linodeVPC.Status.Ready = false
+					k8sClient.Status().Update(ctx, &linodeVPC)
+				}),
+				OneOf(
+					Path(Result("", func(ctx context.Context, mck Mock) {
+						reconciler.Client = k8sClient
+						_, err := reconciler.reconcile(ctx, cScope, mck.Logger())
+						Expect(err).NotTo(HaveOccurred())
+						Expect(rec.ConditionTrue(&linodeCluster, ConditionPreflightLinodeVPCReady)).To(BeFalse())
+					})),
+				),
+			),
 			Path(
 				Call("cluster is not created because there was an error creating nb", func(ctx context.Context, mck Mock) {
 					cScope.LinodeClient = mck.LinodeClient
+					linodeVPC.Status.Ready = true
+					k8sClient.Status().Update(ctx, &linodeVPC)
 					mck.LinodeClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()).
 						Return(nil, errors.New("failed to ensure nodebalancer"))
 				}),
@@ -207,8 +250,9 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc
 					clusterKey := client.ObjectKeyFromObject(&linodeCluster)
 					Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed())
 					Expect(linodeCluster.Status.Ready).To(BeTrue())
-					Expect(linodeCluster.Status.Conditions).To(HaveLen(1))
+					Expect(linodeCluster.Status.Conditions).To(HaveLen(2))
 					Expect(linodeCluster.Status.Conditions[0].Type).To(Equal(clusterv1.ReadyCondition))
+					Expect(linodeCluster.Status.Conditions[1].Type).To(Equal(ConditionPreflightLinodeVPCReady))
 
 					By("checking NB id")
 					Expect(linodeCluster.Spec.Network.NodeBalancerID).To(Equal(&nodebalancerID))

controller/linodemachine_controller.go

@@ -29,6 +29,7 @@ import (
 	"github.com/linode/linodego"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/client-go/tools/record"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -59,6 +60,8 @@ const (
 	defaultDiskFilesystem = string(linodego.FilesystemExt4)
 
 	// conditions for preflight instance creation
+	ConditionPreflightBootstrapDataSecretReady  clusterv1.ConditionType = "PreflightBootstrapDataSecretReady"
+	ConditionPreflightLinodeFirewallReady       clusterv1.ConditionType = "PreflightLinodeFirewallReady"
 	ConditionPreflightMetadataSupportConfigured clusterv1.ConditionType = "PreflightMetadataSupportConfigured"
 	ConditionPreflightCreated                   clusterv1.ConditionType = "PreflightCreated"
 	ConditionPreflightRootDiskResizing          clusterv1.ConditionType = "PreflightRootDiskResizing"
@@ -212,10 +215,12 @@ func (r *LinodeMachineReconciler) reconcile(ctx context.Context, logger logr.Log
 	}
 
 	// Make sure bootstrap data is available and populated.
-	if machineScope.Machine.Spec.Bootstrap.DataSecretName == nil {
+	if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightBootstrapDataSecretReady) && machineScope.Machine.Spec.Bootstrap.DataSecretName == nil {
 		logger.Info("Bootstrap data secret is not yet available")
-		conditions.MarkFalse(machineScope.LinodeMachine, ConditionPreflightMetadataSupportConfigured, WaitingForBootstrapDataReason, clusterv1.ConditionSeverityInfo, "")
+		conditions.MarkFalse(machineScope.LinodeMachine, ConditionPreflightBootstrapDataSecretReady, WaitingForBootstrapDataReason, clusterv1.ConditionSeverityInfo, "")
 		return ctrl.Result{}, nil
+	} else {
+		conditions.MarkTrue(machineScope.LinodeMachine, ConditionPreflightBootstrapDataSecretReady)
 	}
 
 	// Update
@@ -229,7 +234,7 @@ func (r *LinodeMachineReconciler) reconcile(ctx context.Context, logger logr.Log
 	return r.reconcileCreate(ctx, logger, machineScope)
 }
 
-//nolint:cyclop // can't make it simpler with existing API
+//nolint:cyclop,gocognit // can't make it simpler with existing API
 func (r *LinodeMachineReconciler) reconcileCreate(
 	ctx context.Context,
 	logger logr.Logger,
@@ -242,6 +247,16 @@ func (r *LinodeMachineReconciler) reconcileCreate(
 		return ctrl.Result{}, err
 	}
 
+	if machineScope.LinodeMachine.Spec.FirewallRef != nil {
+		if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightLinodeFirewallReady) && machineScope.LinodeMachine.Spec.ProviderID == nil {
+			res, err := r.reconcilePreflightLinodeFirewallCheck(ctx, logger, machineScope)
+			if err != nil || !res.IsZero() {
+				conditions.MarkFalse(machineScope.LinodeMachine, ConditionPreflightLinodeFirewallReady, string("linode firewall not yet available"), clusterv1.ConditionSeverityError, "")
+				return res, err
+			}
+		}
+	}
+
 	if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightMetadataSupportConfigured) && machineScope.LinodeMachine.Spec.ProviderID == nil {
 		res, err := r.reconcilePreflightMetadataSupportConfigure(ctx, logger, machineScope)
 		if err != nil || !res.IsZero() {
@@ -287,6 +302,34 @@ func (r *LinodeMachineReconciler) reconcileCreate(
 	return ctrl.Result{}, nil
 }
 
+func (r *LinodeMachineReconciler) reconcilePreflightLinodeFirewallCheck(ctx context.Context, logger logr.Logger, machineScope *scope.MachineScope) (ctrl.Result, error) {
+	name := machineScope.LinodeMachine.Spec.FirewallRef.Name
+	namespace := machineScope.LinodeMachine.Spec.FirewallRef.Namespace
+	if namespace == "" {
+		namespace = machineScope.LinodeMachine.Namespace
+	}
+	linodeFirewall := infrav1alpha2.LinodeFirewall{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+			Name:      name,
+		},
+	}
+	if err := machineScope.Client.Get(ctx, client.ObjectKeyFromObject(&linodeFirewall), &linodeFirewall); err != nil {
+		logger.Error(err, "Failed to find linode Firewall")
+		if reconciler.RecordDecayingCondition(machineScope.LinodeMachine,
+			ConditionPreflightLinodeFirewallReady, string(cerrs.CreateMachineError), err.Error(),
+			reconciler.DefaultTimeout(r.ReconcileTimeout, reconciler.DefaultMachineControllerWaitForPreflightTimeout)) {
+			return ctrl.Result{}, err
+		}
+		return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerRetryDelay}, nil
+	} else if !linodeFirewall.Status.Ready {
+		logger.Info("Linode firewall not yet ready")
+		return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerRetryDelay}, nil
+	}
+	conditions.MarkTrue(machineScope.LinodeMachine, ConditionPreflightLinodeFirewallReady)
+	return ctrl.Result{}, nil
+}
+
 func (r *LinodeMachineReconciler) reconcilePreflightMetadataSupportConfigure(ctx context.Context, logger logr.Logger, machineScope *scope.MachineScope) (ctrl.Result, error) {
 	region, err := machineScope.LinodeClient.GetRegion(ctx, machineScope.LinodeMachine.Spec.Region)
 	if err != nil {