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

oci_push rule fails on osx when pushing a linux image #706

Open
codesuki opened this issue Oct 1, 2024 · 8 comments
Open

oci_push rule fails on osx when pushing a linux image #706

codesuki opened this issue Oct 1, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@codesuki
Copy link

codesuki commented Oct 1, 2024

While adding support for oci_image to rules_k8s I ran into the following issue.

I am pushing an image with --platforms=@io_bazel_rules_go//go/toolchain:linux_amd64 from my local osx machine and get the following error

push_push_api_image.sh: line 52: ../aspect_bazel_lib~~toolchains~jq_linux_amd64/jq: cannot execute binary file

Some investigation showed that I should set --extra_execution_platforms=@platforms//host, but this fails with the same error.

Investigating some more I found bazelbuild/bazel#19645 and https://github.com/bazel-contrib/rules_oci/blob/main/oci/private/push.bzl#L142-L155.
Together with the the fact that I am using Bazel 7.x and this commit bazelbuild/bazel@c602cec

Subsequent settings of --extra_execution_platforms now override previous settings, instead of adding them to a list.

I figured I can make it work with the snippet below. Instead of overwriting extra_execution_platforms, we preserve previous contents.

# Helper rule for ensuring that the crane and yq toolchains are actually
# resolved for the architecture we are targeting.
def _transition_to_target_impl(settings, _attr):
    return {
        # String conversion is needed to prevent a crash with Bazel 6.x.
        "//command_line_option:extra_execution_platforms": [
            str(platform)
            for platform in settings["//command_line_option:extra_execution_platforms"] + settings["//command_line_option:platforms"]
        ],
    }

_transition_to_target = transition(
    implementation = _transition_to_target_impl,
    inputs = ["//command_line_option:platforms", "//command_line_option:extra_execution_platforms"],
    outputs = ["//command_line_option:extra_execution_platforms"],
)

I am not sure if this is the 'real' fix, but if it were then it would at least need a check for the Bazel version to do the right thing.

@fishy
Copy link

fishy commented Oct 2, 2024

We are seeing the same issue. I think maybe there's a better way to cross-build container image for linux-amd64 on other host platforms than adding --platforms=@io_bazel_rules_go//go/toolchain:linux_amd64 to the command line, but I didn't find doc describing other ways in rules_oci. Currently we reverted back to rules_oci version 1.7.6 which is the last version it works on mac without any additional hack.

@codesuki can you describe more about how the transition solution works?

@codesuki
Copy link
Author

codesuki commented Oct 3, 2024

Reading all these issues it certainly feels like the underlying issues are not resolved even with that transition.

Anyhow, for why the transition broke for me with Bazel 7 was the change I linked to above.

The transition code in rules_oci adds the platform value to the extra_execution_platforms. In Bazel < 7 this worked because it was additive.
So when I specify --extra_execution_platforms=@platforms//host to make oci_push work the transition just added to it.
With the Bazel 7 change it overrides and loses my --extra_execution_platforms=@platforms//host which results in oci_push failing because it's built for linux.

@lionelfleury
Copy link

lionelfleury commented Oct 3, 2024

Having the exact same problem when wanting to build a linux/amd64 container on a darwin/arm64 machine.

I think this is a bug because binaries for push are transitioned to target platform. They should reflect execution platform instead...

@ahans
Copy link
Contributor

ahans commented Oct 18, 2024

It also happens when trying to push linux/arm64 images from a linux/amd64 host.

@ahans
Copy link
Contributor

ahans commented Oct 18, 2024

FWIW, @codesuki's patch fixes it for me (with Bazel 7.3.2), so thanks a lot for that! However, reverting #590 fixes it as well without requiring --extra_execution_platforms=@platforms//host. I wonder why that was introduced in the first place and what's the use case here? Isn't oci_push something that we would always run on the host and not on the target?

@manuelnaranjo
Copy link

It also happens when trying to push linux/arm64 images from a linux/amd64 host.

Ok this sounds like something that's trivial to add a unit test for then, and validate that things work as expected. Cross compiling from darwin is not so trivial as it requires darwin based runners

@thesayyn
Copy link
Collaborator

I think i'll defer to @EdSchouten

@manuelnaranjo
Copy link

I was able to update the go example from aspect to newer versions of all the rules, and didn't fail in the attempt, so I think this issue is resolved aspect-build/bazel-examples#377, it's not merged yet, but I tested in our perl images and it worked

ellie-idb added a commit to devzero-inc/rules_oci that referenced this issue Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants