Merge pull request #3081 from Ankitasw/awsmachine-controller-coverage

Increase unit test coverage for AWSMachine controller

Author: k8s-ci-robot

controllers/awscluster_controller.go

@@ -70,7 +70,7 @@ var (
 type AWSClusterReconciler struct {
 	client.Client
 	Recorder              record.EventRecorder
-	ec2ServiceFactory     func(scope.EC2Scope) services.EC2MachineInterface
+	ec2ServiceFactory     func(scope.EC2Scope) services.EC2Interface
 	networkServiceFactory func(scope.ClusterScope) services.NetworkInterface
 	elbServiceFactory     func(scope.ELBScope) services.ELBInterface
 	securityGroupFactory  func(scope.ClusterScope) services.SecurityGroupInterface
@@ -79,7 +79,7 @@ type AWSClusterReconciler struct {
 }
 
 // getEC2Service factory func is added for testing purpose so that we can inject mocked EC2Service to the AWSClusterReconciler.
-func (r *AWSClusterReconciler) getEC2Service(scope scope.EC2Scope) services.EC2MachineInterface {
+func (r *AWSClusterReconciler) getEC2Service(scope scope.EC2Scope) services.EC2Interface {
 	if r.ec2ServiceFactory != nil {
 		return r.ec2ServiceFactory(scope)
 	}

controllers/awscluster_controller_test.go

@@ -117,7 +117,7 @@ func TestAWSClusterReconciler_IntegrationTests(t *testing.T) {
 
 		ec2Svc := ec2Service.NewService(cs)
 		ec2Svc.EC2Client = ec2Mock
-		reconciler.ec2ServiceFactory = func(scope scope.EC2Scope) services.EC2MachineInterface {
+		reconciler.ec2ServiceFactory = func(scope scope.EC2Scope) services.EC2Interface {
 			return ec2Svc
 		}
 		testSecurityGroupRoles := []infrav1.SecurityGroupRole{
@@ -259,7 +259,7 @@ func TestAWSClusterReconciler_IntegrationTests(t *testing.T) {
 
 		ec2Svc := ec2Service.NewService(cs)
 		ec2Svc.EC2Client = ec2Mock
-		reconciler.ec2ServiceFactory = func(ec2Scope scope.EC2Scope) services.EC2MachineInterface {
+		reconciler.ec2ServiceFactory = func(ec2Scope scope.EC2Scope) services.EC2Interface {
 			return ec2Svc
 		}
 

controllers/awscluster_controller_unit_test.go

@@ -133,7 +133,7 @@ func TestAWSClusterReconcileOperations(t *testing.T) {
 	var (
 		reconciler AWSClusterReconciler
 		mockCtrl   *gomock.Controller
-		ec2Svc     *mock_services.MockEC2MachineInterface
+		ec2Svc     *mock_services.MockEC2Interface
 		elbSvc     *mock_services.MockELBInterface
 		networkSvc *mock_services.MockNetworkInterface
 		sgSvc      *mock_services.MockSecurityGroupInterface
@@ -156,7 +156,7 @@ func TestAWSClusterReconcileOperations(t *testing.T) {
 		csClient := fake.NewClientBuilder().WithObjects(awsCluster, secret).Build()
 
 		mockCtrl = gomock.NewController(t)
-		ec2Svc = mock_services.NewMockEC2MachineInterface(mockCtrl)
+		ec2Svc = mock_services.NewMockEC2Interface(mockCtrl)
 		elbSvc = mock_services.NewMockELBInterface(mockCtrl)
 		networkSvc = mock_services.NewMockNetworkInterface(mockCtrl)
 		sgSvc = mock_services.NewMockSecurityGroupInterface(mockCtrl)
@@ -165,7 +165,7 @@ func TestAWSClusterReconcileOperations(t *testing.T) {
 
 		reconciler = AWSClusterReconciler{
 			Client: csClient,
-			ec2ServiceFactory: func(scope.EC2Scope) services.EC2MachineInterface {
+			ec2ServiceFactory: func(scope.EC2Scope) services.EC2Interface {
 				return ec2Svc
 			},
 			elbServiceFactory: func(elbScope scope.ELBScope) services.ELBInterface {

controllers/awsmachine_controller.go

@@ -66,7 +66,8 @@ type AWSMachineReconciler struct {
 	client.Client
 	Log                          logr.Logger
 	Recorder                     record.EventRecorder
-	ec2ServiceFactory            func(scope.EC2Scope) services.EC2MachineInterface
+	ec2ServiceFactory            func(scope.EC2Scope) services.EC2Interface
+	elbServiceFactory            func(scope.ELBScope) services.ELBInterface
 	secretsManagerServiceFactory func(cloud.ClusterScoper) services.SecretInterface
 	SSMServiceFactory            func(cloud.ClusterScoper) services.SecretInterface
 	Endpoints                    []scope.ServiceEndpoint
@@ -78,7 +79,7 @@ const (
 	AWSManagedControlPlaneRefKind = "AWSManagedControlPlane"
 )
 
-func (r *AWSMachineReconciler) getEC2Service(scope scope.EC2Scope) services.EC2MachineInterface {
+func (r *AWSMachineReconciler) getEC2Service(scope scope.EC2Scope) services.EC2Interface {
 	if r.ec2ServiceFactory != nil {
 		return r.ec2ServiceFactory(scope)
 	}
@@ -111,6 +112,14 @@ func (r *AWSMachineReconciler) getSecretService(machineScope *scope.MachineScope
 	return nil, errors.New("invalid secret backend")
 }
 
+func (r *AWSMachineReconciler) getELBService(scope scope.ELBScope) services.ELBInterface {
+	if r.elbServiceFactory != nil {
+		return r.elbServiceFactory(scope)
+	}
+
+	return elb.NewService(scope)
+}
+
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=awsmachines,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=awsmachines/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines;machines/status,verbs=get;list;watch
@@ -382,7 +391,7 @@ func (r *AWSMachineReconciler) reconcileDelete(machineScope *scope.MachineScope,
 // findInstance queries the EC2 apis and retrieves the instance if it exists.
 // If providerID is empty, finds instance by tags and if it cannot be found, returns empty instance with nil error.
 // If providerID is set, either finds the instance by ID or returns error.
-func (r *AWSMachineReconciler) findInstance(scope *scope.MachineScope, ec2svc services.EC2MachineInterface) (*infrav1.Instance, error) {
+func (r *AWSMachineReconciler) findInstance(scope *scope.MachineScope, ec2svc services.EC2Interface) (*infrav1.Instance, error) {
 	var instance *infrav1.Instance
 
 	// Parse the ProviderID.
@@ -525,7 +534,7 @@ func (r *AWSMachineReconciler) reconcileNormal(_ context.Context, machineScope *
 	}
 
 	// reconcile the deletion of the bootstrap data secret now that we have updated instance state
-	if deleteSecretErr := r.deleteEncryptedBootstrapDataSecret(machineScope, clusterScope); err != nil {
+	if deleteSecretErr := r.deleteEncryptedBootstrapDataSecret(machineScope, clusterScope); deleteSecretErr != nil {
 		r.Log.Error(deleteSecretErr, "unable to delete secrets")
 		return ctrl.Result{}, deleteSecretErr
 	}
@@ -613,7 +622,7 @@ func (r *AWSMachineReconciler) deleteEncryptedBootstrapDataSecret(machineScope *
 	return nil
 }
 
-func (r *AWSMachineReconciler) createInstance(ec2svc services.EC2MachineInterface, machineScope *scope.MachineScope, clusterScope cloud.ClusterScoper) (*infrav1.Instance, error) {
+func (r *AWSMachineReconciler) createInstance(ec2svc services.EC2Interface, machineScope *scope.MachineScope, clusterScope cloud.ClusterScoper) (*infrav1.Instance, error) {
 	machineScope.Info("Creating EC2 instance")
 
 	userData, userDataErr := r.resolveUserData(machineScope, clusterScope)
@@ -678,7 +687,7 @@ func (r *AWSMachineReconciler) reconcileLBAttachment(machineScope *scope.Machine
 		return nil
 	}
 
-	elbsvc := elb.NewService(clusterScope)
+	elbsvc := r.getELBService(clusterScope)
 
 	// In order to prevent sending request to a "not-ready" control plane machines, it is required to remove the machine
 	// from the ELB as soon as the machine gets deleted or when the machine is in a not running state.
@@ -877,7 +886,7 @@ func (r *AWSMachineReconciler) indexAWSMachineByInstanceID(o client.Object) []st
 	return nil
 }
 
-func (r *AWSMachineReconciler) ensureStorageTags(ec2svc services.EC2MachineInterface, instance *infrav1.Instance, machine *infrav1.AWSMachine) {
+func (r *AWSMachineReconciler) ensureStorageTags(ec2svc services.EC2Interface, instance *infrav1.Instance, machine *infrav1.AWSMachine) {
 	annotations, err := r.machineAnnotationJSON(machine, VolumeTagsLastAppliedAnnotation)
 	if err != nil {
 		r.Log.Error(err, "Failed to fetch the annotations for volume tags")