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

Add env files for Kind-in-Prow integration test jobs #5077

Merged
merged 1 commit into from
Jul 14, 2022

Conversation

abayer
Copy link
Contributor

@abayer abayer commented Jul 5, 2022

Changes

The existing test/e2e-tests-kind.env is specifically for the PipelineRun approach. These new files are for running the e2e tests, via kind, in Prow, which will replace the current approach of spinning up GKE clusters.

There are two new env files - one each for stable and alpha. Originally, there were four, splitting stable and alpha into YAML and non-YAML jobs, but once we moved the kind jobs to dedicated n1-standard-4 nodes, rather than sharing nodes with all other jobs, the performance difference between running all of the tests and just the YAML or non-YAML wasn't significant enough to merit the split (22-26 minutes for running all of the tests, vs 15-18 minutes for each of the YAML and non-YAML jobs).

Additionally, examples/v1beta1/taskruns/git-volume.yaml is moved to examples/v1beta1/taskruns/no-ci/git-volume.yaml. This is because Kind nodes don't have git installed, which is necessary for git volumes to work. Also, --ignore-path=/product_uuid has been added to the Kaniko args in examples/v1beta1/pipelineruns/pipelinerun.yaml and test/yamls/v1beta1/pipelineruns/pipelinerun.yaml to work around an issue with Kaniko multi-stage builds on Kind (GoogleContainerTools/kaniko#2164).

/kind misc

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Docs included if any changes are user facing
  • Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Release notes block below has been filled in
    (if there are no user facing changes, use release note "NONE")

Release Notes

NONE

@tekton-robot
Copy link
Collaborator

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@tekton-robot tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesnt merit a release note. kind/misc Categorizes issue or PR as a miscellaneuous one. labels Jul 5, 2022
@tekton-robot tekton-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 5, 2022
@afrittoli
Copy link
Member

For kind jobs, perhaps we could use dedicated cluster/job for YAML tests? The YAML ones seems to be the more flaky ones, so it could help isolate the issue. @lbernick

@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

@afrittoli Good idea - I'm mainly opening this PR to test tektoncd/plumbing#1118, so we can tweak before actually merging anything into Pipeline.

@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

/test pull-tekton-pipeline-kind-integration-tests

2 similar comments
@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

/test pull-tekton-pipeline-kind-integration-tests

@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

/test pull-tekton-pipeline-kind-integration-tests

@abayer abayer force-pushed the prep-for-prowjob-kind-e2e-tests branch from d9bf8fb to 9b657cb Compare July 5, 2022 14:45
@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

/test pull-tekton-pipeline-kind-integration-tests

@abayer abayer force-pushed the prep-for-prowjob-kind-e2e-tests branch from 9b657cb to 3d519ce Compare July 5, 2022 18:11
@tekton-robot tekton-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 5, 2022
@abayer abayer force-pushed the prep-for-prowjob-kind-e2e-tests branch from 3d519ce to fd8684a Compare July 5, 2022 18:24
@abayer abayer changed the title Add env file for Kind-in-Prow integration test job Add env files for Kind-in-Prow integration test jobs Jul 5, 2022
@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

/test pull-tekton-pipeline-kind-integration-tests
/test pull-tekton-pipeline-kind-alpha-integration-tests
/test pull-tekton-pipeline-kind-yaml-tests
/test pull-tekton-pipeline-kind-alpha-yaml-tests

@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

/test pull-tekton-pipeline-build-tests
/test pull-tekton-pipeline-integration-tests
/test pull-tekton-pipeline-unit-tests

@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

/test pull-tekton-pipeline-alpha-integration-tests

@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

/test pull-tekton-pipeline-unit-tests

@tekton-robot
Copy link
Collaborator

@abayer: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-tekton-pipeline-alpha-integration-tests
  • /test pull-tekton-pipeline-build-tests
  • /test pull-tekton-pipeline-integration-tests
  • /test tekton-pipeline-unit-tests

The following commands are available to trigger optional jobs:

  • /test pull-tekton-pipeline-go-coverage
  • /test pull-tekton-pipeline-kind-alpha-integration-tests
  • /test pull-tekton-pipeline-kind-alpha-yaml-tests
  • /test pull-tekton-pipeline-kind-integration-tests
  • /test pull-tekton-pipeline-kind-yaml-tests

Use /test all to run the following jobs that were automatically triggered:

  • pull-tekton-pipeline-alpha-integration-tests
  • pull-tekton-pipeline-build-tests
  • pull-tekton-pipeline-go-coverage
  • pull-tekton-pipeline-integration-tests
  • pull-tekton-pipeline-unit-tests

In response to this:

/test pull-tekton-pipeline-unit-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

