From 87551071a177a22bbbfe7f8953eec6f872892033 Mon Sep 17 00:00:00 2001
From: Jan Safranek <jsafrane@redhat.com>
Date: Tue, 18 Jul 2017 13:35:44 +0200
Subject: [PATCH] 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.
---
 pkg/util/mount/BUILD                 |  1 +
 pkg/util/mount/nsenter_mount.go      | 19 +++++++-
 pkg/util/mount/nsenter_mount_test.go | 67 ++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 2 deletions(-)
 create mode 100644 pkg/util/mount/nsenter_mount_test.go

diff --git a/pkg/util/mount/BUILD b/pkg/util/mount/BUILD
index 906c610f98d..3e5a2d7203a 100644
--- a/pkg/util/mount/BUILD
+++ b/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",
diff --git a/pkg/util/mount/nsenter_mount.go b/pkg/util/mount/nsenter_mount.go
index 4af8ef0d82e..5a7654d371e 100644
--- a/pkg/util/mount/nsenter_mount.go
+++ b/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.
diff --git a/pkg/util/mount/nsenter_mount_test.go b/pkg/util/mount/nsenter_mount_test.go
new file mode 100644
index 00000000000..946c0feb2d3
--- /dev/null
+++ b/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)
+		}
+	}
+}