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

Create roles by default for e2e test cluster creation #994

Merged
merged 23 commits into from
May 27, 2020

Conversation

bnapolitan
Copy link
Contributor

@bnapolitan bnapolitan commented May 26, 2020

#858

  • Force creating role ARN and S3 bucket when creating test cluster.
  • Removed a couple slow tests as a start for time optimization.

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

@mogren mogren self-requested a review May 26, 2020 17:10
@mogren mogren added the testing label May 26, 2020
@bnapolitan bnapolitan marked this pull request as draft May 26, 2020 17:12
@bnapolitan bnapolitan marked this pull request as ready for review May 26, 2020 19:53
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.

Nice! 😄

Two more changes needed:

  • Add env variables that we can override
  • Squash commits

scripts/run-integration-tests.sh Show resolved Hide resolved
scripts/lib/cluster.sh Outdated Show resolved Hide resolved
bnapolitan and others added 9 commits May 27, 2020 12:28
Create role when spinning up tester.

Change to role create in second field.

Insert my own arn.

Set create for s3 tester bucket.

Create s3 bucket empty name.

Revert back to role creation.

Region changes from CircleCI environment variable, and comment out default test.

See if commented tests prevent the large download from occurring (and can we move that download to earlier in the process?

Test polling for available pods.

Revert polling solution and skip slow conformance tests.
Autogenerate per-region manifests from a common template in order to
prevent drift across versions.

Also: Separate in-development (unreleased) manifest changes from the
most recent release, by creating a `config/master` directory.  The idea
is that on each new vX.Y release, we:
- copy the latest version of config/master to config/vX.Y
- set version = `vX.Y` near top of vX.Y/manifests.jsonnet
- re-run `make generate`

I have confirmed (using a yaml diff tool) that the generated manifests
are identical to latest config/v1.6, except for:
- re-ordering of documents within the top-level YAML stream
- re-ordering of container `env` array
- "v1.6.1" docker tag (intentionally) replaced with "latest" (because
  this is config/master)
Default to testing upgrades to `config/master` manifests in
`run-integration-tests.sh`.  Follow-on to aws#986.

Correct internal comment re starting version: we test upgrades from
whatever version is installed by `eks create cluster`.

Also: Switch to shell `${foo:=bar}` idiom to reduce duplication (and
chance for typos) in `foo=${foo:-bar}`.
Signed-off-by: Gyuho Lee <leegyuho@amazon.com>
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!

Comment on lines +81 to +86
: ${ROLE_CREATE:=true}
: ${ROLE_ARN:=""}

# S3 bucket initialization
: ${S3_BUCKET_CREATE:=true}
: ${S3_BUCKET_NAME:=""}
Copy link
Contributor

Choose a reason for hiding this comment

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

Great, thanks!

@mogren mogren merged commit e1dfc72 into aws:master May 27, 2020
@bnapolitan bnapolitan deleted the test-circle-ci branch May 27, 2020 20:07
@mogren mogren added this to the v1.7.0 milestone Jul 8, 2020
@mogren mogren changed the title Create roles when upping cluster. Create roles by default for e2e test cluster creation Jul 8, 2020
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