Skip to content

Commit 3c88bf6

Browse files
rfranzkerfranzke
committed
Fix flaws in Azure CSI translation
1 parent 0a7675e commit 3c88bf6

File tree

6 files changed

+345
-45
lines changed

6 files changed

+345
-45
lines changed

‎staging/src/k8s.io/csi-translation-lib/CONTRIBUTING.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22

33
Do not open pull requests directly against this repository, they will be ignored. Instead, please open pull requests against [kubernetes/kubernetes](https://git.k8s.io/kubernetes/). Please follow the same [contributing guide](https://git.k8s.io/kubernetes/CONTRIBUTING.md) you would follow for any other pull request made to kubernetes/kubernetes.
44

5-
This repository is published from [kubernetes/kubernetes/staging/src/k8s.io/csi-api](https://git.k8s.io/kubernetes/staging/src/k8s.io/csi-api) by the [kubernetes publishing-bot](https://git.k8s.io/publishing-bot).
5+
This repository is published from [kubernetes/kubernetes/staging/src/k8s.io/csi-translation-lib](https://git.k8s.io/kubernetes/staging/src/k8s.io/csi-translation-lib) by the [kubernetes publishing-bot](https://git.k8s.io/publishing-bot).
66

77
Please see [Staging Directory and Publishing](https://git.k8s.io/community/contributors/devel/sig-architecture/staging.md) for more information.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,5 +49,6 @@ go_test(
4949
deps = [
5050
"//staging/src/k8s.io/api/core/v1:go_default_library",
5151
"//staging/src/k8s.io/api/storage/v1:go_default_library",
52+
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
5253
],
5354
)

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

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -110,32 +110,36 @@ func (t *azureDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
110110
return nil, fmt.Errorf("pv is nil or Azure Disk source not defined on pv")
111111
}
112112

113-
azureSource := pv.Spec.PersistentVolumeSource.AzureDisk
114-
115-
// refer to https://github.com/kubernetes-sigs/azuredisk-csi-driver/blob/master/docs/driver-parameters.md
116-
csiSource := &v1.CSIPersistentVolumeSource{
117-
Driver: AzureDiskDriverName,
118-
VolumeHandle: azureSource.DataDiskURI,
119-
ReadOnly: *azureSource.ReadOnly,
120-
FSType: *azureSource.FSType,
121-
VolumeAttributes: map[string]string{azureDiskKind: "Managed"},
122-
}
113+
var (
114+
azureSource = pv.Spec.PersistentVolumeSource.AzureDisk
115+
116+
// refer to https://github.com/kubernetes-sigs/azuredisk-csi-driver/blob/master/docs/driver-parameters.md
117+
csiSource = &v1.CSIPersistentVolumeSource{
118+
Driver: AzureDiskDriverName,
119+
VolumeAttributes: map[string]string{azureDiskKind: "Managed"},
120+
VolumeHandle: azureSource.DataDiskURI,
121+
}
122+
)
123123

124124
if azureSource.CachingMode != nil {
125125
csiSource.VolumeAttributes[azureDiskCachingMode] = string(*azureSource.CachingMode)
126126
}
127127

128128
if azureSource.FSType != nil {
129+
csiSource.FSType = *azureSource.FSType
129130
csiSource.VolumeAttributes[azureDiskFSType] = *azureSource.FSType
130131
}
131132

132133
if azureSource.Kind != nil {
133134
csiSource.VolumeAttributes[azureDiskKind] = string(*azureSource.Kind)
134135
}
135136

137+
if azureSource.ReadOnly != nil {
138+
csiSource.ReadOnly = *azureSource.ReadOnly
139+
}
140+
136141
pv.Spec.PersistentVolumeSource.AzureDisk = nil
137142
pv.Spec.PersistentVolumeSource.CSI = csiSource
138-
pv.Spec.AccessModes = backwardCompatibleAccessModes(pv.Spec.AccessModes)
139143

140144
return pv, nil
141145
}

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

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ import (
2020
"fmt"
2121
"reflect"
2222
"testing"
23+
24+
corev1 "k8s.io/api/core/v1"
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2326
)
2427

2528
func TestIsManagedDisk(t *testing.T) {
@@ -91,3 +94,139 @@ func TestGetDiskName(t *testing.T) {
9194
}
9295
}
9396
}
97+
98+
func TestTranslateAzureDiskInTreeStorageClassToCSI(t *testing.T) {
99+
translator := NewAzureDiskCSITranslator()
100+
101+
cases := []struct {
102+
name string
103+
volume *corev1.Volume
104+
expVol *corev1.PersistentVolume
105+
expErr bool
106+
}{
107+
{
108+
name: "empty volume",
109+
expErr: true,
110+
},
111+
{
112+
name: "no azure disk volume",
113+
volume: &corev1.Volume{},
114+
expErr: true,
115+
},
116+
{
117+
name: "azure disk volume",
118+
volume: &corev1.Volume{
119+
VolumeSource: corev1.VolumeSource{
120+
AzureDisk: &corev1.AzureDiskVolumeSource{
121+
DiskName: "diskname",
122+
DataDiskURI: "datadiskuri",
123+
},
124+
},
125+
},
126+
expVol: &corev1.PersistentVolume{
127+
ObjectMeta: metav1.ObjectMeta{
128+
Name: "disk.csi.azure.com-diskname",
129+
},
130+
Spec: corev1.PersistentVolumeSpec{
131+
PersistentVolumeSource: corev1.PersistentVolumeSource{
132+
CSI: &corev1.CSIPersistentVolumeSource{
133+
Driver: "disk.csi.azure.com",
134+
VolumeHandle: "datadiskuri",
135+
VolumeAttributes: map[string]string{azureDiskKind: "Managed"},
136+
},
137+
},
138+
AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
139+
},
140+
},
141+
},
142+
}
143+
144+
for _, tc := range cases {
145+
t.Logf("Testing %v", tc.name)
146+
got, err := translator.TranslateInTreeInlineVolumeToCSI(tc.volume)
147+
if err != nil && !tc.expErr {
148+
t.Errorf("Did not expect error but got: %v", err)
149+
}
150+
151+
if err == nil && tc.expErr {
152+
t.Errorf("Expected error, but did not get one.")
153+
}
154+
155+
if !reflect.DeepEqual(got, tc.expVol) {
156+
t.Errorf("Got parameters: %v, expected :%v", got, tc.expVol)
157+
}
158+
}
159+
}
160+
161+
func TestTranslateAzureDiskInTreePVToCSI(t *testing.T) {
162+
translator := NewAzureDiskCSITranslator()
163+
164+
cachingMode := corev1.AzureDataDiskCachingMode("cachingmode")
165+
fsType := "fstype"
166+
readOnly := true
167+
168+
cases := []struct {
169+
name string
170+
volume *corev1.PersistentVolume
171+
expVol *corev1.PersistentVolume
172+
expErr bool
173+
}{
174+
{
175+
name: "empty volume",
176+
expErr: true,
177+
},
178+
{
179+
name: "no azure file volume",
180+
volume: &corev1.PersistentVolume{},
181+
expErr: true,
182+
},
183+
{
184+
name: "azure file volume",
185+
volume: &corev1.PersistentVolume{
186+
Spec: corev1.PersistentVolumeSpec{
187+
PersistentVolumeSource: corev1.PersistentVolumeSource{
188+
AzureDisk: &corev1.AzureDiskVolumeSource{
189+
CachingMode: &cachingMode,
190+
DataDiskURI: "datadiskuri",
191+
FSType: &fsType,
192+
ReadOnly: &readOnly,
193+
},
194+
},
195+
},
196+
},
197+
expVol: &corev1.PersistentVolume{
198+
Spec: corev1.PersistentVolumeSpec{
199+
PersistentVolumeSource: corev1.PersistentVolumeSource{
200+
CSI: &corev1.CSIPersistentVolumeSource{
201+
Driver: "disk.csi.azure.com",
202+
FSType: "fstype",
203+
ReadOnly: true,
204+
VolumeAttributes: map[string]string{
205+
azureDiskCachingMode: "cachingmode",
206+
azureDiskFSType: fsType,
207+
azureDiskKind: "Managed",
208+
},
209+
VolumeHandle: "datadiskuri",
210+
},
211+
},
212+
},
213+
},
214+
},
215+
}
216+
217+
for _, tc := range cases {
218+
t.Logf("Testing %v", tc.name)
219+
got, err := translator.TranslateInTreePVToCSI(tc.volume)
220+
if err != nil && !tc.expErr {
221+
t.Errorf("Did not expect error but got: %v", err)
222+
}
223+
224+
if err == nil && tc.expErr {
225+
t.Errorf("Expected error, but did not get one.")
226+
}
227+
228+
if !reflect.DeepEqual(got, tc.expVol) {
229+
t.Errorf("Got parameters: %v, expected :%v", got, tc.expVol)
230+
}
231+
}
232+
}

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

Lines changed: 41 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -58,32 +58,36 @@ func (t *azureFileCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.St
5858
// and converts the AzureFile source to a CSIPersistentVolumeSource
5959
func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error) {
6060
if volume == nil || volume.AzureFile == nil {
61-
return nil, fmt.Errorf("volume is nil or AWS EBS not defined on volume")
61+
return nil, fmt.Errorf("volume is nil or Azure File not defined on volume")
6262
}
6363

64-
azureSource := volume.AzureFile
65-
66-
pv := &v1.PersistentVolume{
67-
ObjectMeta: metav1.ObjectMeta{
68-
// Must be unique per disk as it is used as the unique part of the
69-
// staging path
70-
Name: fmt.Sprintf("%s-%s", AzureFileDriverName, azureSource.ShareName),
71-
},
72-
Spec: v1.PersistentVolumeSpec{
73-
PersistentVolumeSource: v1.PersistentVolumeSource{
74-
CSI: &v1.CSIPersistentVolumeSource{
75-
VolumeHandle: fmt.Sprintf(volumeIDTemplate, "", azureSource.SecretName, azureSource.ShareName),
76-
ReadOnly: azureSource.ReadOnly,
77-
VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName},
78-
NodePublishSecretRef: &v1.SecretReference{
79-
Name: azureSource.ShareName,
80-
Namespace: "default",
64+
var (
65+
azureSource = volume.AzureFile
66+
67+
pv = &v1.PersistentVolume{
68+
ObjectMeta: metav1.ObjectMeta{
69+
// Must be unique per disk as it is used as the unique part of the
70+
// staging path
71+
Name: fmt.Sprintf("%s-%s", AzureFileDriverName, azureSource.ShareName),
72+
},
73+
Spec: v1.PersistentVolumeSpec{
74+
PersistentVolumeSource: v1.PersistentVolumeSource{
75+
CSI: &v1.CSIPersistentVolumeSource{
76+
Driver: AzureFileDriverName,
77+
VolumeHandle: fmt.Sprintf(volumeIDTemplate, "", azureSource.SecretName, azureSource.ShareName),
78+
ReadOnly: azureSource.ReadOnly,
79+
VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName},
80+
NodePublishSecretRef: &v1.SecretReference{
81+
Name: azureSource.ShareName,
82+
Namespace: "default",
83+
},
8184
},
8285
},
86+
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteMany},
8387
},
84-
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteMany},
85-
},
86-
}
88+
}
89+
)
90+
8791
return pv, nil
8892
}
8993

