CNI Accptance test on Kind #1445

curtbushko · 2022-08-24T15:23:49Z

Background: In order to run acceptance tests on kind there needs to be some extra setup. Kind needs the Calico CNI plugin to be installed first and Kind needs its network to match what Calico expects.

Some tests have been disabled as running them with CNI is redundant to other acceptance tests.

There is no magic here. Helm install the CNI plugin and the tests are run. Everything should work the same as normal.

Changes proposed in this PR:

Added a new make target for installing Calico in Kind
Added a helper make target for setting up local Kind with Calico just in case anyone wants to run it.
Added a kind.config for setting up the Kind cluster
Added a new -enable-cni flag to the acceptance test config so that it can be passed through from CircleCI
Added a new nightly circleci job for running the CNI kind tests
I had missed a bats test for the connect-inject template that I merged in a previous PR

Note to Iryna, I added connect-inject-init user uid to the redirect PR last night so not to muddle this PR.

How I've tested this PR:

"nightly" circle CI and local runs

How I expect reviewers to test this PR:

👀

Checklist:

Tests added
CHANGELOG entry added

HashiCorp engineers only, community PRs should not add a changelog entry.
Entries should use present tense (e.g. Add support for...)

curtbushko · 2022-08-24T16:19:40Z

.circleci/config.yml

+      - store_artifacts:
+          path: /tmp/test-results
+      - slack/status:
+          # temporarily sending to #cni-acceptance-tests channel


Will remove before merge.

curtbushko · 2022-08-24T16:19:52Z

.circleci/config.yml

-      - schedule:
-          cron: "0 0 * * *"
-          filters:
-            branches:


Will fix before merge.

curtbushko · 2022-08-24T16:20:09Z

.circleci/config.yml

@@ -1027,29 +1076,33 @@ workflows:
      - dev-upload-docker:
          requires:
            - build-distros-linux
-      - cleanup-gcp-resources


Will fix before merge.

curtbushko · 2022-08-24T16:20:20Z

.circleci/config.yml

-            - cleanup-azure-resources
-            - dev-upload-docker
-      - acceptance-kind-1-23:
+      # - acceptance-gke-1-20:


Will fix before merge.

curtbushko · 2022-08-24T16:27:33Z

charts/consul/test/unit/cni-daemonset.bats

@@ -135,7 +135,7 @@ rollingUpdate:
      --set 'connectInject.enabled=true' \
      . | tee /dev/stderr |
      yq -rc '.spec.template.spec.containers[0].resources' | tee /dev/stderr)
-  [ "${actual}" = '{"limits":{"cpu":"75m","memory":"75Mi"},"requests":{"cpu":"50m","memory":"50Mi"}}' ]
+  [ "${actual}" = '{"limits":{"cpu":"100m","memory":"100Mi"},"requests":{"cpu":"75m","memory":"75Mi"}}' ]


This needed to be increased because the CNI installer was getting OOMKilled on Kind.

curtbushko · 2022-08-24T16:28:23Z

charts/consul/test/unit/connect-inject-clusterrole.bats

@@ -51,6 +51,9 @@ load _helpers
  local actual=$(echo $object | yq -r '.resources[| index("namespaces")' | tee /dev/stderr)
  [ "${actual}" != null ]

+  local actual=$(echo $object | yq -r '.resources[| index("nodes")' | tee /dev/stderr)


I had added 'node' back to the connect inject cluster role in a previous PR (yesterday) and I missed the bats test for it.

curtbushko · 2022-08-24T16:28:44Z

charts/consul/values.yaml

        memory: "75Mi"
        cpu: "75m"
+      limits:


This needed to be increased because the CNI installer was getting OOMKilled on Kind.

ishustava · 2022-08-24T17:23:49Z

.circleci/config.yml

@@ -878,6 +890,42 @@ jobs:
          fail_only: true
          failure_message: "Acceptance tests against Kind with Kubernetes v1.23 failed. Check the logs at: ${CIRCLE_BUILD_URL}"

+  acceptance-kind-cni-1-23:


could we add a job for the test-and-build pipeline as well so that it runs on every PR?

ishustava · 2022-08-24T17:24:38Z

.circleci/config.yml

+          command: |
+            kind create cluster --config=acceptance/framework/environment/cni-kind/kind.config --name dc1 --image kindest/node:<< parameters.version >>
+            make kind-cni-calico
+            sleep 2


why do we need to sleep here? wouldn't finish setup while the other kind cluster is being created?

ishustava · 2022-08-24T17:27:20Z

acceptance/framework/config/config.go

@@ -83,6 +85,11 @@ func (t *TestConfig) HelmValuesFromConfig() (map[string]string, error) {
 		setIfNotEmpty(helmValues, "global.enablePodSecurityPolicies", "true")
 	}

