test: Flakiness Fix for Kubelet Configuration (#2511)

Author: jonathan-innis

.github/ISSUE_TEMPLATE/bug.yaml

@@ -2,13 +2,17 @@ name: Bug Report
 description: Report a bug in Karpenter
 labels: bug
 body:
-  - type: input
+  - type: textarea
     id: version
     attributes:
       label: Version
       description: |
         https://github.com/aws/karpenter/releases
-      placeholder: ex. kubectl version | grep Server
+      value: |
+        <!-- helm ls -A --all -o json | jq '.[] | select(.name=="karpenter") | .app_version' -r -->
+        __Karpenter Version:__ v0.0.0
+        <!-- kubectl version | grep Server -->
+        __Kubernetes Version:__ v1.0.0
     validations:
       required: true
 

Makefile

@@ -1,13 +1,13 @@
 export K8S_VERSION ?= 1.23.x
 export KUBEBUILDER_ASSETS ?= ${HOME}/.kubebuilder/bin
+export CLUSTER_NAME ?= $(shell kubectl config view --minify -o jsonpath='{.clusters[].name}' | rev | cut -d"/" -f1 | rev | cut -d"." -f1)
 
 ## Inject the app version into project.Version
 LDFLAGS ?= -ldflags=-X=github.com/aws/karpenter/pkg/utils/project.Version=$(shell git describe --tags --always)
 GOFLAGS ?= $(LDFLAGS)
 WITH_GOFLAGS = GOFLAGS="$(GOFLAGS)"
 
 ## Extra helm options
-CLUSTER_NAME ?= $(shell kubectl config view --minify -o jsonpath='{.clusters[].name}' | rev | cut -d"/" -f1 | rev | cut -d"." -f1)
 CLUSTER_ENDPOINT ?= $(shell kubectl config view --minify -o jsonpath='{.clusters[].cluster.server}')
 AWS_ACCOUNT_ID ?= $(shell aws sts get-caller-identity --query Account --output text)
 KARPENTER_IAM_ROLE_ARN ?= arn:aws:iam::${AWS_ACCOUNT_ID}:role/${CLUSTER_NAME}-karpenter
@@ -20,6 +20,7 @@ TEST_FILTER ?= .*
 
 # CR for local builds of Karpenter
 SYSTEM_NAMESPACE ?= karpenter
+KARPENTER_VERSION ?= $(shell git tag --sort=committerdate | tail -1)
 KO_DOCKER_REPO ?= ${AWS_ACCOUNT_ID}.dkr.ecr.${AWS_DEFAULT_REGION}.amazonaws.com/karpenter
 GETTING_STARTED_SCRIPT_DIR = website/content/en/preview/getting-started/getting-started-with-eksctl/scripts
 
@@ -49,7 +50,7 @@ battletest: ## Run randomized, racing, code coveraged, tests
 
 e2etests: ## Run the e2e suite against your local cluster
 	go clean -testcache
-	CLUSTER_NAME=${CLUSTER_NAME} go test -p 1 -timeout 180m -v ./test/suites/... -run=${TEST_FILTER}
+	go test -p 1 -timeout 180m -v ./test/suites/... -run=${TEST_FILTER}
 
 benchmark:
 	go test -tags=test_performance -run=NoTests -bench=. ./...

pkg/controllers/consolidation/suite_test.go

@@ -1367,7 +1367,7 @@ var _ = Describe("Parallelization", func() {
 			nodes := &v1.NodeList{}
 			g.Expect(env.Client.List(ctx, nodes)).To(Succeed())
 			g.Expect(len(nodes.Items)).To(Equal(2))
-		}).Should(Succeed())
+		}, time.Second*10).Should(Succeed())
 
 		// Add a new pending pod that should schedule while node is not yet deleted
 		pods := ExpectProvisionedNoBinding(ctx, env.Client, provisioningController, test.UnschedulablePod())

test/suites/integration/ttl_test.go test/suites/integration/emptiness_test.go

@@ -29,7 +29,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-var _ = Describe("TTL Empty", func() {
+var _ = Describe("Emptiness", func() {
 	BeforeEach(func() { env.BeforeEach() })
 	AfterEach(func() { env.AfterEach() })
 
@@ -60,35 +60,3 @@ var _ = Describe("TTL Empty", func() {
 		}
 	})
 })
-
-var _ = Describe("TTL Expired", func() {
-	It("should terminate an expired node", func() {
-		provider := test.AWSNodeTemplate(v1alpha1.AWSNodeTemplateSpec{AWS: awsv1alpha1.AWS{
-			SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": env.ClusterName},
-			SubnetSelector:        map[string]string{"karpenter.sh/discovery": env.ClusterName},
-		}})
-		provisioner := test.Provisioner(test.ProvisionerOptions{
-			ProviderRef: &v1alpha5.ProviderRef{Name: provider.Name},
-		})
-
-		const numPods = 1
-		deployment := test.Deployment(test.DeploymentOptions{Replicas: numPods})
-
-		env.ExpectCreated(provisioner, provider, deployment)
-		env.EventuallyExpectHealthyPodCount(labels.SelectorFromSet(deployment.Spec.Selector.MatchLabels), numPods)
-		env.ExpectCreatedNodeCount("==", 1)
-
-		persistedDep := deployment.DeepCopy()
-		deployment.Spec.Replicas = ptr.Int32(0)
-		Expect(env.Client.Patch(env, deployment, client.MergeFrom(persistedDep))).To(Succeed())
-
-		persistedProv := provisioner.DeepCopy()
-		provisioner.Spec.TTLSecondsUntilExpired = ptr.Int64(10)
-		Expect(env.Client.Patch(env, provisioner, client.MergeFrom(persistedProv))).To(Succeed())
-
-		nodes := env.Monitor.GetCreatedNodes()
-		for i := range nodes {
-			env.EventuallyExpectNotFound(&nodes[i])
-		}
-	})
-})

test/suites/integration/kubelet_config_test.go

@@ -15,26 +15,24 @@ limitations under the License.
 package integration_test
 
 import (
+	"math"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	appsv1 "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"knative.dev/pkg/ptr"
 
 	"github.com/aws/karpenter/pkg/apis/awsnodetemplate/v1alpha1"
 	"github.com/aws/karpenter/pkg/apis/provisioning/v1alpha5"
 	awsv1alpha1 "github.com/aws/karpenter/pkg/cloudprovider/aws/apis/v1alpha1"
+	"github.com/aws/karpenter/pkg/scheduling"
 	"github.com/aws/karpenter/pkg/test"
 )
 
 var _ = Describe("KubeletConfiguration Overrides", func() {
 	It("should schedule pods onto separate nodes when maxPods is set", func() {
-		// Get the total number of daemonsets so that we see how many pods will be taken up
-		// by daemonset overhead
-		dsList := &appsv1.DaemonSetList{}
-		Expect(env.Client.List(env.Context, dsList)).To(Succeed())
-		dsCount := len(dsList.Items)
-
 		provider := test.AWSNodeTemplate(v1alpha1.AWSNodeTemplateSpec{AWS: awsv1alpha1.AWS{
 			SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": env.ClusterName},
 			SubnetSelector:        map[string]string{"karpenter.sh/discovery": env.ClusterName},
@@ -43,26 +41,36 @@ var _ = Describe("KubeletConfiguration Overrides", func() {
 		// MaxPods needs to account for the daemonsets that will run on the nodes
 		provisioner := test.Provisioner(test.ProvisionerOptions{
 			ProviderRef: &v1alpha5.ProviderRef{Name: provider.Name},
-			Kubelet: &v1alpha5.KubeletConfiguration{
-				MaxPods: ptr.Int32(1 + int32(dsCount)),
+			Requirements: []v1.NodeSelectorRequirement{
+				{
+					Key:      v1.LabelOSStable,
+					Operator: v1.NodeSelectorOpIn,
+					Values:   []string{string(v1.Linux)},
+				},
 			},
 		})
 
+		// Get the DS pod count and use it to calculate the DS pod overhead
+		dsCount := getDaemonSetPodCount(provisioner)
+		provisioner.Spec.KubeletConfiguration = &v1alpha5.KubeletConfiguration{
+			MaxPods: ptr.Int32(1 + int32(dsCount)),
+		}
+
 		pods := []*v1.Pod{test.Pod(), test.Pod(), test.Pod()}
 		env.ExpectCreated(provisioner, provider)
 		for _, pod := range pods {
 			env.ExpectCreated(pod)
 		}
 		env.EventuallyExpectHealthy(pods...)
 		env.ExpectCreatedNodeCount("==", 3)
+
+		nodeNames := sets.NewString()
+		for _, pod := range pods {
+			nodeNames.Insert(pod.Spec.NodeName)
+		}
+		Expect(len(nodeNames)).To(BeNumerically("==", 3))
 	})
 	It("should schedule pods onto separate nodes when podsPerCore is set", func() {
-		// Get the total number of daemonsets so that we see how many pods will be taken up
-		// by daemonset overhead
-		dsList := &appsv1.DaemonSetList{}
-		Expect(env.Client.List(env.Context, dsList)).To(Succeed())
-		dsCount := len(dsList.Items)
-
 		provider := test.AWSNodeTemplate(v1alpha1.AWSNodeTemplateSpec{AWS: awsv1alpha1.AWS{
 			SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": env.ClusterName},
 			SubnetSelector:        map[string]string{"karpenter.sh/discovery": env.ClusterName},
@@ -71,25 +79,46 @@ var _ = Describe("KubeletConfiguration Overrides", func() {
 		// This will have 4 pods available on each node (2 taken by daemonset pods)
 		provisioner := test.Provisioner(test.ProvisionerOptions{
 			ProviderRef: &v1alpha5.ProviderRef{Name: provider.Name},
-			Kubelet: &v1alpha5.KubeletConfiguration{
-				PodsPerCore: ptr.Int32(2 + int32(dsCount)),
-			},
 			Requirements: []v1.NodeSelectorRequirement{
 				{
 					Key:      awsv1alpha1.LabelInstanceCPU,
 					Operator: v1.NodeSelectorOpIn,
-					Values:   []string{"1"},
+					Values:   []string{"2"},
+				},
+				{
+					Key:      v1.LabelOSStable,
+					Operator: v1.NodeSelectorOpIn,
+					Values:   []string{string(v1.Linux)},
 				},
 			},
 		})
 
+		// Get the DS pod count and use it to calculate the DS pod overhead
+		// We calculate podsPerCore to split the test pods and the DS pods between two nodes:
+		//   1. If # of DS pods is odd, we will have i.e. ceil((3+2)/2) = 3
+		//      Since we restrict node to two cores, we will allow 6 pods. One node will have 3
+		//      DS pods and 3 test pods. Other node will have 1 test pod and 3 DS pods
+		//   2. If # of DS pods is even, we will have i.e. ceil((4+2)/2) = 3
+		//      Since we restrict node to two cores, we will allow 6 pods. Both nodes will have
+		//      4 DS pods and 2 test pods.
+		dsCount := getDaemonSetPodCount(provisioner)
+		provisioner.Spec.KubeletConfiguration = &v1alpha5.KubeletConfiguration{
+			PodsPerCore: ptr.Int32(int32(math.Ceil(float64(2+dsCount) / 2))),
+		}
+
 		pods := []*v1.Pod{test.Pod(), test.Pod(), test.Pod(), test.Pod()}
 		env.ExpectCreated(provisioner, provider)
 		for _, pod := range pods {
 			env.ExpectCreated(pod)
 		}
 		env.EventuallyExpectHealthy(pods...)
 		env.ExpectCreatedNodeCount("==", 2)
+
+		nodeNames := sets.NewString()
+		for _, pod := range pods {
+			nodeNames.Insert(pod.Spec.NodeName)
+		}
+		Expect(len(nodeNames)).To(BeNumerically("==", 2))
 	})
 	It("should ignore podsPerCore value when Bottlerocket is used", func() {
 		provider := test.AWSNodeTemplate(v1alpha1.AWSNodeTemplateSpec{AWS: awsv1alpha1.AWS{
@@ -108,7 +137,7 @@ var _ = Describe("KubeletConfiguration Overrides", func() {
 				{
 					Key:      awsv1alpha1.LabelInstanceCPU,
 					Operator: v1.NodeSelectorOpIn,
-					Values:   []string{"1"},
+					Values:   []string{"2"},
 				},
 			},
 		})
@@ -122,3 +151,24 @@ var _ = Describe("KubeletConfiguration Overrides", func() {
 		env.ExpectCreatedNodeCount("==", 1)
 	})
 })
+
+// Performs the same logic as the scheduler to get the number of daemonset
+// pods that we estimate we will need to schedule as overhead to each node
+func getDaemonSetPodCount(provisioner *v1alpha5.Provisioner) int {
+	daemonSetList := &appsv1.DaemonSetList{}
+	Expect(env.Client.List(env.Context, daemonSetList)).To(Succeed())
+
+	count := 0
+	for _, daemonSet := range daemonSetList.Items {
+		p := &v1.Pod{Spec: daemonSet.Spec.Template.Spec}
+		nodeTemplate := scheduling.NewNodeTemplate(provisioner)
+		if err := nodeTemplate.Taints.Tolerates(p); err != nil {
+			continue
+		}
+		if err := nodeTemplate.Requirements.Compatible(scheduling.NewPodRequirements(p)); err != nil {
+			continue
+		}
+		count++
+	}
+	return count
+}