@@ -94,24 +98,28 @@ func (t *azureFileCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
9498
return nil, fmt.Errorf("pv is nil or Azure File source not defined on pv")
9599
}
96100

97-
azureSource := pv.Spec.PersistentVolumeSource.AzureFile
101+
var (
102+
azureSource = pv.Spec.PersistentVolumeSource.AzureFile
103+
volumeID = fmt.Sprintf(volumeIDTemplate, "", azureSource.SecretName, azureSource.ShareName)
98104

99-
volumeID := fmt.Sprintf(volumeIDTemplate, "", azureSource.SecretName, azureSource.ShareName)
100-
// refer to https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/driver-parameters.md
101-
csiSource := &v1.CSIPersistentVolumeSource{
102-
VolumeHandle: volumeID,
103-
ReadOnly: azureSource.ReadOnly,
104-
VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName},
105-
}
105+
// refer to https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/driver-parameters.md
106+
csiSource = &v1.CSIPersistentVolumeSource{
107+
Driver: AzureFileDriverName,
108+
NodePublishSecretRef: &v1.SecretReference{
109+
Name: azureSource.ShareName,
110+
},
111+
ReadOnly: azureSource.ReadOnly,
112+
VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName},
113+
VolumeHandle: volumeID,
114+
}
115+
)
106116

107-
csiSource.NodePublishSecretRef = &v1.SecretReference{
108-
Name: azureSource.ShareName,
109-
Namespace: *azureSource.SecretNamespace,
117+
if azureSource.SecretNamespace != nil {
118+
csiSource.NodePublishSecretRef.Namespace = *azureSource.SecretNamespace
110119
}
111120

112121
pv.Spec.PersistentVolumeSource.AzureFile = nil
113122
pv.Spec.PersistentVolumeSource.CSI = csiSource
114-
pv.Spec.AccessModes = backwardCompatibleAccessModes(pv.Spec.AccessModes)
115123

116124
return pv, nil
117125
}

0 commit comments

Comments
 (0)