Merge pull request #47400 from chakri-nelluri/fixgetvolumename

Automatic merge from submit-queue (batch tested with PRs 47204, 46808, 47432, 47400, 47099)

Remove broken getvolumename and pass PV or volume name to attach call

Cherry-picking https://github.com/kubernetes/kubernetes/pull/46249 to master

What this PR does / why we need it:
Flex getvolumename is broken in 1.6. It needs to be fixed comprehensively in 1.7 release. Removing the api in 1.6. Also pass PV or volume name to the driver during attach call. Detach uses PV or volume name, so plugin can use that information to map to PV.
Which issue this PR fixes (optional, in fixes #(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #
Fixes - #44737

examples/volumes/flexvolume/lvm

@@ -24,7 +24,6 @@ usage() {
 	err "\t$0 waitforattach <mount device> <json params>"
 	err "\t$0 mountdevice <mount dir> <mount device> <json params>"
 	err "\t$0 unmountdevice <mount dir>"
-	err "\t$0 getvolumename <json params>"
 	err "\t$0 isattached <json params> <nodename>"
 	exit 1
 }
@@ -138,17 +137,6 @@ unmountdevice() {
 	exit 0
 }
 
-getvolumename() {
-	JSON_PARAMS=$1
-	DMDEV=$(getdevice)
-
-	# get lvm device UUID
-	UUID=`lvs -o lv_uuid --noheadings ${DMDEV} 2>/dev/null|tr -d " "`
-
-	log "{\"status\": \"Success\", \"volumeName\":\"${UUID}\"}"
-	exit 0
-}
-
 isattached() {
 	log "{\"status\": \"Success\", \"attached\":true}"
 	exit 0
@@ -183,15 +171,12 @@ case "$op" in
 	unmountdevice)
 		unmountdevice $*
 		;;
-	getvolumename)
-		getvolumename $*
-		;;
 	isattached)
                 isattached $*
                 ;;
 	*)
-		err "{ \"status\": \"Not supported\" }"
-		exit 1
+		log "{ \"status\": \"Not supported\" }"
+		exit 0
 esac
 
 exit 1

examples/volumes/flexvolume/nfs

@@ -21,7 +21,6 @@ usage() {
 	err "\t$0 init"
 	err "\t$0 mount <mount dir> <json params>"
 	err "\t$0 unmount <mount dir>"
-	err "\t$0 getvolumename <json params>"
 	exit 1
 }
 
@@ -81,14 +80,6 @@ unmount() {
 	exit 0
 }
 
-getvolumename() {
-	NFS_SERVER=$(echo $1 | jq -r '.server')
-	SHARE=$(echo $1 | jq -r '.share')
-
-	log "{\"status\": \"Success\", \"volumeName\": \"${NFS_SERVER}/${SHARE}\"}"
-	exit 0
-}
-
 op=$1
 
 if [ "$op" = "init" ]; then
@@ -113,8 +104,8 @@ case "$op" in
 		getvolumename $*
 		;;
 	*)
-		err "{ \"status\": \"Not supported\" }"
-		exit 1
+		log "{ \"status\": \"Not supported\" }"
+		exit 0
 esac
 
 exit 1

pkg/volume/flexvolume/attacher.go

