chore: Wait for instance termination before deleting nodeclaim

.github/workflows/e2e-matrix.yaml

@@ -62,6 +62,12 @@ jobs:
         suite:
           - name: Integration
             region: ${{ inputs.region }}
+          - name: Ami
+            region: ${{ inputs.region }}
+          - name: Scheduling
+            region: ${{ inputs.region }}
+          - name: Storage
+            region: ${{ inputs.region }}
           - name: NodeClaim
             region: ${{ inputs.region }}
           - name: Consolidation

.github/workflows/e2e-soak-trigger.yaml

@@ -39,14 +39,16 @@ jobs:
     needs: [resolve_cluster]
     strategy:
       fail-fast: false
-      matrix: ${{ fromJson(needs.resolve_cluster.outputs.PREEXISTING_CLUSTERS) }}
+      matrix:
+        cluster: ${{ fromJson(needs.resolve_cluster.outputs.PREEXISTING_CLUSTERS) }}
+        suite: [Integration, Ami, Scheduling, Storage]
     uses: ./.github/workflows/e2e.yaml
     with:
-      suite: Integration
+      suite: ${{ matrix.suite }}
       region: eu-north-1
       workflow_trigger: "soak"
-      cluster_name: ${{ matrix.cluster_name }}
-      cleanup: ${{ matrix.cluster_cleanup }}
-      git_ref: ${{ matrix.git_ref }}
+      cluster_name: ${{ matrix.cluster.cluster_name }}
+      cleanup: ${{ matrix.cluster.cluster_cleanup }}
+      git_ref: ${{ matrix.cluster.git_ref }}
     secrets:
       SLACK_WEBHOOK_SOAK_URL: ${{ secrets.SLACK_WEBHOOK_SOAK_URL }}

.github/workflows/e2e-upgrade.yaml

@@ -54,6 +54,11 @@ jobs:
     permissions:
       id-token: write # aws-actions/[email protected]
       statuses: write # ./.github/actions/commit-status/start
+    strategy:
+      fail-fast: false
+      max-parallel: 100
+      matrix:
+        suite: [Integration, Ami, Scheduling, Storage]
     name: suite-upgrade
     runs-on: ubuntu-latest
     steps:
@@ -124,9 +129,11 @@ jobs:
           cluster_name: ${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }}
           git_ref: ${{ inputs.to_git_ref }}
       - name: run the Upgrade test suite
+        env:
+          TEST_SUITE: ${{ matrix.suite }}
         run: |
           aws eks update-kubeconfig --name ${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }}
-          CLUSTER_NAME=${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }} INTERRUPTION_QUEUE=${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }} CLUSTER_ENDPOINT="$(aws eks describe-cluster --name ${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }} --query "cluster.endpoint" --output text)" TEST_SUITE="Integration" make e2etests
+          CLUSTER_NAME=${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }} INTERRUPTION_QUEUE=${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }} CLUSTER_ENDPOINT="$(aws eks describe-cluster --name ${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }} --query "cluster.endpoint" --output text)" TEST_SUITE="$TEST_SUITE"} make e2etests
       - name: notify slack of success or failure
         uses: ./.github/actions/e2e/slack/notify
         if: (success() || failure()) && github.event_name != 'workflow_run' && inputs.workflow_trigger != 'versionCompatibility'

Makefile

@@ -75,11 +75,11 @@ e2etests: ## Run the e2e suite against your local cluster
 		go test \
 		-p 1 \
 		-count 1 \
-		-timeout 3h \
+		-timeout 5h \
 		-v \
 		./suites/$(shell echo $(TEST_SUITE) | tr A-Z a-z)/... \
 		--ginkgo.focus="${FOCUS}" \
-		--ginkgo.timeout=3h \
+		--ginkgo.timeout=5h \
 		--ginkgo.grace-period=3m \
 		--ginkgo.vv
 

go.mod

@@ -116,3 +116,5 @@ require (
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
 )
+
+replace sigs.k8s.io/karpenter => github.com/jigisha620/karpenter v0.0.0-20240515173114-ed0c151a1215

go.sum

@@ -218,6 +218,8 @@ github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4=
 github.com/imdario/mergo v0.3.16/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY=
 github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
 github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
