Fix non-amd64 arch config for debian images #1839

sozercan · 2021-01-12T18:34:33Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

Non-AMD64 debian images are not marked correct arch in image config. Looks like they are just build as amd64 images with manifest list pointing to different architectures.

To verify:

$ docker buildx imagetools inspect  k8s.gcr.io/build-image/debian-iptables:buster-v1.3.0 
Name:      k8s.gcr.io/build-image/debian-iptables:buster-v1.3.0
...
  Name:      k8s.gcr.io/build-image/debian-iptables:buster-v1.3.0@sha256:3d7ede6013b0516f1ec3852590895d4a7b6ec8f5e15bebc1a55237bba4538da2
  MediaType: application/vnd.docker.distribution.manifest.v2+json
  Platform:  linux/arm64
...

$ docker pull k8s.gcr.io/build-image/debian-iptables:buster-v1.3.0@sha256:3d7ede6013b0516f1ec3852590895d4a7b6ec8f5e15bebc1a55237bba4538da2
k8s.gcr.io/build-image/debian-iptables@sha256:3d7ede6013b0516f1ec3852590895d4a7b6ec8f5e15bebc1a55237bba4538da2: Pulling from build-image/debian-iptables
Digest: sha256:3d7ede6013b0516f1ec3852590895d4a7b6ec8f5e15bebc1a55237bba4538da2
Status: Downloaded newer image for k8s.gcr.io/build-image/debian-iptables@sha256:3d7ede6013b0516f1ec3852590895d4a7b6ec8f5e15bebc1a55237bba4538da2
k8s.gcr.io/build-image/debian-iptables:buster-v1.3.0@sha256:3d7ede6013b0516f1ec3852590895d4a7b6ec8f5e15bebc1a55237bba4538da2

$ docker inspect -f '{{json .Architecture}}' k8s.gcr.io/build-image/debian-iptables:buster-v1.3.0@sha256:3d7ede6013b0516f1ec3852590895d4a7b6ec8f5e15bebc1a55237bba4538da2
"amd64"

This causes an issue with building Kubernetes with Docker v20.10.
image with reference k8s.gcr.io/build-image/debian-iptables:buster-v1.3.0 was found but does not match the specified platform: wanted linux/arm64, actual: linux/amd64
docker/for-linux#1170

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Mark debian images with their corresponding architectures

cc @justaugustus

k8s-ci-robot · 2021-01-12T18:34:41Z

Welcome @sozercan!

It looks like this is your first PR to kubernetes/release 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/release has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2021-01-12T18:34:41Z

Hi @sozercan. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

hasheddan

/ok-to-test

hasheddan · 2021-01-13T13:16:34Z

images/build/debian-base/Makefile

@@ -92,10 +92,15 @@ else
 endif
 	mv $(TEMP_DIR)/$(CONFIG)/Dockerfile.build.tmp $(TEMP_DIR)/$(CONFIG)/Dockerfile.build

-	docker build --pull -t $(BUILD_IMAGE) -f $(TEMP_DIR)/$(CONFIG)/Dockerfile.build $(TEMP_DIR)/$(CONFIG)
+	docker buildx build \


We may run into issues here with pulling the base image when using buildkit and gcr mirror (see kubernetes/kubernetes#97982 for more info)

saschagrunert · 2021-01-13T13:59:21Z

Hey @sozercan, thank you for the contribution of this fix! 🙏

So, I don't think that we need to use buildkit here. We could stick to a FROM --platform=linux/<ARCH> approach within the Dockerfile's.

Going one step back, it also looks like that the base image for debian-iptables aka debian-base has exactly the same issue, which may be the reason that the created manifest just points to one architecture (amd64).

> podman inspect k8s.gcr.io/build-image/debian-base-arm64:buster-v1.2.0 | jq .[].Architecture
"amd64"

I think we should start fixing the debian-base image before 🙃

Edit: We're building the debian-base images from scratch, which means that the mentioned FROM --platform ... will always fallback to the host platform. In this case I only see a fix in introducing buildkit for this image, otherwise we would not get the right architecture into the manifest. 🤷

Referencing discussion: https://kubernetes.slack.com/archives/CJH2GBF7Y/p1610548545313800

dims · 2021-01-13T17:41:26Z

@hasheddan @saschagrunert do we want to bump priority on review for this one?

saschagrunert · 2021-01-13T17:56:53Z

@hasheddan @saschagrunert do we want to bump priority on review for this one?

@sozercan did you had a chance to verify that your changes work even with a "wrong" debian-base? If so then we could conclude on using buildkit for both images and merge this one.

sozercan · 2021-01-13T18:54:01Z

@saschagrunert yes, if it's built with --platform (or built on matching host) it'll add correct arch, even if base image arch doesn't match.

$ ARCH=arm64 make build
cp -r ./buster /tmp/tmp.n7NZDnwBht/
cd /tmp/tmp.n7NZDnwBht/buster && sed -i "s|BASEIMAGE|k8s.gcr.io/build-image/debian-base-arm64:buster-v1.2.0|g" Dockerfile
...
 => => writing image sha256:e5e283e4c90d9ec8dbf7b62520392511e1ae4f0e6d783cfb93aa026f21cb0656

$ docker inspect -f '{{json .Architecture}}' e5e283e4c90d9ec8dbf7b62520392511e1ae4f0e6d783cfb93aa026f21cb0656
"arm64"

justaugustus · 2021-01-13T19:57:19Z

/lgtm
/approve
/priority critical-urgent

I'll work on image updates in a follow-up.

k8s-ci-robot · 2021-01-13T19:57:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justaugustus, sozercan

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:

~~images/build/debian-base/OWNERS~~ [justaugustus]
~~images/build/debian-hyperkube-base/OWNERS~~ [justaugustus]
~~images/build/debian-iptables/OWNERS~~ [justaugustus]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

This reverts commit 1fd673e, reversing changes made to ce6512e.

Revert "Merge pull request #1839 from sozercan/fix-arch"

This reverts commit 1fd673e, reversing changes made to ce6512e.

setting platform config arch

8d4e44f

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. sig/release Categorizes an issue or PR as relevant to SIG Release. labels Jan 12, 2021

k8s-ci-robot requested review from hasheddan and MrHohn January 12, 2021 18:34

hasheddan reviewed Jan 12, 2021

View reviewed changes

hasheddan reviewed Jan 13, 2021

View reviewed changes

hakman mentioned this pull request Jan 13, 2021

Error importing kube-poroxy.tar for ARM64 with containerd #1842

Closed

saschagrunert mentioned this pull request Jan 13, 2021

WIP: Fix container image architecture for conformance image kubernetes/kubernetes#98016

Closed

k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed needs-priority labels Jan 13, 2021

k8s-ci-robot assigned justaugustus Jan 13, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 13, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 13, 2021

k8s-ci-robot merged commit 1fd673e into kubernetes:master Jan 13, 2021

k8s-ci-robot added this to the v1.21 milestone Jan 13, 2021

sozercan deleted the fix-arch branch January 13, 2021 20:21

justaugustus added a commit to justaugustus/release that referenced this pull request Jan 15, 2021

Revert "Merge pull request kubernetes#1839 from sozercan/fix-arch"

c252c58

This reverts commit 1fd673e, reversing changes made to ce6512e.

justaugustus mentioned this pull request Jan 15, 2021

Revert "Merge pull request #1839 from sozercan/fix-arch" #1849

Merged

k8s-ci-robot added a commit that referenced this pull request Jan 15, 2021

Merge pull request #1849 from justaugustus/revert-base-buildx

ce5ecf2

Revert "Merge pull request #1839 from sozercan/fix-arch"

justaugustus mentioned this pull request Jan 20, 2021

releng: Promote debian-base v1.3.0 images kubernetes/k8s.io#1555

Merged

wespanther pushed a commit to wespanther/release that referenced this pull request Mar 15, 2021

Revert "Merge pull request kubernetes#1839 from sozercan/fix-arch"

9073b27

This reverts commit 1fd673e, reversing changes made to ce6512e.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix non-amd64 arch config for debian images #1839

Fix non-amd64 arch config for debian images #1839

sozercan commented Jan 12, 2021 •

edited

Loading

k8s-ci-robot commented Jan 12, 2021

k8s-ci-robot commented Jan 12, 2021

hasheddan left a comment

hasheddan Jan 13, 2021

saschagrunert commented Jan 13, 2021 •

edited

Loading

dims commented Jan 13, 2021

saschagrunert commented Jan 13, 2021

sozercan commented Jan 13, 2021 •

edited

Loading

justaugustus commented Jan 13, 2021

k8s-ci-robot commented Jan 13, 2021

Fix non-amd64 arch config for debian images #1839

Fix non-amd64 arch config for debian images #1839

Conversation

sozercan commented Jan 12, 2021 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

k8s-ci-robot commented Jan 12, 2021

k8s-ci-robot commented Jan 12, 2021

hasheddan left a comment

Choose a reason for hiding this comment

hasheddan Jan 13, 2021

Choose a reason for hiding this comment

saschagrunert commented Jan 13, 2021 • edited Loading

dims commented Jan 13, 2021

saschagrunert commented Jan 13, 2021

sozercan commented Jan 13, 2021 • edited Loading

justaugustus commented Jan 13, 2021

k8s-ci-robot commented Jan 13, 2021

sozercan commented Jan 12, 2021 •

edited

Loading

saschagrunert commented Jan 13, 2021 •

edited

Loading

sozercan commented Jan 13, 2021 •

edited

Loading