@@ -33,6 +33,7 @@ var _ volume.Attacher = &flexVolumeAttacher{}
 
 // Attach is part of the volume.Attacher interface
 func (a *flexVolumeAttacher) Attach(spec *volume.Spec, hostName types.NodeName) (string, error) {
+
 	call := a.plugin.NewDriverCall(attachCmd)
 	call.AppendSpec(spec, a.plugin.host, nil)
 	call.Append(string(hostName))

pkg/volume/flexvolume/detacher.go

@@ -34,14 +34,15 @@ type flexVolumeDetacher struct {
 var _ volume.Detacher = &flexVolumeDetacher{}
 
 // Detach is part of the volume.Detacher interface.
-func (d *flexVolumeDetacher) Detach(deviceName string, hostName types.NodeName) error {
+func (d *flexVolumeDetacher) Detach(pvOrVolumeName string, hostName types.NodeName) error {
+
 	call := d.plugin.NewDriverCall(detachCmd)
-	call.Append(deviceName)
+	call.Append(pvOrVolumeName)
 	call.Append(string(hostName))
 
 	_, err := call.Run()
 	if isCmdNotSupportedErr(err) {
-		return (*detacherDefaults)(d).Detach(deviceName, hostName)
+		return (*detacherDefaults)(d).Detach(pvOrVolumeName, hostName)
 	}
 	return err
 }

pkg/volume/flexvolume/driver-call.go

@@ -46,11 +46,12 @@ const (
 	unmountCmd = "unmount"
 
 	// Option keys
-	optionFSType    = "kubernetes.io/fsType"
-	optionReadWrite = "kubernetes.io/readwrite"
-	optionKeySecret = "kubernetes.io/secret"
-	optionFSGroup   = "kubernetes.io/fsGroup"
-	optionMountsDir = "kubernetes.io/mountsDir"
+	optionFSType         = "kubernetes.io/fsType"
+	optionReadWrite      = "kubernetes.io/readwrite"
+	optionKeySecret      = "kubernetes.io/secret"
+	optionFSGroup        = "kubernetes.io/fsGroup"
+	optionMountsDir      = "kubernetes.io/mountsDir"
+	optionPVorVolumeName = "kubernetes.io/pvOrVolumeName"
 
 	optionKeyPodName      = "kubernetes.io/pod.name"
 	optionKeyPodNamespace = "kubernetes.io/pod.namespace"
@@ -174,6 +175,8 @@ func NewOptionsForDriver(spec *volume.Spec, host volume.VolumeHost, extraOptions
 		options[optionReadWrite] = "rw"
 	}
 
+	options[optionPVorVolumeName] = spec.Name()
+
 	for key, value := range extraOptions {
 		options[key] = value
 	}

pkg/volume/flexvolume/plugin.go

@@ -21,11 +21,12 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/golang/glog"
+
 	"k8s.io/apimachinery/pkg/types"
 	api "k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/util/exec"
 	"k8s.io/kubernetes/pkg/util/mount"
-	utilstrings "k8s.io/kubernetes/pkg/util/strings"
 	"k8s.io/kubernetes/pkg/volume"
 )
 
@@ -70,13 +71,21 @@ func (plugin *flexVolumePlugin) GetVolumeName(spec *volume.Spec) (string, error)
 	call := plugin.NewDriverCall(getVolumeNameCmd)
 	call.AppendSpec(spec, plugin.host, nil)
 
-	status, err := call.Run()
+	_, err := call.Run()
 	if isCmdNotSupportedErr(err) {
 		return (*pluginDefaults)(plugin).GetVolumeName(spec)
 	} else if err != nil {
 		return "", err
 	}
-	return utilstrings.EscapeQualifiedNameForDisk(status.VolumeName), nil
+
+	name, err := (*pluginDefaults)(plugin).GetVolumeName(spec)
+	if err != nil {
+		return "", err
+	}
+
+	glog.Warning(logPrefix(plugin), "GetVolumeName is not supported yet. Defaulting to PV or volume name: ", name)
+
+	return name, nil
 }
 
 // CanSupport is part of the volume.VolumePlugin interface.

pkg/volume/flexvolume/plugin_test.go

@@ -41,7 +41,7 @@ func TestGetVolumeName(t *testing.T) {
 	spec := fakeVolumeSpec()
 	plugin, _ := testPlugin()
 	plugin.runner = fakeRunner(
-		assertDriverCall(t, fakeVolumeNameOutput("/dev/sdx"), getVolumeNameCmd,
+		assertDriverCall(t, fakeVolumeNameOutput(spec.Name()), getVolumeNameCmd,
 			specJson(plugin, spec, nil)),
 	)
 
@@ -49,7 +49,7 @@ func TestGetVolumeName(t *testing.T) {
 	if err != nil {
 		t.Errorf("GetVolumeName() failed: %v", err)
 	}
-	expectedName := "~dev~sdx"
+	expectedName := spec.Name()
 	if name != expectedName {
 		t.Errorf("GetVolumeName() returned %v instead of %v", name, expectedName)
 	}