Updated git-clone task #2469
Updated git-clone task #2469
Conversation
|
@matejvasek: The label(s) 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-sigs/prow repository. |
knative-prow
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: matejvasek 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 |
knative-prow
bot
added
the
approved
knative-prow
bot
added
the
size/XS
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2469 +/- ##
===========================================
- Coverage 61.19% 50.64% -10.56%
===========================================
Files 130 128 -2
Lines 15354 11933 -3421
===========================================
- Hits 9396 6043 -3353
- Misses 5032 5183 +151
+ Partials 926 707 -219
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
knative-prow
bot
added
size/S
matejvasek
force-pushed
the
matejvasek-updated-git-clone-task
branch
from
825d5ca to
2242f30
Compare
matejvasek
force-pushed
the
matejvasek-updated-git-clone-task
branch
from
ded4ed3 to
20bd907
Compare
matejvasek
requested review from
matzew,
lkingland and
gauron99
and removed request for
rhuss
knative-prow
bot
removed
the
do-not-merge/work-in-progress
|
PTAL @lkingland @matzew @gauron99 |
|
/hold |
knative-prow
bot
added
the
do-not-merge/hold
| @@ -66,7 +66,7 @@ spec: | |||
| default: "1001" | |||
| - name: GROUP_ID | |||
| description: The group ID of the builder image user. | |||
| default: "0" | |||
| default: "65532" | |||
There was a problem hiding this comment.
What happens if this setting were just removed? This default group ID, and runAsGroup below, and the fsGroup setting? Is it possible that explicitly setting these to 0 is what caused the problem in the first place?
There was a problem hiding this comment.
The primary problem is that that uid is changed, not gid. This causes that git-fetch task on second run cannot clean up the volume. I fixed it by setting gid to match gid of git-fetch task (and also fsGroup) and by setting write access to said group on line 111.
TBH I have no idea why the gid here was set to 0 here. Zbynek implemented it that way long ago.
IMO the gid should be 1000 to match buildpack builder, but it works also with 65532.
I suspect I could just s/65532/1000/g on this PR and it still would work. But I am not sure.
There was a problem hiding this comment.
This whole ceremony here can be ignored on OCP. OCP automatically sets fsGroup and also adds write perms for the group.
Actually on OCP you must not set podTemplate.securityContext, otherwise there will be error.
There was a problem hiding this comment.
The primary problem is that that uid is changed, not gid.
This and the fact that set gid works oddly or not at all.
There was a problem hiding this comment.
Thanks for the clarification. Let me know if this, or the rootfull git-clone works out better for you and I'll review/approve
|
@lkingland we may also opt not to do this update and keep rootfull git-clone. We just will have to also make func-utils image rootfull to. See #2471 |
|
This Pull Request is stale because it has been open for 90 days with |
github-actions
bot
added
the
lifecycle/stale
Changes
This changes are also already included in the "Go s2i on cluster build" PR.