Merge pull request #62886 from msau42/fix-localssd-fsgroup

Automatic merge from submit-queue (batch tested with PRs 62780, 62886). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Only count local mounts that are from other pods

**What this PR does / why we need it**:
In GCE, we mount the same local SSD in two different paths (for backwards compatability).  This makes the fsGroup conflict check fail because it thinks the 2nd mount is from another pod.  For the fsgroup check, we only want to detect if other pods are mounting the same volume, so this PR filters the mount list to only those mounts under "/var/lib/kubelet".

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #62867

**Release note**:

```release-note
NONE
```

pkg/controller/volume/attachdetach/attach_detach_controller.go

@@ -522,6 +522,10 @@ func (adc *attachDetachController) GetVolumeDevicePluginDir(podUID string) strin
 	return ""
 }
 
+func (adc *attachDetachController) GetPodsDir() string {
+	return ""
+}
+
 func (adc *attachDetachController) GetPodVolumeDir(podUID types.UID, pluginName, volumeName string) string {
 	return ""
 }

pkg/controller/volume/expand/expand_controller.go

@@ -227,6 +227,10 @@ func (expc *expandController) GetVolumeDevicePluginDir(pluginName string) string
 	return ""
 }
 
+func (expc *expandController) GetPodsDir() string {
+	return ""
+}
+
 func (expc *expandController) GetPodVolumeDir(podUID types.UID, pluginName string, volumeName string) string {
 	return ""
 }

pkg/controller/volume/persistentvolume/volume_host.go

@@ -42,6 +42,10 @@ func (ctrl *PersistentVolumeController) GetVolumeDevicePluginDir(pluginName stri
 	return ""
 }
 
+func (ctrl *PersistentVolumeController) GetPodsDir() string {
+	return ""
+}
+
 func (ctrl *PersistentVolumeController) GetPodVolumeDir(podUID types.UID, pluginName string, volumeName string) string {
 	return ""
 }

pkg/kubelet/volume_host.go

@@ -93,6 +93,10 @@ func (kvh *kubeletVolumeHost) GetVolumeDevicePluginDir(pluginName string) string
 	return kvh.kubelet.getVolumeDevicePluginDir(pluginName)
 }
 
+func (kvh *kubeletVolumeHost) GetPodsDir() string {
+	return kvh.kubelet.getPodsDir()
+}
+
 func (kvh *kubeletVolumeHost) GetPodVolumeDir(podUID types.UID, pluginName string, volumeName string) string {
 	dir := kvh.kubelet.getPodVolumeDir(podUID, pluginName, volumeName)
 	if runtime.GOOS == "windows" {

pkg/volume/local/local.go

@@ -19,8 +19,9 @@ package local
 import (
 	"fmt"
 	"os"
-	"path"
+	"path/filepath"
 	"runtime"
+	"strings"
 
 	"github.com/golang/glog"
 
@@ -31,7 +32,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/events"
 	"k8s.io/kubernetes/pkg/util/keymutex"
 	"k8s.io/kubernetes/pkg/util/mount"
-	"k8s.io/kubernetes/pkg/util/strings"
+	stringsutil "k8s.io/kubernetes/pkg/util/strings"
 	"k8s.io/kubernetes/pkg/volume"
 	"k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/validation"
@@ -219,7 +220,7 @@ type localVolume struct {
 }
 
 func (l *localVolume) GetPath() string {
-	return l.plugin.host.GetPodVolumeDir(l.podUID, strings.EscapeQualifiedNameForDisk(localVolumePluginName), l.volName)
+	return l.plugin.host.GetPodVolumeDir(l.podUID, stringsutil.EscapeQualifiedNameForDisk(localVolumePluginName), l.volName)
 }
 
 type localVolumeMounter struct {
@@ -280,6 +281,8 @@ func (m *localVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
 			return err
 		}
 
+		// Only count mounts from other pods
+		refs = m.filterPodMounts(refs)
 		if len(refs) > 0 {
 			fsGroupNew := int64(*fsGroup)
 			fsGroupSame, fsGroupOld, err := volume.IsSameFSGroup(m.globalPath, fsGroupNew)
@@ -344,6 +347,17 @@ func (m *localVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
 	return nil
 }
 
+// filterPodMounts only returns mount paths inside the kubelet pod directory
+func (m *localVolumeMounter) filterPodMounts(refs []string) []string {
+	filtered := []string{}
+	for _, r := range refs {
+		if strings.HasPrefix(r, m.plugin.host.GetPodsDir()+string(os.PathSeparator)) {
+			filtered = append(filtered, r)
+		}
+	}
+	return filtered
+}
+
 type localVolumeUnmounter struct {
 	*localVolume
 }
@@ -392,7 +406,7 @@ func (u *localVolumeUnmapper) TearDownDevice(mapPath, devicePath string) error {
 // GetGlobalMapPath returns global map path and error.
 // path: plugins/kubernetes.io/kubernetes.io/local-volume/volumeDevices/{volumeName}
 func (lv *localVolume) GetGlobalMapPath(spec *volume.Spec) (string, error) {
-	return path.Join(lv.plugin.host.GetVolumeDevicePluginDir(strings.EscapeQualifiedNameForDisk(localVolumePluginName)),
+	return filepath.Join(lv.plugin.host.GetVolumeDevicePluginDir(stringsutil.EscapeQualifiedNameForDisk(localVolumePluginName)),
 		lv.volName), nil
 }
 
@@ -401,5 +415,5 @@ func (lv *localVolume) GetGlobalMapPath(spec *volume.Spec) (string, error) {
 // volName: local-pv-ff0d6d4
 func (lv *localVolume) GetPodDeviceMapPath() (string, string) {
 	return lv.plugin.host.GetPodVolumeDeviceDir(lv.podUID,
-		strings.EscapeQualifiedNameForDisk(localVolumePluginName)), lv.volName
+		stringsutil.EscapeQualifiedNameForDisk(localVolumePluginName)), lv.volName
 }

pkg/volume/local/local_test.go

@@ -22,6 +22,8 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"path/filepath"
+	"reflect"
 	"runtime"
 	"testing"
 
@@ -447,3 +449,57 @@ func TestUnsupportedPlugins(t *testing.T) {
 		t.Errorf("Provisionable plugin found, expected none")
 	}
 }
+
+func TestFilterPodMounts(t *testing.T) {
+	tmpDir, plug := getPlugin(t)
+	defer os.RemoveAll(tmpDir)
+
+	pod := &v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: types.UID("poduid")}}
+	mounter, err := plug.NewMounter(getTestVolume(false, tmpDir, false), pod, volume.VolumeOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	lvMounter, ok := mounter.(*localVolumeMounter)
+	if !ok {
+		t.Fatal("mounter is not localVolumeMounter")
+	}
+
+	host := volumetest.NewFakeVolumeHost(tmpDir, nil, nil)
+	podsDir := host.GetPodsDir()
+
+	cases := map[string]struct {
+		input    []string
+		expected []string
+	}{
+		"empty": {
+			[]string{},
+			[]string{},
+		},
+		"not-pod-mount": {
+			[]string{"/mnt/outside"},
+			[]string{},
+		},
+		"pod-mount": {
+			[]string{filepath.Join(podsDir, "pod-mount")},
+			[]string{filepath.Join(podsDir, "pod-mount")},
+		},
+		"not-directory-prefix": {
+			[]string{podsDir + "pod-mount"},
+			[]string{},
+		},
+		"mix": {
+			[]string{"/mnt/outside",
+				filepath.Join(podsDir, "pod-mount"),
+				"/another/outside",
+				filepath.Join(podsDir, "pod-mount2")},
+			[]string{filepath.Join(podsDir, "pod-mount"),
+				filepath.Join(podsDir, "pod-mount2")},
+		},
+	}
+	for name, test := range cases {
+		output := lvMounter.filterPodMounts(test.input)
+		if !reflect.DeepEqual(output, test.expected) {
+			t.Errorf("%v failed: output %+v doesn't equal expected %+v", name, output, test.expected)
+		}
+	}
+}

pkg/volume/plugins.go

@@ -249,6 +249,10 @@ type VolumeHost interface {
 	// ex. plugins/kubernetes.io/{PluginName}/{DefaultKubeletVolumeDevicesDirName}/{volumePluginDependentPath}/
 	GetVolumeDevicePluginDir(pluginName string) string
 
+	// GetPodsDir returns the absolute path to a directory where all the pods
+	// information is stored
+	GetPodsDir() string
+
 	// GetPodVolumeDir returns the absolute path a directory which
 	// represents the named volume under the named plugin for the given
 	// pod.  If the specified pod does not exist, the result of this call

pkg/volume/testing/testing.go

@@ -95,6 +95,10 @@ func (f *fakeVolumeHost) GetVolumeDevicePluginDir(pluginName string) string {
 	return path.Join(f.rootDir, "plugins", pluginName, "volumeDevices")
 }
 
+func (f *fakeVolumeHost) GetPodsDir() string {
+	return path.Join(f.rootDir, "pods")
+}
+
 func (f *fakeVolumeHost) GetPodVolumeDir(podUID types.UID, pluginName, volumeName string) string {
 	return path.Join(f.rootDir, "pods", string(podUID), "volumes", pluginName, volumeName)
 }