fix: azure file csi migration failure

This commit is contained in:
andyzhangx 2020-04-20 11:13:47 +00:00
parent 3c88bf6b17
commit 17d2e00e68
4 changed files with 149 additions and 60 deletions

View File

@ -5,9 +5,11 @@ module k8s.io/csi-translation-lib
go 1.13 go 1.13
require ( require (
github.com/stretchr/testify v1.4.0
k8s.io/api v0.0.0 k8s.io/api v0.0.0
k8s.io/apimachinery v0.0.0 k8s.io/apimachinery v0.0.0
k8s.io/cloud-provider v0.0.0 k8s.io/cloud-provider v0.0.0
k8s.io/klog v1.0.0
) )
replace ( replace (

View File

@ -19,6 +19,7 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/cloud-provider/volume:go_default_library", "//staging/src/k8s.io/cloud-provider/volume:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
], ],
) )
@ -50,5 +51,6 @@ go_test(
"//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/api/storage/v1:go_default_library", "//staging/src/k8s.io/api/storage/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/github.com/stretchr/testify/assert:go_default_library",
], ],
) )

View File

@ -18,11 +18,13 @@ package plugins
import ( import (
"fmt" "fmt"
"regexp"
"strings" "strings"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
storage "k8s.io/api/storage/v1" storage "k8s.io/api/storage/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog"
) )
const ( const (
@ -32,14 +34,19 @@ const (
AzureFileInTreePluginName = "kubernetes.io/azure-file" AzureFileInTreePluginName = "kubernetes.io/azure-file"
separator = "#" separator = "#"
volumeIDTemplate = "%s#%s#%s" volumeIDTemplate = "%s#%s#%s#%s"
// Parameter names defined in azure file CSI driver, refer to // Parameter names defined in azure file CSI driver, refer to
// https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/driver-parameters.md // https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/driver-parameters.md
azureFileShareName = "shareName" azureFileShareName = "shareName"
secretNameTemplate = "azure-storage-account-%s-secret"
defaultSecretNamespace = "default"
) )
var _ InTreePlugin = &azureFileCSITranslator{} var _ InTreePlugin = &azureFileCSITranslator{}
var secretNameFormatRE = regexp.MustCompile(`azure-storage-account-(.+)-secret`)
// azureFileCSITranslator handles translation of PV spec from In-tree // azureFileCSITranslator handles translation of PV spec from In-tree
// Azure File to CSI Azure File and vice versa // Azure File to CSI Azure File and vice versa
type azureFileCSITranslator struct{} type azureFileCSITranslator struct{}
@ -61,9 +68,14 @@ func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol
return nil, fmt.Errorf("volume is nil or Azure File not defined on volume") return nil, fmt.Errorf("volume is nil or Azure File not defined on volume")
} }
var ( azureSource := volume.AzureFile
azureSource = volume.AzureFile accountName, err := getStorageAccountName(azureSource.SecretName)
if err != nil {
klog.Warningf("getStorageAccountName(%s) returned with error: %v", azureSource.SecretName, err)
accountName = azureSource.SecretName
}
var (
pv = &v1.PersistentVolume{ pv = &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
// Must be unique per disk as it is used as the unique part of the // Must be unique per disk as it is used as the unique part of the
@ -74,12 +86,12 @@ func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol
PersistentVolumeSource: v1.PersistentVolumeSource{ PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{ CSI: &v1.CSIPersistentVolumeSource{
Driver: AzureFileDriverName, Driver: AzureFileDriverName,
VolumeHandle: fmt.Sprintf(volumeIDTemplate, "", azureSource.SecretName, azureSource.ShareName), VolumeHandle: fmt.Sprintf(volumeIDTemplate, "", accountName, azureSource.ShareName, ""),
ReadOnly: azureSource.ReadOnly, ReadOnly: azureSource.ReadOnly,
VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName}, VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName},
NodePublishSecretRef: &v1.SecretReference{ NodeStageSecretRef: &v1.SecretReference{
Name: azureSource.ShareName, Name: azureSource.SecretName,
Namespace: "default", Namespace: defaultSecretNamespace,
}, },
}, },
}, },
@ -98,15 +110,21 @@ func (t *azureFileCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
return nil, fmt.Errorf("pv is nil or Azure File source not defined on pv") return nil, fmt.Errorf("pv is nil or Azure File source not defined on pv")
} }
var ( azureSource := pv.Spec.PersistentVolumeSource.AzureFile
azureSource = pv.Spec.PersistentVolumeSource.AzureFile accountName, err := getStorageAccountName(azureSource.SecretName)
volumeID = fmt.Sprintf(volumeIDTemplate, "", azureSource.SecretName, azureSource.ShareName) if err != nil {
klog.Warningf("getStorageAccountName(%s) returned with error: %v", azureSource.SecretName, err)
accountName = azureSource.SecretName
}
volumeID := fmt.Sprintf(volumeIDTemplate, "", accountName, azureSource.ShareName, "")
var (
// refer to https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/driver-parameters.md // refer to https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/driver-parameters.md
csiSource = &v1.CSIPersistentVolumeSource{ csiSource = &v1.CSIPersistentVolumeSource{
Driver: AzureFileDriverName, Driver: AzureFileDriverName,
NodePublishSecretRef: &v1.SecretReference{ NodeStageSecretRef: &v1.SecretReference{
Name: azureSource.ShareName, Name: azureSource.SecretName,
Namespace: defaultSecretNamespace,
}, },
ReadOnly: azureSource.ReadOnly, ReadOnly: azureSource.ReadOnly,
VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName}, VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName},
@ -115,7 +133,7 @@ func (t *azureFileCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
) )
if azureSource.SecretNamespace != nil { if azureSource.SecretNamespace != nil {
csiSource.NodePublishSecretRef.Namespace = *azureSource.SecretNamespace csiSource.NodeStageSecretRef.Namespace = *azureSource.SecretNamespace
} }
pv.Spec.PersistentVolumeSource.AzureFile = nil pv.Spec.PersistentVolumeSource.AzureFile = nil
@ -137,22 +155,21 @@ func (t *azureFileCSITranslator) TranslateCSIPVToInTree(pv *v1.PersistentVolume)
ReadOnly: csiSource.ReadOnly, ReadOnly: csiSource.ReadOnly,
} }
if csiSource.NodePublishSecretRef != nil && csiSource.NodePublishSecretRef.Name != "" { if csiSource.NodeStageSecretRef != nil && csiSource.NodeStageSecretRef.Name != "" {
azureSource.SecretName = csiSource.NodePublishSecretRef.Name azureSource.SecretName = csiSource.NodeStageSecretRef.Name
azureSource.SecretNamespace = &csiSource.NodePublishSecretRef.Namespace azureSource.SecretNamespace = &csiSource.NodeStageSecretRef.Namespace
if csiSource.VolumeAttributes != nil { if csiSource.VolumeAttributes != nil {
if shareName, ok := csiSource.VolumeAttributes[azureFileShareName]; ok { if shareName, ok := csiSource.VolumeAttributes[azureFileShareName]; ok {
azureSource.ShareName = shareName azureSource.ShareName = shareName
} }
} }
} else { } else {
_, _, fileShareName, err := getFileShareInfo(csiSource.VolumeHandle) _, storageAccount, fileShareName, _, err := getFileShareInfo(csiSource.VolumeHandle)
if err != nil { if err != nil {
return nil, err return nil, err
} }
azureSource.ShareName = fileShareName azureSource.ShareName = fileShareName
// to-do: for dynamic provision scenario in CSI, it uses cluster's identity to get storage account key azureSource.SecretName = fmt.Sprintf(secretNameTemplate, storageAccount)
// secret for the file share is not created, we may create a serect here
} }
pv.Spec.CSI = nil pv.Spec.CSI = nil
@ -190,12 +207,25 @@ func (t *azureFileCSITranslator) RepairVolumeHandle(volumeHandle, nodeID string)
} }
// get file share info according to volume id, e.g. // get file share info according to volume id, e.g.
// input: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41" // input: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41#diskname.vhd"
// output: rg, f5713de20cde511e8ba4900, pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41 // output: rg, f5713de20cde511e8ba4900, pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41, diskname.vhd
func getFileShareInfo(id string) (string, string, string, error) { func getFileShareInfo(id string) (string, string, string, string, error) {
segments := strings.Split(id, separator) segments := strings.Split(id, separator)
if len(segments) < 3 { if len(segments) < 3 {
return "", "", "", fmt.Errorf("error parsing volume id: %q, should at least contain two #", id) return "", "", "", "", fmt.Errorf("error parsing volume id: %q, should at least contain two #", id)
} }
return segments[0], segments[1], segments[2], nil var diskName string
if len(segments) > 3 {
diskName = segments[3]
}
return segments[0], segments[1], segments[2], diskName, nil
}
// get storage account name from secret name
func getStorageAccountName(secretName string) (string, error) {
matches := secretNameFormatRE.FindStringSubmatch(secretName)
if len(matches) != 2 {
return "", fmt.Errorf("could not get account name from %s, correct format: %s", secretName, secretNameFormatRE)
}
return matches[1], nil
} }

