Merge pull request kubernetes#84754 from davidz627/fix/uniqueName

Update inline volume translated PV Name to be unique per disk so that staging paths are unique

Author: k8s-ci-robot

staging/src/k8s.io/csi-translation-lib/BUILD

@@ -20,6 +20,7 @@ go_test(
     deps = [
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
         "//staging/src/k8s.io/csi-translation-lib/plugins:go_default_library",
     ],
 )

staging/src/k8s.io/csi-translation-lib/go.sum

@@ -78,8 +78,9 @@ func (t *awsElasticBlockStoreCSITranslator) TranslateInTreeInlineVolumeToCSI(vol
 	}
 	pv := &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{
-			// A.K.A InnerVolumeSpecName required to match for Unmount
-			Name: volume.Name,
+			// Must be unique per disk as it is used as the unique part of the
+			// staging path
+			Name: fmt.Sprintf("%s-%s", AWSEBSDriverName, ebsSource.VolumeID),
 		},
 		Spec: v1.PersistentVolumeSpec{
 			PersistentVolumeSource: v1.PersistentVolumeSource{

staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go

@@ -70,8 +70,9 @@ func (t *azureDiskCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol
 	azureSource := volume.AzureDisk
 	pv := &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{
-			// A.K.A InnerVolumeSpecName required to match for Unmount
-			Name: volume.Name,
+			// Must be unique per disk as it is used as the unique part of the
+			// staging path
+			Name: fmt.Sprintf("%s-%s", AzureDiskDriverName, azureSource.DiskName),
 		},
 		Spec: v1.PersistentVolumeSpec{
 			PersistentVolumeSource: v1.PersistentVolumeSource{

staging/src/k8s.io/csi-translation-lib/plugins/azure_disk.go

@@ -65,8 +65,9 @@ func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol
 
 	pv := &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{
-			// A.K.A InnerVolumeSpecName required to match for Unmount
-			Name: volume.Name,
+			// Must be unique per disk as it is used as the unique part of the
+			// staging path
+			Name: fmt.Sprintf("%s-%s", AzureFileDriverName, azureSource.ShareName),
 		},
 		Spec: v1.PersistentVolumeSpec{
 			PersistentVolumeSource: v1.PersistentVolumeSource{

staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go

@@ -212,8 +212,9 @@ func (g *gcePersistentDiskCSITranslator) TranslateInTreeInlineVolumeToCSI(volume
 	fsMode := v1.PersistentVolumeFilesystem
 	return &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{
-			// A.K.A InnerVolumeSpecName required to match for Unmount
-			Name: volume.Name,
+			// Must be unique per disk as it is used as the unique part of the
+			// staging path
+			Name: fmt.Sprintf("%s-%s", GCEPDDriverName, pdSource.PDName),
 		},
 		Spec: v1.PersistentVolumeSpec{
 			PersistentVolumeSource: v1.PersistentVolumeSource{

staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go

@@ -58,8 +58,9 @@ func (t *osCinderCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volu
 	cinderSource := volume.Cinder
 	pv := &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{
-			// A.K.A InnerVolumeSpecName required to match for Unmount
-			Name: volume.Name,
+			// Must be unique per disk as it is used as the unique part of the
+			// staging path
+			Name: fmt.Sprintf("%s-%s", CinderDriverName, cinderSource.VolumeID),
 		},
 		Spec: v1.PersistentVolumeSpec{
 			PersistentVolumeSource: v1.PersistentVolumeSource{

staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go

@@ -17,11 +17,13 @@ limitations under the License.
 package csitranslation
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/csi-translation-lib/plugins"
 )
 
@@ -312,6 +314,81 @@ func makeTopology(key string, values ...string) *v1.NodeSelectorRequirement {
 	}
 }
 
+func TestTranslateInTreeInlineVolumeToCSINameUniqueness(t *testing.T) {
+	for driverName := range inTreePlugins {
+		t.Run(driverName, func(t *testing.T) {
+			ctl := New()
+			vs1, err := generateUniqueVolumeSource(driverName)
+			if err != nil {
+				t.Fatalf("Couldn't generate random source: %v", err)
+			}
+			pv1, err := ctl.TranslateInTreeInlineVolumeToCSI(&v1.Volume{
+				VolumeSource: vs1,
+			})
+			if err != nil {
+				t.Fatalf("Error when translating to CSI: %v", err)
+			}
+			vs2, err := generateUniqueVolumeSource(driverName)
+			if err != nil {
+				t.Fatalf("Couldn't generate random source: %v", err)
+			}
+			pv2, err := ctl.TranslateInTreeInlineVolumeToCSI(&v1.Volume{
+				VolumeSource: vs2,
+			})
+			if err != nil {
+				t.Fatalf("Error when translating to CSI: %v", err)
+			}
+			if pv1 == nil || pv2 == nil {
+				t.Fatalf("Did not expect either pv1: %v or pv2: %v to be nil", pv1, pv2)
+			}
+			if pv1.Name == pv2.Name {
+				t.Errorf("PV name %s not sufficiently unique for different volumes", pv1.Name)
+			}
+		})
+
+	}
+}
+
+func generateUniqueVolumeSource(driverName string) (v1.VolumeSource, error) {
+	switch driverName {
+	case plugins.GCEPDDriverName:
+		return v1.VolumeSource{
+			GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{
+				PDName: string(uuid.NewUUID()),
+			},
+		}, nil
+	case plugins.AWSEBSDriverName:
+		return v1.VolumeSource{
+			AWSElasticBlockStore: &v1.AWSElasticBlockStoreVolumeSource{
+				VolumeID: string(uuid.NewUUID()),
+			},
+		}, nil
+
+	case plugins.CinderDriverName:
+		return v1.VolumeSource{
+			Cinder: &v1.CinderVolumeSource{
+				VolumeID: string(uuid.NewUUID()),
+			},
+		}, nil
+	case plugins.AzureDiskDriverName:
+		return v1.VolumeSource{
+			AzureDisk: &v1.AzureDiskVolumeSource{
+				DiskName:    string(uuid.NewUUID()),
+				DataDiskURI: string(uuid.NewUUID()),
+			},
+		}, nil
+	case plugins.AzureFileDriverName:
+		return v1.VolumeSource{
+			AzureFile: &v1.AzureFileVolumeSource{
+				SecretName: string(uuid.NewUUID()),
+				ShareName:  string(uuid.NewUUID()),
+			},
+		}, nil
+	default:
+		return v1.VolumeSource{}, fmt.Errorf("couldn't find logic for driver: %v", driverName)
+	}
+}
+
 func TestPluginNameMappings(t *testing.T) {
 	testCases := []struct {
 		name             string