refactoring to reduce cyclomatic complexity in machine controller

.golangci.yml

@@ -51,7 +51,7 @@ linters-settings:
     local-prefixes: github.com/crossplane/provider-template
 
   cyclop:
-    max-complexity: 17
+    max-complexity: 15
 
   maligned:
     # print struct with more effective memory layout or not, false by default
@@ -168,7 +168,7 @@ linters:
     - nestif
     - nilerr
     - nilnil
-    # - nlreturn
+    - nlreturn
     - noctx
     - nolintlint
     - paralleltest

cloud/services/loadbalancers.go

@@ -111,7 +111,6 @@ func AddNodeToNB(
 	ctx context.Context,
 	logger logr.Logger,
 	machineScope *scope.MachineScope,
-	clusterScope *scope.ClusterScope,
 ) error {
 	// Update the NB backend with the new instance if it's a control plane node
 	if !kutil.IsControlPlaneMachine(machineScope.Machine) {
@@ -132,8 +131,8 @@ func AddNodeToNB(
 	}
 
 	lbPort := defaultLBPort
-	if clusterScope.LinodeCluster.Spec.Network.LoadBalancerPort != 0 {
-		lbPort = clusterScope.LinodeCluster.Spec.Network.LoadBalancerPort
+	if machineScope.LinodeCluster.Spec.Network.LoadBalancerPort != 0 {
+		lbPort = machineScope.LinodeCluster.Spec.Network.LoadBalancerPort
 	}
 	if machineScope.LinodeCluster.Spec.Network.NodeBalancerConfigID == nil {
 		err := errors.New("nil NodeBalancer Config ID")

config/crd/bases/infrastructure.cluster.x-k8s.io_linodemachines.yaml

@@ -93,9 +93,9 @@ spec:
                   CredentialsRef is a reference to a Secret that contains the credentials
                   to use for provisioning this machine. If not supplied then these
                   credentials will be used in-order:
-                    1. Machine
-                    2. Cluster
-                    2. Controller
+                    1. LinodeMachine
+                    2. Owner LinodeCluster
+                    3. Controller
                 properties:
                   name:
                     description: name is unique within a namespace to reference a

config/crd/bases/infrastructure.cluster.x-k8s.io_linodemachinetemplates.yaml

@@ -80,9 +80,9 @@ spec:
                           CredentialsRef is a reference to a Secret that contains the credentials
                           to use for provisioning this machine. If not supplied then these
                           credentials will be used in-order:
-                            1. Machine
-                            2. Cluster
-                            2. Controller
+                            1. LinodeMachine
+                            2. Owner LinodeCluster
+                            3. Controller
                         properties:
                           name:
                             description: name is unique within a namespace to reference

controller/linodemachine_controller.go

@@ -103,76 +103,31 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	linodeMachine := &infrav1alpha1.LinodeMachine{}
 	if err := r.Client.Get(ctx, req.NamespacedName, linodeMachine); err != nil {
 		if err = client.IgnoreNotFound(err); err != nil {
-			log.Error(err, "Failed to fetch Linode machine")
+			log.Error(err, "Failed to fetch LinodeMachine")
 		}
 
 		return ctrl.Result{}, err
 	}
 
-	machine, err := kutil.GetOwnerMachine(ctx, r.Client, linodeMachine.ObjectMeta)
-	switch {
-	case err != nil:
-		if err = client.IgnoreNotFound(err); err != nil {
-			log.Error(err, "Failed to fetch owner machine")
-		}
-
+	machine, err := r.getOwnerMachine(ctx, *linodeMachine, log)
+	if err != nil || machine == nil {
 		return ctrl.Result{}, err
-	case machine == nil:
-		log.Info("Machine Controller has not yet set OwnerRef, skipping reconciliation")
-
-		return ctrl.Result{}, nil
-	case skippedMachinePhases[machine.Status.Phase]:
-
-		return ctrl.Result{}, nil
-	default:
-		match := false
-		for i := range linodeMachine.OwnerReferences {
-			if match = linodeMachine.OwnerReferences[i].UID == machine.UID; match {
-				break
-			}
-		}
-
-		if !match {
-			log.Info("Failed to find the referenced owner machine, skipping reconciliation", "references", linodeMachine.OwnerReferences, "machine", machine.ObjectMeta)
-
-			return ctrl.Result{}, nil
-		}
 	}
+	log = log.WithValues("LinodeMachine: ", machine.Name)
 
-	log = log.WithValues("Linode machine: ", machine.Name)
-
-	cluster, err := kutil.GetClusterFromMetadata(ctx, r.Client, machine.ObjectMeta)
-	switch {
-	case err != nil:
-		if err = client.IgnoreNotFound(err); err != nil {
-			log.Error(err, "Failed to fetch cluster by label")
-		}
-
-		return ctrl.Result{}, err
-	case cluster == nil:
-		err = errors.New("missing cluster")
-
-		log.Error(err, "Missing cluster")
-
-		return ctrl.Result{}, err
-	case cluster.Spec.InfrastructureRef == nil:
-		err = errors.New("missing infrastructure reference")
-
-		log.Error(err, "Missing infrastructure reference")
-
+	cluster, err := r.getClusterFromMetadata(ctx, *machine, log)
+	if err != nil || cluster == nil {
 		return ctrl.Result{}, err
 	}
 
-	linodeCluster := &infrav1alpha1.LinodeCluster{}
-
 	machineScope, err := scope.NewMachineScope(
 		ctx,
 		r.LinodeApiKey,
 		scope.MachineScopeParams{
 			Client:        r.Client,
 			Cluster:       cluster,
 			Machine:       machine,
-			LinodeCluster: linodeCluster,
+			LinodeCluster: &infrav1alpha1.LinodeCluster{},
 			LinodeMachine: linodeMachine,
 		},
 	)
@@ -182,29 +137,13 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, fmt.Errorf("failed to create machine scope: %w", err)
 	}
 
-	clusterScope, err := scope.NewClusterScope(
-		ctx,
-		r.LinodeApiKey,
-		scope.ClusterScopeParams{
-			Client:        r.Client,
-			Cluster:       cluster,
-			LinodeCluster: linodeCluster,
-		},
-	)
-	if err != nil {
-		log.Error(err, "Failed to create cluster scope")
-
-		return ctrl.Result{}, fmt.Errorf("failed to create cluster scope: %w", err)
-	}
-
-	return r.reconcile(ctx, log, machineScope, clusterScope)
+	return r.reconcile(ctx, log, machineScope)
 }
 
 func (r *LinodeMachineReconciler) reconcile(
 	ctx context.Context,
 	logger logr.Logger,
 	machineScope *scope.MachineScope,
-	clusterScope *scope.ClusterScope,
 ) (res ctrl.Result, err error) {
 	res = ctrl.Result{}
 
@@ -290,7 +229,7 @@ func (r *LinodeMachineReconciler) reconcile(
 
 		return
 	}
-	linodeInstance, err = r.reconcileCreate(ctx, logger, machineScope, clusterScope)
+	linodeInstance, err = r.reconcileCreate(ctx, logger, machineScope)
 
 	return
 }
@@ -299,7 +238,6 @@ func (r *LinodeMachineReconciler) reconcileCreate(
 	ctx context.Context,
 	logger logr.Logger,
 	machineScope *scope.MachineScope,
-	clusterScope *scope.ClusterScope,
 ) (*linodego.Instance, error) {
 	logger.Info("creating machine")
 
@@ -332,17 +270,6 @@ func (r *LinodeMachineReconciler) reconcileCreate(
 			return nil, err
 		}
 
-		if machineScope.LinodeCluster.Spec.VPCRef != nil {
-			iface, err := r.getVPCInterfaceConfig(ctx, machineScope, createOpts.Interfaces, logger)
-			if err != nil {
-				logger.Error(err, "Failed to get VPC interface config")
-
-				return nil, err
-			}
-
-			createOpts.Interfaces = append(createOpts.Interfaces, *iface)
-		}
-
 		linodeInstance, err = machineScope.LinodeClient.CreateInstance(ctx, *createOpts)
 		if err != nil {
 			logger.Error(err, "Failed to create Linode machine instance")
@@ -370,31 +297,12 @@ func (r *LinodeMachineReconciler) reconcileCreate(
 		return nil, err
 	}
 
-	if linodeInstance == nil {
-		err = errors.New("missing instance")
-
-		logger.Error(err, "Panic! Failed to create isntance")
-
-		return nil, err
-	}
-
 	machineScope.LinodeMachine.Status.Ready = true
 	machineScope.LinodeMachine.Spec.InstanceID = &linodeInstance.ID
 	machineScope.LinodeMachine.Spec.ProviderID = util.Pointer(fmt.Sprintf("linode://%d", linodeInstance.ID))
+	machineScope.LinodeMachine.Status.Addresses = buildInstanceAddrs(linodeInstance)
 
-	machineScope.LinodeMachine.Status.Addresses = []clusterv1.MachineAddress{}
-	for _, addr := range linodeInstance.IPv4 {
-		addrType := clusterv1.MachineExternalIP
-		if addr.IsPrivate() {
-			addrType = clusterv1.MachineInternalIP
-		}
-		machineScope.LinodeMachine.Status.Addresses = append(machineScope.LinodeMachine.Status.Addresses, clusterv1.MachineAddress{
-			Type:    addrType,
-			Address: addr.String(),
-		})
-	}
-
-	if err = services.AddNodeToNB(ctx, logger, machineScope, clusterScope); err != nil {
+	if err = services.AddNodeToNB(ctx, logger, machineScope); err != nil {
 		logger.Error(err, "Failed to add instance to Node Balancer backend")
 
 		return linodeInstance, err

controller/linodemachine_controller_helpers.go

@@ -46,7 +46,7 @@ import (
 // The decoded user_data must not exceed 16384 bytes per the Linode API
 const maxBootstrapDataBytes = 16384
 
-func (*LinodeMachineReconciler) newCreateConfig(ctx context.Context, machineScope *scope.MachineScope, tags []string, logger logr.Logger) (*linodego.InstanceCreateOptions, error) {
+func (r *LinodeMachineReconciler) newCreateConfig(ctx context.Context, machineScope *scope.MachineScope, tags []string, logger logr.Logger) (*linodego.InstanceCreateOptions, error) {
 	var err error
 
 	createConfig := linodeMachineSpecToInstanceCreateConfig(machineScope.LinodeMachine.Spec)
@@ -94,9 +94,93 @@ func (*LinodeMachineReconciler) newCreateConfig(ctx context.Context, machineScop
 		createConfig.RootPass = uuid.NewString()
 	}
 
+	if machineScope.LinodeCluster.Spec.VPCRef != nil {
+		iface, err := r.getVPCInterfaceConfig(ctx, machineScope, createConfig.Interfaces, logger)
+		if err != nil {
+			logger.Error(err, "Failed to get VPC interface config")
+
+			return nil, err
+		}
+		createConfig.Interfaces = append(createConfig.Interfaces, *iface)
+	}
+
 	return createConfig, nil
 }
 
+func buildInstanceAddrs(linodeInstance *linodego.Instance) []clusterv1.MachineAddress {
+	addrs := []clusterv1.MachineAddress{}
+	for _, addr := range linodeInstance.IPv4 {
+		addrType := clusterv1.MachineExternalIP
+		if addr.IsPrivate() {
+			addrType = clusterv1.MachineInternalIP
+		}
+		addrs = append(addrs, clusterv1.MachineAddress{
+			Type:    addrType,
+			Address: addr.String(),
+		})
+	}
+
+	return addrs
+}
+
+func (r *LinodeMachineReconciler) getOwnerMachine(ctx context.Context, linodeMachine infrav1alpha1.LinodeMachine, log logr.Logger) (*clusterv1.Machine, error) {
+	machine, err := kutil.GetOwnerMachine(ctx, r.Client, linodeMachine.ObjectMeta)
+	switch {
+	case err != nil:
+		if err = client.IgnoreNotFound(err); err != nil {
+			log.Error(err, "Failed to fetch owner machine")
+		}
+
+		return nil, err
+	case machine == nil:
+		log.Info("Machine Controller has not yet set OwnerRef, skipping reconciliation")
+
+		return nil, err
+	case skippedMachinePhases[machine.Status.Phase]:
+
+		return nil, err
+	default:
+		match := false
+		for i := range linodeMachine.OwnerReferences {
+			if match = linodeMachine.OwnerReferences[i].UID == machine.UID; match {
+				break
+			}
+		}
+		if !match {
+			log.Info("Failed to find the referenced owner machine, skipping reconciliation", "references", linodeMachine.OwnerReferences, "machine", machine.ObjectMeta)
+
+			return nil, err
+		}
+	}
+
+	return machine, nil
+}
+
+func (r *LinodeMachineReconciler) getClusterFromMetadata(ctx context.Context, machine clusterv1.Machine, log logr.Logger) (*clusterv1.Cluster, error) {
+	cluster, err := kutil.GetClusterFromMetadata(ctx, r.Client, machine.ObjectMeta)
+	switch {
+	case err != nil:
+		if err = client.IgnoreNotFound(err); err != nil {
+			log.Error(err, "Failed to fetch cluster by label")
+		}
+
+		return nil, err
+	case cluster == nil:
+		err = errors.New("missing cluster")
+		log.Error(err, "Missing cluster")
+
+		return nil, err
+	case cluster.Spec.InfrastructureRef == nil:
+		err = errors.New("missing infrastructure reference")
+
+		log.Error(err, "Missing infrastructure reference")
+
+		return nil, err
+	}
+
+	return cluster, nil
+}
+
 func (r *LinodeMachineReconciler) linodeClusterToLinodeMachines(logger logr.Logger) handler.MapFunc {
 	logger = logger.WithName("LinodeMachineReconciler").WithName("linodeClusterToLinodeMachines")