-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
build all non-distro-packaged go binaries ourselves and respect .go-version #3201
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder 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 |
cool |
I've run |
2cf344a
to
c385c0c
Compare
I've updated this to also include LICENSE and notice files via https://github.com/google/go-license Some other Kubernetes projects are doing this and we haven't been, this is easier to do now that we're cloning and building these packages. debian packages that we're just installing the usual way already include comparable info in a standard way. |
- streamlines patching go version related CVEs - ensures binaries with CGO are linked to the library version we'll be using
110cc1f
to
5c8ec48
Compare
2fa880f
to
68dfb5b
Compare
I've tested all of these builds locally in quick mode (single-architecture only w/o pushing). |
# set by makefile to .go-version | ||
ARG GO_VERSION | ||
RUN eval "$(gimme "${GO_VERSION}")" \ | ||
&& GOBIN=/usr/local/bin go install github.com/google/go-licenses@latest |
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.
Latest?
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.
yes, the most recent tagged release. the API surface of this tool is small
# stage for building runc | ||
FROM go-build as build-runc | ||
ARG GO_VERSION | ||
ARG RUNC_VERSION="v1.1.7" |
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.
Didn't containerd have a specific version for runc that we obtained from one of the files on the repo?
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.
We did do that, but that version is just a recommendation / what they're using in CI in containerd*, we want to be able to control the pace of upgrading runc.
* their own integration tests, maybe not k8s e2e ...
We may even have to split this up and use multiple versions depending on k8s version, see kubernetes/kubernetes#117691 (comment)
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.
It's recommended to use a version >= https://github.com/containerd/containerd/blob/main/script/setup/runc-version, but that may actually be broken with Kubernetes or we may want to be ahead.
runc versions will need more thought following the thread I linked above, but for now sticking to the current stable tag.
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.
should add: 1.1.7 is the version used by containerd 1.6.21 https://github.com/containerd/containerd/releases/tag/v1.6.21
semi-related: they're no longer including runc with containerd releases containerd/containerd#6541
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.
turns out we actually need to be able to stick to an older version at times. #3220
older is fine, newer binary may not be. at least as long as we have to support cgroups v1
/lgtm |
This has serious perf issues in the arm64 cross-build. Working on a fix |
follow-up in #3214 |
Pros:
Cons:
make
TODO:
.go-version
andimages/Makefile.common.in
Reworkimages/kindnetd
,images/local-path-provisioner
to also respect.go-version
w/ gimmeWhen those are complete, go CVEs can be addressed in two PRs: