Merge pull request #4698 from AndiDog/asg-recon-patch-cherrypick

🐛 There is no need to check instance refresh if ASG is not found (cherry-pick to release-2.3)

Author: k8s-ci-robot

exp/controllers/awsmachinepool_controller.go

@@ -226,13 +226,30 @@ func (r *AWSMachinePoolReconciler) reconcileNormal(ctx context.Context, machineP
 	ec2Svc := r.getEC2Service(ec2Scope)
 	asgsvc := r.getASGService(clusterScope)
 
+	// Find existing ASG
+	asg, err := r.findASG(machinePoolScope, asgsvc)
+	if err != nil {
+		conditions.MarkUnknown(machinePoolScope.AWSMachinePool, expinfrav1.ASGReadyCondition, expinfrav1.ASGNotFoundReason, err.Error())
+		return err
+	}
+
 	canUpdateLaunchTemplate := func() (bool, error) {
 		// If there is a change: before changing the template, check if there exist an ongoing instance refresh,
 		// because only 1 instance refresh can be "InProgress". If template is updated when refresh cannot be started,
 		// that change will not trigger a refresh. Do not start an instance refresh if only userdata changed.
+		if asg == nil {
+			// If the ASG hasn't been created yet, there is no need to check if we can start the instance refresh.
+			// But we want to update the LaunchTemplate because an error in the LaunchTemplate may be blocking the ASG creation.
+			return true, nil
+		}
 		return asgsvc.CanStartASGInstanceRefresh(machinePoolScope)
 	}
 	runPostLaunchTemplateUpdateOperation := func() error {
+		// skip instance refresh if ASG is not created yet
+		if asg == nil {
+			machinePoolScope.Debug("ASG does not exist yet, skipping instance refresh")
+			return nil
+		}
 		// skip instance refresh if explicitly disabled
 		if machinePoolScope.AWSMachinePool.Spec.RefreshPreferences != nil && machinePoolScope.AWSMachinePool.Spec.RefreshPreferences.Disable {
 			machinePoolScope.Debug("instance refresh disabled, skipping instance refresh")
@@ -259,13 +276,6 @@ func (r *AWSMachinePoolReconciler) reconcileNormal(ctx context.Context, machineP
 	// set the LaunchTemplateReady condition
 	conditions.MarkTrue(machinePoolScope.AWSMachinePool, expinfrav1.LaunchTemplateReadyCondition)
 
-	// Find existing ASG
-	asg, err := r.findASG(machinePoolScope, asgsvc)
-	if err != nil {
-		conditions.MarkUnknown(machinePoolScope.AWSMachinePool, expinfrav1.ASGReadyCondition, expinfrav1.ASGNotFoundReason, err.Error())
-		return err
-	}
-
 	if asg == nil {
 		// Create new ASG
 		if err := r.createPool(machinePoolScope, clusterScope); err != nil {

exp/controllers/awsmachinepool_controller_test.go

@@ -208,8 +208,6 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
 				defer teardown(t, g)
 				getASG(t, g)
 
-				ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any())
-
 				_ = reconciler.reconcileNormal(context.Background(), ms, cs, cs)
 
 				g.Expect(ms.AWSMachinePool.Finalizers).To(ContainElement(expinfrav1.MachinePoolFinalizer))
@@ -253,11 +251,18 @@ func TestAWSMachinePoolReconciler(t *testing.T) {
 				t.Helper()
 				ms.AWSMachinePool.Spec.ProviderID = id
 			}
+			getASG := func(t *testing.T, g *WithT) {
+				t.Helper()
+
+				ec2Svc.EXPECT().GetLaunchTemplate(gomock.Any()).Return(nil, "", nil).AnyTimes()
+				asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(nil, nil).AnyTimes()
+			}
 			t.Run("should look up by provider ID when one exists", func(t *testing.T) {
 				g := NewWithT(t)
 				setup(t, g)
 				defer teardown(t, g)
 				setProviderID(t, g)
+				getASG(t, g)
 
 				expectedErr := errors.New("no connection available ")
 				ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(expectedErr)

pkg/cloud/services/autoscaling/autoscalinggroup_test.go

@@ -1091,7 +1091,7 @@ func TestServiceCanStartASGInstanceRefresh(t *testing.T) {
 				m.DescribeInstanceRefreshesWithContext(context.TODO(), gomock.Eq(&autoscaling.DescribeInstanceRefreshesInput{
 					AutoScalingGroupName: aws.String("machinePoolName"),
 				})).
-					Return(nil, awserrors.NewNotFound("not found"))
+					Return(nil, awserrors.NewConflict("some error"))
 			},
 		},
 		{