/test tekton-pipeline-unit-tests

@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

/retest

1 similar comment
@abayer
Copy link
Contributor Author

abayer commented Jul 5, 2022

/retest

@abayer
Copy link
Contributor Author

abayer commented Jul 14, 2022

/test optional-kind

2 similar comments
@abayer
Copy link
Contributor Author

abayer commented Jul 14, 2022

/test optional-kind

@abayer
Copy link
Contributor Author

abayer commented Jul 14, 2022

/test optional-kind

@abayer abayer force-pushed the prep-for-prowjob-kind-e2e-tests branch from c101eb1 to d545ea4 Compare July 14, 2022 13:52
@tekton-robot tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 14, 2022
@abayer
Copy link
Contributor Author

abayer commented Jul 14, 2022

/hold cancel

Ok, this is good to go! Once it's merged, I'll take tektoncd/plumbing#1144 off hold and then we'll switch over for real.

@tekton-robot tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 14, 2022
@abayer
Copy link
Contributor Author

abayer commented Jul 14, 2022

/test pull-tekton-pipeline-kind-integration-tests
/test pull-tekton-pipeline-kind-alpha-integration-tests

@abayer
Copy link
Contributor Author

abayer commented Jul 14, 2022

ko resolve -R -f config/ \
| sed -e 's%"level": "info"%"level": "debug"%' \
local ko_target="$(mktemp)"
ko resolve -R -f config/ > "${ko_target}" || fail_test "Pipeline image resolve failed"
Copy link
Member

Choose a reason for hiding this comment

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

heh, nice :)


# Run these _after_ the integration tests b/c they don't quite work all the way
# and they cause a lot of noise in the logs, making it harder to debug integration
# test failures.
if [ "${RUN_YAML_TESTS}" == "true" ]; then
go_test_e2e -parallel=4 -mod=readonly -tags=examples -timeout=20m ./test/ || failed=1
Copy link
Member

Choose a reason for hiding this comment

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

Why is the parallel=4 flag removed? Does it mean it will run as many as possible in parallel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put that in there a while ago to try to improve stability - it didn't really make a difference. I saw no difference in stability when I removed it here, and a slight improvement in speed when I was using n2-standard-8s, so hey. Of course, now that we're on n2-standard-4s, with 4 cores, the end result is going to be the same as if -parallel=4 was there. =)

Copy link
Member

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

Thanks for this!!
It seems like the minimal status envs are not used yet, but I don't mind having them in.
/approve

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2022
The existing `test/e2e-tests-kind.env` is specifically for the `PipelineRun` approach. These new files are for running the e2e tests, via `kind`, in Prow.

There are four new env files - one for just the go e2e tests each for `stable` and `alpha`, and one for just the yaml tests each for `stable` and `alpha`.

Additionally, `examples/v1beta1/taskruns/git-volume.yaml` is moved to `examples/v1beta1/taskruns/no-ci/git-volume.yaml`. This is because Kind nodes don't have `git` installed, which is necessary for git volumes to work. Also, `--ignore-path=/product_uuid` has been added to the Kaniko args in `examples/v1beta1/pipelineruns/pipelinerun.yaml` and `test/yamls/v1beta1/pipelineruns/pipelinerun.yaml` to work around an issue with Kaniko multi-stage builds on Kind (GoogleContainerTools/kaniko#2164).

Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
@abayer abayer force-pushed the prep-for-prowjob-kind-e2e-tests branch from d545ea4 to f537778 Compare July 14, 2022 16:26
@abayer
Copy link
Contributor Author

abayer commented Jul 14, 2022

Oops, I forgot to remove the now unused test/e2e-tests-kind-prow-*-yaml.env files. That's done now.

@tekton-robot
Copy link
Collaborator

tekton-robot commented Jul 14, 2022

@abayer: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-tekton-pipeline-kind-yaml-tests c101eb1 link false /test pull-tekton-pipeline-kind-yaml-tests
pull-tekton-pipeline-kind-alpha-integration-tests c101eb1 link false /test pull-tekton-pipeline-kind-alpha-integration-tests
pull-tekton-pipeline-kind-alpha-yaml-tests c101eb1 link false /test pull-tekton-pipeline-kind-alpha-yaml-tests
pull-tekton-pipeline-kind-integration-tests c101eb1 link false /test pull-tekton-pipeline-kind-integration-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@abayer
Copy link
Contributor Author

abayer commented Jul 14, 2022

/retest

@dibyom
Copy link
Member

dibyom commented Jul 14, 2022

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2022
@tekton-robot tekton-robot merged commit 6542823 into tektoncd:main Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/misc Categorizes issue or PR as a miscellaneuous one. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesnt merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants