Skip to content

Commit 17d2e00

Browse files
andyzhangxandyzhangx
committed
fix: azure file csi migration failure
1 parent 3c88bf6 commit 17d2e00

File tree

4 files changed

+149
-60
lines changed

4 files changed

+149
-60
lines changed

‎staging/src/k8s.io/csi-translation-lib/go.mod‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ module k8s.io/csi-translation-lib
55
go 1.13
66

77
require (
8+
github.com/stretchr/testify v1.4.0
89
k8s.io/api v0.0.0
910
k8s.io/apimachinery v0.0.0
1011
k8s.io/cloud-provider v0.0.0
12+
k8s.io/klog v1.0.0
1113
)
1214

1315
replace (

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ go_library(
1919
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
2020
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
2121
"//staging/src/k8s.io/cloud-provider/volume:go_default_library",
22+
"//vendor/k8s.io/klog:go_default_library",
2223
],
2324
)
2425

@@ -50,5 +51,6 @@ go_test(
5051
"//staging/src/k8s.io/api/core/v1:go_default_library",
5152
"//staging/src/k8s.io/api/storage/v1:go_default_library",
5253
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
54+
"//vendor/github.com/stretchr/testify/assert:go_default_library",
5355
],
5456
)

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

Lines changed: 54 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,13 @@ package plugins
1818

1919
import (
2020
"fmt"
21+
"regexp"
2122
"strings"
2223

2324
v1 "k8s.io/api/core/v1"
2425
storage "k8s.io/api/storage/v1"
2526
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"k8s.io/klog"
2628
)
2729

2830
const (
@@ -32,14 +34,19 @@ const (
3234
AzureFileInTreePluginName = "kubernetes.io/azure-file"
3335

3436
separator = "#"
35-
volumeIDTemplate = "%s#%s#%s"
37+
volumeIDTemplate = "%s#%s#%s#%s"
3638
// Parameter names defined in azure file CSI driver, refer to
3739
// https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/driver-parameters.md
3840
azureFileShareName = "shareName"
41+
42+
secretNameTemplate = "azure-storage-account-%s-secret"
43+
defaultSecretNamespace = "default"
3944
)
4045

4146
var _ InTreePlugin = &azureFileCSITranslator{}
4247

48+
var secretNameFormatRE = regexp.MustCompile(`azure-storage-account-(.+)-secret`)
49+
4350
// azureFileCSITranslator handles translation of PV spec from In-tree
4451
// Azure File to CSI Azure File and vice versa
4552
type azureFileCSITranslator struct{}
@@ -61,9 +68,14 @@ func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol
6168
return nil, fmt.Errorf("volume is nil or Azure File not defined on volume")
6269
}
6370

