[build/debian*] Cleanup of image building and allow building on Darwin #80909
Conversation
k8s-ci-robot
added
release-note-none
|
/retest |
k8s-ci-robot
added
size/L
hoegaarden
changed the title
|
/retest |
|
/assign @tallclair @thockin |
There was a problem hiding this comment.
All I know about this is what I hacked. @tallclair has fingerprints on this..
|
/assign @mkumatag |
hoegaarden
changed the title
- Use multistage builds for the base - Don't sed Dockerfiles, use build args instead - Make (multiarch) builds work on Darwin - Hide not so useful output
When trying to run the container builds on GCB in parallel steps I see IO errors, which I believe are triggered when we try to register binfmt rules in parallel. When `SKIP_REGISTER_MULTIARCH` is a non-empty string, the registration will be skipped. I also took the opportiny to split that out into its own target. ---- Example errors from a GCB run: ``` Step #6 - "s390x": /workspace/go/src/k8s.io/kubernetes/build/debian-hyperkube-base/../../third_party/multiarch/qemu-user-static/register/qemu-binfmt-conf.sh: 246: echo: echo: I/O error Step #6 - "s390x": /workspace/go/src/k8s.io/kubernetes/build/debian-hyperkube-base/../../third_party/multiarch/qemu-user-static/register/qemu-binfmt-conf.sh: 246: echo: echo: I/O error Step #6 - "s390x": /workspace/go/src/k8s.io/kubernetes/build/debian-hyperkube-base/../../third_party/multiarch/qemu-user-static/register/qemu-binfmt-conf.sh: 246: echo: echo: I/O error Step #6 - "s390x": /workspace/go/src/k8s.io/kubernetes/build/debian-hyperkube-base/../../third_party/multiarch/qemu-user-static/register/qemu-binfmt-conf.sh: 246: echo: echo: I/O error Step #6 - "s390x": /workspace/go/src/k8s.io/kubernetes/build/debian-hyperkube-base/../../third_party/multiarch/qemu-user-static/register/qemu-binfmt-conf.sh: 246: echo: echo: I/O error Step #6 - "s390x": /workspace/go/src/k8s.io/kubernetes/build/debian-hyperkube-base/../../third_party/multiarch/qemu-user-static/register/qemu-binfmt-conf.sh: 246: echo: echo: I/O error ```
|
/retest |
|
Whoa. I didn't know this PR existed. |
k8s-ci-robot
added
area/release-eng
justaugustus
added
the
priority/important-soon
k8s-ci-robot
removed
the
needs-priority
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: hoegaarden The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The config was deliberatly called `debian-images-cloudbuild.yaml` to signal that this is only responsible for the debian-* images. The images for all supported architectures are baked in parallel and pushed to the registry ones all images have been created successfully.
| ## optional | ||
| _K8S_GIT_URL: https://github.com/kubernetes/kubernetes | ||
| _K8S_GIT_BRANCH: master | ||
| _BUILDER_IMAGE: gcr.io/k8s-staging-release-test/k8s-cloud-builder |
There was a problem hiding this comment.
@justaugustus 🤷♂️
| @@ -12,8 +12,96 @@ | |||
| # See the License for the specific language governing permissions and | |||
| # limitations under the License. | |||
|
|
|||
| FROM scratch | |||
| ARG BUILD_BASE | |||
There was a problem hiding this comment.
@justaugustus I know you requested to do a git rm Dockerfile{.build,} or such, to see a nicer diff. However, git / GitHub show the diff always the same if I change/delete the two files in one commit.
| ARCH?=amd64 | ||
| SKIP_REGISTER_MULTIARCH?= | ||
| ALL_ARCH = amd64 arm arm64 ppc64le s390x | ||
|
|
||
| TEMP_DIR:=$(shell mktemp -d) | ||
| QEMUVERSION=v2.9.1 |
There was a problem hiding this comment.
Out of curiosity, why not bump the version of qemu in this PR ? Lot of improvements have been made in the last releases : https://www.qemu.org/2019/04/24/qemu-4-0-0/.
There was a problem hiding this comment.
I thought about that, but I would rather bump the version in a separate PR than "hiding" that upgrade in this "refactoring PR".
|
To be honest, I'm tempted to abandon debian-base in favor of the upstream debian -slim image.
Before making this decision, I would want to compare 2 data points:
Any other arguments against migrating to debian-slim? |
|
I really like that idea, @tallclair! @hoegaarden @cpanato @ameukam -- Can you work on what Tim mentioned above together? |
|
@hoegaarden @cpanato @ameukam -- Any update w.r.t. Tim's suggestion? |
|
TL;DR: I think the base images might still make sense, and it might be more useful to invest in making the building/promotion process open and invest in processes and tooling around that. What I have learned:
What I assume or believe I to have heard (might be totally wrong though):
What I don't know:
Personal take:
Images in k/k based on k8s.gcr.io/debian-* imagesreferences to k8s.gcr.io/debian-* on github
|
k8s-ci-robot
added
the
needs-rebase
|
@hoegaarden: PR needs rebase. 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/test-infra repository. |
|
Hello @hoegaarden, @cpanato & @ameukam ! |
|
@hoegaarden can you expand on why you think we should still maintain the k8s.gcr.io/debian-* images? As I mentioned in #80909 (comment), the 2 reasons we historically used a custom base image were:
2 seems like the only valid reason to me, but I'm not convinced that the pain of maintaining an intermediate image is worth the benefit we get from dropping ~10 packages out of ~85. |
|
Some data answering the questions in #80909 (comment): closest build of buster-slim to debian-base:v2.0.0 (built 2019-11-01) is debian:buster-20191014-slim
Packages removed in debian-base: Comparison:
Observations:
|
|
@hoegaarden -- I'm going to take a run at removing the debian images in #88603 given @tallclair's analysis. Depending on how much headway I make/availability I may need someone to pick up that PR and carry it over the finish line. Stay tuned. |
|
@hoegaarden, @tallclair & @justaugustus: are you still confident that you will complete this for milestone 1.18? Would like to leave a friendly reminder that code freeze happens in 1 day (5 March EOD). Thanks! |
|
We're not going to land this for 1.18 and it may end up obsoleted altogether. |
|
Apologies for allowing this to go stale. When I have some time, I'm going to come back to this and see if we can pull out some additional functionality from what @hoegaarden set up, but for now: |
|
@justaugustus: Closed this PR. 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/test-infra repository. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
seding of the Dockerfilebinfmt_miscmultiarch setupWhich issue(s) this PR fixes:
none
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: