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

e2e tests #606

Closed
wants to merge 1 commit into from
Closed

e2e tests #606

wants to merge 1 commit into from

Conversation

akutz
Copy link
Contributor

@akutz akutz commented Feb 23, 2019

What this PR does / why we need it:
This patch updates documentation, the Makefile, and creates some new programs under the hack directory in order to make e2e testing as easy as possible, both locally and as a invocation point for Prow jobs.

This is a work-in-progress, and the PR is being opened as a point of discussion and to provide an opportunity for the community to give feedback early in the process.

Blocked by:

  1. boskos: AWS janitor region via env var kubernetes/test-infra#11425
  2. boskos: AWS janitor not cleaning up all of the resources kubernetes/test-infra#11471
  3. AWS janitor clean -all option kubernetes/test-infra#11482
  4. aws-janitor: Support for CloudFormation Stacks kubernetes/test-infra#11504

/hold

Which issue(s) this PR fixes:
Partially-Sorta-Kinda-Phixes #272 (see Special notes)

Special notes for your reviewer:
This PR alone will not phix #272 as a period Prow job will need to be created. However, the content of this PR will be how Prow initiates the e2e testing.

Release note:

Execute unit tests with "make test", integration tests with "make integration", and end-to-end tests with "make e2e".

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 23, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: akutz
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: vincepri

If they are not already assigned, you can assign the PR to them by writing /assign @vincepri in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 23, 2019
@akutz akutz changed the title Easy e2e tests WiP: Easy e2e tests Feb 23, 2019
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 23, 2019
@akutz akutz changed the title WiP: Easy e2e tests WiP: e2e tests Feb 23, 2019
@akutz akutz force-pushed the feature/easy-e2e branch 4 times, most recently from b73a326 to 654721e Compare February 23, 2019 21:20
docs/development.md Outdated Show resolved Hide resolved
@akutz akutz force-pushed the feature/easy-e2e branch 3 times, most recently from c46f48d to 031bc0a Compare February 25, 2019 22:41
@akutz
Copy link
Contributor Author

akutz commented Feb 25, 2019

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 25, 2019
@akutz akutz changed the title WiP: e2e tests e2e tests Feb 25, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 25, 2019
akutz added a commit to akutz/test-infra that referenced this pull request Feb 26, 2019
This patch deploys a new Boskos janitor as a service in the same cluster
running Prow. The new service is named `boskos-janitor-aws`.

Related to kubernetes-sigs/cluster-api-provider-aws#272 and
kubernetes-sigs/cluster-api-provider-aws#606.

/area boskos
akutz added a commit to akutz/test-infra that referenced this pull request Feb 26, 2019
This patch deploys a new Boskos janitor as a service in the same cluster
running Prow. The new service is named `boskos-janitor-aws`.

Related to kubernetes-sigs/cluster-api-provider-aws#272 and
kubernetes-sigs/cluster-api-provider-aws#606.

/area boskos
Copy link
Member

@detiber detiber left a comment

Choose a reason for hiding this comment

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

Some minor nits.

The PR description both says that it fixes #272 and that it doesn't fully resolve #272, can you update the description?

docs/development.md Outdated Show resolved Hide resolved
hack/e2e-aws-janitor.sh Outdated Show resolved Hide resolved
hack/e2e-aws-resources-list.sh Outdated Show resolved Hide resolved
hack/e2e.sh Outdated Show resolved Hide resolved
@akutz
Copy link
Contributor Author

akutz commented Feb 26, 2019

Hi Jason,

Regarding the PR description, there will be no single PR in this repo that fixes 272 as it will take a series of PRs across this and test infra. That’s why I said it partially fixed the issue. To me it’s a problem with the fixes notation that it only applies to a single issue. How do you recommend we use that when a resolution takes more than one PR across multiple repos? That would typically be an epic, but I don’t see that you’ve got ZenHub or GH projects set up to track epics.

.PHONY: integration
integration: generate verify ## Run integraion tests
bazel test --define='gotags=integration' --test_output all //test/integration/...

Copy link
Contributor

@ashish-amarnath ashish-amarnath Feb 26, 2019

Choose a reason for hiding this comment

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

Suggested change
# WARNING: DO NOT SET THE `JANITOR_ENABLED` TO 1 UNLESS YOU WISH TO TEARDOWN RESOURCES IN AWS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @ashish-amarnath,

Are you suggesting the definition of a target named WARNING? If so, do you mean the text should be echoed? I am sorry, but I do not understand the proposed change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I missed this comment.
The suggestion was to add a comment to caution users not to change the JANITOR_ENABLED flag.

@randomvariable
Copy link
Member

@akutz I think @detiber means you should remove the Fixes: #272 wording, as Github will automatically close the issue. Use Partially addresses #272 or similar so it doesn't trip Github trying to be too helpful.

akutz added a commit to akutz/test-infra that referenced this pull request Feb 26, 2019
This patch deploys a new Boskos janitor as a service in the same cluster
running Prow. The new service is named `boskos-janitor-aws`.

Related to kubernetes-sigs/cluster-api-provider-aws#272 and
kubernetes-sigs/cluster-api-provider-aws#606.

/area boskos
akutz added a commit to akutz/test-infra that referenced this pull request Feb 26, 2019
This patch deploys a new Boskos janitor as a service in the same cluster
running Prow. The new service is named `boskos-janitor-aws`.

Related to kubernetes-sigs/cluster-api-provider-aws#272 and
kubernetes-sigs/cluster-api-provider-aws#606.

/area boskos
akutz added a commit to akutz/test-infra that referenced this pull request Feb 26, 2019
This patch deploys a new Boskos janitor as a service in the same cluster
running Prow. The new service is named `boskos-janitor-aws`.

Related to kubernetes-sigs/cluster-api-provider-aws#272 and
kubernetes-sigs/cluster-api-provider-aws#606.

/area boskos
akutz added a commit to akutz/test-infra that referenced this pull request Feb 26, 2019
This patch deploys a new Boskos janitor as a service in the same cluster
running Prow. The new service is named `boskos-janitor-aws`.

Related to kubernetes-sigs/cluster-api-provider-aws#272 and
kubernetes-sigs/cluster-api-provider-aws#606.

/area boskos
@akutz
Copy link
Contributor Author

akutz commented Feb 26, 2019

@akutz I think @detiber means you should remove the Fixes: #272 wording, as Github will automatically close the issue. Use Partially addresses #272 or similar so it doesn't trip Github trying to be too helpful.

Hi @randomvariable,

I understand what he means. I just wanted to point out that the PR template pushes people to indicates fixes, even when it's partially applicable. I'll remove it though.

@randomvariable
Copy link
Member

Enjoying the terminology

@akutz
Copy link
Contributor Author

akutz commented Feb 26, 2019

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 26, 2019

# Prevent a disallowed AWS key from being used.
if grep -iqF "$(echo "${AWS_ACCESS_KEY_ID-}" | \
{ md5sum || md5; } | awk '{print $1}')" hack/e2e-aws-disallowed.txt; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix stderr

This patch updates documentation, the Makefile, and creates some new
programs under the `hack` directory in order to make e2e testing as easy
as possible, both locally and as a invocation point for Prow jobs.
@vincepri
Copy link
Member

vincepri commented Mar 7, 2019

/milestone v1alpha1

@k8s-ci-robot k8s-ci-robot added this to the v1alpha1 milestone Mar 7, 2019
@detiber
Copy link
Member

detiber commented Mar 14, 2019

closing, since #658 has merged

@detiber detiber closed this Mar 14, 2019
detiber pushed a commit to detiber/test-infra that referenced this pull request Mar 20, 2019
This patch deploys a new Boskos janitor as a service in the same cluster
running Prow. The new service is named `boskos-janitor-aws`.

Related to kubernetes-sigs/cluster-api-provider-aws#272 and
kubernetes-sigs/cluster-api-provider-aws#606.

/area boskos
ixdy pushed a commit to ixdy/boskos that referenced this pull request May 14, 2020
This patch deploys a new Boskos janitor as a service in the same cluster
running Prow. The new service is named `boskos-janitor-aws`.

Related to kubernetes-sigs/cluster-api-provider-aws#272 and
kubernetes-sigs/cluster-api-provider-aws#606.

/area boskos
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants