Skip to content

Commit 529d57b

Browse files
pwschuurmanpwschuurman
committed
Replace MutatorConfig with CreatePVCPrimeFn that allows custom creation of a PVC
1 parent 4e8feb2 commit 529d57b

File tree

2 files changed

+66
-92
lines changed

2 files changed

+66
-92
lines changed

‎populator-machinery/controller.go

+25-45
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ const (
7979
reasonPopulateOperationFailed = "PopulateOperationFailed"
8080
reasonPopulateOperationFinished = "PopulateOperationFinished"
8181
reasonPVCPrimeCreationError = "PopulatorPVCPrimeCreationError"
82-
reasonPVCPrimeMutatorError = "reasonPVCPrimeMutatorError"
8382
reasonWaitForDataPopulationFinished = "PopulatorWaitForDataPopulationFinished"
8483
reasonStorageClassCreationError = "PopulatorStorageClassCreationError"
8584
reasonDataSourceNotFound = "PopulatorDataSourceNotFound"
@@ -117,7 +116,6 @@ type controller struct {
117116
referenceGrantSynced cache.InformerSynced
118117
podConfig *PodConfig
119118
providerFunctionConfig *ProviderFunctionConfig
120-
mutatorConfig *MutatorConfig
121119
crossNamespace bool
122120
providerMetricManager *ProviderMetricManager
123121
}
@@ -142,9 +140,6 @@ type VolumePopulatorConfig struct {
142140
// ProviderFunctionConfig is the configuration for invoking provider functions. Either PodConfig or ProviderFunctionConfig should
143141
// be specified. PodConfig and ProviderFunctionConfig can't be provided at the same time
144142
ProviderFunctionConfig *ProviderFunctionConfig
145-
// MutatorConfig is the configuration for invoking mutator functions. You can specify your own mutator functions to modify the
146-
// Kubernetes resources used for volume population
147-
MutatorConfig *MutatorConfig
148143
// ProviderMetricManager is the manager for provider specific metric handling
149144
ProviderMetricManager *ProviderMetricManager
150145
// CrossNamespace indicates if the populator supports data sources located in namespaces different than the PVC's namespace.
@@ -170,6 +165,9 @@ type PodConfig struct {
170165
}
171166

172167
type ProviderFunctionConfig struct {
168+
// CreatePVCPrimeFn is the provider specific PVCPRime creation function
169+
// Returns a PVC object that should be created by the caller, otherwise an error if creation failed and should be retried.
170+
CreatePVCPrimeFn func(context.Context, PopulatorParams) (*corev1.PersistentVolumeClaim, error)
173171
// PopulateFn is the provider specific data population function
174172
PopulateFn func(context.Context, PopulatorParams) error
175173
// PopulateCompleteFn is the provider specific data population completeness check function, return true when data transfer gets completed
@@ -192,19 +190,6 @@ type PopulatorParams struct {
192190
Recorder record.EventRecorder
193191
}
194192

195-
type MutatorConfig struct {
196-
// PvcPrimeMutator is the mutator function for pvcPrime. The function gets called to modify the PVC object before pvcPrime gets created.
197-
PvcPrimeMutator func(PvcPrimeMutatorParams) (*corev1.PersistentVolumeClaim, error)
198-
}
199-
200-
// PvcPrimeMutatorParams includes the parameters passing to the PvcPrimeMutator function
201-
type PvcPrimeMutatorParams struct {
202-
// PvcPrime is the temporary PVC created by volume populator
203-
PvcPrime *corev1.PersistentVolumeClaim
204-
// StorageClass is the original StorageClass Pvc refer to
205-
StorageClass *storagev1.StorageClass
206-
}
207-
208193
func RunController(masterURL, kubeconfig, imageName, httpEndpoint, metricsPath, namespace, prefix string,
209194
gk schema.GroupKind, gvr schema.GroupVersionResource, mountPath, devicePath string,
210195
populatorArgs func(bool, *unstructured.Unstructured) ([]string, error),
@@ -308,7 +293,6 @@ func RunControllerWithConfig(vpcfg VolumePopulatorConfig) {
308293
referenceGrantSynced: referenceGrants.Informer().HasSynced,
309294
podConfig: vpcfg.PodConfig,
310295
providerFunctionConfig: vpcfg.ProviderFunctionConfig,
311-
mutatorConfig: vpcfg.MutatorConfig,
312296
crossNamespace: vpcfg.CrossNamespace,
313297
providerMetricManager: vpcfg.ProviderMetricManager,
314298
}
@@ -714,37 +698,33 @@ func (c *controller) syncPvc(ctx context.Context, key, pvcNamespace, pvcName str
714698
// TODO: Handle PVC' update while the original PVC changed
715699
// If the PVC is unbound and PVC' doesn't exist yet, create PVC'
716700
if "" == pvc.Spec.VolumeName && pvcPrime == nil {
717-
pvcPrime = &corev1.PersistentVolumeClaim{
718-
ObjectMeta: metav1.ObjectMeta{
719-
Name: pvcPrimeName,
720-
Namespace: c.populatorNamespace,
721-
},
722-
Spec: corev1.PersistentVolumeClaimSpec{
723-
AccessModes: pvc.Spec.AccessModes,
724-
Resources: pvc.Spec.Resources,
725-
StorageClassName: pvc.Spec.StorageClassName,
726-
VolumeMode: pvc.Spec.VolumeMode,
727-
},
728-
}
729-
if waitForFirstConsumer {
730-
pvcPrime.Annotations = map[string]string{
731-
annSelectedNode: nodeName,
732-
}
733-
}
734-
if c.mutatorConfig != nil && c.mutatorConfig.PvcPrimeMutator != nil {
735-
mp := PvcPrimeMutatorParams{
736-
PvcPrime: pvcPrime,
737-
StorageClass: storageClass,
738-
}
739-
pvcPrime, err = c.mutatorConfig.PvcPrimeMutator(mp)
740-
if err != nil {
741-
c.recorder.Eventf(pvc, corev1.EventTypeWarning, reasonPVCPrimeMutatorError, "Failed to mutate populator pvcPrime: %s", err)
701+
if c.providerFunctionConfig != nil && c.providerFunctionConfig.CreatePVCPrimeFn != nil {
702+
if pvcPrime, err = c.providerFunctionConfig.CreatePVCPrimeFn(ctx, *params); err != nil {
703+
c.recorder.Eventf(pvc, corev1.EventTypeWarning, reasonPVCPrimeCreationError, "Failed to create PVC Prime: %s", err)
742704
return err
743705
}
744706
if pvcPrime == nil {
745-
c.recorder.Eventf(pvc, corev1.EventTypeWarning, reasonPVCPrimeMutatorError, "pvcPrime must not be nil")
707+
c.recorder.Eventf(pvc, corev1.EventTypeWarning, reasonPVCPrimeCreationError, "pvcPrime must not be nil")
746708
return fmt.Errorf("pvcPrime must not be nil")
747709
}
710+
} else {
711+
pvcPrime = &corev1.PersistentVolumeClaim{
712+
ObjectMeta: metav1.ObjectMeta{
713+
Name: pvcPrimeName,
714+
Namespace: c.populatorNamespace,
715+
},
716+
Spec: corev1.PersistentVolumeClaimSpec{
717+
AccessModes: pvc.Spec.AccessModes,
718+
Resources: pvc.Spec.Resources,
719+
StorageClassName: pvc.Spec.StorageClassName,
720+
VolumeMode: pvc.Spec.VolumeMode,
721+
},
722+
}
723+
if waitForFirstConsumer {
724+
pvcPrime.Annotations = map[string]string{
725+
annSelectedNode: nodeName,
726+
}
727+
}
748728
}
749729
pvcPrime, err = c.kubeClient.CoreV1().PersistentVolumeClaims(c.populatorNamespace).Create(ctx, pvcPrime, metav1.CreateOptions{})
750730
if err != nil {

‎populator-machinery/controller_test.go

+41-47
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,14 @@ type testCase struct {
5858
initialObjects []runtime.Object
5959
// Args for the populator pod
6060
populatorArgs func(b bool, u *unstructured.Unstructured) ([]string, error)
61+
// Provider specific PVC prime creation function
62+
pvcPrimeCreateFn func(context.Context, PopulatorParams) (*corev1.PersistentVolumeClaim, error)
6163
// Provider specific data population function
6264
populateFn func(context.Context, PopulatorParams) error
6365
// Provider specific data population completeness check function, return true when data transfer gets completed.
6466
populateCompleteFn func(context.Context, PopulatorParams) (bool, error)
6567
// The original PVC gets deleted or not
6668
pvcDeleted bool
67-
// PvcPrimeMutator is the mutator function for pvcPrime
68-
pvcPrimeMutator func(PvcPrimeMutatorParams) (*v1.PersistentVolumeClaim, error)
6969
// Expected errors
7070
expectedResult error
7171
// Expected keys in the notifyMap
@@ -105,7 +105,7 @@ const (
105105
testProvisioner = "test.provisioner"
106106
testPopulationOperationStartFailed = "Test populate operation start failed"
107107
testPopulateCompleteFailed = "Test populate operation complete failed"
108-
testMutatePVCPrimeFailed = "Test mutate pvcPrime failed"
108+
testPvcPrimeCreateFailed = "Test create pvcPrime failed"
109109
dataSourceKey = "unstructured/" + testPvcNamespace + "/" + testDataSourceName
110110
storageClassKey = "sc/" + testStorageClassName
111111
podKey = "pod/" + testVpWorkingNamespace + "/" + testPodName
@@ -252,18 +252,18 @@ func populateCompleteSuccess(ctx context.Context, p PopulatorParams) (bool, erro
252252
return true, nil
253253
}
254254

255-
func pvcPrimeMutateAccessModeRWX(mp PvcPrimeMutatorParams) (*v1.PersistentVolumeClaim, error) {
256-
accessMode := v1.ReadWriteMany
257-
mp.PvcPrime.Spec.AccessModes[0] = accessMode
258-
return mp.PvcPrime, nil
255+
func pvcPrimeCreateError(ctx context.Context, p PopulatorParams) (*corev1.PersistentVolumeClaim, error) {
256+
return nil, fmt.Errorf(testPvcPrimeCreateFailed)
259257
}
260258

261-
func pvcPrimeMutateError(mp PvcPrimeMutatorParams) (*v1.PersistentVolumeClaim, error) {
262-
return mp.PvcPrime, fmt.Errorf(testMutatePVCPrimeFailed)
259+
func pvcPrimeCreateNil(ctx context.Context, p PopulatorParams) (*corev1.PersistentVolumeClaim, error) {
260+
return nil, nil
263261
}
264262

265-
func pvcPrimeMutatePVCPrimeNil(mp PvcPrimeMutatorParams) (*v1.PersistentVolumeClaim, error) {
266-
return nil, nil
263+
func pvcPrimeCreateSuccess(pvcPrime *corev1.PersistentVolumeClaim) func(ctx context.Context, p PopulatorParams) (*corev1.PersistentVolumeClaim, error) {
264+
return func(ctx context.Context, p PopulatorParams) (*corev1.PersistentVolumeClaim, error) {
265+
return pvcPrime, nil
266+
}
267267
}
268268

269269
func initTest(test testCase) (
@@ -310,19 +310,14 @@ func initTest(test testCase) (
310310
}
311311
}
312312

313-
var providerFunctionConfig *ProviderFunctionConfig
313+
var providerFunctionConfig *ProviderFunctionConfig = &ProviderFunctionConfig{}
314314
if test.populateFn != nil || test.populateCompleteFn != nil {
315-
providerFunctionConfig = &ProviderFunctionConfig{
316-
PopulateFn: test.populateFn,
317-
PopulateCompleteFn: test.populateCompleteFn,
318-
}
315+
providerFunctionConfig.PopulateFn = test.populateFn
316+
providerFunctionConfig.PopulateCompleteFn = test.populateCompleteFn
319317
}
320318

321-
var mutatorConfig *MutatorConfig
322-
if test.pvcPrimeMutator != nil {
323-
mutatorConfig = &MutatorConfig{
324-
PvcPrimeMutator: test.pvcPrimeMutator,
325-
}
319+
if test.pvcPrimeCreateFn != nil {
320+
providerFunctionConfig.CreatePVCPrimeFn = test.pvcPrimeCreateFn
326321
}
327322

328323
c := &controller{
@@ -350,7 +345,6 @@ func initTest(test testCase) (
350345
referenceGrantSynced: referenceGrants.Informer().HasSynced,
351346
podConfig: podConfig,
352347
providerFunctionConfig: providerFunctionConfig,
353-
mutatorConfig: mutatorConfig,
354348
}
355349
return c, pvcInformer, unstInformer, scInformer, podInformer, pvInformer
356350
}
@@ -367,7 +361,7 @@ func compareResult(want error, got error) bool {
367361

368362
func compareNotifyMap(want []string, got map[string]*stringSet) error {
369363
if len(want) != len(got) {
370-
return fmt.Errorf("The number of keys expected is different from actual. Expect %v, got %v", len(want), len(got))
364+
return fmt.Errorf("The number of keys expected is different from actual. Expect %v, got %v. Want %v: Got: %v", len(want), len(got), want, got)
371365
}
372366
for _, key := range want {
373367
if got[key] == nil {
@@ -682,10 +676,10 @@ func TestSyncPvcWithPopulatorPod(t *testing.T) {
682676
ust(),
683677
sc(testStorageClassName, storagev1.VolumeBindingImmediate),
684678
},
685-
populatorArgs: populatorArgs,
686-
pvcPrimeMutator: pvcPrimeMutateAccessModeRWX,
687-
expectedResult: nil,
688-
expectedKeys: []string{podKey, pvcPrimeKey},
679+
populatorArgs: populatorArgs,
680+
pvcPrimeCreateFn: pvcPrimeCreateSuccess(pvc(testPvcPrimeName, testVpWorkingNamespace, "", testStorageClassName, "", "", []string{}, nil, "", v1.ReadWriteMany)),
681+
expectedResult: nil,
682+
expectedKeys: []string{podKey, pvcPrimeKey},
689683
expectedObjects: &vpObjects{
690684
pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer, vpFinalizer},
691685
dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce),
@@ -710,10 +704,10 @@ func TestSyncPvcWithPopulatorPod(t *testing.T) {
710704
ust(),
711705
sc(testStorageClassName, storagev1.VolumeBindingImmediate),
712706
},
713-
populatorArgs: populatorArgs,
714-
pvcPrimeMutator: pvcPrimeMutateError,
715-
expectedResult: fmt.Errorf(testMutatePVCPrimeFailed),
716-
expectedKeys: []string{podKey, pvcPrimeKey},
707+
populatorArgs: populatorArgs,
708+
pvcPrimeCreateFn: pvcPrimeCreateError,
709+
expectedResult: fmt.Errorf(testPvcPrimeCreateFailed),
710+
expectedKeys: []string{podKey, pvcPrimeKey},
717711
expectedObjects: &vpObjects{
718712
pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer},
719713
dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce),
@@ -730,10 +724,10 @@ func TestSyncPvcWithPopulatorPod(t *testing.T) {
730724
ust(),
731725
sc(testStorageClassName, storagev1.VolumeBindingImmediate),
732726
},
733-
populatorArgs: populatorArgs,
734-
pvcPrimeMutator: pvcPrimeMutatePVCPrimeNil,
735-
expectedResult: fmt.Errorf("pvcPrime must not be nil"),
736-
expectedKeys: []string{podKey, pvcPrimeKey},
727+
populatorArgs: populatorArgs,
728+
pvcPrimeCreateFn: pvcPrimeCreateNil,
729+
expectedResult: fmt.Errorf("pvcPrime must not be nil"),
730+
expectedKeys: []string{podKey, pvcPrimeKey},
737731
expectedObjects: &vpObjects{
738732
pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer},
739733
dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce),
@@ -1076,10 +1070,10 @@ func TestSyncPvcWithProviderImplementation(t *testing.T) {
10761070
ust(),
10771071
sc(testStorageClassName, storagev1.VolumeBindingImmediate),
10781072
},
1079-
pvcPrimeMutator: pvcPrimeMutateAccessModeRWX,
1080-
populateFn: populateOperationStartError,
1081-
expectedResult: nil,
1082-
expectedKeys: []string{pvcPrimeKey},
1073+
pvcPrimeCreateFn: pvcPrimeCreateSuccess(pvc(testPvcPrimeName, testVpWorkingNamespace, "", testStorageClassName, "", "", []string{}, nil, "", v1.ReadWriteMany)),
1074+
populateFn: populateOperationStartError,
1075+
expectedResult: nil,
1076+
expectedKeys: []string{pvcPrimeKey},
10831077
expectedObjects: &vpObjects{
10841078
pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer, vpFinalizer},
10851079
dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce),
@@ -1103,10 +1097,10 @@ func TestSyncPvcWithProviderImplementation(t *testing.T) {
11031097
ust(),
11041098
sc(testStorageClassName, storagev1.VolumeBindingImmediate),
11051099
},
1106-
pvcPrimeMutator: pvcPrimeMutateError,
1107-
populateFn: populateOperationStartError,
1108-
expectedResult: fmt.Errorf(testMutatePVCPrimeFailed),
1109-
expectedKeys: []string{pvcPrimeKey},
1100+
pvcPrimeCreateFn: pvcPrimeCreateError,
1101+
populateFn: populateOperationStartError,
1102+
expectedResult: fmt.Errorf(testPvcPrimeCreateFailed),
1103+
expectedKeys: []string{pvcPrimeKey},
11101104
expectedObjects: &vpObjects{
11111105
pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer},
11121106
dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce),
@@ -1123,10 +1117,10 @@ func TestSyncPvcWithProviderImplementation(t *testing.T) {
11231117
ust(),
11241118
sc(testStorageClassName, storagev1.VolumeBindingImmediate),
11251119
},
1126-
pvcPrimeMutator: pvcPrimeMutatePVCPrimeNil,
1127-
populateFn: populateOperationStartError,
1128-
expectedResult: fmt.Errorf("pvcPrime must not be nil"),
1129-
expectedKeys: []string{pvcPrimeKey},
1120+
pvcPrimeCreateFn: pvcPrimeCreateNil,
1121+
populateFn: populateOperationStartError,
1122+
expectedResult: fmt.Errorf("pvcPrime must not be nil"),
1123+
expectedKeys: []string{pvcPrimeKey},
11301124
expectedObjects: &vpObjects{
11311125
pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer},
11321126
dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce),

0 commit comments

Comments
 (0)