libdocker.GenerateMountBindings(): Disable RRO mounts when bumping up the Docker API client package to v25 (API v1.44)

[AkihiroSuda]

cri-dockerd/libdocker/helpers.go

Lines 205 to 261 in 8ae7a0e

	// generateMountBindings converts the mount list to a list of strings that
	// can be understood by docker.
	// '<HostPath>:<ContainerPath>[:options]', where 'options'
	// is a comma-separated list of the following strings:
	// 'ro', if the path is read only
	// 'Z', if the volume requires SELinux relabeling
	// propagation mode such as 'rslave'
	func GenerateMountBindings(mounts []*v1.Mount, terminationMessagePath string) []string {
	if terminationMessagePath == "" {
	terminationMessagePath = core.TerminationMessagePathDefault
	}
	result := make([]string, 0, len(mounts))
	for _, m := range mounts {
	if runtime.GOOS == "windows" && isSingleFileMount(m.HostPath, m.ContainerPath, terminationMessagePath) {
	logrus.Debugf("skipping mount :%s:%s", m.HostPath, m.ContainerPath)
	continue
	}
	bind := fmt.Sprintf("%s:%s", m.HostPath, m.ContainerPath)
	var attrs []string
	if m.Readonly {
	attrs = append(attrs, "ro")
	}
	// Only request relabeling if the pod provides an SELinux context. If the pod
	// does not provide an SELinux context relabeling will label the volume with
	// the container's randomly allocated MCS label. This would restrict access
	// to the volume to the container which mounts it first.
	if m.SelinuxRelabel {
	attrs = append(attrs, "Z")
	}
	switch m.Propagation {
	case v1.MountPropagation_PROPAGATION_PRIVATE:
	// noop, dockerd will decide the propagation.
	//
	// dockerd's default propagation is "rprivate": https://github.com/moby/moby/blob/v20.10.23/volume/mounts/linux_parser.go#L145
	//
	// However, dockerd automatically changes the default propagation to "rslave"
	// when the mount source contains the daemon root (/var/lib/docker):
	// - https://github.com/moby/moby/blob/v20.10.23/daemon/volumes.go#L137-L143
	// - https://github.com/moby/moby/blob/v20.10.23/daemon/volumes_linux.go#L11-L36
	//
	// This behavior was introduced in Docker 18.03: https://github.com/moby/moby/pull/36055
	case v1.MountPropagation_PROPAGATION_BIDIRECTIONAL:
	attrs = append(attrs, "rshared")
	case v1.MountPropagation_PROPAGATION_HOST_TO_CONTAINER:
	attrs = append(attrs, "rslave")
	default:
	logrus.Infof("Unknown propagation mode for hostPath %s", m.HostPath)
	// let dockerd decide the propagation
	}
	if len(attrs) > 0 {
	bind = fmt.Sprintf("%s:%s", bind, strings.Join(attrs, ","))
	}
	result = append(result, bind)
	}
	return result
	}

Docker v25 (API v1.44) treats read-only mounts as recursively read-only by default: moby/moby#45278 (comment)

But this appeared to be too much breaking for Kubernetes: kubernetes/enhancements#3858 (comment)

So cri-dockerd has to disable RRO by setting BindOptions.ReadOnlyNonRecursive.

[AkihiroSuda]

(This needs migrating from the legacy bind strings to the modern mount structs, but migrating "Z" isn't straightforward)

[AkihiroSuda]

PR:

• Do not make read-only mounts recursively read-only by default (also updates Docker client module to v25) #311

[afbjorklund]

Should probably have been 0.4.0, and not 0.3.12

[neersighted]

Hindsight being what it is, probably. At the time this seemed like a pretty simple bugfix for a regression caused by an unintended API change.

[afbjorklund]

Could still be a good idea to bump the next release to 0.4.0, once all the fixes are in and tested and so forth?

• Calico CNI installation fails with cridocker v0.3.12 #345 (comment)

Then I suppose there could be a 0.3 maintenance branch, and a 0.3.13 with the commit reverted (as suggested).

• Bump up minimum Docker API to v1.42 (Docker v23) #346 (comment)

[neersighted]

That seems reasonable to me (though I don't think there is any need for the 0.3 maintenance branch); cc @nwneisen.