Test: Implemented ginkgo test against local kubeconfig #2015

ellistarn · 2022-06-28T23:46:19Z

Fixes #

Description
Enables e2e tests against a local cluster.

test resources are all tagged with a special label for cleanup
Made progress on CI infra setup (critical addons only, role name, etc), but gaps remain

 ~/w/g/s/g/aws/karpenter │ on testinfra !1  make e2etests                                2 ✘ │ at 18:00:08
ginkgo -r test -- --environment-name=karpenter-test-infrastructure
Running Suite: Integration Suite
================================
Random Seed: 1656550823
Will run 1 of 1 specs

• [SLOW TEST:85.879 seconds]
Sanity Checks
/Users/etarn/workspaces/go/src/github.com/aws/karpenter/test/suites/integration/suite_test.go:30
  should provision nodes
  /Users/etarn/workspaces/go/src/github.com/aws/karpenter/test/suites/integration/suite_test.go:31
------------------------------

Ran 1 of 1 Specs in 87.084 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
PASS

Ginkgo ran 1 suite in 1m33.141041379s
Test Suite Passed

How was this change tested?

Does this change impact docs?

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

Release Note

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

netlify · 2022-06-28T23:46:30Z

✅ Deploy Preview for karpenter-docs-prod canceled.

Name	Link
🔨 Latest commit	`53d0a77`
🔍 Latest deploy log	https://app.netlify.com/sites/karpenter-docs-prod/deploys/62bf328911719e0008e158d9

test/pkg/environment/expectations.go

tzneal · 2022-06-30T01:48:02Z

test/pkg/environment/expectations.go

