-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Use ko://
in e2e tests to exercices current code 🙃
#2902
Use ko://
in e2e tests to exercices current code 🙃
#2902
Conversation
/cc @praveen4g0 |
This PR cannot be merged: expecting exactly one kind/ label Available
|
1 similar comment
This PR cannot be merged: expecting exactly one kind/ label Available
|
/area testing |
This PR cannot be merged: expecting exactly one kind/ label Available
|
2 similar comments
This PR cannot be merged: expecting exactly one kind/ label Available
|
This PR cannot be merged: expecting exactly one kind/ label Available
|
This PR cannot be merged: expecting exactly one kind/ label Available
|
Ah, finally a reason to be using ko instead of kubectl in the example tests XD
I don't think so, at least the current test logic uses ko so that seems okay - i guess if you were casually applying an example you'd have trouble. Maybe we could at least update the docs? https://github.com/tektoncd/pipeline/tree/master/examples#examples |
Indeed 😊 |
Legit failure 🤔
|
/test pull-tekton-pipeline-integration-tests |
1bad4b1
to
c62e278
Compare
This PR cannot be merged: expecting exactly one kind/ label Available
|
c62e278
to
c79d293
Compare
This PR cannot be merged: expecting exactly one kind/ label Available
|
c79d293
to
7b27f96
Compare
Rotten issues close after 30d of inactivity. /close Send feedback to tektoncd/plumbing. |
Stale issues rot after 30d of inactivity. /lifecycle rotten Send feedback to tektoncd/plumbing. |
@tekton-robot: Closed this PR. In response to this:
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. |
/remove-lifecycle rotten |
@vdemeester: Reopened this PR. In response to this:
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. |
7b27f96
to
c728c08
Compare
Right now, some examples are exercicing old version of the code, and definitely never the changes that would be on the images in a PR. Using `ko://` instead helps with that (e2e tests are using `ko` to apply the samples). This link `.ko.yaml` in test folder to use the proper base image (as `ko create …` seems to be running from the `test` folder). Signed-off-by: Vincent Demeester <[email protected]>
3b76484
to
0e1ea9b
Compare
securityContext: | ||
runAsUser: 0 |
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.
Because kaniko
run as root, this has to run as root too, otherwise permission denied
😓
@@ -0,0 +1 @@ | |||
../.ko.yaml |
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 is a symbolic link to .ko.yaml
so that it uses the overrides from there instead of nothing.
/hold cancel |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pritidesai, 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 |
Changes
Right now, some examples are exercicing old version of the code, and
definitely never the changes that would be on the images in a PR.
Using
ko://
instead helps with that (e2e tests are usingko
toapply the samples).
Fixes #2883
/hold
Note: this means one cannot do a
kubectl apply -f …
on those files, which may be a problem ? 🤔Most likely need #2675 to be in too 👼
Signed-off-by: Vincent Demeester [email protected]
/cc @bobcatfish @pritidesai @afrittoli @sbwsg
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmd
dir, please updatethe release Task to build and release this image.
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