+	if t.EnableCNI {
+		setIfNotEmpty(helmValues, "connectInject.enabled", "true")


I don't think we should set connectInject. For tproxy we're only setting the tproxy value so that for tests that don't use connect inject you don't end up running it because of the global flag.

That makes sense.

Changed.

ishustava · 2022-08-24T17:28:01Z

acceptance/framework/flags/flags.go

@@ -85,6 +87,10 @@ func (t *TestFlags) init() {
 	flag.BoolVar(&t.flagEnablePodSecurityPolicies, "enable-pod-security-policies", false,
 		"If true, the test suite will run tests with pod security policies enabled.")

+	flag.BoolVar(&t.flagEnableCNI, "enable-cni", false,
+		"If true, the test suite will run tests with consul-cni plugin enabled. "+
+			"In general, this will only run against tests that are mesh related (connect, dns, mesh-gateway, peering, etc")


dns test is not a mesh test

ishustava · 2022-08-24T17:29:06Z

acceptance/tests/connect/connect_inject_namespaces_test.go

@@ -71,6 +73,7 @@ func TestConnectInjectNamespaces(t *testing.T) {
 			helmValues := map[string]string{
 				"global.enableConsulNamespaces": "true",
 				"connectInject.enabled":         "true",
+				"connectInject.cni.enabled":     strconv.FormatBool(cfg.EnableCNI),


why do we need to set it here? wouldn't it be set by the framework?

You are correct. I am setting this all over the place and there is no need. I only discovered my mistake after I started the GKE acceptance tests.

I will remove and shrink this PR by quite a bit.

These have been removed.

…connectInject.cni.enabled from several files as the framework will set this when the flag is passed in

ishustava · 2022-08-25T14:54:02Z

.circleci/config.yml

@@ -1007,18 +1054,22 @@ workflows:
          context: consul-ci
          requires:
            - dev-upload-docker
+      - acceptance-kind-cni-1-23:


Also, we should use the same kind version as other acceptance tests in this pipeline

Suggested change

- acceptance-kind-cni-1-23:

- acceptance-tproxy-cni:

ishustava

Great work Curt! Approving, but I left one small comment

- Added a new make target for installing Calico CNI in Kind. The target installs Calico CNI using config files located under /acceptance tests - Added a helper make target for setting up local Kind with Calico just in case anyone wants to run it. - Added a kind.config for setting up the Kind cluster - Added an -enable-cni flag to the acceptance test config so that it can be passed through from CircleCI - Added a nightly circleci job for running the CNI kind tests - I had missed a bats test for the connect-inject template that I merged in a previous PR

curtbushko added 5 commits August 24, 2022 11:07

Add CNI acceptance tests on Kind

8ef2c28

missed setting cni.enabled for one mesh test

e49970c

fix lint

be0ad22

missed the resource increase bats test for cni-daemonset

e9d029e

revert a line in circle ci config

691f10a

curtbushko commented Aug 24, 2022

View reviewed changes

.circleci/config.yml Outdated

- schedule:

cron: "0 0 * * *"

filters:

branches:

Copy link

Contributor Author

curtbushko Aug 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix before merge.

curtbushko commented Aug 24, 2022

View reviewed changes

curtbushko requested review from ishustava and t-eckert August 24, 2022 17:18

curtbushko added theme/kubernetes theme/testing Related to testing labels Aug 24, 2022

ishustava reviewed Aug 24, 2022

View reviewed changes

curtbushko added 2 commits August 24, 2022 15:48

Add CNI Kind tests to test-and-build, remove unneeded sleeps, remove …

b95c83c

…connectInject.cni.enabled from several files as the framework will set this when the flag is passed in

disable for tests that do no use connectInject.enabled

bce75ed

curtbushko requested a review from ishustava August 24, 2022 21:49

curtbushko self-assigned this Aug 25, 2022

This was referenced Aug 25, 2022

CNI Acceptance test on GKE #1446

Merged

CNI Acceptance tests on EKS #1447

Merged

ishustava reviewed Aug 25, 2022

View reviewed changes

ishustava approved these changes Aug 25, 2022

View reviewed changes

curtbushko mentioned this pull request Aug 25, 2022

CNI acceptance tests on AKS #1449

Merged

2 tasks

curtbushko added 3 commits August 25, 2022 11:49

use a separate target for test-and-build for acceptance-tproxy-cni

a3814e2

check-in the calico yaml files because calicos website is unstable

92ab438

turn on nightly schedules and the rest of the jobs

d3feb5e

t-eckert approved these changes Aug 25, 2022

View reviewed changes

curtbushko merged commit ff76ee9 into cni Aug 25, 2022

curtbushko deleted the curtbushko/cni-acceptance-kind branch August 25, 2022 19:23

curtbushko restored the curtbushko/cni-acceptance-kind branch August 25, 2022 19:28

curtbushko deleted the curtbushko/cni-acceptance-kind branch August 26, 2022 02:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CNI Accptance test on Kind #1445

CNI Accptance test on Kind #1445

curtbushko commented Aug 24, 2022

curtbushko Aug 24, 2022 •

edited

Loading

curtbushko Aug 24, 2022

curtbushko Aug 24, 2022

curtbushko Aug 24, 2022

curtbushko Aug 24, 2022

curtbushko Aug 24, 2022

curtbushko Aug 24, 2022

ishustava Aug 24, 2022

curtbushko Aug 24, 2022

ishustava Aug 24, 2022

curtbushko Aug 24, 2022

ishustava Aug 24, 2022

curtbushko Aug 24, 2022 •

edited

Loading

ishustava Aug 24, 2022

curtbushko Aug 24, 2022

ishustava Aug 24, 2022

curtbushko Aug 24, 2022

curtbushko Aug 24, 2022

ishustava Aug 25, 2022

ishustava left a comment

CNI Accptance test on Kind #1445

CNI Accptance test on Kind #1445

Conversation

curtbushko commented Aug 24, 2022

curtbushko Aug 24, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

curtbushko Aug 24, 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

ishustava left a comment

Choose a reason for hiding this comment

curtbushko Aug 24, 2022 •

edited

Loading

curtbushko Aug 24, 2022 •

edited

Loading