Merge pull request #103439 from andyzhangx/azure-managed-disk-translation-issue

fix azure disk translation issue due to lower case `managed` kind

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

@@ -44,6 +44,7 @@ const (
 var (
 	managedDiskPathRE   = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(?:.*)/providers/Microsoft.Compute/disks/(.+)`)
 	unmanagedDiskPathRE = regexp.MustCompile(`http(?:.*)://(?:.*)/vhds/(.+)`)
+	managed             = string(v1.AzureManagedDisk)
 )
 
 var _ InTreePlugin = &azureDiskCSITranslator{}
@@ -99,6 +100,9 @@ func (t *azureDiskCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol
 	}
 
 	azureSource := volume.AzureDisk
+	if azureSource.Kind != nil && !strings.EqualFold(string(*azureSource.Kind), managed) {
+		return nil, fmt.Errorf("kind(%v) is not supported in csi migration", *azureSource.Kind)
+	}
 	pv := &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{
 			// Must be unique per disk as it is used as the unique part of the
@@ -110,7 +114,7 @@ func (t *azureDiskCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol
 				CSI: &v1.CSIPersistentVolumeSource{
 					Driver:           AzureDiskDriverName,
 					VolumeHandle:     azureSource.DataDiskURI,
-					VolumeAttributes: map[string]string{azureDiskKind: "Managed"},
+					VolumeAttributes: map[string]string{azureDiskKind: managed},
 				},
 			},
 			AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
@@ -127,9 +131,7 @@ func (t *azureDiskCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol
 		pv.Spec.PersistentVolumeSource.CSI.FSType = *azureSource.FSType
 		pv.Spec.PersistentVolumeSource.CSI.VolumeAttributes[azureDiskFSType] = *azureSource.FSType
 	}
-	if azureSource.Kind != nil {
+	pv.Spec.PersistentVolumeSource.CSI.VolumeAttributes[azureDiskKind] = managed
-		pv.Spec.PersistentVolumeSource.CSI.VolumeAttributes[azureDiskKind] = string(*azureSource.Kind)
-	}
 
 	return pv, nil
 }
@@ -147,11 +149,15 @@ func (t *azureDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
 		// refer to https://github.com/kubernetes-sigs/azuredisk-csi-driver/blob/master/docs/driver-parameters.md
 		csiSource = &v1.CSIPersistentVolumeSource{
 			Driver:           AzureDiskDriverName,
-			VolumeAttributes: map[string]string{azureDiskKind: "Managed"},
+			VolumeAttributes: map[string]string{azureDiskKind: managed},
 			VolumeHandle:     azureSource.DataDiskURI,
 		}
 	)
 
+	if azureSource.Kind != nil && !strings.EqualFold(string(*azureSource.Kind), managed) {
+		return nil, fmt.Errorf("kind(%v) is not supported in csi migration", *azureSource.Kind)
+	}
+
 	if azureSource.CachingMode != nil {
 		csiSource.VolumeAttributes[azureDiskCachingMode] = string(*azureSource.CachingMode)
 	}
@@ -160,10 +166,7 @@ func (t *azureDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
 		csiSource.FSType = *azureSource.FSType
 		csiSource.VolumeAttributes[azureDiskFSType] = *azureSource.FSType
 	}
-
+	csiSource.VolumeAttributes[azureDiskKind] = managed
-	if azureSource.Kind != nil {
-		csiSource.VolumeAttributes[azureDiskKind] = string(*azureSource.Kind)
-	}
 
 	if azureSource.ReadOnly != nil {
 		csiSource.ReadOnly = *azureSource.ReadOnly
@@ -208,11 +211,7 @@ func (t *azureDiskCSITranslator) TranslateCSIPVToInTree(pv *v1.PersistentVolume)
 		if fsType, ok := csiSource.VolumeAttributes[azureDiskFSType]; ok && fsType != "" {
 			azureSource.FSType = &fsType
 		}
-
+		azureSource.Kind = &managed
-		if kind, ok := csiSource.VolumeAttributes[azureDiskKind]; ok && kind != "" {
-			diskKind := v1.AzureDataDiskKind(kind)
-			azureSource.Kind = &diskKind
-		}
 	}
 
 	pv.Spec.CSI = nil

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

@@ -98,6 +98,7 @@ func TestGetDiskName(t *testing.T) {
 }
 
 func TestTranslateAzureDiskInTreeStorageClassToCSI(t *testing.T) {
+	sharedBlobDiskKind := v1.AzureDedicatedBlobDisk
 	translator := NewAzureDiskCSITranslator()
 
 	cases := []struct {
@@ -141,6 +142,19 @@ func TestTranslateAzureDiskInTreeStorageClassToCSI(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "azure disk volume with non-managed kind",
+			volume: &corev1.Volume{
+				VolumeSource: corev1.VolumeSource{
+					AzureDisk: &corev1.AzureDiskVolumeSource{
+						DiskName:    "diskname",
+						DataDiskURI: "datadiskuri",
+						Kind:        &sharedBlobDiskKind,
+					},
+				},
+			},
+			expErr: true,
+		},
 	}
 
 	for _, tc := range cases {
@@ -163,9 +177,11 @@ func TestTranslateAzureDiskInTreeStorageClassToCSI(t *testing.T) {
 func TestTranslateAzureDiskInTreePVToCSI(t *testing.T) {
 	translator := NewAzureDiskCSITranslator()
 
+	sharedBlobDiskKind := v1.AzureDedicatedBlobDisk
 	cachingMode := corev1.AzureDataDiskCachingMode("cachingmode")
 	fsType := "fstype"
 	readOnly := true
+	diskURI := "/subscriptions/12/resourceGroups/23/providers/Microsoft.Compute/disks/name"
 
 	cases := []struct {
 		name   string
@@ -178,18 +194,18 @@ func TestTranslateAzureDiskInTreePVToCSI(t *testing.T) {
 			expErr: true,
 		},
 		{
-			name:   "no azure file volume",
+			name:   "no azure disk volume",
 			volume: &corev1.PersistentVolume{},
 			expErr: true,
 		},
 		{
-			name: "azure file volume",
+			name: "azure disk volume",
 			volume: &corev1.PersistentVolume{
 				Spec: corev1.PersistentVolumeSpec{
 					PersistentVolumeSource: corev1.PersistentVolumeSource{
 						AzureDisk: &corev1.AzureDiskVolumeSource{
 							CachingMode: &cachingMode,
-							DataDiskURI: "datadiskuri",
+							DataDiskURI: diskURI,
 							FSType:      &fsType,
 							ReadOnly:    &readOnly,
 						},
@@ -208,12 +224,29 @@ func TestTranslateAzureDiskInTreePVToCSI(t *testing.T) {
 								azureDiskFSType:      fsType,
 								azureDiskKind:        "Managed",
 							},
-							VolumeHandle: "datadiskuri",
+							VolumeHandle: diskURI,
 						},
 					},
 				},
 			},
 		},
+		{
+			name: "azure disk volume with non-managed kind",
+			volume: &corev1.PersistentVolume{
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						AzureDisk: &corev1.AzureDiskVolumeSource{
+							CachingMode: &cachingMode,
+							DataDiskURI: diskURI,
+							FSType:      &fsType,
+							ReadOnly:    &readOnly,
+							Kind:        &sharedBlobDiskKind,
+						},
+					},
+				},
+			},
+			expErr: true,
+		},
 	}
 
 	for _, tc := range cases {
@@ -233,6 +266,74 @@ func TestTranslateAzureDiskInTreePVToCSI(t *testing.T) {
 	}
 }
 
+func TestTranslateTranslateCSIPVToInTree(t *testing.T) {
+	cachingMode := corev1.AzureDataDiskCachingMode("cachingmode")
+	fsType := "fstype"
+	readOnly := true
+	diskURI := "/subscriptions/12/resourceGroups/23/providers/Microsoft.Compute/disks/name"
+	managed := v1.AzureManagedDisk
+
+	translator := NewAzureDiskCSITranslator()
+	cases := []struct {
+		name   string
+		volume *corev1.PersistentVolume
+		expVol *corev1.PersistentVolume
+		expErr bool
+	}{
+		{
+			name: "azure disk volume",
+			volume: &corev1.PersistentVolume{
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						CSI: &corev1.CSIPersistentVolumeSource{
+							Driver:   "disk.csi.azure.com",
+							FSType:   "fstype",
+							ReadOnly: true,
+							VolumeAttributes: map[string]string{
+								azureDiskCachingMode: "cachingmode",
+								azureDiskFSType:      fsType,
+								azureDiskKind:        "managed",
+							},
+							VolumeHandle: diskURI,
+						},
+					},
+				},
+			},
+			expVol: &corev1.PersistentVolume{
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						AzureDisk: &corev1.AzureDiskVolumeSource{
+							CachingMode: &cachingMode,
+							DataDiskURI: diskURI,
+							FSType:      &fsType,
+							ReadOnly:    &readOnly,
+							Kind:        &managed,
+							DiskName:    "name",
+						},
+					},
+				},
+			},
+			expErr: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Logf("Testing %v", tc.name)
+		got, err := translator.TranslateCSIPVToInTree(tc.volume)
+		if err != nil && !tc.expErr {
+			t.Errorf("Did not expect error but got: %v", err)
+		}
+
+		if err == nil && tc.expErr {
+			t.Errorf("Expected error, but did not get one.")
+		}
+
+		if !reflect.DeepEqual(got, tc.expVol) {
+			t.Errorf("Got parameters: %v, expected :%v", got, tc.expVol)
+		}
+	}
+}
+
 func TestTranslateInTreeStorageClassToCSI(t *testing.T) {
 	translator := NewAzureDiskCSITranslator()