From 82dfad9be5bdf77214595d4dd6ac8ab4d33b69e5 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Fri, 15 Jan 2021 03:47:36 +0000 Subject: [PATCH] fix azure file migration issue refactor fix comments fix annoation nil map add ut --- .../csi-translation-lib/plugins/azure_file.go | 53 ++++--- .../plugins/azure_file_test.go | 139 ++++++++++++++++-- 2 files changed, 164 insertions(+), 28 deletions(-) diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go b/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go index 94ef714fe274..6ce3be461f73 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go @@ -37,11 +37,11 @@ const ( volumeIDTemplate = "%s#%s#%s#%s" // Parameter names defined in azure file CSI driver, refer to // https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/driver-parameters.md - azureFileShareName = "shareName" - - secretNameTemplate = "azure-storage-account-%s-secret" - defaultSecretNamespace = "default" - + shareNameField = "sharename" + secretNameField = "secretname" + secretNamespaceField = "secretnamespace" + secretNameTemplate = "azure-storage-account-%s-secret" + defaultSecretNamespace = "default" resourceGroupAnnotation = "kubernetes.io/azure-file-resource-group" ) @@ -90,7 +90,7 @@ func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol Driver: AzureFileDriverName, VolumeHandle: fmt.Sprintf(volumeIDTemplate, "", accountName, azureSource.ShareName, ""), ReadOnly: azureSource.ReadOnly, - VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName}, + VolumeAttributes: map[string]string{shareNameField: azureSource.ShareName}, NodeStageSecretRef: &v1.SecretReference{ Name: azureSource.SecretName, Namespace: defaultSecretNamespace, @@ -135,7 +135,7 @@ func (t *azureFileCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume) Namespace: defaultSecretNamespace, }, ReadOnly: azureSource.ReadOnly, - VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName}, + VolumeAttributes: map[string]string{shareNameField: azureSource.ShareName}, VolumeHandle: volumeID, } ) @@ -163,31 +163,48 @@ func (t *azureFileCSITranslator) TranslateCSIPVToInTree(pv *v1.PersistentVolume) ReadOnly: csiSource.ReadOnly, } + for k, v := range csiSource.VolumeAttributes { + switch strings.ToLower(k) { + case shareNameField: + azureSource.ShareName = v + case secretNameField: + azureSource.SecretName = v + case secretNamespaceField: + ns := v + azureSource.SecretNamespace = &ns + } + } + resourceGroup := "" if csiSource.NodeStageSecretRef != nil && csiSource.NodeStageSecretRef.Name != "" { azureSource.SecretName = csiSource.NodeStageSecretRef.Name azureSource.SecretNamespace = &csiSource.NodeStageSecretRef.Namespace - if csiSource.VolumeAttributes != nil { - if shareName, ok := csiSource.VolumeAttributes[azureFileShareName]; ok { - azureSource.ShareName = shareName - } - } - } else { + } + if azureSource.ShareName == "" || azureSource.SecretName == "" { rg, storageAccount, fileShareName, _, err := getFileShareInfo(csiSource.VolumeHandle) if err != nil { return nil, err } - azureSource.ShareName = fileShareName - azureSource.SecretName = fmt.Sprintf(secretNameTemplate, storageAccount) + if azureSource.ShareName == "" { + azureSource.ShareName = fileShareName + } + if azureSource.SecretName == "" { + azureSource.SecretName = fmt.Sprintf(secretNameTemplate, storageAccount) + } resourceGroup = rg } + if azureSource.SecretNamespace == nil { + ns := defaultSecretNamespace + azureSource.SecretNamespace = &ns + } + pv.Spec.CSI = nil pv.Spec.AzureFile = azureSource + if pv.ObjectMeta.Annotations == nil { + pv.ObjectMeta.Annotations = map[string]string{} + } if resourceGroup != "" { - if pv.ObjectMeta.Annotations == nil { - pv.ObjectMeta.Annotations = map[string]string{} - } pv.ObjectMeta.Annotations[resourceGroupAnnotation] = resourceGroup } diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/azure_file_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/azure_file_test.go index 66c4dc144539..47767e7a22c8 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/azure_file_test.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/azure_file_test.go @@ -140,7 +140,7 @@ func TestTranslateAzureFileInTreeStorageClassToCSI(t *testing.T) { Namespace: "default", }, ReadOnly: true, - VolumeAttributes: map[string]string{azureFileShareName: "sharename"}, + VolumeAttributes: map[string]string{shareNameField: "sharename"}, VolumeHandle: "#secretname#sharename#", }, }, @@ -217,7 +217,7 @@ func TestTranslateAzureFileInTreePVToCSI(t *testing.T) { Name: "secretname", Namespace: secretNamespace, }, - VolumeAttributes: map[string]string{azureFileShareName: "sharename"}, + VolumeAttributes: map[string]string{shareNameField: "sharename"}, VolumeHandle: "#secretname#sharename#", }, }, @@ -256,7 +256,7 @@ func TestTranslateAzureFileInTreePVToCSI(t *testing.T) { Name: "secretname", Namespace: secretNamespace, }, - VolumeAttributes: map[string]string{azureFileShareName: "sharename"}, + VolumeAttributes: map[string]string{shareNameField: "sharename"}, VolumeHandle: "rg#secretname#sharename#", }, }, @@ -285,9 +285,18 @@ func TestTranslateAzureFileInTreePVToCSI(t *testing.T) { func TestTranslateCSIPVToInTree(t *testing.T) { translator := NewAzureFileCSITranslator() + secretName := "secretname" secretNamespace := "secretnamespace" + shareName := "sharename" + defaultNS := "default" mp := make(map[string]string) - mp["shareName"] = "unit-test" + mp["shareName"] = shareName + + secretMap := make(map[string]string) + secretMap["shareName"] = shareName + secretMap["secretName"] = secretName + secretMap["secretNamespace"] = secretNamespace + cases := []struct { name string volume *corev1.PersistentVolume @@ -315,13 +324,16 @@ func TestTranslateCSIPVToInTree(t *testing.T) { }, }, expVol: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, Spec: corev1.PersistentVolumeSpec{ PersistentVolumeSource: corev1.PersistentVolumeSource{ AzureFile: &corev1.AzureFilePersistentVolumeSource{ SecretName: "ut", SecretNamespace: &secretNamespace, ReadOnly: true, - ShareName: "unit-test", + ShareName: shareName, }, }, }, @@ -334,7 +346,7 @@ func TestTranslateCSIPVToInTree(t *testing.T) { Spec: corev1.PersistentVolumeSpec{ PersistentVolumeSource: corev1.PersistentVolumeSource{ CSI: &corev1.CSIPersistentVolumeSource{ - VolumeHandle: "unit-test", + VolumeHandle: shareName, ReadOnly: true, VolumeAttributes: mp, }, @@ -344,7 +356,76 @@ func TestTranslateCSIPVToInTree(t *testing.T) { expErr: true, }, { - name: "translate from volume handle", + name: "translate from VolumeAttributes", + volume: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "file.csi.azure.com-sharename", + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + CSI: &corev1.CSIPersistentVolumeSource{ + VolumeHandle: "rg#st#pvc-file-dynamic#diskname.vhd", + ReadOnly: true, + VolumeAttributes: mp, + }, + }, + }, + }, + expVol: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "file.csi.azure.com-sharename", + Annotations: map[string]string{resourceGroupAnnotation: "rg"}, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + AzureFile: &corev1.AzureFilePersistentVolumeSource{ + SecretName: "azure-storage-account-st-secret", + ShareName: shareName, + SecretNamespace: &defaultNS, + ReadOnly: true, + }, + }, + }, + }, + expErr: false, + }, + { + name: "translate from SecretMap VolumeAttributes", + volume: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "file.csi.azure.com-sharename", + Annotations: map[string]string{}, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + CSI: &corev1.CSIPersistentVolumeSource{ + VolumeHandle: "rg#st#pvc-file-dynamic#diskname.vhd", + ReadOnly: true, + VolumeAttributes: secretMap, + }, + }, + }, + }, + expVol: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "file.csi.azure.com-sharename", + Annotations: map[string]string{}, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + AzureFile: &corev1.AzureFilePersistentVolumeSource{ + SecretName: secretName, + SecretNamespace: &secretNamespace, + ShareName: shareName, + ReadOnly: true, + }, + }, + }, + }, + expErr: false, + }, + { + name: "translate from NodeStageSecretRef", volume: &corev1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: "file.csi.azure.com-sharename", @@ -355,6 +436,43 @@ func TestTranslateCSIPVToInTree(t *testing.T) { VolumeHandle: "rg#st#pvc-file-dynamic#diskname.vhd", ReadOnly: true, VolumeAttributes: mp, + NodeStageSecretRef: &corev1.SecretReference{ + Name: secretName, + Namespace: secretNamespace, + }, + }, + }, + }, + }, + expVol: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "file.csi.azure.com-sharename", + Annotations: map[string]string{}, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + AzureFile: &corev1.AzureFilePersistentVolumeSource{ + SecretName: secretName, + ShareName: shareName, + SecretNamespace: &secretNamespace, + ReadOnly: true, + }, + }, + }, + }, + expErr: false, + }, + { + name: "translate from VolumeHandle", + volume: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "file.csi.azure.com-sharename", + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + CSI: &corev1.CSIPersistentVolumeSource{ + VolumeHandle: "rg#st#pvc-file-dynamic#diskname.vhd", + ReadOnly: true, }, }, }, @@ -367,9 +485,10 @@ func TestTranslateCSIPVToInTree(t *testing.T) { Spec: corev1.PersistentVolumeSpec{ PersistentVolumeSource: corev1.PersistentVolumeSource{ AzureFile: &corev1.AzureFilePersistentVolumeSource{ - SecretName: "azure-storage-account-st-secret", - ShareName: "pvc-file-dynamic", - ReadOnly: true, + SecretName: "azure-storage-account-st-secret", + ShareName: "pvc-file-dynamic", + SecretNamespace: &defaultNS, + ReadOnly: true, }, }, },