-
Notifications
You must be signed in to change notification settings - Fork 348
Update kubernetes version and support mount propagation #218
Conversation
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.
LGTM with one small nit. Ready to merge after nit addressed.
pkg/server/container_create.go
Outdated
case runtime.MountPropagation_PROPAGATION_HOST_TO_CONTAINER: | ||
options = append(options, "rslave") | ||
default: | ||
glog.Warningf("unknown propagation mode for hostPath %q", mount.HostPath) |
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.
nit: s/unknown/Unknown
Log should start with capitalized letter. :)
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.
@Random-Liu Thank for review, Done.
Based on the comment kubernetes/kubernetes#46444 (comment), there will be a conformance test for mount propagation. Please make sure we enable that conformance test in our node e2e after they add it. :) |
@miaoyq Could you add a CRI validation test for this? kubernetes-sigs/cri-tools#131 Just mount in container and host, and check whether the visibility of the mounts in different mode. :) |
I will work for this. : ) |
fixex containerd#185 Signed-off-by: Yanqiang Miao <[email protected]>
LGTM, will merge after test passes. |
The test has passed, I don't know why the label is still yellow. Merge anyway. |
fixes #185
/cc @Random-Liu
Signed-off-by: Yanqiang Miao [email protected]