View File

@ -23,52 +23,77 @@ import (
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/stretchr/testify/assert"
) )
func TestGetFileShareInfo(t *testing.T) { func TestGetFileShareInfo(t *testing.T) {
tests := []struct { tests := []struct {
options string id string
expected1 string resourceGroupName string
expected2 string accountName string
expected3 string fileShareName string
expected4 error diskName string
expectedError error
}{ }{
{ {
options: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41", id: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41#diskname.vhd",
expected1: "rg", resourceGroupName: "rg",
expected2: "f5713de20cde511e8ba4900", accountName: "f5713de20cde511e8ba4900",
expected3: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41", fileShareName: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
expected4: nil, diskName: "diskname.vhd",
expectedError: nil,
}, },
{ {
options: "rg#f5713de20cde511e8ba4900", id: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
expected1: "", resourceGroupName: "rg",
expected2: "", accountName: "f5713de20cde511e8ba4900",
expected3: "", fileShareName: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
expected4: fmt.Errorf("error parsing volume id: \"rg#f5713de20cde511e8ba4900\", should at least contain two #"), diskName: "",
expectedError: nil,
}, },
{ {
options: "rg", id: "rg#f5713de20cde511e8ba4900",
expected1: "", resourceGroupName: "",
expected2: "", accountName: "",
expected3: "", fileShareName: "",
expected4: fmt.Errorf("error parsing volume id: \"rg\", should at least contain two #"), diskName: "",
expectedError: fmt.Errorf("error parsing volume id: \"rg#f5713de20cde511e8ba4900\", should at least contain two #"),
}, },
{ {
options: "", id: "rg",
expected1: "", resourceGroupName: "",
expected2: "", accountName: "",
expected3: "", fileShareName: "",
expected4: fmt.Errorf("error parsing volume id: \"\", should at least contain two #"), diskName: "",
expectedError: fmt.Errorf("error parsing volume id: \"rg\", should at least contain two #"),
},
{
id: "",
resourceGroupName: "",
accountName: "",
fileShareName: "",
diskName: "",
expectedError: fmt.Errorf("error parsing volume id: \"\", should at least contain two #"),
}, },
} }
for _, test := range tests { for _, test := range tests {
result1, result2, result3, result4 := getFileShareInfo(test.options) resourceGroupName, accountName, fileShareName, diskName, expectedError := getFileShareInfo(test.id)
if !reflect.DeepEqual(result1, test.expected1) || !reflect.DeepEqual(result2, test.expected2) || if resourceGroupName != test.resourceGroupName {
!reflect.DeepEqual(result3, test.expected3) || !reflect.DeepEqual(result4, test.expected4) { t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, resourceGroupName, test.resourceGroupName)
t.Errorf("input: %q, getFileShareInfo result1: %q, expected1: %q, result2: %q, expected2: %q, result3: %q, expected3: %q, result4: %q, expected4: %q", test.options, result1, test.expected1, result2, test.expected2, }
result3, test.expected3, result4, test.expected4) if accountName != test.accountName {
t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, accountName, test.accountName)
}
if fileShareName != test.fileShareName {
t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, fileShareName, test.fileShareName)
}
if diskName != test.diskName {
t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, diskName, test.diskName)
}
if !reflect.DeepEqual(expectedError, test.expectedError) {
t.Errorf("getFileShareInfo(%q) returned with: %v, expected: %v", test.id, expectedError, test.expectedError)
} }
} }
} }
@ -110,13 +135,13 @@ func TestTranslateAzureFileInTreeStorageClassToCSI(t *testing.T) {
PersistentVolumeSource: corev1.PersistentVolumeSource{ PersistentVolumeSource: corev1.PersistentVolumeSource{
CSI: &corev1.CSIPersistentVolumeSource{ CSI: &corev1.CSIPersistentVolumeSource{
Driver: "file.csi.azure.com", Driver: "file.csi.azure.com",
NodePublishSecretRef: &corev1.SecretReference{ NodeStageSecretRef: &corev1.SecretReference{
Name: "sharename", Name: "secretname",
Namespace: "default", Namespace: "default",
}, },
ReadOnly: true, ReadOnly: true,
VolumeAttributes: map[string]string{azureFileShareName: "sharename"}, VolumeAttributes: map[string]string{azureFileShareName: "sharename"},
VolumeHandle: "#secretname#sharename", VolumeHandle: "#secretname#sharename#",
}, },
}, },
AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany},
@ -188,12 +213,12 @@ func TestTranslateAzureFileInTreePVToCSI(t *testing.T) {
CSI: &corev1.CSIPersistentVolumeSource{ CSI: &corev1.CSIPersistentVolumeSource{
Driver: "file.csi.azure.com", Driver: "file.csi.azure.com",
ReadOnly: true, ReadOnly: true,
NodePublishSecretRef: &corev1.SecretReference{ NodeStageSecretRef: &corev1.SecretReference{
Name: "sharename", Name: "secretname",
Namespace: secretNamespace, Namespace: secretNamespace,
}, },
VolumeAttributes: map[string]string{azureFileShareName: "sharename"}, VolumeAttributes: map[string]string{azureFileShareName: "sharename"},
VolumeHandle: "#secretname#sharename", VolumeHandle: "#secretname#sharename#",
}, },
}, },
}, },
@ -217,3 +242,33 @@ func TestTranslateAzureFileInTreePVToCSI(t *testing.T) {
} }
} }
} }
func TestGetStorageAccount(t *testing.T) {
tests := []struct {
secretName string
expectedError bool
expectedResult string
}{
{
secretName: "azure-storage-account-accountname-secret",
expectedError: false,
expectedResult: "accountname",
},
{
secretName: "azure-storage-account-accountname-dup-secret",
expectedError: false,
expectedResult: "accountname-dup",
},
{
secretName: "invalid",
expectedError: true,
expectedResult: "",
},
}
for i, test := range tests {
accountName, err := getStorageAccountName(test.secretName)
assert.Equal(t, test.expectedError, err != nil, "TestCase[%d]", i)
assert.Equal(t, test.expectedResult, accountName, "TestCase[%d]", i)
}
}