Merge pull request #49111 from jsafrane/nsenter-path-space

Automatic merge from submit-queue

Fix findmnt parsing in containerized kubelet

NsEnterMounter should not stop parsing findmnt output on the first space but on the last one, just in case the mount point name itself contains a space.

Fixes #49106

```release-note
Fixed unmounting of vSphere volumes when kubelet runs in a container.
```

@kubernetes/sig-storage-pr-reviews

pkg/util/mount/BUILD

@@ -29,6 +29,7 @@ go_test(
     name = "go_default_test",
     srcs = [
         "mount_linux_test.go",
+        "nsenter_mount_test.go",
         "safe_format_and_mount_test.go",
     ],
     library = ":go_default_library",

pkg/util/mount/nsenter_mount.go

@@ -201,8 +201,11 @@ func (n *NsenterMounter) IsLikelyNotMountPoint(file string) (bool, error) {
 		// It's safer to assume that it's not a mount point.
 		return true, nil
 	}
-	mountTarget := strings.Split(string(out), " ")[0]
-	mountTarget = strings.TrimSuffix(mountTarget, "\n")
+	mountTarget, err := parseFindMnt(string(out))
+	if err != nil {
+		return false, err
+	}
+
 	glog.V(5).Infof("IsLikelyNotMountPoint findmnt output for path %s: %v:", file, mountTarget)
 
 	if mountTarget == file {
@@ -213,6 +216,18 @@ func (n *NsenterMounter) IsLikelyNotMountPoint(file string) (bool, error) {
 	return true, nil
 }
 
+// parse output of "findmnt -o target,fstype" and return just the target
+func parseFindMnt(out string) (string, error) {
+	// cut trailing newline
+	out = strings.TrimSuffix(out, "\n")
+	// cut everything after the last space - it's the filesystem type
+	i := strings.LastIndex(out, " ")
+	if i == -1 {
+		return "", fmt.Errorf("error parsing findmnt output, expected at least one space: %q", out)
+	}
+	return out[:i], nil
+}
+
 // DeviceOpened checks if block device in use by calling Open with O_EXCL flag.
 // Returns true if open returns errno EBUSY, and false if errno is nil.
 // Returns an error if errno is any error other than EBUSY.

pkg/util/mount/nsenter_mount_test.go

@@ -0,0 +1,67 @@
+// +build linux
+
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package mount
+
+import "testing"
+
+func TestParseFindMnt(t *testing.T) {
+	tests := []struct {
+		input       string
+		target      string
+		expectError bool
+	}{
+		{
+			// standard mount name, e.g. for AWS
+			"/var/lib/kubelet/plugins/kubernetes.io/aws-ebs/mounts/aws/us-east-1d/vol-020f82b0759f72389 ext4\n",
+			"/var/lib/kubelet/plugins/kubernetes.io/aws-ebs/mounts/aws/us-east-1d/vol-020f82b0759f72389",
+			false,
+		},
+		{
+			// mount name with space, e.g. vSphere
+			"/var/lib/kubelet/plugins/kubernetes.io/vsphere-volume/mounts/[datastore1] kubevols/kubernetes-dynamic-pvc-4aacaa9b-6ba5-11e7-8f64-0050569f1b82.vmdk ext2\n",
+			"/var/lib/kubelet/plugins/kubernetes.io/vsphere-volume/mounts/[datastore1] kubevols/kubernetes-dynamic-pvc-4aacaa9b-6ba5-11e7-8f64-0050569f1b82.vmdk",
+			false,
+		},
+		{
+			// hypotetic mount with several spaces
+			"/var/lib/kubelet/plugins/kubernetes.io/vsphere-volume/mounts/[ d a t a s t o r e 1 ] kubevols/kubernetes-dynamic-pvc-4aacaa9b-6ba5-11e7-8f64-0050569f1b82.vmdk ext2\n",
+			"/var/lib/kubelet/plugins/kubernetes.io/vsphere-volume/mounts/[ d a t a s t o r e 1 ] kubevols/kubernetes-dynamic-pvc-4aacaa9b-6ba5-11e7-8f64-0050569f1b82.vmdk",
+			false,
+		},
+		{
+			// invalid output - no filesystem type
+			"/var/lib/kubelet/plugins/kubernetes.io/vsphere-volume/mounts/blabla",
+			"",
+			true,
+		},
+	}
+
+	for i, test := range tests {
+		target, err := parseFindMnt(test.input)
+		if test.expectError && err == nil {
+			t.Errorf("test %d expected error, got nil", i)
+		}
+		if !test.expectError && err != nil {
+			t.Errorf("test %d returned error: %s", i, err)
+		}
+		if target != test.target {
+			t.Errorf("test %d expected %q, got %q", i, test.target, target)
+		}
+	}
+}