-
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
Readd mounting volumes in workingdir #1609
Readd mounting volumes in workingdir #1609
Conversation
f5cff36
to
f824728
Compare
/retest |
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.
Sounds like we could use an integration test for this. I can add one after this or you can add one in this PR, up to you.
Thanks for catching this!
Mounting volume was dropped when we did the large refactoring in d7f492c which basically making it ineffective and fails when not running as root. Readding it in there Closes tektoncd#1608 Signed-off-by: Chmouel Boudjnah <[email protected]>
f824728
to
d9030d2
Compare
@imjasonh good point on the integration test, but this make me wonder, do we. need the workingdir ? since it seems that nobody has realised it wasn't and nothing catastrophic has been discovered since then without it ? |
Nobody may have noticed it was broken in the past few days, but most people don't run so aggressively from head either (plus, Kubecon). If we decide to change the behavior, we should do it slowly over a couple releases with ample notice, but I don't really think we would since it's easy enough to support (when I'm not breaking it! 🤣) /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ImJasonH 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 |
I think the implicit mounts are quite handy to pass content between containers. |
Mounting volume was dropped when we did the large refactoring in
d7f492c which basically making
it ineffective and fails when not running as root.
Readding it in there
Closes #1608
/cc @imjasonh
Changes
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
Release Notes