64-
var (
65-
azureSource = volume.AzureFile
71+
azureSource := volume.AzureFile
72+
accountName, err := getStorageAccountName(azureSource.SecretName)
73+
if err != nil {
74+
klog.Warningf("getStorageAccountName(%s) returned with error: %v", azureSource.SecretName, err)
75+
accountName = azureSource.SecretName
76+
}
6677

78+
var (
6779
pv = &v1.PersistentVolume{
6880
ObjectMeta: metav1.ObjectMeta{
6981
// 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
7486
PersistentVolumeSource: v1.PersistentVolumeSource{
7587
CSI: &v1.CSIPersistentVolumeSource{
7688
Driver: AzureFileDriverName,
77-
VolumeHandle: fmt.Sprintf(volumeIDTemplate, "", azureSource.SecretName, azureSource.ShareName),
89+
VolumeHandle: fmt.Sprintf(volumeIDTemplate, "", accountName, azureSource.ShareName, ""),
7890
ReadOnly: azureSource.ReadOnly,
7991
VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName},
80-
NodePublishSecretRef: &v1.SecretReference{
81-
Name: azureSource.ShareName,
82-
Namespace: "default",
92+
NodeStageSecretRef: &v1.SecretReference{
93+
Name: azureSource.SecretName,
94+
Namespace: defaultSecretNamespace,
8395
},
8496
},
8597
},
@@ -98,15 +110,21 @@ func (t *azureFileCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
98110
return nil, fmt.Errorf("pv is nil or Azure File source not defined on pv")
99111
}
100112

101-
var (
102-
azureSource = pv.Spec.PersistentVolumeSource.AzureFile
103-
volumeID = fmt.Sprintf(volumeIDTemplate, "", azureSource.SecretName, azureSource.ShareName)
113+
azureSource := pv.Spec.PersistentVolumeSource.AzureFile
114+
accountName, err := getStorageAccountName(azureSource.SecretName)
115+
if err != nil {
116+
klog.Warningf("getStorageAccountName(%s) returned with error: %v", azureSource.SecretName, err)
117+
accountName = azureSource.SecretName
118+
}
119+
volumeID := fmt.Sprintf(volumeIDTemplate, "", accountName, azureSource.ShareName, "")
104120

121+
var (
105122
// refer to https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/driver-parameters.md
106123
csiSource = &v1.CSIPersistentVolumeSource{
107124
Driver: AzureFileDriverName,
108-
NodePublishSecretRef: &v1.SecretReference{
109-
Name: azureSource.ShareName,
125+
NodeStageSecretRef: &v1.SecretReference{
126+
Name: azureSource.SecretName,
127+
Namespace: defaultSecretNamespace,
110128
},
111129
ReadOnly: azureSource.ReadOnly,
112130
VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName},
@@ -115,7 +133,7 @@ func (t *azureFileCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
115133
)
116134

117135
if azureSource.SecretNamespace != nil {
118-
csiSource.NodePublishSecretRef.Namespace = *azureSource.SecretNamespace
136+
csiSource.NodeStageSecretRef.Namespace = *azureSource.SecretNamespace
119137
}
120138

121139
pv.Spec.PersistentVolumeSource.AzureFile = nil
@@ -137,22 +155,21 @@ func (t *azureFileCSITranslator) TranslateCSIPVToInTree(pv *v1.PersistentVolume)
137155
ReadOnly: csiSource.ReadOnly,
138156
}
139157

140-
if csiSource.NodePublishSecretRef != nil && csiSource.NodePublishSecretRef.Name != "" {
141-
azureSource.SecretName = csiSource.NodePublishSecretRef.Name
142-
azureSource.SecretNamespace = &csiSource.NodePublishSecretRef.Namespace
158+
if csiSource.NodeStageSecretRef != nil && csiSource.NodeStageSecretRef.Name != "" {
159+
azureSource.SecretName = csiSource.NodeStageSecretRef.Name
160+
azureSource.SecretNamespace = &csiSource.NodeStageSecretRef.Namespace
143161
if csiSource.VolumeAttributes != nil {
144162
if shareName, ok := csiSource.VolumeAttributes[azureFileShareName]; ok {
145163
azureSource.ShareName = shareName
146164
}
147165
}
148166
} else {
149-
_, _, fileShareName, err := getFileShareInfo(csiSource.VolumeHandle)
167+
_, storageAccount, fileShareName, _, err := getFileShareInfo(csiSource.VolumeHandle)
150168
if err != nil {
151169
return nil, err
152170
}
153171
azureSource.ShareName = fileShareName
154-
// to-do: for dynamic provision scenario in CSI, it uses cluster's identity to get storage account key
155-
// secret for the file share is not created, we may create a serect here
172+
azureSource.SecretName = fmt.Sprintf(secretNameTemplate, storageAccount)
156173
}
157174

158175
pv.Spec.CSI = nil
@@ -190,12 +207,25 @@ func (t *azureFileCSITranslator) RepairVolumeHandle(volumeHandle, nodeID string)
190207
}
191208

192209
// get file share info according to volume id, e.g.
193-
// input: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41"
194-
// output: rg, f5713de20cde511e8ba4900, pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41
195-
func getFileShareInfo(id string) (string, string, string, error) {
210+
// input: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41#diskname.vhd"
211+
// output: rg, f5713de20cde511e8ba4900, pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41, diskname.vhd
212+
func getFileShareInfo(id string) (string, string, string, string, error) {
196213
segments := strings.Split(id, separator)
197214
if len(segments) < 3 {
198-
return "", "", "", fmt.Errorf("error parsing volume id: %q, should at least contain two #", id)
215+
return "", "", "", "", fmt.Errorf("error parsing volume id: %q, should at least contain two #", id)
216+
}
217+
var diskName string
218+
if len(segments) > 3 {
219+
diskName = segments[3]
220+
}
221+
return segments[0], segments[1], segments[2], diskName, nil
222+
}
223+
224+
// get storage account name from secret name
225+
func getStorageAccountName(secretName string) (string, error) {
226+
matches := secretNameFormatRE.FindStringSubmatch(secretName)
227+
if len(matches) != 2 {
228+
return "", fmt.Errorf("could not get account name from %s, correct format: %s", secretName, secretNameFormatRE)
199229
}
200-
return segments[0], segments[1], segments[2], nil
230+
return matches[1], nil
201231
}

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

Lines changed: 91 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -23,52 +23,77 @@ import (
2323

2424
corev1 "k8s.io/api/core/v1"
2525
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
27+
"github.com/stretchr/testify/assert"
2628
)
2729

2830
func TestGetFileShareInfo(t *testing.T) {
2931
tests := []struct {
30-
options string
31-
expected1 string
32-
expected2 string
33-
expected3 string
34-
expected4 error
32+
id string
33+
resourceGroupName string
34+
accountName string
35+
fileShareName string
36+
diskName string
37+
expectedError error
3538
}{
3639
{
37-
options: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
38-
expected1: "rg",
39-
expected2: "f5713de20cde511e8ba4900",
40-
expected3: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
41-
expected4: nil,
40+
id: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41#diskname.vhd",
41+
resourceGroupName: "rg",
42+
accountName: "f5713de20cde511e8ba4900",
43+
fileShareName: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
44+
diskName: "diskname.vhd",
45+
expectedError: nil,
46+
},
47+
{
48+
id: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
49+
resourceGroupName: "rg",
50+
accountName: "f5713de20cde511e8ba4900",
51+
fileShareName: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
52+
diskName: "",
53+
expectedError: nil,
4254
},
4355
{
44-
options: "rg#f5713de20cde511e8ba4900",
45-
expected1: "",
46-
expected2: "",
47-
expected3: "",
48-
expected4: fmt.Errorf("error parsing volume id: \"rg#f5713de20cde511e8ba4900\", should at least contain two #"),
56+
id: "rg#f5713de20cde511e8ba4900",
57+
resourceGroupName: "",
58+
accountName: "",
59+
fileShareName: "",
60+
diskName: "",
61+
expectedError: fmt.Errorf("error parsing volume id: \"rg#f5713de20cde511e8ba4900\", should at least contain two #"),
4962
},
5063
{
51-
options: "rg",
52-
expected1: "",
53-
expected2: "",
54-
expected3: "",
55-
expected4: fmt.Errorf("error parsing volume id: \"rg\", should at least contain two #"),
64+
id: "rg",
65+
resourceGroupName: "",
66+
accountName: "",
67+
fileShareName: "",
68+
diskName: "",
69+
expectedError: fmt.Errorf("error parsing volume id: \"rg\", should at least contain two #"),
5670
},
5771
{
58-
options: "",
59-
expected1: "",
60-
expected2: "",
61-
expected3: "",
62-
expected4: fmt.Errorf("error parsing volume id: \"\", should at least contain two #"),
72+
id: "",
73+
resourceGroupName: "",
74+
accountName: "",
75+
fileShareName: "",
76+
diskName: "",
77+
expectedError: fmt.Errorf("error parsing volume id: \"\", should at least contain two #"),
6378
},
6479
}
6580

6681
for _, test := range tests {
67-
result1, result2, result3, result4 := getFileShareInfo(test.options)
68-
if !reflect.DeepEqual(result1, test.expected1) || !reflect.DeepEqual(result2, test.expected2) ||
69-
!reflect.DeepEqual(result3, test.expected3) || !reflect.DeepEqual(result4, test.expected4) {
70-
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,
71-
result3, test.expected3, result4, test.expected4)
82+
resourceGroupName, accountName, fileShareName, diskName, expectedError := getFileShareInfo(test.id)
83+
if resourceGroupName != test.resourceGroupName {
84+
t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, resourceGroupName, test.resourceGroupName)
85+
}
86+
if accountName != test.accountName {
87+
t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, accountName, test.accountName)
88+
}
89+
if fileShareName != test.fileShareName {
90+
t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, fileShareName, test.fileShareName)
91+
}
92+
if diskName != test.diskName {
93+
t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, diskName, test.diskName)
94+
}
95+
if !reflect.DeepEqual(expectedError, test.expectedError) {
96+
t.Errorf("getFileShareInfo(%q) returned with: %v, expected: %v", test.id, expectedError, test.expectedError)
7297
}
7398
}
7499
}
@@ -110,13 +135,13 @@ func TestTranslateAzureFileInTreeStorageClassToCSI(t *testing.T) {
110135
PersistentVolumeSource: corev1.PersistentVolumeSource{
111136
CSI: &corev1.CSIPersistentVolumeSource{
112137
Driver: "file.csi.azure.com",
113-
NodePublishSecretRef: &corev1.SecretReference{
114-
Name: "sharename",
138+
NodeStageSecretRef: &corev1.SecretReference{
139+
Name: "secretname",
115140
Namespace: "default",
116141
},
117142
ReadOnly: true,
118143
VolumeAttributes: map[string]string{azureFileShareName: "sharename"},
119-
VolumeHandle: "#secretname#sharename",
144+
VolumeHandle: "#secretname#sharename#",
120145
},
121146
},
122147
AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany},
@@ -188,12 +213,12 @@ func TestTranslateAzureFileInTreePVToCSI(t *testing.T) {
188213
CSI: &corev1.CSIPersistentVolumeSource{
189214
Driver: "file.csi.azure.com",
190215
ReadOnly: true,
191-
NodePublishSecretRef: &corev1.SecretReference{
192-
Name: "sharename",
216+
NodeStageSecretRef: &corev1.SecretReference{
217+
Name: "secretname",
193218
Namespace: secretNamespace,
194219
},
195220
VolumeAttributes: map[string]string{azureFileShareName: "sharename"},
196-
VolumeHandle: "#secretname#sharename",
221+
VolumeHandle: "#secretname#sharename#",
197222
},
198223
},
199224
},
@@ -217,3 +242,33 @@ func TestTranslateAzureFileInTreePVToCSI(t *testing.T) {
217242
}
218243
}
219244
}
245+
246+
func TestGetStorageAccount(t *testing.T) {
247+
tests := []struct {
248+
secretName string
249+
expectedError bool
250+
expectedResult string
251+
}{
252+
{
253+
secretName: "azure-storage-account-accountname-secret",
254+
expectedError: false,
255+
expectedResult: "accountname",
256+
},
257+
{
258+
secretName: "azure-storage-account-accountname-dup-secret",
259+
expectedError: false,
260+
expectedResult: "accountname-dup",
261+
},
262+
{
263+
secretName: "invalid",
264+
expectedError: true,
265+
expectedResult: "",
266+
},
267+
}
268+
269+
for i, test := range tests {
270+
accountName, err := getStorageAccountName(test.secretName)
271+
assert.Equal(t, test.expectedError, err != nil, "TestCase[%d]", i)
272+
assert.Equal(t, test.expectedResult, accountName, "TestCase[%d]", i)
273+
}
274+
}

0 commit comments

Comments
 (0)