Skip to content

Commit 566362d

Browse files
AndrewSirenkoAndrewSirenko
committed
Add modifycontroller SyncPVC unit tests
1 parent ce8f3b7 commit 566362d

File tree

4 files changed

+151
-160
lines changed

4 files changed

+151
-160
lines changed

‎pkg/modifycontroller/controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ func (ctrl *modifyController) sync() {
235235
}
236236
}
237237

238-
// syncPVC checks if a pvc requests resizing, and execute the resize operation if requested.
238+
// syncPVC checks if a pvc requests modification, and execute the ModifyVolume operation if requested.
239239
func (ctrl *modifyController) syncPVC(key string) error {
240240
klog.V(4).InfoS("Started PVC processing for modify controller", "key", key)
241241

@@ -260,7 +260,7 @@ func (ctrl *modifyController) syncPVC(key string) error {
260260
}
261261

262262
// Only trigger modify volume if the following conditions are met
263-
// 1. Non empty vac name
263+
// 1. Non-empty vac name
264264
// 2. PVC is in Bound state
265265
// 3. PV CSI driver name matches local driver
266266
vacName := pvc.Spec.VolumeAttributesClassName

‎pkg/modifycontroller/controller_test.go

Lines changed: 135 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func TestController(t *testing.T) {
3737
}{
3838
{
3939
name: "Modify called",
40-
pvc: createTestPVC(pvcName, "target-vac" /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
40+
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
4141
pv: basePV,
4242
vacExists: true,
4343
callCSIModify: true,
@@ -61,57 +61,13 @@ func TestController(t *testing.T) {
6161
for _, test := range tests {
6262
t.Run(test.name, func(t *testing.T) {
6363
// Setup
64-
client := csi.NewMockClient("foo", true, true, true, true, true, false)
65-
driverName, _ := client.GetDriverName(context.TODO())
66-
67-
var initialObjects []runtime.Object
68-
initialObjects = append(initialObjects, test.pvc)
69-
initialObjects = append(initialObjects, test.pv)
70-
// existing vac set in the pvc and pv
71-
initialObjects = append(initialObjects, testVacObject)
72-
if test.vacExists {
73-
initialObjects = append(initialObjects, targetVacObject)
74-
}
75-
76-
kubeClient, informerFactory := fakeK8s(initialObjects)
77-
pvInformer := informerFactory.Core().V1().PersistentVolumes()
78-
pvcInformer := informerFactory.Core().V1().PersistentVolumeClaims()
79-
vacInformer := informerFactory.Storage().V1beta1().VolumeAttributesClasses()
80-
81-
csiModifier, err := modifier.NewModifierFromClient(client, 15*time.Second, kubeClient, informerFactory, false, driverName)
82-
if err != nil {
83-
t.Fatalf("Test %s: Unable to create modifier: %v", test.name, err)
84-
}
85-
86-
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeAttributesClass, true)
87-
controller := NewModifyController(driverName,
88-
csiModifier, kubeClient,
89-
time.Second, false, informerFactory,
90-
workqueue.DefaultTypedControllerRateLimiter[string]())
64+
client := csi.NewMockClient(testDriverName, true, true, true, true, true, false)
9165

92-
ctrlInstance, _ := controller.(*modifyController)
93-
94-
stopCh := make(chan struct{})
95-
informerFactory.Start(stopCh)
96-
97-
ctx := context.TODO()
66+
initialObjects := []runtime.Object{test.pvc, test.pv, testVacObject, targetVacObject}
67+
ctrlInstance, ctx := setupFakeK8sEnvironment(t, client, initialObjects)
9868
defer ctx.Done()
99-
go controller.Run(1, ctx)
100-
101-
for _, obj := range initialObjects {
102-
switch obj.(type) {
103-
case *v1.PersistentVolume:
104-
pvInformer.Informer().GetStore().Add(obj)
105-
case *v1.PersistentVolumeClaim:
106-
pvcInformer.Informer().GetStore().Add(obj)
107-
case *storagev1beta1.VolumeAttributesClass:
108-
vacInformer.Informer().GetStore().Add(obj)
109-
default:
110-
t.Fatalf("Test %s: Unknown initalObject type: %+v", test.name, obj)
111-
}
112-
}
113-
time.Sleep(time.Second * 2)
114-
_, _, err, _ = ctrlInstance.modify(test.pvc, test.pv)
69+
70+
_, _, err, _ := ctrlInstance.modify(test.pvc, test.pv)
11571
if err != nil {
11672
t.Fatalf("for %s: unexpected error: %v", test.name, err)
11773
}
@@ -141,14 +97,14 @@ func TestModifyPVC(t *testing.T) {
14197
}{
14298
{
14399
name: "Modify succeeded",
144-
pvc: createTestPVC(pvcName, "target-vac" /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
100+
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
145101
pv: basePV,
146102
modifyFailure: false,
147103
expectFailure: false,
148104
},
149105
{
150106
name: "Modify failed",
151-
pvc: createTestPVC(pvcName, "target-vac" /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
107+
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
152108
pv: basePV,
153109
modifyFailure: true,
154110
expectFailure: true,
@@ -157,77 +113,152 @@ func TestModifyPVC(t *testing.T) {
157113

158114
for _, test := range tests {
159115
t.Run(test.name, func(t *testing.T) {
160-
client := csi.NewMockClient("mock", true, true, true, true, true, false)
116+
client := csi.NewMockClient(testDriverName, true, true, true, true, true, false)
161117
if test.modifyFailure {
162118
client.SetModifyFailed()
163119
}
164-
driverName, _ := client.GetDriverName(context.TODO())
165120

166-
initialObjects := []runtime.Object{}
167-
if test.pvc != nil {
168-
initialObjects = append(initialObjects, test.pvc)
121+
initialObjects := []runtime.Object{test.pvc, test.pv, testVacObject, targetVacObject}
122+
ctrlInstance, ctx := setupFakeK8sEnvironment(t, client, initialObjects)
123+
defer ctx.Done()
124+
125+
_, _, err, _ := ctrlInstance.modify(test.pvc, test.pv)
126+
127+
if test.expectFailure && err == nil {
128+
t.Errorf("for %s expected error got nothing", test.name)
169129
}
170-
if test.pv != nil {
171-
test.pv.Spec.PersistentVolumeSource.CSI.Driver = driverName
172-
initialObjects = append(initialObjects, test.pv)
130+
131+
if !test.expectFailure {
132+
if err != nil {
133+
t.Errorf("for %s, unexpected error: %v", test.name, err)
134+
}
173135
}
136+
})
137+
}
138+
}
174139

175-
// existing vac set in the pvc and pv
176-
initialObjects = append(initialObjects, testVacObject)
177-
// new vac used in modify volume
178-
initialObjects = append(initialObjects, targetVacObject)
140+
func TestSyncPVC(t *testing.T) {
141+
basePVC := createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/)
142+
basePV := createTestPV(1, pvcName, pvcNamespace, "foobaz" /*pvcUID*/, &fsVolumeMode, testVac)
179143

180-
kubeClient, informerFactory := fakeK8s(initialObjects)
181-
pvInformer := informerFactory.Core().V1().PersistentVolumes()
182-
pvcInformer := informerFactory.Core().V1().PersistentVolumeClaims()
183-
vacInformer := informerFactory.Storage().V1beta1().VolumeAttributesClasses()
144+
otherDriverPV := createTestPV(1, pvcName, pvcNamespace, "foobaz" /*pvcUID*/, &fsVolumeMode, testVac)
145+
otherDriverPV.Spec.PersistentVolumeSource.CSI.Driver = "some-other-driver"
184146

185-
csiModifier, err := modifier.NewModifierFromClient(client, 15*time.Second, kubeClient, informerFactory, false, driverName)
186-
if err != nil {
187-
t.Fatalf("Test %s: Unable to create modifier: %v", test.name, err)
188-
}
147+
unboundPVC := createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/)
148+
unboundPVC.Status.Phase = v1.ClaimPending
189149

190-
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeAttributesClass, true)
191-
controller := NewModifyController(driverName,
192-
csiModifier, kubeClient,
193-
time.Second, false, informerFactory,
194-
workqueue.DefaultTypedControllerRateLimiter[string]())
150+
pvcWithUncreatedPV := createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/)
151+
pvcWithUncreatedPV.Spec.VolumeName = ""
195152

196-
ctrlInstance, _ := controller.(*modifyController)
153+
tests := []struct {
154+
name string
155+
pvc *v1.PersistentVolumeClaim
156+
pv *v1.PersistentVolume
157+
callCSIModify bool
158+
}{
159+
{
160+
name: "Should execute ModifyVolume operation when PVC's VAC changes",
161+
pvc: basePVC,
162+
pv: basePV,
163+
callCSIModify: true,
164+
},
165+
{
166+
name: "Should NOT modify if PVC managed by another CSI Driver",
167+
pvc: basePVC,
168+
pv: otherDriverPV,
169+
callCSIModify: false,
170+
},
171+
{
172+
name: "Should NOT modify if PVC has empty Spec.VACName",
173+
pvc: createTestPVC(pvcName, "" /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
174+
pv: basePV,
175+
callCSIModify: false,
176+
},
177+
{
178+
name: "Should NOT modify if PVC not in bound state",
179+
pvc: unboundPVC,
180+
pv: basePV,
181+
callCSIModify: false,
182+
},
183+
{
184+
name: "Should NOT modify if PVC's PV not created yet",
185+
pvc: pvcWithUncreatedPV,
186+
pv: basePV,
187+
callCSIModify: false,
188+
},
189+
}
197190

198-
stopCh := make(chan struct{})
199-
informerFactory.Start(stopCh)
191+
for _, test := range tests {
192+
t.Run(test.name, func(t *testing.T) {
193+
client := csi.NewMockClient(testDriverName, true, true, true, true, true, false)
200194

201-
ctx := context.TODO()
195+
initialObjects := []runtime.Object{test.pvc, test.pv, testVacObject, targetVacObject}
196+
ctrlInstance, ctx := setupFakeK8sEnvironment(t, client, initialObjects)
202197
defer ctx.Done()
203-
go controller.Run(1, ctx)
204-
205-
for _, obj := range initialObjects {
206-
switch obj.(type) {
207-
case *v1.PersistentVolume:
208-
pvInformer.Informer().GetStore().Add(obj)
209-
case *v1.PersistentVolumeClaim:
210-
pvcInformer.Informer().GetStore().Add(obj)
211-
case *storagev1beta1.VolumeAttributesClass:
212-
vacInformer.Informer().GetStore().Add(obj)
213-
default:
214-
t.Fatalf("Test %s: Unknown initalObject type: %+v", test.name, obj)
215-
}
216-
}
217-
218-
time.Sleep(time.Second * 2)
219198

220-
_, _, err, _ = ctrlInstance.modify(test.pvc, test.pv)
199+
err := ctrlInstance.syncPVC(pvcNamespace + "/" + pvcName)
200+
if err != nil {
201+
t.Errorf("for %s, unexpected error: %v", test.name, err)
202+
}
221203

222-
if test.expectFailure && err == nil {
223-
t.Errorf("for %s expected error got nothing", test.name)
204+
modifyCallCount := client.GetModifyCount()
205+
if test.callCSIModify && modifyCallCount == 0 {
206+
t.Fatalf("for %s: expected csi modify call, no csi modify call was made", test.name)
224207
}
225208

226-
if !test.expectFailure {
227-
if err != nil {
228-
t.Errorf("for %s, unexpected error: %v", test.name, err)
229-
}
209+
if !test.callCSIModify && modifyCallCount > 0 {
210+
t.Fatalf("for %s: expected no csi modify call, received csi modify request", test.name)
230211
}
231212
})
232213
}
233214
}
215+
216+
// setupFakeK8sEnvironment creates fake K8s environment and starts Informers and ModifyController
217+
func setupFakeK8sEnvironment(t *testing.T, client *csi.MockClient, initialObjects []runtime.Object) (*modifyController, context.Context) {
218+
t.Helper()
219+
220+
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeAttributesClass, true)
221+
222+
/* Create fake kubeClient, Informers, and ModifyController */
223+
kubeClient, informerFactory := fakeK8s(initialObjects)
224+
pvInformer := informerFactory.Core().V1().PersistentVolumes()
225+
pvcInformer := informerFactory.Core().V1().PersistentVolumeClaims()
226+
vacInformer := informerFactory.Storage().V1beta1().VolumeAttributesClasses()
227+
228+
driverName, _ := client.GetDriverName(context.TODO())
229+
230+
csiModifier, err := modifier.NewModifierFromClient(client, 15*time.Second, kubeClient, informerFactory, false, driverName)
231+
if err != nil {
232+
t.Fatalf("Test %s: Unable to create modifier: %v", t.Name(), err)
233+
}
234+
235+
controller := NewModifyController(driverName,
236+
csiModifier, kubeClient,
237+
0, false, informerFactory,
238+
workqueue.DefaultTypedControllerRateLimiter[string]())
239+
240+
/* Start informers and ModifyController*/
241+
stopCh := make(chan struct{})
242+
informerFactory.Start(stopCh)
243+
244+
ctx := context.TODO()
245+
go controller.Run(1, ctx)
246+
247+
/* Add initial objects to informer caches */
248+
for _, obj := range initialObjects {
249+
switch obj.(type) {
250+
case *v1.PersistentVolume:
251+
pvInformer.Informer().GetStore().Add(obj)
252+
case *v1.PersistentVolumeClaim:
253+
pvcInformer.Informer().GetStore().Add(obj)
254+
case *storagev1beta1.VolumeAttributesClass:
255+
vacInformer.Informer().GetStore().Add(obj)
256+
default:
257+
t.Fatalf("Test %s: Unknown initalObject type: %+v", t.Name(), obj)
258+
}
259+
}
260+
261+
ctrlInstance, _ := controller.(*modifyController)
262+
263+
return ctrlInstance, ctx
264+
}

‎pkg/modifycontroller/modify_status_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@ import (
2828
const (
2929
pvcName = "foo"
3030
pvcNamespace = "modify"
31+
pvName = "testPV"
3132
)
3233

3334
var (
3435
fsVolumeMode = v1.PersistentVolumeFilesystem
3536
testVac = "test-vac"
3637
targetVac = "target-vac"
38+
testDriverName = "mock"
3739
infeasibleErr = status.Errorf(codes.InvalidArgument, "Parameters in VolumeAttributesClass is invalid")
3840
finalErr = status.Errorf(codes.Internal, "Final error")
3941
pvcConditionInProgress = v1.PersistentVolumeClaimCondition{
@@ -390,7 +392,7 @@ func createTestPV(capacityGB int, pvcName, pvcNamespace string, pvcUID types.UID
390392
},
391393
PersistentVolumeSource: v1.PersistentVolumeSource{
392394
CSI: &v1.CSIPersistentVolumeSource{
393-
Driver: "foo",
395+
Driver: testDriverName,
394396
VolumeHandle: "foo",
395397
},
396398
},

0 commit comments

Comments
 (0)