Added some logs and fixed the unit test

Author: Skarlso

exp/controllers/awsmachinepool_controller.go

@@ -404,6 +404,7 @@ func (r *AWSMachinePoolReconciler) updatePool(machinePoolScope *scope.MachinePoo
 
 	suspendedProcessesSlice := machinePoolScope.AWSMachinePool.Spec.SuspendProcesses.ConvertSetValuesToStringSlice()
 	if !cmp.Equal(existingASG.CurrentlySuspendProcesses, suspendedProcessesSlice) {
+		clusterScope.Info("reconciling processes", "suspend-processes", suspendedProcessesSlice)
 		var (
 			toBeSuspended []string
 			toBeResumed   []string
@@ -431,7 +432,7 @@ func (r *AWSMachinePoolReconciler) updatePool(machinePoolScope *scope.MachinePoo
 			delete(currentlySuspended, k)
 		}
 
-		// Convert them back into lists so
+		// Convert them back into lists to pass them to resume/suspend.
 		for k := range desiredSuspended {
 			toBeSuspended = append(toBeSuspended, k)
 		}
@@ -441,11 +442,13 @@ func (r *AWSMachinePoolReconciler) updatePool(machinePoolScope *scope.MachinePoo
 		}
 
 		if len(toBeSuspended) > 0 {
+			clusterScope.Info("suspending processes", "processes", toBeSuspended)
 			if err := asgSvc.SuspendProcesses(existingASG.Name, toBeSuspended); err != nil {
 				return errors.Wrapf(err, "failed to suspend processes while trying update pool")
 			}
 		}
 		if len(toBeResumed) > 0 {
+			clusterScope.Info("resuming processes", "processes", toBeResumed)
 			if err := asgSvc.ResumeProcesses(existingASG.Name, toBeResumed); err != nil {
 				return errors.Wrapf(err, "failed to resume processes while trying update pool")
 			}

exp/controllers/awsmachinepool_controller_test.go

@@ -266,7 +266,7 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
 					},
 				}
 			}
-			t.Run("it should suspend these processes", func(t *testing.T) {
+			t.Run("it should not call suspend as we don't have an ASG yet", func(t *testing.T) {
 				g := NewWithT(t)
 				setup(t, g)
 				defer teardown(t, g)
@@ -277,7 +277,7 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
 				asgSvc.EXPECT().CreateASG(gomock.Any()).Return(&expinfrav1.AutoScalingGroup{
 					Name: "name",
 				}, nil)
-				asgSvc.EXPECT().SuspendProcesses("name", []string{"Launch", "Terminate"}).Return(nil).AnyTimes()
+				asgSvc.EXPECT().SuspendProcesses("name", []string{"Launch", "Terminate"}).Return(nil).AnyTimes().Times(0)
 
 				_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs)
 				g.Expect(err).To(Succeed())
@@ -290,56 +290,29 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
 					All: true,
 				}
 			}
-			t.Run("it should result in all processes being included except the value all", func(t *testing.T) {
+			t.Run("processes should be suspended during an update call", func(t *testing.T) {
 				g := NewWithT(t)
 				setup(t, g)
 				defer teardown(t, g)
 				setSuspendedProcesses(t, g)
-
+				ms.AWSMachinePool.Spec.SuspendProcesses.All = true
 				ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
-				asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(nil, nil)
-				asgSvc.EXPECT().CreateASG(gomock.Any()).Return(&expinfrav1.AutoScalingGroup{
+				ec2Svc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil)
+				asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(&expinfrav1.AutoScalingGroup{
 					Name: "name",
 				}, nil)
-				asgSvc.EXPECT().SuspendProcesses("name", []string{
+				asgSvc.EXPECT().UpdateASG(gomock.Any()).Return(nil).AnyTimes()
+				asgSvc.EXPECT().SuspendProcesses("name", gomock.InAnyOrder([]string{
+					"ScheduledActions",
 					"Launch",
 					"Terminate",
 					"AddToLoadBalancer",
 					"AlarmNotification",
 					"AZRebalance",
-					"HealthCheck",
 					"InstanceRefresh",
-					"ReplaceUnhealthy",
-					"ScheduledActions",
-				}).Return(nil).AnyTimes()
-
-				_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs)
-				g.Expect(err).To(Succeed())
-			})
-			t.Run("and one or more processes are disabled, it should not list those", func(t *testing.T) {
-				g := NewWithT(t)
-				setup(t, g)
-				defer teardown(t, g)
-				setSuspendedProcesses(t, g)
-				ms.AWSMachinePool.Spec.SuspendProcesses.Processes = &expinfrav1.Processes{
-					Launch:      pointer.Bool(false),
-					AZRebalance: pointer.Bool(true), // this should still be included but not twice...
-				}
-				ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
-				asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(nil, nil)
-				asgSvc.EXPECT().CreateASG(gomock.Any()).Return(&expinfrav1.AutoScalingGroup{
-					Name: "name",
-				}, nil)
-				asgSvc.EXPECT().SuspendProcesses("name", []string{
-					"Terminate",
-					"AddToLoadBalancer",
-					"AlarmNotification",
-					"AZRebalance",
 					"HealthCheck",
-					"InstanceRefresh",
 					"ReplaceUnhealthy",
-					"ScheduledActions",
-				}).Return(nil).AnyTimes()
+				})).Return(nil).AnyTimes().Times(1)
 
 				_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs)
 				g.Expect(err).To(Succeed())
@@ -369,8 +342,8 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
 					CurrentlySuspendProcesses: []string{"Launch", "process3"},
 				}, nil)
 				asgSvc.EXPECT().UpdateASG(gomock.Any()).Return(nil).AnyTimes()
-				asgSvc.EXPECT().SuspendProcesses("name", []string{"Terminate"}).Return(nil).AnyTimes()
-				asgSvc.EXPECT().ResumeProcesses("name", []string{"process3"}).Return(nil).AnyTimes()
+				asgSvc.EXPECT().SuspendProcesses("name", []string{"Terminate"}).Return(nil).AnyTimes().Times(1)
+				asgSvc.EXPECT().ResumeProcesses("name", []string{"process3"}).Return(nil).AnyTimes().Times(1)
 
 				_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs)
 				g.Expect(err).To(Succeed())