diff --git a/pkg/volume/util/util.go b/pkg/volume/util/util.go index e7df9538f32..f6f5a3f9960 100644 --- a/pkg/volume/util/util.go +++ b/pkg/volume/util/util.go @@ -75,6 +75,9 @@ const ( // VolumeDynamicallyCreatedByKey is the key of the annotation on PersistentVolume // object created dynamically VolumeDynamicallyCreatedByKey = "kubernetes.io/createdby" + + // kubernetesPluginPathPrefix is the prefix of kubernetes plugin mount paths. + kubernetesPluginPathPrefix = "/plugins/kubernetes.io/" ) // IsReady checks for the existence of a regular file @@ -635,12 +638,30 @@ func FsUserFrom(pod *v1.Pod) *int64 { // In GCI cluster, if gci mounter is used for mounting, the container started by mounter // script will cause additional mounts created in the container. Since these mounts are // irrelevant to the original mounts, they should be not considered when checking the -// mount references. Current solution is to filter out those mount paths that contain -// the string of original mount path. -// Plan to work on better approach to solve this issue. +// mount references. The current solution is to filter out those mount paths that contain +// the k8s plugin suffix of original mount path. func HasMountRefs(mountPath string, mountRefs []string) bool { + // A mountPath typically is like + // /var/lib/kubelet/plugins/kubernetes.io/some-plugin/mounts/volume-XXXX + // Mount refs can look like + // /home/somewhere/var/lib/kubelet/plugins/kubernetes.io/some-plugin/... + // but if /var/lib/kubelet is mounted to a different device a ref might be like + // /mnt/some-other-place/kubelet/plugins/kubernetes.io/some-plugin/... + // Neither of the above should be counted as a mount ref as those are handled + // by the kubelet. What we're concerned about is a path like + // /data/local/some/manual/mount + // As unmonting could interrupt usage from that mountpoint. + // + // So instead of looking for the entire /var/lib/... path, the plugins/kuberentes.io/ + // suffix is trimmed off and searched for. + // + // If there isn't a /plugins/... path, the whole mountPath is used instead. + pathToFind := mountPath + if i := strings.Index(mountPath, kubernetesPluginPathPrefix); i > -1 { + pathToFind = mountPath[i:] + } for _, ref := range mountRefs { - if !strings.Contains(ref, mountPath) { + if !strings.Contains(ref, pathToFind) { return true } } diff --git a/pkg/volume/util/util_test.go b/pkg/volume/util/util_test.go index 0274f3cbac2..0f3d99c837e 100644 --- a/pkg/volume/util/util_test.go +++ b/pkg/volume/util/util_test.go @@ -260,6 +260,41 @@ func TestFsUserFrom(t *testing.T) { } } +func TestHasMountRefs(t *testing.T) { + testCases := map[string]struct { + mountPath string + mountRefs []string + expected bool + }{ + "plugin mounts only": { + mountPath: "/var/lib/kubelet/plugins/kubernetes.io/some-plugin/mounts/volume-XXXX", + mountRefs: []string{ + "/home/somewhere/var/lib/kubelet/plugins/kubernetes.io/some-plugin/mounts/volume-XXXX", + "/var/lib/kubelet/plugins/kubernetes.io/some-plugin/mounts/volume-XXXX", + "/mnt/kubelet/plugins/kubernetes.io/some-plugin/mounts/volume-XXXX", + "/mnt/plugins/kubernetes.io/some-plugin/mounts/volume-XXXX", + }, + expected: false, + }, + "extra local mount": { + mountPath: "/var/lib/kubelet/plugins/kubernetes.io/some-plugin/mounts/volume-XXXX", + mountRefs: []string{ + "/home/somewhere/var/lib/kubelet/plugins/kubernetes.io/some-plugin/mounts/volume-XXXX", + "/local/data/kubernetes.io/some-plugin/mounts/volume-XXXX", + "/mnt/kubelet/plugins/kubernetes.io/some-plugin/mounts/volume-XXXX", + "/mnt/plugins/kubernetes.io/some-plugin/mounts/volume-XXXX", + }, + expected: true, + }, + } + for name, test := range testCases { + actual := HasMountRefs(test.mountPath, test.mountRefs) + if actual != test.expected { + t.Errorf("for %s expected %v but got %v", name, test.expected, actual) + } + } +} + func TestMountOptionFromSpec(t *testing.T) { scenarios := map[string]struct { volume *volume.Spec