-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix: let kind run with podman and VirtualBox #3079
base: main
Are you sure you want to change the base?
fix: let kind run with podman and VirtualBox #3079
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wheelerlaw 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 |
Welcome @wheelerlaw! |
Hi @wheelerlaw. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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. |
ebdacbb
to
501f06b
Compare
@@ -185,6 +185,9 @@ func runArgsForNode(node *config.Node, clusterIPFamily config.ClusterIPFamily, n | |||
// including some ones podman would otherwise do by default. | |||
// for now this is what we want. in the future we may revisit this. | |||
"--privileged", | |||
// For using podman when VirtualBox is also installed | |||
// See https://github.com/containers/podman/issues/14284 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems to be needed only for rootless, if you see in the commonArgs
method we append optional arguments based on some environment details, we should have a check for rootless somewhere so we can use the same pattern for this option instead of setting it for everything
I'm a little concerned that we have diverging behavior here but I supposed that's always going to be the case with rootless. I haven't seen similar issues with rootless runc + docker yet. Trying to think what other implications carrying through the groups might have. |
Just circling back to this now. I've been looking at the code for a little bit, and I am having a hard time understanding why the |
provision.go splits out the lengthy logic for organizing node creation from the rest of the provider logic.
That debt has happened over time but seems largely orthogonal, and has no relation to being in another file or not ..? caching info for the duration of the process may also be a mis-feature. execing something like |
Thinking about this again, is there any reason to expect that this is desired? containers/podman#14284 (comment) also comes to mind as a potential issue. |
If a user has VirtualBox installed on the same machine they have
kind
installed, and they are using the podman experimental provider, they will likely be faced with the following error when they try to runkind create cluster
:This is because the devices in the
/dev/vboxusb
belong to thevboxusers
group. If the user executing thekind
command is in that group, podman attempts to bind mount those hardware devices in the container when the--privileged
flag is passed to podman. However,crun
makes asetgroups
syscall to reset the user groups so when it comes time for the bind mount to actually occur, it fails because the namespaced user in the container is no longer in that group and therefore does not have permission tostat
the devices.This change passes the
--group-add keep-groups
to podman (which then passes therun.oci.keep_original_groups=1
annotation tocrun
) so that the groups are preserved so the bind mount doesn't fail.(related to containers/podman#14284)