Skip to content

Commit 1cac213

Browse files
jigisha620edibble21
authored andcommitted
chore: Update instance provider delete to check if the instance is terminated
1 parent 1752709 commit 1cac213

File tree

3 files changed

+17
-15
lines changed

3 files changed

+17
-15
lines changed

go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ require (
4343
k8s.io/klog/v2 v2.130.1
4444
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738
4545
sigs.k8s.io/controller-runtime v0.20.1
46-
sigs.k8s.io/karpenter v1.2.1-0.20250128194523-2a09110a1cb6
46+
sigs.k8s.io/karpenter v1.2.1-0.20250207011955-403034a0cbd9
4747
sigs.k8s.io/yaml v1.4.0
4848
)
4949

go.sum

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

pkg/providers/instance/instance.go

+14-12
Original file line numberDiff line numberDiff line change
@@ -174,19 +174,21 @@ func (p *DefaultProvider) List(ctx context.Context) ([]*Instance, error) {
174174
}
175175

176176
func (p *DefaultProvider) Delete(ctx context.Context, id string) error {
177-
if _, err := p.ec2Batcher.TerminateInstances(ctx, &ec2.TerminateInstancesInput{
178-
InstanceIds: []string{id},
179-
}); err != nil {
180-
if awserrors.IsNotFound(err) {
181-
return cloudprovider.NewNodeClaimNotFoundError(fmt.Errorf("instance already terminated"))
182-
}
183-
if _, e := p.Get(ctx, id); e != nil {
184-
if cloudprovider.IsNodeClaimNotFoundError(e) {
185-
return e
186-
}
187-
err = multierr.Append(err, e)
177+
out, err := p.Get(ctx, id)
178+
if err != nil {
179+
return err
180+
}
181+
// Check if the instance is already shutting-down to reduce the number of terminate-instance calls we make thereby
182+
// reducing our overall QPS. Due to EC2's eventual consistency model, the result of the terminate-instance or
183+
// describe-instance call may return a not found error even when the instance is not terminated -
184+
// https://docs.aws.amazon.com/ec2/latest/devguide/eventual-consistency.html. In this case, the instance will get
185+
// picked up by the garbage collection controller and will be cleaned up eventually.
186+
if out.State != ec2types.InstanceStateNameShuttingDown {
187+
if _, err := p.ec2Batcher.TerminateInstances(ctx, &ec2.TerminateInstancesInput{
188+
InstanceIds: []string{id},
189+
}); err != nil {
190+
return err
188191
}
189-
return fmt.Errorf("terminating instance, %w", err)
190192
}
191193
return nil
192194
}

0 commit comments

Comments
 (0)