reduce cognitive complexity, update tests and use ccm v0.4.3 for upda…

…ted helm chart
linode · Apr 24, 2024 · aab43d7 · aab43d7
1 parent bb49160
commit aab43d7
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 13 deletions.
diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go
@@ -61,6 +61,7 @@ const (
 	ConditionPreflightAdditionalDisksCreated clusterv1.ConditionType = "PreflightAdditionalDisksCreated"
 	ConditionPreflightConfigured             clusterv1.ConditionType = "PreflightConfigured"
 	ConditionPreflightBootTriggered          clusterv1.ConditionType = "PreflightBootTriggered"
+	ConditionPreflightNBConfigured           clusterv1.ConditionType = "PreflightNBConfigured"
 	ConditionPreflightReady                  clusterv1.ConditionType = "PreflightReady"
 )
 
@@ -248,7 +249,6 @@ func (r *LinodeMachineReconciler) reconcile(
 	return
 }
 
-//nolint:cyclop // keep top-level preflight condition checks in the same function for readability
 func (r *LinodeMachineReconciler) reconcileCreate(
 	ctx context.Context,
 	logger logr.Logger,
@@ -312,8 +312,17 @@ func (r *LinodeMachineReconciler) reconcileCreate(
 		return ctrl.Result{}, err
 	}
 
+	return r.reconcileInstanceCreate(ctx, logger, machineScope, linodeInstance)
+}
+
+func (r *LinodeMachineReconciler) reconcileInstanceCreate(
+	ctx context.Context,
+	logger logr.Logger,
+	machineScope *scope.MachineScope,
+	linodeInstance *linodego.Instance,
+) (ctrl.Result, error) {
 	if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightConfigured) {
-		if err = r.configureDisks(ctx, logger, machineScope, linodeInstance.ID); err != nil {
+		if err := r.configureDisks(ctx, logger, machineScope, linodeInstance.ID); err != nil {
 			if reconciler.RecordDecayingCondition(machineScope.LinodeMachine,
 				ConditionPreflightConfigured, string(cerrs.CreateMachineError), err.Error(),
 				reconciler.DefaultMachineControllerPreflightTimeout(r.ReconcileTimeout)) {
@@ -327,7 +336,7 @@ func (r *LinodeMachineReconciler) reconcileCreate(
 	}
 
 	if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightBootTriggered) {
-		if err = machineScope.LinodeClient.BootInstance(ctx, linodeInstance.ID, 0); err != nil {
+		if err := machineScope.LinodeClient.BootInstance(ctx, linodeInstance.ID, 0); err != nil {
 			logger.Error(err, "Failed to boot instance")
 
 			if reconciler.RecordDecayingCondition(machineScope.LinodeMachine,
@@ -342,19 +351,23 @@ func (r *LinodeMachineReconciler) reconcileCreate(
 		conditions.MarkTrue(machineScope.LinodeMachine, ConditionPreflightBootTriggered)
 	}
 
-	if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightReady) {
-		if err = services.AddNodeToNB(ctx, logger, machineScope); err != nil {
+	if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightNBConfigured) {
+		if err := services.AddNodeToNB(ctx, logger, machineScope); err != nil {
 			logger.Error(err, "Failed to add instance to Node Balancer backend")
 
 			if reconciler.RecordDecayingCondition(machineScope.LinodeMachine,
-				ConditionPreflightReady, string(cerrs.CreateMachineError), err.Error(),
+				ConditionPreflightNBConfigured, string(cerrs.CreateMachineError), err.Error(),
 				reconciler.DefaultMachineControllerPreflightTimeout(r.ReconcileTimeout)) {
 				return ctrl.Result{}, err
 			}
 
 			return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerWaitForRunningDelay}, nil
 		}
 
+		conditions.MarkTrue(machineScope.LinodeMachine, ConditionPreflightNBConfigured)
+	}
+
+	if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightReady) {
 		addrs, err := r.buildInstanceAddrs(ctx, machineScope, linodeInstance.ID)
 		if err != nil {
 			logger.Error(err, "Failed to get instance ip addresses")

diff --git a/controller/linodemachine_controller_test.go b/controller/linodemachine_controller_test.go
@@ -493,6 +493,7 @@ var _ = Describe("create", Label("machine", "create"), func() {
 				Return(&linodego.InstanceIPAddressResponse{
 					IPv4: &linodego.InstanceIPv4Response{
 						Private: []*linodego.InstanceIP{{Address: "192.168.0.2"}},
+						Public:  []*linodego.InstanceIP{{Address: "172.0.0.2"}},
 					},
 				}, nil)
 			createNB := mockLinodeClient.EXPECT().
@@ -509,6 +510,7 @@ var _ = Describe("create", Label("machine", "create"), func() {
 				Return(&linodego.InstanceIPAddressResponse{
 					IPv4: &linodego.InstanceIPv4Response{
 						Private: []*linodego.InstanceIP{{Address: "192.168.0.2"}},
+						Public:  []*linodego.InstanceIP{{Address: "172.0.0.2"}},
 					},
 				}, nil)
 			mockLinodeClient.EXPECT().
@@ -518,6 +520,10 @@ var _ = Describe("create", Label("machine", "create"), func() {
 					Devices: &linodego.InstanceConfigDeviceMap{
 						SDA: &linodego.InstanceConfigDevice{DiskID: 100},
 					},
+					Interfaces: []linodego.InstanceConfigInterface{{
+						VPCID: ptr.To(1),
+						IPv4:  &linodego.VPCIPv4{VPC: "10.0.0.2"},
+					}},
 				}}, nil)
 
 			_, err = reconciler.reconcileCreate(ctx, logger, &mScope)
@@ -531,10 +537,20 @@ var _ = Describe("create", Label("machine", "create"), func() {
 			Expect(*linodeMachine.Status.InstanceState).To(Equal(linodego.InstanceOffline))
 			Expect(*linodeMachine.Spec.InstanceID).To(Equal(123))
 			Expect(*linodeMachine.Spec.ProviderID).To(Equal("linode://123"))
-			Expect(linodeMachine.Status.Addresses).To(Equal([]clusterv1.MachineAddress{{
-				Type:    clusterv1.MachineInternalIP,
-				Address: "192.168.0.2",
-			}}))
+			Expect(linodeMachine.Status.Addresses).To(Equal([]clusterv1.MachineAddress{
+				{
+					Type:    clusterv1.MachineExternalIP,
+					Address: "172.0.0.2",
+				},
+				{
+					Type:    clusterv1.MachineInternalIP,
+					Address: "10.0.0.2",
+				},
+				{
+					Type:    clusterv1.MachineInternalIP,
+					Address: "192.168.0.2",
+				},
+			}))
 
 			Expect(testLogs.String()).To(ContainSubstring("creating machine"))
 			Expect(testLogs.String()).To(ContainSubstring("Linode instance already exists"))

diff --git a/templates/addons/provider-linode/linode-ccm.yaml b/templates/addons/provider-linode/linode-ccm.yaml
@@ -9,7 +9,7 @@ spec:
   repoURL: https://linode.github.io/linode-cloud-controller-manager/
   chartName: ccm-linode
   namespace: kube-system
-  version: ${LINODE_CCM_VERSION:=v0.4.1}
+  version: ${LINODE_CCM_VERSION:=v0.4.3}
   options:
     waitForJobs: true
     wait: true

diff --git a/templates/flavors/k3s/secret.yaml b/templates/flavors/k3s/secret.yaml
@@ -39,7 +39,7 @@ stringData:
      name: ccm-linode
     spec:
      targetNamespace: kube-system
-     version: ${LINODE_CCM_VERSION:=v0.4.1}
+     version: ${LINODE_CCM_VERSION:=v0.4.3}
      chart: ccm-linode
      repo: https://linode.github.io/linode-cloud-controller-manager/
      bootstrap: true

diff --git a/templates/flavors/rke2/secret.yaml b/templates/flavors/rke2/secret.yaml
@@ -21,7 +21,7 @@ stringData:
      name: ccm-linode
     spec:
      targetNamespace: kube-system
-     version: ${LINODE_CCM_VERSION:=v0.4.1}
+     version: ${LINODE_CCM_VERSION:=v0.4.3}
      chart: ccm-linode
      repo: https://linode.github.io/linode-cloud-controller-manager/
      bootstrap: true