chore: Update instance provider delete to check if the instance is te…

…rminated
aws · Feb 7, 2025 · a5f6c8d · a5f6c8d
1 parent 4693be5
commit a5f6c8d
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 15 deletions.
diff --git a/go.mod b/go.mod
@@ -43,7 +43,7 @@ require (
 	k8s.io/klog/v2 v2.130.1
 	k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738
 	sigs.k8s.io/controller-runtime v0.20.1
-	sigs.k8s.io/karpenter v1.2.1-0.20250128194523-2a09110a1cb6
+	sigs.k8s.io/karpenter v1.2.1-0.20250207011955-403034a0cbd9
 	sigs.k8s.io/yaml v1.4.0
 )
 

diff --git a/go.sum b/go.sum
@@ -336,8 +336,8 @@ sigs.k8s.io/controller-runtime v0.20.1 h1:JbGMAG/X94NeM3xvjenVUaBjy6Ui4Ogd/J5Ztj
 sigs.k8s.io/controller-runtime v0.20.1/go.mod h1:BrP3w158MwvB3ZbNpaAcIKkHQ7YGpYnzpoSTZ8E14WU=
 sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 h1:/Rv+M11QRah1itp8VhT6HoVx1Ray9eB4DBr+K+/sCJ8=
 sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3/go.mod h1:18nIHnGi6636UCz6m8i4DhaJ65T6EruyzmoQqI2BVDo=
-sigs.k8s.io/karpenter v1.2.1-0.20250128194523-2a09110a1cb6 h1:AWuTX1D+2+q9sZT2IkMHauj3ZivwVzixZftlO7lJ7ZQ=
-sigs.k8s.io/karpenter v1.2.1-0.20250128194523-2a09110a1cb6/go.mod h1:0PV2k6Ua1Sc04M6NIOfVXLNGyFnvdwDxaIJriic2L5o=
+sigs.k8s.io/karpenter v1.2.1-0.20250207011955-403034a0cbd9 h1:/phqkLkjx+iIPoUpFzZQBzGAEYlDmFvgXrFjeH/Cw1M=
+sigs.k8s.io/karpenter v1.2.1-0.20250207011955-403034a0cbd9/go.mod h1:S+qNY3XwugJTu+UvgAdeNUxWuwQP/gS0uefdrV5wFLE=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.2 h1:MdmvkGuXi/8io6ixD5wud3vOLwc1rj0aNqRlpuvjmwA=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.2/go.mod h1:N8f93tFZh9U6vpxwRArLiikrE5/2tiu1w1AGfACIGE4=
 sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=

diff --git a/pkg/providers/instance/instance.go b/pkg/providers/instance/instance.go
@@ -174,19 +174,21 @@ func (p *DefaultProvider) List(ctx context.Context) ([]*Instance, error) {
 }
 
 func (p *DefaultProvider) Delete(ctx context.Context, id string) error {
-	if _, err := p.ec2Batcher.TerminateInstances(ctx, &ec2.TerminateInstancesInput{
-		InstanceIds: []string{id},
-	}); err != nil {
-		if awserrors.IsNotFound(err) {
-			return cloudprovider.NewNodeClaimNotFoundError(fmt.Errorf("instance already terminated"))
-		}
-		if _, e := p.Get(ctx, id); e != nil {
-			if cloudprovider.IsNodeClaimNotFoundError(e) {
-				return e
-			}
-			err = multierr.Append(err, e)
+	out, err := p.Get(ctx, id)
+	if err != nil {
+		return err
+	}
+	// Check if the instance is already shutting-down to reduce the number of terminate-instance calls we make thereby
+	// reducing our overall QPS. Due to EC2's eventual consistency model, the result of the terminate-instance or
+	// describe-instance call may return a not found error even when the instance is not terminated -
+	// https://docs.aws.amazon.com/ec2/latest/devguide/eventual-consistency.html. In this case, the instance will get
+	// picked up by the garbage collection controller and will be cleaned up eventually.
+	if out.State != ec2types.InstanceStateNameShuttingDown {
+		if _, err := p.ec2Batcher.TerminateInstances(ctx, &ec2.TerminateInstancesInput{
+			InstanceIds: []string{id},
+		}); err != nil {
+			return err
 		}
-		return fmt.Errorf("terminating instance, %w", err)
 	}
 	return nil
 }