Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Test cases for EnvVars check on CNI daemonset #1431

Merged
merged 13 commits into from
May 6, 2021

Conversation

cgchinmay
Copy link
Contributor

What type of PR is this?
Added ginkgo test for testing env vars on cni daemonset

What does this PR do / Why do we need it:
Need CNI automation using Ginkgo framework

If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:

cd test/integration-new/cni/env_vars
ginkgo -v --failOnPending -- --cluster-kubeconfig=<kubeconfig> --cluster-name=<cluster>--aws-region=<region>--aws-vpc-id=<vpc-id>

Testing done on this change:

Running Suite: Cni Suite
========================
Random Seed: 1618598445
Will run 3 of 3 specs

cni env test CNI Environment Variables 
  Verifying that secondary ENI is created
  /Users/cgadgil/Documents/amazon-vpc-cni-k8s/test/integration-new/cni/env_vars/env_test.go:22
•
------------------------------
cni env test CNI Environment Variables 
  Changing AWS_VPC_ENI_MTU and AWS_VPC_K8S_CNI_VETHPREFIX
  /Users/cgadgil/Documents/amazon-vpc-cni-k8s/test/integration-new/cni/env_vars/env_test.go:36
STEP: getting the aws-node daemon set in namesapce kube-system
STEP: setting the environment variables on the ds to map[AWS_VPC_ENI_MTU:1300 AWS_VPC_K8S_CNI_VETHPREFIX:veth]
STEP: updating the daemon set with new environment variable
STEP: Deploying a BusyBox deployment
STEP: Validating new MTU value
STEP: Validating new VETH Prefix
STEP: Deleting BusyBox Deployment
STEP: Restoring old value on daemonset
STEP: getting the aws-node daemon set in namesapce kube-system
STEP: setting the environment variables on the ds to map[AWS_VPC_ENI_MTU:1300 AWS_VPC_K8S_CNI_VETHPREFIX:veth]
STEP: updating the daemon set with new environment variable
•
------------------------------
cni env test CNI Environment Variables 
  Changing AWS_VPC_K8S_CNI_LOG_FILE
  /Users/cgadgil/Documents/amazon-vpc-cni-k8s/test/integration-new/cni/env_vars/env_test.go:101
STEP: getting the aws-node daemon set in namesapce kube-system
STEP: setting the environment variables on the ds to map[AWS_VPC_K8S_CNI_LOG_FILE:/host/var/log/aws-routed-eni/ipamd_test.log]
STEP: updating the daemon set with new environment variable
STEP: Restoring old value on daemonset
STEP: getting the aws-node daemon set in namesapce kube-system
STEP: setting the environment variables on the ds to map[AWS_VPC_K8S_CNI_LOG_FILE:/host/var/log/aws-routed-eni/ipamd-logs/ipamd.log]
STEP: updating the daemon set with new environment variable

• [SLOW TEST:88.605 seconds]
cni env test
/Users/cgadgil/Documents/amazon-vpc-cni-k8s/test/integration-new/cni/env_vars/env_test.go:19
  CNI Environment Variables
  /Users/cgadgil/Documents/amazon-vpc-cni-k8s/test/integration-new/cni/env_vars/env_test.go:21
    Changing AWS_VPC_K8S_CNI_LOG_FILE
    /Users/cgadgil/Documents/amazon-vpc-cni-k8s/test/integration-new/cni/env_vars/env_test.go:101
------------------------------

Ran 3 of 3 Specs in 99.584 seconds
SUCCESS! -- 3 Passed | 0 Failed | 0 Pending | 0 Skipped
PASS

Will this break upgrades or downgrades. Has updating a running cluster been tested?:
No

Does this change require updates to the CNI daemonset config files to work?:

Assumes using latest CNI

Does this PR introduce any user-facing change?:

No

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

Copy link
Contributor

@jayanthvn jayanthvn left a comment

Choose a reason for hiding this comment

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

LGTM :)..will wait for Abhinav to review.

test/framework/options.go Outdated Show resolved Hide resolved
test/framework/resources/k8s/resources/deployment.go Outdated Show resolved Hide resolved
test/integration-new/cni/env_vars/env_suite_test.go Outdated Show resolved Hide resolved
test/integration-new/cni/env_vars/env_test.go Outdated Show resolved Hide resolved
test/integration-new/cni/env_vars/env_test.go Outdated Show resolved Hide resolved
test/integration-new/cni/env_vars/env_test.go Outdated Show resolved Hide resolved
test/integration-new/cni/env_vars/env_test.go Outdated Show resolved Hide resolved
test/framework/options.go Outdated Show resolved Hide resolved
test/framework/resources/k8s/manifest/container.go Outdated Show resolved Hide resolved
test/framework/resources/k8s/manifest/deployment.go Outdated Show resolved Hide resolved
test/integration-new/cni/eni_config/config_suite_test.go Outdated Show resolved Hide resolved
test/integration-new/cni/eni_config/env_test.go Outdated Show resolved Hide resolved
test/integration-new/cni/eni_config/ipleak_test.go Outdated Show resolved Hide resolved
- folder restructuring
test/integration-new/ipamd/ipamd_test.go Outdated Show resolved Hide resolved
test/integration-new/ipamd/ipamd_test.go Outdated Show resolved Hide resolved
test/integration-new/env_vars/env_vars_suite_test.go Outdated Show resolved Hide resolved
# Conflicts:
#	test/integration-new/ipamd/ipamd_suite_test.go
# Conflicts:
#	test/framework/resources/k8s/manifest/deployment.go
#	test/framework/resources/k8s/resources/node.go
#	test/go.mod
Removed unnecessary tests
Addressed PR comments
Copy link
Contributor

@abhipth abhipth left a comment

Choose a reason for hiding this comment

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

LGTM

Following improvements can be made in subsequent iterations

  • Use test helper docker image to test MTU
  • Instead of deploying a deployment with single replica and getting the pod from the deployment. Deploy a single pod. The utility method for the same will be available once the PR is merged.

test/integration-new/ipamd/ipamd_test.go Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants