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

In a container, try to register binfmt_misc #5732

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

nalind
Copy link
Member

@nalind nalind commented Sep 11, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

If we're running a command in a working container whose platform doesn't match our own, attempt to register any emulators for which we find configurations of the type included in Fedora's qemu-user-static packages.

How to verify it

In-container tests, provided the CI node is new enough, will start being able to use emulation if they couldn't before.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

  • Linux 6.6 and later allow binfmt_misc to be mounted inside user namespaces.
  • This should be helpful for people who are using our Fedora-based images on systems which don't provide their own qemu-user-static binaries.

Does this PR introduce a user-facing change?

buildah container images will now attempt to use the qemu-user-static binaries they include, if they also include configuration for them, for running binaries built for non-native architectures.

@nalind nalind added do-not-merge/hold No New Tests Allow PR to proceed without adding regression tests labels Sep 11, 2024
@openshift-ci openshift-ci bot added kind/feature Categorizes issue or PR as related to a new feature. approved labels Sep 11, 2024
cmd/buildah/main.go Outdated Show resolved Hide resolved
@nalind nalind force-pushed the binfmt-container branch 2 times, most recently from 81b6534 to 4aedfc1 Compare September 12, 2024 14:27
return fmt.Errorf("looking for binfmt.d configuration in %q: %w", searchDir, err)
}
for _, conf := range globs {
f, err := os.Open(conf)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we only be doing this if the image has different ARCH then the local arch?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. At that point we're too far from the CLI to make it something that can be manually controlled very easily, but then maybe we're fine without that.

@nalind nalind force-pushed the binfmt-container branch 3 times, most recently from f3a7b41 to 2e91082 Compare September 12, 2024 15:21
If we're running a command in a working container whose platform doesn't
match our own, attempt to register any emulators for which we find
configurations of the type included in Fedora's qemu-user-static
packages.

Signed-off-by: Nalin Dahyabhai <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Sep 13, 2024

LGTM
@flouthoc @giuseppe @mheon PTAL

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Copy link
Contributor

openshift-ci bot commented Sep 13, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, nalind

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@nalind
Copy link
Member Author

nalind commented Sep 13, 2024

Oops, forgot I left a hold on this.
/unhold

@openshift-merge-bot openshift-merge-bot bot merged commit b4d5c2f into containers:main Sep 13, 2024
32 checks passed
@nalind nalind deleted the binfmt-container branch September 13, 2024 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved kind/feature Categorizes issue or PR as related to a new feature. lgtm No New Tests Allow PR to proceed without adding regression tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants