chore: Wait for instance termination before deleting nodeclaim #5979

jigisha620 · 2024-04-03T22:02:09Z

Fixes #N/A

Description
The termination controller calls cloudProvider.Delete(nodeClaim) which tries to terminate the underlying instance. It will remove the finalizer once the cloudProvider.Delete() call succeeds. However, the current implementation does not wait until the instance state is actually terminated and proceeds to remove the finalizer once the ec2.TerminateInstance() call has succeeded.

The change in this PR will check if the instance state is not terminated (check the instance state code)and will return a retryable error that will effectively re-enqueue the node into the reconciliation loop of the termination controller.

How was this change tested?

Does this change impact docs?

Yes, PR includes docs updates
Yes, issue opened: #
No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

netlify · 2024-04-03T22:02:26Z

✅ Deploy Preview for karpenter-docs-prod canceled.

Name	Link
🔨 Latest commit	`e86e82f`
🔍 Latest deploy log	https://app.netlify.com/sites/karpenter-docs-prod/deploys/6669e9725494310008766ee0

go.mod

coveralls · 2024-04-05T17:38:18Z

Pull Request Test Coverage Report for Build 9487862175

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 82.481%

Totals
Change from base Build 9475149132:	0.0%
Covered Lines:	5551
Relevant Lines:	6730

💛 - Coveralls

engedaam

I think we should add and E2E test to validate this behavior on top of the unit test

jigisha620

/karpenter snapshot

github-actions · 2024-04-05T20:57:00Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-38f448ea4e204cfed5b15be8adf16e4d927729fc.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-38f448ea4e204cfed5b15be8adf16e4d927729fc" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

engedaam

Seems like there are failures on the E2E tests

test/suites/nodeclaim/nodeclaim_test.go

jigisha620

/karpenter snapshot

github-actions · 2024-04-07T22:54:54Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-914b6e4747199b12ddede43caf83a146a1536253.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-914b6e4747199b12ddede43caf83a146a1536253" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

pkg/controllers/nodeclaim/garbagecollection/controller.go

pkg/providers/instance/instance.go

jigisha620

/karpenter snapshot

github-actions · 2024-04-08T18:47:01Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-dfb89b16de62a40debed37190f1275f89755476a.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-dfb89b16de62a40debed37190f1275f89755476a" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

jigisha620

/karpenter snapshot

github-actions · 2024-04-16T00:23:45Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-99f8d1d95653dde8948a2336b3e051c0045b9d85.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-99f8d1d95653dde8948a2336b3e051c0045b9d85" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

jigisha620

/karpenter snapshot

github-actions · 2024-04-16T17:01:11Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-64229eceb520f4bc662bee61797b3c7acc40a1d6.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-64229eceb520f4bc662bee61797b3c7acc40a1d6" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

jigisha620

/karpenter snapshot

github-actions · 2024-04-16T22:24:10Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-a3ec526edfbad290c78c7801c953496824af4415.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-a3ec526edfbad290c78c7801c953496824af4415" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

github-actions · 2024-05-10T13:44:14Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-af9dea06058254f3b5a8fe6bf70138e9e788a9fa.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-af9dea06058254f3b5a8fe6bf70138e9e788a9fa" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

jigisha620

/karpenter snapshot

github-actions · 2024-05-11T13:11:36Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-a41e7042c35124016e121d192acd5f768e8de8cb.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-a41e7042c35124016e121d192acd5f768e8de8cb" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

jigisha620

/karpenter snapshot

github-actions · 2024-05-13T13:10:49Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-6c090c2e94eeb7ca211a5988689f99657558ba36.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-6c090c2e94eeb7ca211a5988689f99657558ba36" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

jigisha620

/karpenter snapshot

github-actions · 2024-05-16T14:19:33Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-2fb768a3be071fc59d76220c42409f54a9aa4825.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-2fb768a3be071fc59d76220c42409f54a9aa4825" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

github-actions · 2024-06-01T12:04:07Z

This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity.

jigisha620

/karpenter snapshot

github-actions · 2024-06-12T00:39:59Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-200d9b6b4c840f3ce2b61b3214fd4612b28356c7.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-200d9b6b4c840f3ce2b61b3214fd4612b28356c7" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

jigisha620

/karpenter snapshot

github-actions · 2024-06-12T03:47:31Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-237c35c29be1aac5035696b92d28fdd642699b06.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-237c35c29be1aac5035696b92d28fdd642699b06" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

test/suites/expiration/suite_test.go

jmdeal

LGTM 🚀

jigisha620 requested a review from a team as a code owner April 3, 2024 22:02

jigisha620 requested a review from engedaam April 3, 2024 22:02

jigisha620 force-pushed the feat-instance-termination branch from 9f4fa97 to 550e2dd Compare April 3, 2024 23:02

engedaam reviewed Apr 3, 2024

View reviewed changes

go.mod Outdated Show resolved Hide resolved

jigisha620 force-pushed the feat-instance-termination branch from 550e2dd to af11d0d Compare April 5, 2024 17:33

engedaam reviewed Apr 5, 2024

View reviewed changes

jigisha620 force-pushed the feat-instance-termination branch from af11d0d to 38f448e Compare April 5, 2024 20:46

jigisha620 commented Apr 5, 2024

View reviewed changes

engedaam reviewed Apr 5, 2024

View reviewed changes

test/suites/nodeclaim/nodeclaim_test.go Outdated Show resolved Hide resolved

test/suites/nodeclaim/nodeclaim_test.go Outdated Show resolved Hide resolved

test/suites/nodeclaim/nodeclaim_test.go Outdated Show resolved Hide resolved

