-
Notifications
You must be signed in to change notification settings - Fork 741
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
Conversation
There was a problem hiding this 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
12bdad1
to
f784b4a
Compare
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
f784b4a
to
5b7a46f
Compare
There was a problem hiding this 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 push
ing an image and the eks-k8s-tester binary will simply pull the image that was in the ECR repo for v1.5.5
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...
5b7a46f
to
3fb53de
Compare
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
3fb53de
to
cf66674
Compare
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.