+		for _, namespace := range namespaces.Items {
+			wg.Add(1)
+			go func(object client.Object, namespace string) {
+				env.Expect(env.Client.DeleteAllOf(env, object,


Maybe add &client.DeleteAllOfOptions{DeleteOptions: client.DeleteOptions{GracePeriodSeconds: ptr.Int64(0)}})

I wanted this to be closer to natural usage of clusters and include normal deletion flows rather than force deletion.

test/pkg/environment/expectations.go

pkg/test/provisioner.go

test/pkg/environment/environment.go

njtran

Let’s discuss how we want to separate tasks, clusterTasks, and pipelines in the filesystem. If someone’s importing all the files from GitHub this is quite easy and nice to find the file definitions in the repo as you've done it.

As the amount of configs increase: if someone’s doing this manually, they have to figure out how to find all related tasks then apply them manually. I think we should keep ClusterTasks in their own ClusterTasks folder and then any tasks in their respective suites. WDYT?

test/README.md

test/infrastructure/scripts/step-02-eksctl-cluster.sh

njtran · 2022-06-30T15:46:11Z

test/infrastructure/scripts/step-06-karpenter.sh

-helm upgrade --install --namespace karpenter --create-namespace \
-  karpenter karpenter/karpenter \
-  --version ${KARPENTER_VERSION} \
+helm upgrade --install --namespace karpenter --create-namespace karpenter --version v0.13.1 \


Let's stick to v0.9.0 here. That's the highest version of Karpenter tested in KIT right now while they figure out this issue. Vending the Karpenter installation for KIT can be possible in the future and where they would establish the Karpenter version we could use.

This is a really bad precedent to set. We can't be behind on karpenter in karpenter testing. Can't we set a higher ttl?

Vending the Karpenter installation for KIT can be possible in the future and where they would establish the Karpenter version we could use.

Is this something like kitctl?

Possibly. If this is an issue with pre-binding then I want to be able to figure this out. This was a fairly large change in assumptions for Karpenter so I don't think we'll be sticking here for long. We use a TTL of 180 right now.

test/infrastructure/scripts/step-07-provisioners.sh

njtran · 2022-06-30T15:57:34Z

test/tasks/before-karpenter-suite.yaml

+    - name: karpenter-version
+      default: latest
+    - name: account-id
+      default: "767520670908"


Let's remove this

njtran · 2022-06-30T15:58:01Z

test/tasks/before-karpenter-suite.yaml

+      kind: ClusterConfig
+      metadata:
+        name: $(params.cluster-name)
+        version: 1.22


eksctl v0.86.0 doesn't work with 1.22

njtran · 2022-06-30T15:58:51Z

test/tasks/before-karpenter-suite.yaml

+        withOIDC: true
+      EOF
+  - name: kubeconfig
+    image: docker.io/njtran/scratch:latest


you can use

Suggested change

image: docker.io/njtran/scratch:latest

image: public.ecr.aws/d2q6r4b4/minimal:latest

njtran · 2022-06-30T16:01:47Z

test/tasks/before-karpenter-suite.yaml

+        TEMPOUT=$(mktemp)
+        curl -fsSL https://karpenter.sh/"$(params.karpenter-version)"/getting-started/getting-started-with-eksctl/cloudformation.yaml  > $TEMPOUT \
+        && aws cloudformation deploy \
+          --stack-name "Karpenter-$(params.cluster-name)" \


This is going to collide with our karpenter installation if using the same cluster name as the management cluster. Can we make it the following? It may be better to change up the host-cluster setup too so that we don't run into the chance of IAM Role name conflicts.

Suggested change

--stack-name "Karpenter-$(params.cluster-name)" \

--stack-name "KarpenterTesting-$(params.cluster-name)" \

njtran · 2022-06-30T16:02:08Z

test/tasks/go-suite.yaml

+    - name: repository
+    - name: path


Can you put descriptions for these

njtran · 2022-06-30T17:51:19Z

test/infrastructure/scripts/step-07-provisioners.sh

    tags:
      kit.aws/substrate: ${CLUSTER_NAME}
    subnetSelector:
      karpenter.sh/discovery: ${CLUSTER_NAME}
    securityGroupSelector:
-      kubernetes.io/cluster/${CLUSTER_NAME}: owned
+      karpenter.sh/discovery: ${CLUSTER_NAME}


Sorry for the Churn. AWS Load Balancer discovers security groups like this. Without this tag discovery, KIT Guest clusters will not connect.

Suggested change

karpenter.sh/discovery: ${CLUSTER_NAME}

kubernetes.io/cluster/${CLUSTER_NAME}: owned

njtran · 2022-06-30T17:54:06Z

test/suites/integration/pipeline.yaml

+  name: karpenter-integration-suite
+spec:
+  params:
+    - name: commit


Can you add description for this

I think it's self documenting as is.

njtran · 2022-06-30T17:55:44Z

test/suites/integration/pipeline.yaml

+  tasks:
+  - name: github-action
+    taskRef:
+      name: github-action


Can we self-reference the type of object it is to make it clear?

Suggested change

name: github-action

name: github-action-cluster-task

Not a big fan of adding types to object names. deployments/foo-deployment stutters

njtran · 2022-06-30T18:12:47Z

pkg/test/pods.go

@@ -93,7 +93,7 @@ func Pod(overrides ...PodOptions) *v1.Pod {
 			Tolerations:               options.Tolerations,
 			InitContainers: []v1.Container{{
 				Name:      strings.ToLower(sequentialRandomName()),
-				Image:     options.Image,
+				Image:     "alpine",


Why are we hardcoding the image here if we're allowing pod images to be defined in the first place? Looks like we also have a default above on L78?

Pause doesn't work as an init container. Needed something that could exit.

njtran · 2022-06-30T18:13:41Z

pkg/test/provisioner.go

 			ProviderRef:          options.ProviderRef,
 			Taints:               options.Taints,
 			StartupTaints:        options.StartupTaints,
 			Labels:               options.Labels,
 			Limits:               &v1alpha5.Limits{Resources: options.Limits},
+			TTLSecondsAfterEmpty: ptr.Int64(10),


Any reason why we're picking 10 here? Can we parameterize empty and expired? I can imagine these are functions we'll need to test too.

Eventually sure -- I wanted to default for 10 until we need to override.

njtran · 2022-06-30T18:15:22Z

pkg/test/provisioner.go

 	provisioner.SetDefaults(context.Background())
-	_ = provisioner.Validate(context.Background())
+	if err := provisioner.Validate(context.Background()); err != nil {
+		logging.FromContext(context.TODO()).Info("TODO: Fix the tests that cause this")


Can we report the error here too? I'm unsure if this is a logged TODO that should be used when running tests or a TODO for yourself.

A ton of existing tests break if I fail here, since the error was originally squashed incorrectly. I wanted to log and clean it up later, since I'm OOTO for a few days and wanted to unblock folks.

njtran · 2022-06-30T18:38:53Z

test/infrastructure/scripts/step-01-config.sh

@@ -1,6 +1,5 @@
-export CLUSTER_NAME="${CLUSTER_NAME:-karpenter-test-cluster}"
+export CLUSTER_NAME="${CLUSTER_NAME:-karpenter-test-infrastructure}"


this would make the stack eksctl-karpenter-test-management-cluster

Suggested change

export CLUSTER_NAME="${CLUSTER_NAME:-karpenter-test-infrastructure}"

export CLUSTER_NAME="${CLUSTER_NAME:-karpenter-test-management}"

karpenter-test-management is less idiomatic than karpenter-test-infrastructure to me.

njtran

I think we have some disagreements on what the CI Infra should be right now. I think the ginkgo work looks great and am happy to approve that if we scope this one down.

test/pkg/environment/environment.go

njtran

Nice! Just a nit or two

njtran · 2022-07-01T16:26:20Z

test/pkg/environment/expectations.go

+
+var (
+	EnvironmentLabelName = "testing.karpenter.sh/environment-name"
+	CleanableObjects     = []client.Object{


How do we ensure that Nodes are cleaned up? I think the TTL on the Provisioners is meant to do that, but if a test fails to delete nodes (which could happen), then we want our Cleanup to also include those.

Ah I forgot that Nodes are owned by provisioners now, so this should be good.

We definitely need to test scale down, too. I think this library will need to be greatly expanded upon (e.g. wait for things to cleanup). This is just to get us started.

njtran · 2022-07-01T16:33:34Z

test/suites/integration/suite_test.go

+
+var _ = Describe("Sanity Checks", func() {
+	It("should provision nodes", func() {
+		provider := test.AWSNodeTemplate(test.AWSNodeTemplateOptions{AWS: v1alpha1.AWS{


If the Karpenter being tested here doesn't have a defaultInstanceProfile, this should fail. I think it's fine to leave as is, but I think we should make sure we can test both modes eventually.

Co-authored-by: Nick Tran <[email protected]>

ellistarn requested a review from a team as a code owner June 28, 2022 23:46

ellistarn requested a review from bwagner5 June 28, 2022 23:46

ellistarn marked this pull request as draft June 28, 2022 23:48

ellistarn force-pushed the testinfra branch 8 times, most recently from 4fa3c65 to 19db495 Compare June 30, 2022 00:48

ellistarn marked this pull request as ready for review June 30, 2022 00:56

ellistarn force-pushed the testinfra branch 2 times, most recently from 0210f59 to cbe1b62 Compare June 30, 2022 01:01

tzneal reviewed Jun 30, 2022

View reviewed changes

ellistarn force-pushed the testinfra branch 2 times, most recently from b468169 to 5a6c55a Compare June 30, 2022 04:41

njtran reviewed Jun 30, 2022

View reviewed changes

ellistarn force-pushed the testinfra branch 2 times, most recently from a86af3d to 3118682 Compare June 30, 2022 18:11

njtran reviewed Jun 30, 2022

View reviewed changes

Implemented ginkgo test against local kubeconfig

723b721

ellistarn force-pushed the testinfra branch from 3118682 to 723b721 Compare June 30, 2022 23:21

njtran suggested changes Jul 1, 2022

View reviewed changes

PR Comments

9d5c9bd

njtran reviewed Jul 1, 2022

View reviewed changes

test/pkg/environment/environment.go Outdated Show resolved Hide resolved

njtran reviewed Jul 1, 2022

View reviewed changes

Update test/pkg/environment/environment.go

53d0a77

Co-authored-by: Nick Tran <[email protected]>

njtran approved these changes Jul 1, 2022

View reviewed changes

njtran merged commit 4c35c0f into aws:main Jul 1, 2022

ellistarn deleted the testinfra branch July 7, 2022 17:19

This was referenced Jul 13, 2022

Fix failing lint checks #2128

Closed

feat: Adding Custom AMIFamily and validations #2154

Merged

suket22 mentioned this pull request Jul 21, 2022

docs: Add docs for customAMIs, tests for UserData #2169

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test: Implemented ginkgo test against local kubeconfig #2015

Test: Implemented ginkgo test against local kubeconfig #2015

ellistarn commented Jun 28, 2022 •

edited

Loading

netlify bot commented Jun 28, 2022 •

edited

Loading

tzneal Jun 30, 2022

ellistarn Jun 30, 2022

njtran left a comment

njtran Jun 30, 2022

ellistarn Jun 30, 2022

ellistarn Jun 30, 2022

njtran Jun 30, 2022

njtran Jun 30, 2022

njtran Jun 30, 2022

njtran Jun 30, 2022

njtran Jun 30, 2022

njtran Jun 30, 2022

njtran Jun 30, 2022

njtran Jun 30, 2022

ellistarn Jun 30, 2022

njtran Jun 30, 2022

ellistarn Jun 30, 2022 •

edited

Loading

njtran Jun 30, 2022

ellistarn Jun 30, 2022

njtran Jun 30, 2022

ellistarn Jun 30, 2022

njtran Jun 30, 2022

ellistarn Jun 30, 2022

njtran Jun 30, 2022

ellistarn Jun 30, 2022

njtran left a comment •

edited

Loading

njtran left a comment

njtran Jul 1, 2022

njtran Jul 1, 2022

ellistarn Jul 1, 2022

njtran Jul 1, 2022

	image: docker.io/njtran/scratch:latest
	image: public.ecr.aws/d2q6r4b4/minimal:latest

	--stack-name "Karpenter-$(params.cluster-name)" \
	--stack-name "KarpenterTesting-$(params.cluster-name)" \

	karpenter.sh/discovery: ${CLUSTER_NAME}
	kubernetes.io/cluster/${CLUSTER_NAME}: owned

		@@ -1,6 +1,5 @@
		export CLUSTER_NAME="${CLUSTER_NAME:-karpenter-test-cluster}"
		export CLUSTER_NAME="${CLUSTER_NAME:-karpenter-test-infrastructure}"

Test: Implemented ginkgo test against local kubeconfig #2015

Test: Implemented ginkgo test against local kubeconfig #2015

Conversation

ellistarn commented Jun 28, 2022 • edited Loading

netlify bot commented Jun 28, 2022 • edited Loading

✅ Deploy Preview for karpenter-docs-prod canceled.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njtran left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ellistarn Jun 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njtran left a comment • edited Loading

Choose a reason for hiding this comment

njtran left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ellistarn commented Jun 28, 2022 •

edited

Loading

netlify bot commented Jun 28, 2022 •

edited

Loading

ellistarn Jun 30, 2022 •

edited

Loading

njtran left a comment •

edited

Loading