jigisha620 force-pushed the feat-instance-termination branch from 38f448e to 914b6e4 Compare April 7, 2024 22:40

jigisha620 commented Apr 7, 2024

View reviewed changes

jonathan-innis reviewed Apr 8, 2024

View reviewed changes

jigisha620 force-pushed the feat-instance-termination branch from 914b6e4 to dfb89b1 Compare April 8, 2024 18:34

jigisha620 commented Apr 8, 2024

View reviewed changes

jigisha620 force-pushed the feat-instance-termination branch 3 times, most recently from 285ad8f to 99f8d1d Compare April 16, 2024 00:13

jigisha620 commented Apr 16, 2024

View reviewed changes

jigisha620 force-pushed the feat-instance-termination branch from 99f8d1d to 64229ec Compare April 16, 2024 16:27

jigisha620 commented Apr 16, 2024

View reviewed changes

jigisha620 force-pushed the feat-instance-termination branch from 64229ec to a3ec526 Compare April 16, 2024 21:19

jigisha620 commented Apr 16, 2024

View reviewed changes

jigisha620 force-pushed the feat-instance-termination branch 2 times, most recently from 23e2b3d to a41e704 Compare May 11, 2024 13:00

jigisha620 commented May 11, 2024

View reviewed changes

jigisha620 force-pushed the feat-instance-termination branch from a41e704 to 6c090c2 Compare May 13, 2024 12:57

jigisha620 commented May 13, 2024

View reviewed changes

jigisha620 force-pushed the feat-instance-termination branch 2 times, most recently from 84aeae1 to 2fb768a Compare May 16, 2024 14:07

jigisha620 commented May 16, 2024

View reviewed changes

jigisha620 force-pushed the feat-instance-termination branch from 2fb768a to a8fd4db Compare May 17, 2024 14:22

github-actions bot added the lifecycle/stale label Jun 1, 2024

jigisha620 force-pushed the feat-instance-termination branch 3 times, most recently from 9796457 to 200d9b6 Compare June 12, 2024 00:30

jigisha620 commented Jun 12, 2024

View reviewed changes

jigisha620 force-pushed the feat-instance-termination branch from 200d9b6 to 237c35c Compare June 12, 2024 03:34

jigisha620 commented Jun 12, 2024

View reviewed changes

jmdeal reviewed Jun 12, 2024

View reviewed changes

test/suites/expiration/suite_test.go Show resolved Hide resolved

github-actions bot removed the lifecycle/stale label Jun 12, 2024

chore: Wait for instance termination before deleting nodeclaim

e86e82f

jigisha620 force-pushed the feat-instance-termination branch from 237c35c to e86e82f Compare June 12, 2024 18:31

jmdeal approved these changes Jun 12, 2024

View reviewed changes

jigisha620 merged commit 90aece4 into aws:main Jun 12, 2024
18 checks passed

AndrewSirenko mentioned this pull request Jun 13, 2024

docs: RFC for disrupted EBS-Backed StatefulSet delays #6336

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Wait for instance termination before deleting nodeclaim #5979

chore: Wait for instance termination before deleting nodeclaim #5979

jigisha620 commented Apr 3, 2024

netlify bot commented Apr 3, 2024 •

edited

Loading

coveralls commented Apr 5, 2024 •

edited

Loading

engedaam left a comment •

edited

Loading

jigisha620 left a comment

github-actions bot commented Apr 5, 2024

engedaam left a comment

jigisha620 left a comment

github-actions bot commented Apr 7, 2024

jigisha620 left a comment

github-actions bot commented Apr 8, 2024

jigisha620 left a comment

github-actions bot commented Apr 16, 2024

jigisha620 left a comment

github-actions bot commented Apr 16, 2024

jigisha620 left a comment

github-actions bot commented Apr 16, 2024

github-actions bot commented May 10, 2024

jigisha620 left a comment

github-actions bot commented May 11, 2024

jigisha620 left a comment

github-actions bot commented May 13, 2024

jigisha620 left a comment

github-actions bot commented May 16, 2024

github-actions bot commented Jun 1, 2024

jigisha620 left a comment

github-actions bot commented Jun 12, 2024

jigisha620 left a comment

github-actions bot commented Jun 12, 2024

jmdeal left a comment

chore: Wait for instance termination before deleting nodeclaim #5979

chore: Wait for instance termination before deleting nodeclaim #5979

Conversation

jigisha620 commented Apr 3, 2024

netlify bot commented Apr 3, 2024 • edited Loading

✅ Deploy Preview for karpenter-docs-prod canceled.

coveralls commented Apr 5, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9487862175

Details

💛 - Coveralls

engedaam left a comment • edited Loading

Choose a reason for hiding this comment

jigisha620 left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 5, 2024

engedaam left a comment

Choose a reason for hiding this comment

jigisha620 left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 7, 2024

jigisha620 left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 8, 2024

jigisha620 left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 16, 2024

jigisha620 left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 16, 2024

jigisha620 left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 16, 2024

github-actions bot commented May 10, 2024

jigisha620 left a comment

Choose a reason for hiding this comment

github-actions bot commented May 11, 2024

jigisha620 left a comment

Choose a reason for hiding this comment

github-actions bot commented May 13, 2024

jigisha620 left a comment

Choose a reason for hiding this comment

github-actions bot commented May 16, 2024

github-actions bot commented Jun 1, 2024

jigisha620 left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 12, 2024

jigisha620 left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 12, 2024

jmdeal left a comment

Choose a reason for hiding this comment

netlify bot commented Apr 3, 2024 •

edited

Loading

coveralls commented Apr 5, 2024 •

edited

Loading

engedaam left a comment •

edited

Loading