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

integration tests running with updated aws-k8s-tester #766

Merged
merged 3 commits into from
Jan 20, 2020

Conversation

jaypipes
Copy link
Contributor

@jaypipes jaypipes commented Dec 19, 2019

Checks that there is an ECR repo before we use aws-k8s-tester to actually
create the test cluster. In addition, trap errors in the main
run-integration-tests.sh script and deprovision cluster if an error
occurred after creating the cluster.

Adds some guardrails to the scripts/run-integration_tests.sh script by
checking (before creating an EKS cluster) that AWS credentials are
present and that the required ECR repository is readable.

The names of environment variables accepted by aws-k8s-tester changed
when the managed node group functionality was introduced. This commit
updates the integration testing scripts to call aws-k8s-tester (v.0.5.4
which is the release needed with the fix for aws/aws-k8s-tester#70) with
these updated environment variables.

We decrease the number of parallel builds of the echo job from 100 to
3 and the number of completions for that job from 1000 to 30. This
decreases the setup time of the cluster by about 10 minutes.

Finally, I added in a short-circuit to prevent double-deprovisioning of
the cluster if, say, a stacktrace occurred when running the
aws-k8s-tester tool.

Issue #686
Issue #784
Issue #786

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

Copy link
Contributor

@mogren mogren left a comment

Choose a reason for hiding this comment

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

LGTM

if [[ "$PROVISION" = true ]]; then
up-test-cluster
__cluster_created=1
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@jaypipes jaypipes force-pushed the e2e-test-cleanup branch 2 times, most recently from 12bdad1 to f784b4a Compare December 20, 2019 16:50
@jaypipes jaypipes changed the title do pre-checks before creating EKS test cluster WIP/DNM - do pre-checks before creating EKS test cluster Dec 21, 2019
Adds some guardrails to the scripts/run-integration_tests.sh script by
checking (before creating an EKS cluster) that AWS credentials are
present and that the required ECR repository is readable.
Ensure that there is an ECR repo before we use aws-k8s-tester to actually
create the test cluster. In addition, trap errors in the main
run-integration-tests.sh script and deprovision cluster if an error
occurred after creating the cluster.

Updates the required aws-k8s-tester release to 0.5.2 to handle
aws/aws-k8s-tester#66

Issue aws#765
@jaypipes jaypipes changed the title WIP/DNM - do pre-checks before creating EKS test cluster integration tests running with updated aws-k8s-tester Jan 3, 2020
@mogren mogren added the testing label Jan 8, 2020
Copy link
Contributor

@mogren mogren left a comment

Choose a reason for hiding this comment

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

# login to the ECR registry
eval $(aws ecr get-login --region $AWS_REGION --no-include-email) >/dev/null 2>&1
ensure_ecr_repo "$AWS_ACCOUNT_ID" "$AWS_ECR_REPO_NAME"
make docker IMAGE=$IMAGE_NAME VERSION=$IMAGE_VERSION
Copy link
Contributor

@SaranBalaji90 SaranBalaji90 Jan 16, 2020

Choose a reason for hiding this comment

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

This doesn't pull source code for a given tag(version) right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it doesn't. It runs docker build -t aws-vpc-cni:$IMAGE_VERSION. If IMAGE_VERSION envvar has been overridden, we should probably set the BUILD envvar to false automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Either that, or check that IMAGE_VERSION is overridden and if so, do a git checkout $IMAGE_VERSION here to switch the working tree to that git tag.

Copy link
Contributor

Choose a reason for hiding this comment

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

using git-checkout makes sense. Can you help me understand how disabling BUILD will help? Do you mean to say, if CNI image is already available in my repo then disabling BUILD will help?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, exactly. if you've already pushed, say, a v1.5.5 CNI image built from that git tag/branch, then setting BUILD=false will prevent the run-integration-tests.sh script from docker pushing an image and the eks-k8s-tester binary will simply pull the image that was in the ECR repo for v1.5.5

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok makes sense. Thanks for clarifying.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In short, I need to make some changes to this script to facilitate the not-building-from-local-checkout scenarios...

The names of environment variables accepted by aws-k8s-tester changed
when the managed node group functionality was introduced. This commit
updates the integration testing scripts to call aws-k8s-tester (v.0.5.4
which is the release needed with the fix for aws/aws-k8s-tester#70) with
these updated environment variables.

We decrease the number of parallel builds of the echo job from 100 to
3 and the number of completions for that job from 1000 to 30. This
decreases the setup time of the cluster by about 10 minutes.

Finally, I added in a short-circuit to prevent double-deprovisioning of
the cluster if, say, a stacktrace occurred when running the
aws-k8s-tester tool.

Issue aws#686
Issue aws#784
Issue aws#786
@mogren mogren merged commit 8291df3 into aws:master Jan 20, 2020
@jaypipes jaypipes deleted the e2e-test-cleanup branch January 29, 2020 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants