Merge pull request #58171 from verult/NoPath-FlexDirExec

Automatic merge from submit-queue (batch tested with PRs 58171, 58036, 60540). 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>.

Changing Flexvolume plugin directory on COS in GCE to a durable directory

**What this PR does / why we need it**: The original `/etc/srv/...` directory is in an overlayfs over a path in /tmp, so Flexvolume drivers are erased across node restarts for any reason. Changing it to non-tmpfs location.

Also removing redundant Flexvolume path injection in `config-test.sh` because it's already in `cluster/common.sh`.

**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 #57353

**Release note**:

```release-note
[action required] Default Flexvolume plugin directory for COS images on GCE is changed to `/home/kubernetes/flexvolume`.
```

/assign @roberthbailey @saad-ali 
/cc @chakri-nelluri @wongma7 
/sig storage

cluster/gce/config-default.sh

@@ -125,7 +125,8 @@ NODE_SCOPES="${NODE_SCOPES:-monitoring,logging-write,storage-ro}"
 # Extra docker options for nodes.
 EXTRA_DOCKER_OPTS="${EXTRA_DOCKER_OPTS:-}"
 
-VOLUME_PLUGIN_DIR="${VOLUME_PLUGIN_DIR:-/etc/srv/kubernetes/kubelet-plugins/volume/exec}"
+VOLUME_PLUGIN_DIR="${VOLUME_PLUGIN_DIR:-/home/kubernetes/flexvolume}"
+REMOUNT_VOLUME_PLUGIN_DIR="${REMOUNT_VOLUME_PLUGIN_DIR:-true}"
 
 SERVICE_CLUSTER_IP_RANGE="${SERVICE_CLUSTER_IP_RANGE:-10.0.0.0/16}"  # formerly PORTAL_NET
 ALLOCATE_NODE_CIDRS=true

cluster/gce/config-test.sh

@@ -175,17 +175,8 @@ CONTROLLER_MANAGER_TEST_LOG_LEVEL="${CONTROLLER_MANAGER_TEST_LOG_LEVEL:-$TEST_CL
 SCHEDULER_TEST_LOG_LEVEL="${SCHEDULER_TEST_LOG_LEVEL:-$TEST_CLUSTER_LOG_LEVEL}"
 KUBEPROXY_TEST_LOG_LEVEL="${KUBEPROXY_TEST_LOG_LEVEL:-$TEST_CLUSTER_LOG_LEVEL}"
 
-VOLUME_PLUGIN_DIR="${VOLUME_PLUGIN_DIR:-/etc/srv/kubernetes/kubelet-plugins/volume/exec}"
+VOLUME_PLUGIN_DIR="${VOLUME_PLUGIN_DIR:-/home/kubernetes/flexvolume}"
-
+REMOUNT_VOLUME_PLUGIN_DIR="${REMOUNT_VOLUME_PLUGIN_DIR:-true}"
-# TODO: change this and flex e2e test when default flex volume install path is changed for GCI
-# Set flex dir to one that's readable from controller-manager container and writable by the flex e2e test.
-if [[ "${MASTER_OS_DISTRIBUTION}" == "gci" ]]; then
-    CONTROLLER_MANAGER_TEST_VOLUME_PLUGIN_DIR="--flex-volume-plugin-dir=/etc/srv/kubernetes/kubelet-plugins/volume/exec"
-fi
-# Set flex dir to one that's readable from kubelet and writable by the flex e2e test.
-if [[ "${NODE_OS_DISTRIBUTION}" == "gci" ]] || ([[ "${MASTER_OS_DISTRIBUTION}" == "gci" ]] && [[ "${REGISTER_MASTER_KUBELET}" == "false" ]]); then
-    KUBELET_TEST_VOLUME_PLUGIN_DIR="--volume-plugin-dir=/etc/srv/kubernetes/kubelet-plugins/volume/exec"
-fi
 
 TEST_CLUSTER_DELETE_COLLECTION_WORKERS="${TEST_CLUSTER_DELETE_COLLECTION_WORKERS:---delete-collection-workers=1}"
 TEST_CLUSTER_MAX_REQUESTS_INFLIGHT="${TEST_CLUSTER_MAX_REQUESTS_INFLIGHT:-}"
