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

Bidirectional mount propagation for hostroot #1290

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dougbtv
Copy link
Member

@dougbtv dougbtv commented May 29, 2024

Modifies the hostroot volume in the multus-daemonset-thick.yml example/quickstart deployment file from HostToContainer to Bidirectional. This change enables the volume to be accessible in both directions, which is necessary for users who need to share a mount with another container/pod.

This is motivated by the fact that in thin plugin mode, since all things were run on the host directly, CNI plugins wouldn't be limited by the mount propagation.

One example that has come up recently is userspace CNI interaction with kubevirt, and sharing the usage of the socket as mounted by kubevirt.

This does expose some level of risk (as noted in the mount propagation docs regarding changing of mounts), however, I don't believe it's significantly more than would've been the case in thin plugin mode.

References

…root` volume in the `multus-daemonset-thick.yml` example/quickstart deployment file from `HostToContainer` to `Bidirectional`. This change enables the volume to be accessible in both directions, which is necessary for users who need to share a mount with another container/pod.

This is motivated by the fact that in thin plugin mode, since all things were run on the host directly, CNI plugins wouldn't be limited by the mount propagation.

One example that has come up recently is userspace CNI interaction with kubevirt, and sharing the usage of the socket as mounted by kubevirt.

This does expose some level of risk (as noted in the mount propagation docs regarding changing of mounts), however, I don't believe it's significantly more than would've been the case in thin plugin mode.

References
- [Kubernetes Volume Mount Propagation Documentation](https://kubernetes.io/docs/concepts/storage/volumes/#mount-propagation)
@dougbtv dougbtv changed the title This pull request modifies the volume mount propagation for the `host… Bidirectional mount propagation for hostroot May 29, 2024
@coveralls
Copy link

Coverage Status

coverage: 63.116%. remained the same
when pulling e45ee41 on dougbtv:hostroot-mount-propagation
into 9f5c023 on k8snetworkplumbingwg:master.

@@ -190,7 +190,7 @@ spec:
readOnly: true
- name: hostroot
mountPath: /hostroot
mountPropagation: HostToContainer
mountPropagation: Bidirectional
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't we actually consider these yamls to be just examples ?

Why can't the downstreams that require it to be bi-directional use what they need ?

I'm trying to wrap my head around the fact this is not the recommended configuration, required only for specific use cases, but still, we're advocating / instructing everyone to use it.

Copy link
Member Author

Choose a reason for hiding this comment

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

These are indeed examples, but, I think this more accurately represents a setup that's similar to what happens in a thin plugin mode on the host -- you'd see mounts in both directions. So, I think it's a valid setup and probably better for more cases at least.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I disagree, but don't have more arguments other than this is not the recommended configuration, and is required only for specific use cases.

Do we even know if this will work on openshift ?

Copy link
Member

@s1061123 s1061123 left a comment

Choose a reason for hiding this comment

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

How about to add same config to kind-e2e config? Others are okey!

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 this pull request may close these issues.

None yet

4 participants