Build binary fully in container build #1171
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/S
|
Thanks @mrueg :)! PR looks good; just one minor comment to address |
mrueg
force-pushed
the
build-in-Dockerfile
branch
from
23bd715 to
c6333ef
Compare
Signed-off-by: Manuel Rüger <[email protected]>
Builds in docker now, so tempdir is not needed and we'll provide GOARCH to Docker as a build-arg Signed-off-by: Manuel Rüger <[email protected]>
mrueg
force-pushed
the
build-in-Dockerfile
branch
from
c6333ef to
0be9f6e
Compare
|
If I'm not mistaken this was done intentionally to support the image promotion process on k8s.gcr.io, but I don't remember exactly. Maybe @lilic or @serathius remember? (maybe I'm totally mixing up things though) |
|
Yes, using container multi stage build was my recommendation to make integration with image promotion process easier. Build process for k8s.gcr.io is already done in docker, creating lot of problems with path mounting and permissions. I found that using docker multi stage build solves all those problems. Secondly I found that using locally build binaries lead to broken images #1086 (comment) |
|
@serathius Just to make sure I understand correctly, you also approve of this change right? :) |
|
Yes, it's a great improvement. |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brancz, mrueg 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 |
k8s-ci-robot
added
the
approved
|
(just wanted to make sure there is not some specific reason why we didn't have this :) ) |
What this PR does / why we need it:
As a first step towards #1089, this now builds the kube-state-metrics binary fully in Docker. I'm not sure how images are build via the image-promoter, in order to build correctly there, the image-promoter would need to set a build arg for the arch it needs to build for.
FYI @paulfantom @lilic @tariq1890