diff --git a/exp/controllers/awsmachinepool_controller.go b/exp/controllers/awsmachinepool_controller.go index e35f746a39..827c515fbe 100644 --- a/exp/controllers/awsmachinepool_controller.go +++ b/exp/controllers/awsmachinepool_controller.go @@ -227,13 +227,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") @@ -260,13 +277,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 { diff --git a/exp/controllers/awsmachinepool_controller_test.go b/exp/controllers/awsmachinepool_controller_test.go index 4a459bf707..3489fa9237 100644 --- a/exp/controllers/awsmachinepool_controller_test.go +++ b/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) diff --git a/pkg/cloud/services/autoscaling/autoscalinggroup.go b/pkg/cloud/services/autoscaling/autoscalinggroup.go index cea424063f..fb471c7d0f 100644 --- a/pkg/cloud/services/autoscaling/autoscalinggroup.go +++ b/pkg/cloud/services/autoscaling/autoscalinggroup.go @@ -323,9 +323,6 @@ func (s *Service) CanStartASGInstanceRefresh(scope *scope.MachinePoolScope) (boo describeInput := &autoscaling.DescribeInstanceRefreshesInput{AutoScalingGroupName: aws.String(scope.Name())} refreshes, err := s.ASGClient.DescribeInstanceRefreshesWithContext(context.TODO(), describeInput) if err != nil { - if awserrors.IsNotFound(err) { - return false, nil - } return false, err } hasUnfinishedRefresh := false diff --git a/pkg/cloud/services/autoscaling/autoscalinggroup_test.go b/pkg/cloud/services/autoscaling/autoscalinggroup_test.go index ed8b9f4ec6..8b9ca450a9 100644 --- a/pkg/cloud/services/autoscaling/autoscalinggroup_test.go +++ b/pkg/cloud/services/autoscaling/autoscalinggroup_test.go @@ -1094,17 +1094,6 @@ func TestServiceCanStartASGInstanceRefresh(t *testing.T) { Return(nil, awserrors.NewConflict("some error")) }, }, - { - name: "should NOT return error if describe instance failed due to 'not found'", - wantErr: false, - canStart: false, - expect: func(m *mock_autoscalingiface.MockAutoScalingAPIMockRecorder) { - m.DescribeInstanceRefreshesWithContext(context.TODO(), gomock.Eq(&autoscaling.DescribeInstanceRefreshesInput{ - AutoScalingGroupName: aws.String("machinePoolName"), - })). - Return(nil, awserrors.NewNotFound("not found")) - }, - }, { name: "should return true if no instance available for refresh", wantErr: false,