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
ensure we can remove _output #117510
ensure we can remove _output #117510
Conversation
/sig testing |
@@ -367,6 +367,8 @@ function kube::build::clean() { | |||
|
|||
if [[ -d "${LOCAL_OUTPUT_ROOT}" ]]; then | |||
kube::log::status "Removing _output directory" | |||
# this ensures we can clean _output/local/go/cache which is not rw by default |
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 because we go install
in module mode with GOPATH under _output
, creating these non-writeable module cache files. We could make make clean
aware of the go module path but ... that's a bit messy.
I think it generically makes sense to ensure the directory is recursively writeable before trying to remove it.
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.
interesting!!
/approve |
LGTM label has been added. Git tree hash: 32ec90e5a28c5990266cf6f42d16b2a990698ac4
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder, dims 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 |
oh, great, I was surprised why I had to use sudo and it was because of this |
What type of PR is this?
/kind bug
What this PR does / why we need it:
Fixes
make clean
, which currently fails, if you run something likemake test GOFLAGS="-v -failfast -count=1" KUBE_COVER="y" WHAT=./pkg/kubelet/kuberuntime
(debugging #117362) then afterwardsmake clean
will fail on:AFAICT this issue does not exist on release-1.26 @ 67c667c or release-1.27 @ 523dd02 but it does at HEAD, I think this is related to #117016 which added a
go install
togolang::setup_env
.#117273 may also be related or is at least a similar problem, need to follow up on that separately.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Since this was introduced since 1.27 and has not been released yet, no release note. I don't see this issue on release-1.27 @ 523dd02, and #117016 is so far only in 1.28.
If we cherry-pick #117016 we'll need to pick this as well.
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: