Skip to content
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

Incorrect architecture label for container image #1421

Closed
murilofv opened this issue Feb 14, 2020 · 1 comment · Fixed by #1427
Closed

Incorrect architecture label for container image #1421

murilofv opened this issue Feb 14, 2020 · 1 comment · Fixed by #1427

Comments

@murilofv
Copy link
Contributor

I have an issue when using rules_docker in a different architecture (in my case ppc64le) where all the container images generated get the label as amd64 architecture despite the architecture I am running in.

Steps to recreate

Need a system from a different architecture than amd64.

The following is an example of a WORKSPACE file:

load(
    "@bazel_tools//tools/build_defs/repo:http.bzl",
    "http_archive",
    "http_file",
)

http_archive(
    name = "io_bazel_rules_docker",
    sha256 = "dc97fccceacd4c6be14e800b2a00693d5e8d07f69ee187babfd04a80a9f8e250",
    strip_prefix = "rules_docker-0.14.1",
    urls = ["https://github.com/bazelbuild/rules_docker/releases/download/v0.14.1/rules_docker-v0.14.1.tar.gz"],
)

load(
    "@io_bazel_rules_docker//repositories:repositories.bzl",
    container_repositories = "repositories",
)
container_repositories()

load("@io_bazel_rules_docker//repositories:deps.bzl", container_deps = "deps")

container_deps()

load(
    "@io_bazel_rules_docker//container:container.bzl",
    "container_image",
    "container_pull",
)

http_file(
    name = "go_puller_linux_ppc64le",
    executable = True,
    sha256 = "540f4d7b2a3d627d7c3190f11c4fab5f8aad48bd42a9dffb037786e26270b6bd",
    urls = [
        "https://storage.googleapis.com/builddeps/540f4d7b2a3d627d7c3190f11c4fab5f8aad48bd42a9dffb037786e26270b6bd",
    ],
)

container_pull(
    name = "fedora_ppc64le",
    digest = "sha256:de1e23d807365800621d10efe27fa7eea865ee0bf9f1beca316f919972312695",
    puller_linux = "@go_puller_linux_ppc64le//file:downloaded",
    registry = "index.docker.io",
    repository = "library/fedora",
)

Note: I am using a custom rebuilt puller binary given it is currently only built for amd64

The following is an example of the BUILD.bazel file:

load(
    "@io_bazel_rules_docker//container:container.bzl",
    "container_image",
    "container_push",
)

container_image(
    name = "fedora30_ppc64le",
    base = "@fedora_ppc64le//image",
    visibility = ["//visibility:public"],
)

container_push(
    name = "push-fedora30_ppc64le",
    format = "Docker",
    image = ":fedora30_ppc64le",
    registry = "index.docker.io/murilofv",
    repository = "fedora30",
    tag = "latest",
)

Run the following commands:
bazel build :fedora30_ppc64le
bazel run :push-fedora30_ppc64le

By inspecting it we can see that the architecture doesn't match what it was built in:

$ uname -m
ppc64le

$ docker inspect index.docker.io/murilofv/fedora30:latest | grep Arch
        "Architecture": "amd64",

The image works fine though:

$ docker run -it index.docker.io/murilofv/fedora30:latest bash
[root@8a0ecb4ef47d /]# exit

Aside from being confusing to an user, labeling it incorrectly would be an issue if this container image would be made a part of manifest list to build a multi-arch image.

Potential fix

I did a small patch that seemed to have done the trick for me but I don't know if it is the proper way to address this issue, so I'd appreciate any feedback / suggestions on how to address it. The following is the patch:

container_proper_arch.txt

Basically I am only changing the hardcoded amd64 label in container/go/pkg/compat/config.go by the runtime GOARCH value. If this is an acceptable / proper way I can do a PR for it.

With it I get the proper architecture label and can execute in my system without any issues as well:

$ docker inspect index.docker.io/murilofv/fedora30:latest-patched | grep Arch
        "Architecture": "ppc64le",

$ docker run -it index.docker.io/murilofv/fedora30:latest-patched bash
[root@2ebff5a8164a /]# exit

I put the patched / unpatched container images built in this test in my docker hub: https://hub.docker.com/r/murilofv/fedora30/tags

@smukherj1
Copy link
Collaborator

I think the suggested fix would tie the architecture of the built image to the platform it was built on. I feel like that's not the right approach. Let me know if you disagree.

I'm thinking maybe we should expose the architecture as an attribute on the container_image rule that would default to amd64 but can be overridden by the user. If you have multiple images that want to override the architecture attribute, you could then define a custom container_image macro that sets the new attribute but forwards other attributes to the underlying container_image rule exported by rules_docker. If this proposal sounds agreeable, feel free to send a PR and I'll be happy to review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants