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

Make examples runnable with kubectl 🐇 #3564

Merged
merged 1 commit into from
Dec 2, 2020

Conversation

vdemeester
Copy link
Member

@vdemeester vdemeester commented Nov 26, 2020

Changes

This remove the use of ko:// in the examples folder, using the
:latest release image for those. This allow users to apply those
with kubectl directly.

To keep testing the "currently" built image, this introduce a
test/yamls folder where we use ko://. This updates the tests to
run both (examples and test/yamls).

Fixes #3498

Signed-off-by: Vincent Demeester vdemeest@redhat.com

/kind misc
/area testing

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes tests (if functionality changed/added)
  • Includes docs (if user facing)
  • Commit messages follow commit message best practices
  • Release notes block has been filled in or deleted (only if no user facing changes)

See the contribution guide for more details.

Double check this list of stuff that's easy to miss:

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.

Release Notes

NONE

@tekton-robot tekton-robot added release-note-none Denotes a PR that doesnt merit a release note. kind/misc Categorizes issue or PR as a miscellaneuous one. area/testing Issues or PRs related to testing labels Nov 26, 2020
@tekton-robot tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Nov 26, 2020
@vdemeester
Copy link
Member Author

/cc @afrittoli

This remove the use of `ko://` in the examples folder, using the
`:latest` release image for those. This allow users to apply those
with `kubectl` directly.

To keep testing the "currently" built image, this introduce a
`test/yamls` folder where we use `ko://`. This updates the tests to
run both (`examples` and `test/yamls`).

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
@dlorenc
Copy link
Contributor

dlorenc commented Nov 26, 2020

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 26, 2020
@chmouel
Copy link
Member

chmouel commented Nov 26, 2020

Is there any reasons why people would want to run the test without KO ? Just wondering why

@vdemeester
Copy link
Member Author

Is there any reasons why people would want to run the test without KO ? Just wondering why

See the linked issue, most people assume that the root examples folder is kubectl appliable (no need for ko, nor having the tektoncd source really).

@pritidesai
Copy link
Member

@vdemeester do we need to remove both yamls (pipelinerun-with-final-tasks.yaml and pipelinerun.yaml ) from examples/ folder since they exists under test/yamls/ now?

@vdemeester
Copy link
Member Author

@vdemeester do we need to remove both yamls (pipelinerun-with-final-tasks.yaml and pipelinerun.yaml ) from examples/ folder since they exists under test/yamls/ now?

So, I didn't because they are still "valid" example for users to look at, they are just using "normal" images instead of using the ko:// notation.

@@ -135,7 +142,7 @@ func exampleTest(path string, waitValidateFunc waitFunc, kind string) func(t *te
t.Skipf("Couldn't substitute environment: %v", err)
}

out, err := koCreate(subbedInput, namespace)
Copy link

Choose a reason for hiding this comment

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

Super tiny nit: there's a comment on line 151, // Parse from koCreate for now that I think is no longer right with this change? Maybe // Parse from createFunc for now?

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbwsg

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 Dec 2, 2020
@vdemeester
Copy link
Member Author

/retest

@tekton-robot tekton-robot merged commit d702ca1 into tektoncd:master Dec 2, 2020
@vdemeester vdemeester deleted the 3498-examples-no-ko branch December 2, 2020 15:32
@pritidesai pritidesai mentioned this pull request Dec 8, 2020
4 tasks
@lbernick lbernick mentioned this pull request Jun 20, 2023
2 tasks
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. area/testing Issues or PRs related to testing 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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

example/pipelinerun: Error: InvalidImageName
5 participants