@@ -194,7 +185,7 @@ TEST_CLUSTER_RESYNC_PERIOD="${TEST_CLUSTER_RESYNC_PERIOD:---min-resync-period=3m
 # ContentType used by all components to communicate with apiserver.
 TEST_CLUSTER_API_CONTENT_TYPE="${TEST_CLUSTER_API_CONTENT_TYPE:-}"
 
-KUBELET_TEST_ARGS="${KUBELET_TEST_ARGS:-} --max-pods=110 --serialize-image-pulls=false ${TEST_CLUSTER_API_CONTENT_TYPE} ${KUBELET_TEST_VOLUME_PLUGIN_DIR:-}"
+KUBELET_TEST_ARGS="${KUBELET_TEST_ARGS:-} --max-pods=110 --serialize-image-pulls=false ${TEST_CLUSTER_API_CONTENT_TYPE}"
 if [[ "${NODE_OS_DISTRIBUTION}" == "gci" ]] || [[ "${NODE_OS_DISTRIBUTION}" == "ubuntu" ]]; then
   NODE_KUBELET_TEST_ARGS=" --experimental-kernel-memcg-notification=true"
 fi
@@ -202,7 +193,7 @@ if [[ "${MASTER_OS_DISTRIBUTION}" == "gci" ]] || [[ "${MASTER_OS_DISTRIBUTION}"
   MASTER_KUBELET_TEST_ARGS=" --experimental-kernel-memcg-notification=true"
 fi
 APISERVER_TEST_ARGS="${APISERVER_TEST_ARGS:-} --runtime-config=extensions/v1beta1 ${TEST_CLUSTER_DELETE_COLLECTION_WORKERS} ${TEST_CLUSTER_MAX_REQUESTS_INFLIGHT}"
-CONTROLLER_MANAGER_TEST_ARGS="${CONTROLLER_MANAGER_TEST_ARGS:-} ${TEST_CLUSTER_RESYNC_PERIOD} ${TEST_CLUSTER_API_CONTENT_TYPE} ${CONTROLLER_MANAGER_TEST_VOLUME_PLUGIN_DIR:-}"
+CONTROLLER_MANAGER_TEST_ARGS="${CONTROLLER_MANAGER_TEST_ARGS:-} ${TEST_CLUSTER_RESYNC_PERIOD} ${TEST_CLUSTER_API_CONTENT_TYPE}"
 SCHEDULER_TEST_ARGS="${SCHEDULER_TEST_ARGS:-} ${TEST_CLUSTER_API_CONTENT_TYPE}"
 KUBEPROXY_TEST_ARGS="${KUBEPROXY_TEST_ARGS:-} ${TEST_CLUSTER_API_CONTENT_TYPE}"
 

cluster/gce/gci/flexvolume_node_setup.sh

@@ -32,7 +32,7 @@ set -o pipefail
 
 MOUNTER_IMAGE=${1:-}
 MOUNTER_PATH=/home/kubernetes/flexvolume_mounter
-VOLUME_PLUGIN_DIR=/etc/srv/kubernetes/kubelet-plugins/volume/exec
+VOLUME_PLUGIN_DIR=/home/kubernetes/flexvolume
 
 usage() {
   echo "usage: $0 imagename[:tag]"

test/e2e/storage/BUILD

@@ -37,6 +37,7 @@ go_library(
         "//pkg/kubelet/apis:go_default_library",
         "//pkg/kubelet/metrics:go_default_library",
         "//pkg/util/slice:go_default_library",
+        "//pkg/util/version:go_default_library",
         "//pkg/volume/util:go_default_library",
         "//test/e2e/framework:go_default_library",
         "//test/e2e/framework/metrics:go_default_library",
@@ -72,6 +73,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/version:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/authentication/serviceaccount:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",

test/e2e/storage/flexvolume.go

@@ -22,10 +22,15 @@ import (
 	"net"
 	"path"
 
+	"time"
+
 	. "github.com/onsi/ginkgo"
 	"k8s.io/api/core/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/apimachinery/pkg/version"
 	clientset "k8s.io/client-go/kubernetes"
+	versionutil "k8s.io/kubernetes/pkg/util/version"
 	"k8s.io/kubernetes/test/e2e/framework"
 	"k8s.io/kubernetes/test/e2e/generated"
 	"k8s.io/kubernetes/test/e2e/storage/utils"
@@ -39,7 +44,9 @@ const (
 	// On gci, root is read-only and controller-manager containerized. Assume
 	// controller-manager has started with --flex-volume-plugin-dir equal to this
 	// (see cluster/gce/config-test.sh)
-	gciVolumePluginDir = "/etc/srv/kubernetes/kubelet-plugins/volume/exec"
+	gciVolumePluginDir        = "/home/kubernetes/flexvolume"
+	gciVolumePluginDirLegacy  = "/etc/srv/kubernetes/kubelet-plugins/volume/exec"
+	gciVolumePluginDirVersion = "1.10.0"
 )
 
 // testFlexVolume tests that a client pod using a given flexvolume driver
@@ -65,8 +72,8 @@ func testFlexVolume(driver string, cs clientset.Interface, config framework.Volu
 // installFlex installs the driver found at filePath on the node, and restarts
 // kubelet if 'restart' is true. If node is nil, installs on the master, and restarts
 // controller-manager if 'restart' is true.
-func installFlex(node *v1.Node, vendor, driver, filePath string, restart bool) {
+func installFlex(c clientset.Interface, node *v1.Node, vendor, driver, filePath string, restart bool) {
-	flexDir := getFlexDir(node == nil, vendor, driver)
+	flexDir := getFlexDir(c, node, vendor, driver)
 	flexFile := path.Join(flexDir, driver)
 
 	host := ""
@@ -103,8 +110,8 @@ func installFlex(node *v1.Node, vendor, driver, filePath string, restart bool) {
 	}
 }
 
-func uninstallFlex(node *v1.Node, vendor, driver string) {
+func uninstallFlex(c clientset.Interface, node *v1.Node, vendor, driver string) {
-	flexDir := getFlexDir(node == nil, vendor, driver)
+	flexDir := getFlexDir(c, node, vendor, driver)
 
 	host := ""
 	if node != nil {
@@ -117,11 +124,26 @@ func uninstallFlex(node *v1.Node, vendor, driver string) {
 	sshAndLog(cmd, host)
 }
 
-func getFlexDir(master bool, vendor, driver string) string {
+func getFlexDir(c clientset.Interface, node *v1.Node, vendor, driver string) string {
 	volumePluginDir := defaultVolumePluginDir
 	if framework.ProviderIs("gce") {
-		if (master && framework.MasterOSDistroIs("gci")) || (!master && framework.NodeOSDistroIs("gci")) {
+		if node == nil && framework.MasterOSDistroIs("gci") {
-			volumePluginDir = gciVolumePluginDir
+			v, err := getMasterVersion(c)
+			if err != nil {
+				framework.Failf("Error getting master version: %v", err)
+			}
+
+			if v.AtLeast(versionutil.MustParseGeneric(gciVolumePluginDirVersion)) {
+				volumePluginDir = gciVolumePluginDir
+			} else {
+				volumePluginDir = gciVolumePluginDirLegacy
+			}
+		} else if node != nil && framework.NodeOSDistroIs("gci") {
+			if getNodeVersion(node).AtLeast(versionutil.MustParseGeneric(gciVolumePluginDirVersion)) {
+				volumePluginDir = gciVolumePluginDir
+			} else {
+				volumePluginDir = gciVolumePluginDirLegacy
+			}
 		}
 	}
 	flexDir := path.Join(volumePluginDir, fmt.Sprintf("/%s~%s/", vendor, driver))
@@ -137,6 +159,24 @@ func sshAndLog(cmd, host string) {
 	}
 }
 
+func getMasterVersion(c clientset.Interface) (*versionutil.Version, error) {
+	var err error
+	var v *version.Info
+	waitErr := wait.PollImmediate(5*time.Second, 2*time.Minute, func() (bool, error) {
+		v, err = c.Discovery().ServerVersion()
+		return err == nil, nil
+	})
+	if waitErr != nil {
+		return nil, fmt.Errorf("Could not get the master version: %v", waitErr)
+	}
+
+	return versionutil.MustParseSemantic(v.GitVersion), nil
+}
+
+func getNodeVersion(node *v1.Node) *versionutil.Version {
+	return versionutil.MustParseSemantic(node.Status.NodeInfo.KubeletVersion)
+}
+
 var _ = utils.SIGDescribe("Flexvolumes [Disruptive]", func() {
 	f := framework.NewDefaultFramework("flexvolume")
 
@@ -171,7 +211,7 @@ var _ = utils.SIGDescribe("Flexvolumes [Disruptive]", func() {
 		driverInstallAs := driver + "-" + suffix
 
 		By(fmt.Sprintf("installing flexvolume %s on node %s as %s", path.Join(driverDir, driver), node.Name, driverInstallAs))
-		installFlex(&node, "k8s", driverInstallAs, path.Join(driverDir, driver), true /* restart */)
+		installFlex(cs, &node, "k8s", driverInstallAs, path.Join(driverDir, driver), true /* restart */)
 
 		testFlexVolume(driverInstallAs, cs, config, f)
 
@@ -181,7 +221,7 @@ var _ = utils.SIGDescribe("Flexvolumes [Disruptive]", func() {
 		}
 
 		By(fmt.Sprintf("uninstalling flexvolume %s from node %s", driverInstallAs, node.Name))
-		uninstallFlex(&node, "k8s", driverInstallAs)
+		uninstallFlex(cs, &node, "k8s", driverInstallAs)
 	})
 
 	It("should be mountable when attachable", func() {
@@ -189,9 +229,9 @@ var _ = utils.SIGDescribe("Flexvolumes [Disruptive]", func() {
 		driverInstallAs := driver + "-" + suffix
 
 		By(fmt.Sprintf("installing flexvolume %s on node %s as %s", path.Join(driverDir, driver), node.Name, driverInstallAs))
-		installFlex(&node, "k8s", driverInstallAs, path.Join(driverDir, driver), true /* restart */)
+		installFlex(cs, &node, "k8s", driverInstallAs, path.Join(driverDir, driver), true /* restart */)
 		By(fmt.Sprintf("installing flexvolume %s on master as %s", path.Join(driverDir, driver), driverInstallAs))
-		installFlex(nil, "k8s", driverInstallAs, path.Join(driverDir, driver), true /* restart */)
+		installFlex(cs, nil, "k8s", driverInstallAs, path.Join(driverDir, driver), true /* restart */)
 
 		testFlexVolume(driverInstallAs, cs, config, f)
 
@@ -201,9 +241,9 @@ var _ = utils.SIGDescribe("Flexvolumes [Disruptive]", func() {
 		}
 
 		By(fmt.Sprintf("uninstalling flexvolume %s from node %s", driverInstallAs, node.Name))
-		uninstallFlex(&node, "k8s", driverInstallAs)
+		uninstallFlex(cs, &node, "k8s", driverInstallAs)
 		By(fmt.Sprintf("uninstalling flexvolume %s from master", driverInstallAs))
-		uninstallFlex(nil, "k8s", driverInstallAs)
+		uninstallFlex(cs, nil, "k8s", driverInstallAs)
 	})
 
 	It("should install plugin without kubelet restart", func() {
@@ -211,7 +251,7 @@ var _ = utils.SIGDescribe("Flexvolumes [Disruptive]", func() {
 		driverInstallAs := driver + "-" + suffix
 
 		By(fmt.Sprintf("installing flexvolume %s on node %s as %s", path.Join(driverDir, driver), node.Name, driverInstallAs))
-		installFlex(&node, "k8s", driverInstallAs, path.Join(driverDir, driver), false /* restart */)
+		installFlex(cs, &node, "k8s", driverInstallAs, path.Join(driverDir, driver), false /* restart */)
 
 		testFlexVolume(driverInstallAs, cs, config, f)
 
@@ -221,6 +261,6 @@ var _ = utils.SIGDescribe("Flexvolumes [Disruptive]", func() {
 		}
 
 		By(fmt.Sprintf("uninstalling flexvolume %s from node %s", driverInstallAs, node.Name))
-		uninstallFlex(&node, "k8s", driverInstallAs)
+		uninstallFlex(cs, &node, "k8s", driverInstallAs)
 	})
 })