+github.com/jigisha620/karpenter v0.0.0-20240515173114-ed0c151a1215 h1:ulKF3AmZNXpWs73c5U2KGmrS/drsfslozEUeixYZ0t4=
+github.com/jigisha620/karpenter v0.0.0-20240515173114-ed0c151a1215/go.mod h1:Ov8+tDVcF2BIPti+HL0hgoxIGy+rGIymKZAYZprl0Ww=
 github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg=
 github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo=
 github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8=
@@ -761,8 +763,6 @@ sigs.k8s.io/controller-runtime v0.18.2 h1:RqVW6Kpeaji67CY5nPEfRz6ZfFMk0lWQlNrLql
 sigs.k8s.io/controller-runtime v0.18.2/go.mod h1:tuAt1+wbVsXIT8lPtk5RURxqAnq7xkpv2Mhttslg7Hw=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
-sigs.k8s.io/karpenter v0.36.1-0.20240515165354-f26918ee07ab h1:pTvlhY4G5uQFJI7FOcanVukzw+gqHCnxAIQOdrmR90c=
-sigs.k8s.io/karpenter v0.36.1-0.20240515165354-f26918ee07ab/go.mod h1:Ov8+tDVcF2BIPti+HL0hgoxIGy+rGIymKZAYZprl0Ww=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.1/go.mod h1:N8hJocpFajUSSeSJ9bOZ77VzejKZaXsTtZo4/u7Io08=
 sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=

test/pkg/environment/common/environment.go

@@ -75,7 +75,7 @@ func NewEnvironment(t *testing.T) *Environment {
 		ctx = context.WithValue(ctx, GitRefContextKey, val)
 	}
 
-	gomega.SetDefaultEventuallyTimeout(5 * time.Minute)
+	gomega.SetDefaultEventuallyTimeout(16 * time.Minute)
 	gomega.SetDefaultEventuallyPollingInterval(1 * time.Second)
 	return &Environment{
 		Context:    ctx,

test/pkg/environment/common/expectations.go

@@ -781,7 +781,7 @@ func (env *Environment) EventuallyExpectAvgUtilization(resource v1.ResourceName,
 	GinkgoHelper()
 	Eventually(func(g Gomega) {
 		g.Expect(env.Monitor.AvgUtilization(resource)).To(BeNumerically(comparator, value))
-	}, 10*time.Minute).Should(Succeed())
+	}, 12*time.Minute).Should(Succeed())
 }
 
 func (env *Environment) ExpectDaemonSetEnvironmentVariableUpdated(obj client.ObjectKey, name, value string, containers ...string) {

test/suites/integration/ami_test.go → test/suites/ami/ami_test.go

@@ -12,32 +12,58 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package integration_test
+package ami_test
 
 import (
+	"testing"
+
+	corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1"
+
+	"github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1"
+
 	"encoding/base64"
 	"fmt"
 	"os"
 	"strings"
 	"time"
 
-	"github.com/aws/aws-sdk-go/aws"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	awssdk "github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/samber/lo"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	"github.com/aws/karpenter-provider-aws/test/pkg/environment/aws"
+
 	coretest "sigs.k8s.io/karpenter/pkg/test"
+)
 
-	corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1"
+var env *aws.Environment
+var nodeClass *v1beta1.EC2NodeClass
+var nodePool *corev1beta1.NodePool
 
-	"github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1"
-	awsenv "github.com/aws/karpenter-provider-aws/test/pkg/environment/aws"
+func TestIntegration(t *testing.T) {
+	RegisterFailHandler(Fail)
+	BeforeSuite(func() {
+		env = aws.NewEnvironment(t)
+	})
+	AfterSuite(func() {
+		env.Stop()
+	})
+	RunSpecs(t, "Integration")
+}
 
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-)
+var _ = BeforeEach(func() {
+	env.BeforeEach()
+	nodeClass = env.DefaultEC2NodeClass()
+	nodePool = env.DefaultNodePool(nodeClass)
+})
+var _ = AfterEach(func() { env.Cleanup() })
+var _ = AfterEach(func() { env.AfterEach() })
 
 var _ = Describe("AMI", func() {
 	var customAMI string
@@ -79,7 +105,7 @@ var _ = Describe("AMI", func() {
 	})
 	It("should support AMI Selector Terms for Name but fail with incorrect owners", func() {
 		output, err := env.EC2API.DescribeImages(&ec2.DescribeImagesInput{
-			ImageIds: []*string{aws.String(customAMI)},
+			ImageIds: []*string{awssdk.String(customAMI)},
 		})
 		Expect(err).To(BeNil())
 		Expect(output.Images).To(HaveLen(1))
@@ -97,7 +123,7 @@ var _ = Describe("AMI", func() {
 	})
 	It("should support ami selector Name with default owners", func() {
 		output, err := env.EC2API.DescribeImages(&ec2.DescribeImagesInput{
-			ImageIds: []*string{aws.String(customAMI)},
+			ImageIds: []*string{awssdk.String(customAMI)},
 		})
 		Expect(err).To(BeNil())
 		Expect(output.Images).To(HaveLen(1))
@@ -170,7 +196,7 @@ var _ = Describe("AMI", func() {
 					NodeSelectorRequirement: v1.NodeSelectorRequirement{
 						Key:      v1beta1.LabelInstanceFamily,
 						Operator: v1.NodeSelectorOpNotIn,
-						Values:   awsenv.ExcludedInstanceFamilies,
+						Values:   aws.ExcludedInstanceFamilies,
 					},
 				},
 			)
@@ -237,7 +263,7 @@ var _ = Describe("AMI", func() {
 			content, err := os.ReadFile("testdata/al2_userdata_input.sh")
 			Expect(err).ToNot(HaveOccurred())
 			nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyAL2
-			nodeClass.Spec.UserData = aws.String(string(content))
+			nodeClass.Spec.UserData = awssdk.String(string(content))
 			nodePool.Spec.Template.Spec.Taints = []v1.Taint{{Key: "example.com", Value: "value", Effect: "NoExecute"}}
 			nodePool.Spec.Template.Spec.StartupTaints = []v1.Taint{{Key: "example.com", Value: "value", Effect: "NoSchedule"}}
 			pod := coretest.Pod(coretest.PodOptions{Tolerations: []v1.Toleration{{Key: "example.com", Operator: v1.TolerationOpExists}}})
@@ -258,7 +284,7 @@ var _ = Describe("AMI", func() {
 			content, err := os.ReadFile("testdata/al2_no_mime_userdata_input.sh")
 			Expect(err).ToNot(HaveOccurred())
 			nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyAL2
-			nodeClass.Spec.UserData = aws.String(string(content))
+			nodeClass.Spec.UserData = awssdk.String(string(content))
 			nodePool.Spec.Template.Spec.Taints = []v1.Taint{{Key: "example.com", Value: "value", Effect: "NoExecute"}}
 			nodePool.Spec.Template.Spec.StartupTaints = []v1.Taint{{Key: "example.com", Value: "value", Effect: "NoSchedule"}}
 			pod := coretest.Pod(coretest.PodOptions{Tolerations: []v1.Toleration{{Key: "example.com", Operator: v1.TolerationOpExists}}})
@@ -279,7 +305,7 @@ var _ = Describe("AMI", func() {
 			content, err := os.ReadFile("testdata/br_userdata_input.sh")
 			Expect(err).ToNot(HaveOccurred())
 			nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyBottlerocket
-			nodeClass.Spec.UserData = aws.String(string(content))
+			nodeClass.Spec.UserData = awssdk.String(string(content))
 			nodePool.Spec.Template.Spec.Taints = []v1.Taint{{Key: "example.com", Value: "value", Effect: "NoExecute"}}
 			nodePool.Spec.Template.Spec.StartupTaints = []v1.Taint{{Key: "example.com", Value: "value", Effect: "NoSchedule"}}
 			pod := coretest.Pod(coretest.PodOptions{Tolerations: []v1.Toleration{{Key: "example.com", Operator: v1.TolerationOpExists}}})
@@ -303,7 +329,7 @@ var _ = Describe("AMI", func() {
 			content, err := os.ReadFile("testdata/windows_userdata_input.ps1")
 			Expect(err).ToNot(HaveOccurred())
 			nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyWindows2022
-			nodeClass.Spec.UserData = aws.String(string(content))
+			nodeClass.Spec.UserData = awssdk.String(string(content))
 			nodePool.Spec.Template.Spec.Taints = []v1.Taint{{Key: "example.com", Value: "value", Effect: "NoExecute"}}
 			nodePool.Spec.Template.Spec.StartupTaints = []v1.Taint{{Key: "example.com", Value: "value", Effect: "NoSchedule"}}
 
@@ -313,7 +339,7 @@ var _ = Describe("AMI", func() {
 					NodeSelectorRequirement: v1.NodeSelectorRequirement{
 						Key:      v1beta1.LabelInstanceFamily,
 						Operator: v1.NodeSelectorOpNotIn,
-						Values:   awsenv.ExcludedInstanceFamilies,
+						Values:   aws.ExcludedInstanceFamilies,
 					},
 				},
 				corev1beta1.NodeSelectorRequirementWithMinValues{
@@ -325,7 +351,7 @@ var _ = Describe("AMI", func() {
 				},
 			)
 			pod := coretest.Pod(coretest.PodOptions{
-				Image: awsenv.WindowsDefaultImage,
+				Image: aws.WindowsDefaultImage,
 				NodeSelector: map[string]string{
 					v1.LabelOSStable:     string(v1.Windows),
 					v1.LabelWindowsBuild: "10.0.20348",
@@ -354,8 +380,8 @@ func getInstanceAttribute(nodeName string, attribute string) *ec2.DescribeInstan
 	providerIDSplit := strings.Split(node.Spec.ProviderID, "/")
 	instanceID := providerIDSplit[len(providerIDSplit)-1]
 	instanceAttribute, err := env.EC2API.DescribeInstanceAttribute(&ec2.DescribeInstanceAttributeInput{
-		InstanceId: aws.String(instanceID),
-		Attribute:  aws.String(attribute),
+		InstanceId: awssdk.String(instanceID),
+		Attribute:  awssdk.String(attribute),
 	})
 	Expect(err).ToNot(HaveOccurred())
 	return instanceAttribute

test/suites/expiration/suite_test.go

@@ -586,13 +586,6 @@ var _ = Describe("Expiration", func() {
 			// TODO: reduce timeouts when deprovisioning waits are factored out
 			env.EventuallyExpectNodesUntaintedWithTimeout(11*time.Minute, taintedNodes...)
 
-			// The nodeclaims that never registers will be removed
-			Eventually(func(g Gomega) {
-				nodeClaims := &corev1beta1.NodeClaimList{}
-				g.Expect(env.Client.List(env, nodeClaims, client.HasLabels{coretest.DiscoveryLabel})).To(Succeed())
-				g.Expect(len(nodeClaims.Items)).To(BeNumerically("==", int(numPods)))
-			}).WithTimeout(6 * time.Minute).Should(Succeed())
-
 			// Expect all the NodeClaims that existed on the initial provisioning loop are not removed
 			Consistently(func(g Gomega) {
 				nodeClaims := &corev1beta1.NodeClaimList{}

test/suites/integration/termination_test.go

@@ -44,7 +44,7 @@ var _ = Describe("Termination", func() {
 		env.ExpectDeleted(nodes[0])
 		env.EventuallyExpectNotFound(nodes[0])
 		Eventually(func(g Gomega) {
-			g.Expect(lo.FromPtr(env.GetInstanceByID(instanceID).State.Name)).To(Equal("shutting-down"))
+			g.Expect(lo.FromPtr(env.GetInstanceByID(instanceID).State.Name)).To(BeElementOf("terminated", "shutting-down"))
 		}, time.Second*10).Should(Succeed())
 	})
 })

test/suites/interruption/suite_test.go

@@ -130,8 +130,8 @@ var _ = Describe("Interruption", func() {
 		node := env.Monitor.CreatedNodes()[0]
 
 		By("Stopping the EC2 instance without the EKS cluster's knowledge")
-		env.ExpectInstanceStopped(node.Name)                                                   // Make a call to the EC2 api to stop the instance
-		env.EventuallyExpectNotFoundAssertion(node).WithTimeout(time.Minute).Should(Succeed()) // shorten the timeout since we should react faster
+		env.ExpectInstanceStopped(node.Name) // Make a call to the EC2 api to stop the instance
+		env.EventuallyExpectNotFoundAssertion(node).Should(Succeed())
 		env.EventuallyExpectHealthyPodCount(selector, 1)
 	})
 	It("should terminate the node at the API server when the EC2 instance is terminated", func() {
@@ -156,8 +156,8 @@ var _ = Describe("Interruption", func() {
 		node := env.Monitor.CreatedNodes()[0]
 
 		By("Terminating the EC2 instance without the EKS cluster's knowledge")
-		env.ExpectInstanceTerminated(node.Name)                                                // Make a call to the EC2 api to stop the instance
-		env.EventuallyExpectNotFoundAssertion(node).WithTimeout(time.Minute).Should(Succeed()) // shorten the timeout since we should react faster
+		env.ExpectInstanceTerminated(node.Name) // Make a call to the EC2 api to stop the instance
+		env.EventuallyExpectNotFoundAssertion(node).Should(Succeed())
 		env.EventuallyExpectHealthyPodCount(selector, 1)
 	})
 	It("should terminate the node when receiving a scheduled change health event", func() {
@@ -185,7 +185,7 @@ var _ = Describe("Interruption", func() {
 
 		By("Creating a scheduled change health event in the SQS message queue")
 		env.ExpectMessagesCreated(scheduledChangeMessage(env.Region, "000000000000", instanceID))
-		env.EventuallyExpectNotFoundAssertion(node).WithTimeout(time.Minute).Should(Succeed()) // shorten the timeout since we should react faster
+		env.EventuallyExpectNotFoundAssertion(node).Should(Succeed())
 		env.EventuallyExpectHealthyPodCount(selector, 1)
 	})
 })

test/suites/nodeclaim/garbage_collection_test.go

@@ -140,7 +140,7 @@ var _ = Describe("GarbageCollection", func() {
 		// Eventually expect the node and the instance to be removed (shutting-down)
 		env.EventuallyExpectNotFound(node)
 		Eventually(func(g Gomega) {
-			g.Expect(lo.FromPtr(env.GetInstanceByID(aws.StringValue(out.Instances[0].InstanceId)).State.Name)).To(Equal("shutting-down"))
+			g.Expect(lo.FromPtr(env.GetInstanceByID(aws.StringValue(out.Instances[0].InstanceId)).State.Name)).To(BeElementOf("terminated", "shutting-down"))
 		}, time.Second*10).Should(Succeed())
 	})
 	It("should succeed to garbage collect an Instance that was deleted without the cluster's knowledge", func() {

test/suites/nodeclaim/nodeclaim_test.go

@@ -19,15 +19,15 @@ import (
 	"os"
 	"time"
 
-	"github.com/samber/lo"
-	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/karpenter/pkg/utils/resources"
 
+	"github.com/samber/lo"
+	v1 "k8s.io/api/core/v1"
 	corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1"
 	"sigs.k8s.io/karpenter/pkg/test"
-	"sigs.k8s.io/karpenter/pkg/utils/resources"
 
 	"github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1"
 
@@ -225,7 +225,7 @@ var _ = Describe("StandaloneNodeClaim", func() {
 		env.EventuallyExpectNotFound(nodeClaim, node)
 
 		Eventually(func(g Gomega) {
-			g.Expect(lo.FromPtr(env.GetInstanceByID(instanceID).State.Name)).To(Equal("shutting-down"))
+			g.Expect(lo.FromPtr(env.GetInstanceByID(instanceID).State.Name)).To(BeElementOf("terminated", "shutting-down"))
 		}, time.Second*10).Should(Succeed())
 	})
 	It("should delete a NodeClaim from the node termination finalizer", func() {
@@ -264,7 +264,7 @@ var _ = Describe("StandaloneNodeClaim", func() {
 		env.EventuallyExpectNotFound(nodeClaim, node)
 
 		Eventually(func(g Gomega) {
-			g.Expect(lo.FromPtr(env.GetInstanceByID(instanceID).State.Name)).To(Equal("shutting-down"))
+			g.Expect(lo.FromPtr(env.GetInstanceByID(instanceID).State.Name)).To(BeElementOf("terminated", "shutting-down"))
 		}, time.Second*10).Should(Succeed())
 	})
 	It("should create a NodeClaim with custom labels passed through the userData", func() {