From 529d57b6a0eff86a71523807fb404570ff131e23 Mon Sep 17 00:00:00 2001 From: Peter Schuurman Date: Fri, 31 Jan 2025 18:03:08 -0800 Subject: [PATCH] Replace MutatorConfig with CreatePVCPrimeFn that allows custom creation of a PVC --- populator-machinery/controller.go | 70 ++++++++------------ populator-machinery/controller_test.go | 88 ++++++++++++-------------- 2 files changed, 66 insertions(+), 92 deletions(-) diff --git a/populator-machinery/controller.go b/populator-machinery/controller.go index 2fa7bc60..261c6721 100644 --- a/populator-machinery/controller.go +++ b/populator-machinery/controller.go @@ -79,7 +79,6 @@ const ( reasonPopulateOperationFailed = "PopulateOperationFailed" reasonPopulateOperationFinished = "PopulateOperationFinished" reasonPVCPrimeCreationError = "PopulatorPVCPrimeCreationError" - reasonPVCPrimeMutatorError = "reasonPVCPrimeMutatorError" reasonWaitForDataPopulationFinished = "PopulatorWaitForDataPopulationFinished" reasonStorageClassCreationError = "PopulatorStorageClassCreationError" reasonDataSourceNotFound = "PopulatorDataSourceNotFound" @@ -117,7 +116,6 @@ type controller struct { referenceGrantSynced cache.InformerSynced podConfig *PodConfig providerFunctionConfig *ProviderFunctionConfig - mutatorConfig *MutatorConfig crossNamespace bool providerMetricManager *ProviderMetricManager } @@ -142,9 +140,6 @@ type VolumePopulatorConfig struct { // ProviderFunctionConfig is the configuration for invoking provider functions. Either PodConfig or ProviderFunctionConfig should // be specified. PodConfig and ProviderFunctionConfig can't be provided at the same time ProviderFunctionConfig *ProviderFunctionConfig - // MutatorConfig is the configuration for invoking mutator functions. You can specify your own mutator functions to modify the - // Kubernetes resources used for volume population - MutatorConfig *MutatorConfig // ProviderMetricManager is the manager for provider specific metric handling ProviderMetricManager *ProviderMetricManager // CrossNamespace indicates if the populator supports data sources located in namespaces different than the PVC's namespace. @@ -170,6 +165,9 @@ type PodConfig struct { } type ProviderFunctionConfig struct { + // CreatePVCPrimeFn is the provider specific PVCPRime creation function + // Returns a PVC object that should be created by the caller, otherwise an error if creation failed and should be retried. + CreatePVCPrimeFn func(context.Context, PopulatorParams) (*corev1.PersistentVolumeClaim, error) // PopulateFn is the provider specific data population function PopulateFn func(context.Context, PopulatorParams) error // PopulateCompleteFn is the provider specific data population completeness check function, return true when data transfer gets completed @@ -192,19 +190,6 @@ type PopulatorParams struct { Recorder record.EventRecorder } -type MutatorConfig struct { - // PvcPrimeMutator is the mutator function for pvcPrime. The function gets called to modify the PVC object before pvcPrime gets created. - PvcPrimeMutator func(PvcPrimeMutatorParams) (*corev1.PersistentVolumeClaim, error) -} - -// PvcPrimeMutatorParams includes the parameters passing to the PvcPrimeMutator function -type PvcPrimeMutatorParams struct { - // PvcPrime is the temporary PVC created by volume populator - PvcPrime *corev1.PersistentVolumeClaim - // StorageClass is the original StorageClass Pvc refer to - StorageClass *storagev1.StorageClass -} - func RunController(masterURL, kubeconfig, imageName, httpEndpoint, metricsPath, namespace, prefix string, gk schema.GroupKind, gvr schema.GroupVersionResource, mountPath, devicePath string, populatorArgs func(bool, *unstructured.Unstructured) ([]string, error), @@ -308,7 +293,6 @@ func RunControllerWithConfig(vpcfg VolumePopulatorConfig) { referenceGrantSynced: referenceGrants.Informer().HasSynced, podConfig: vpcfg.PodConfig, providerFunctionConfig: vpcfg.ProviderFunctionConfig, - mutatorConfig: vpcfg.MutatorConfig, crossNamespace: vpcfg.CrossNamespace, providerMetricManager: vpcfg.ProviderMetricManager, } @@ -714,37 +698,33 @@ func (c *controller) syncPvc(ctx context.Context, key, pvcNamespace, pvcName str // TODO: Handle PVC' update while the original PVC changed // If the PVC is unbound and PVC' doesn't exist yet, create PVC' if "" == pvc.Spec.VolumeName && pvcPrime == nil { - pvcPrime = &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: pvcPrimeName, - Namespace: c.populatorNamespace, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: pvc.Spec.AccessModes, - Resources: pvc.Spec.Resources, - StorageClassName: pvc.Spec.StorageClassName, - VolumeMode: pvc.Spec.VolumeMode, - }, - } - if waitForFirstConsumer { - pvcPrime.Annotations = map[string]string{ - annSelectedNode: nodeName, - } - } - if c.mutatorConfig != nil && c.mutatorConfig.PvcPrimeMutator != nil { - mp := PvcPrimeMutatorParams{ - PvcPrime: pvcPrime, - StorageClass: storageClass, - } - pvcPrime, err = c.mutatorConfig.PvcPrimeMutator(mp) - if err != nil { - c.recorder.Eventf(pvc, corev1.EventTypeWarning, reasonPVCPrimeMutatorError, "Failed to mutate populator pvcPrime: %s", err) + if c.providerFunctionConfig != nil && c.providerFunctionConfig.CreatePVCPrimeFn != nil { + if pvcPrime, err = c.providerFunctionConfig.CreatePVCPrimeFn(ctx, *params); err != nil { + c.recorder.Eventf(pvc, corev1.EventTypeWarning, reasonPVCPrimeCreationError, "Failed to create PVC Prime: %s", err) return err } if pvcPrime == nil { - c.recorder.Eventf(pvc, corev1.EventTypeWarning, reasonPVCPrimeMutatorError, "pvcPrime must not be nil") + c.recorder.Eventf(pvc, corev1.EventTypeWarning, reasonPVCPrimeCreationError, "pvcPrime must not be nil") return fmt.Errorf("pvcPrime must not be nil") } + } else { + pvcPrime = &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcPrimeName, + Namespace: c.populatorNamespace, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: pvc.Spec.AccessModes, + Resources: pvc.Spec.Resources, + StorageClassName: pvc.Spec.StorageClassName, + VolumeMode: pvc.Spec.VolumeMode, + }, + } + if waitForFirstConsumer { + pvcPrime.Annotations = map[string]string{ + annSelectedNode: nodeName, + } + } } pvcPrime, err = c.kubeClient.CoreV1().PersistentVolumeClaims(c.populatorNamespace).Create(ctx, pvcPrime, metav1.CreateOptions{}) if err != nil { diff --git a/populator-machinery/controller_test.go b/populator-machinery/controller_test.go index d280efda..1cc22f79 100644 --- a/populator-machinery/controller_test.go +++ b/populator-machinery/controller_test.go @@ -58,14 +58,14 @@ type testCase struct { initialObjects []runtime.Object // Args for the populator pod populatorArgs func(b bool, u *unstructured.Unstructured) ([]string, error) + // Provider specific PVC prime creation function + pvcPrimeCreateFn func(context.Context, PopulatorParams) (*corev1.PersistentVolumeClaim, error) // Provider specific data population function populateFn func(context.Context, PopulatorParams) error // Provider specific data population completeness check function, return true when data transfer gets completed. populateCompleteFn func(context.Context, PopulatorParams) (bool, error) // The original PVC gets deleted or not pvcDeleted bool - // PvcPrimeMutator is the mutator function for pvcPrime - pvcPrimeMutator func(PvcPrimeMutatorParams) (*v1.PersistentVolumeClaim, error) // Expected errors expectedResult error // Expected keys in the notifyMap @@ -105,7 +105,7 @@ const ( testProvisioner = "test.provisioner" testPopulationOperationStartFailed = "Test populate operation start failed" testPopulateCompleteFailed = "Test populate operation complete failed" - testMutatePVCPrimeFailed = "Test mutate pvcPrime failed" + testPvcPrimeCreateFailed = "Test create pvcPrime failed" dataSourceKey = "unstructured/" + testPvcNamespace + "/" + testDataSourceName storageClassKey = "sc/" + testStorageClassName podKey = "pod/" + testVpWorkingNamespace + "/" + testPodName @@ -252,18 +252,18 @@ func populateCompleteSuccess(ctx context.Context, p PopulatorParams) (bool, erro return true, nil } -func pvcPrimeMutateAccessModeRWX(mp PvcPrimeMutatorParams) (*v1.PersistentVolumeClaim, error) { - accessMode := v1.ReadWriteMany - mp.PvcPrime.Spec.AccessModes[0] = accessMode - return mp.PvcPrime, nil +func pvcPrimeCreateError(ctx context.Context, p PopulatorParams) (*corev1.PersistentVolumeClaim, error) { + return nil, fmt.Errorf(testPvcPrimeCreateFailed) } -func pvcPrimeMutateError(mp PvcPrimeMutatorParams) (*v1.PersistentVolumeClaim, error) { - return mp.PvcPrime, fmt.Errorf(testMutatePVCPrimeFailed) +func pvcPrimeCreateNil(ctx context.Context, p PopulatorParams) (*corev1.PersistentVolumeClaim, error) { + return nil, nil } -func pvcPrimeMutatePVCPrimeNil(mp PvcPrimeMutatorParams) (*v1.PersistentVolumeClaim, error) { - return nil, nil +func pvcPrimeCreateSuccess(pvcPrime *corev1.PersistentVolumeClaim) func(ctx context.Context, p PopulatorParams) (*corev1.PersistentVolumeClaim, error) { + return func(ctx context.Context, p PopulatorParams) (*corev1.PersistentVolumeClaim, error) { + return pvcPrime, nil + } } func initTest(test testCase) ( @@ -310,19 +310,14 @@ func initTest(test testCase) ( } } - var providerFunctionConfig *ProviderFunctionConfig + var providerFunctionConfig *ProviderFunctionConfig = &ProviderFunctionConfig{} if test.populateFn != nil || test.populateCompleteFn != nil { - providerFunctionConfig = &ProviderFunctionConfig{ - PopulateFn: test.populateFn, - PopulateCompleteFn: test.populateCompleteFn, - } + providerFunctionConfig.PopulateFn = test.populateFn + providerFunctionConfig.PopulateCompleteFn = test.populateCompleteFn } - var mutatorConfig *MutatorConfig - if test.pvcPrimeMutator != nil { - mutatorConfig = &MutatorConfig{ - PvcPrimeMutator: test.pvcPrimeMutator, - } + if test.pvcPrimeCreateFn != nil { + providerFunctionConfig.CreatePVCPrimeFn = test.pvcPrimeCreateFn } c := &controller{ @@ -350,7 +345,6 @@ func initTest(test testCase) ( referenceGrantSynced: referenceGrants.Informer().HasSynced, podConfig: podConfig, providerFunctionConfig: providerFunctionConfig, - mutatorConfig: mutatorConfig, } return c, pvcInformer, unstInformer, scInformer, podInformer, pvInformer } @@ -367,7 +361,7 @@ func compareResult(want error, got error) bool { func compareNotifyMap(want []string, got map[string]*stringSet) error { if len(want) != len(got) { - return fmt.Errorf("The number of keys expected is different from actual. Expect %v, got %v", len(want), len(got)) + 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) } for _, key := range want { if got[key] == nil { @@ -682,10 +676,10 @@ func TestSyncPvcWithPopulatorPod(t *testing.T) { ust(), sc(testStorageClassName, storagev1.VolumeBindingImmediate), }, - populatorArgs: populatorArgs, - pvcPrimeMutator: pvcPrimeMutateAccessModeRWX, - expectedResult: nil, - expectedKeys: []string{podKey, pvcPrimeKey}, + populatorArgs: populatorArgs, + pvcPrimeCreateFn: pvcPrimeCreateSuccess(pvc(testPvcPrimeName, testVpWorkingNamespace, "", testStorageClassName, "", "", []string{}, nil, "", v1.ReadWriteMany)), + expectedResult: nil, + expectedKeys: []string{podKey, pvcPrimeKey}, expectedObjects: &vpObjects{ pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer, vpFinalizer}, dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce), @@ -710,10 +704,10 @@ func TestSyncPvcWithPopulatorPod(t *testing.T) { ust(), sc(testStorageClassName, storagev1.VolumeBindingImmediate), }, - populatorArgs: populatorArgs, - pvcPrimeMutator: pvcPrimeMutateError, - expectedResult: fmt.Errorf(testMutatePVCPrimeFailed), - expectedKeys: []string{podKey, pvcPrimeKey}, + populatorArgs: populatorArgs, + pvcPrimeCreateFn: pvcPrimeCreateError, + expectedResult: fmt.Errorf(testPvcPrimeCreateFailed), + expectedKeys: []string{podKey, pvcPrimeKey}, expectedObjects: &vpObjects{ pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer}, dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce), @@ -730,10 +724,10 @@ func TestSyncPvcWithPopulatorPod(t *testing.T) { ust(), sc(testStorageClassName, storagev1.VolumeBindingImmediate), }, - populatorArgs: populatorArgs, - pvcPrimeMutator: pvcPrimeMutatePVCPrimeNil, - expectedResult: fmt.Errorf("pvcPrime must not be nil"), - expectedKeys: []string{podKey, pvcPrimeKey}, + populatorArgs: populatorArgs, + pvcPrimeCreateFn: pvcPrimeCreateNil, + expectedResult: fmt.Errorf("pvcPrime must not be nil"), + expectedKeys: []string{podKey, pvcPrimeKey}, expectedObjects: &vpObjects{ pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer}, dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce), @@ -1076,10 +1070,10 @@ func TestSyncPvcWithProviderImplementation(t *testing.T) { ust(), sc(testStorageClassName, storagev1.VolumeBindingImmediate), }, - pvcPrimeMutator: pvcPrimeMutateAccessModeRWX, - populateFn: populateOperationStartError, - expectedResult: nil, - expectedKeys: []string{pvcPrimeKey}, + pvcPrimeCreateFn: pvcPrimeCreateSuccess(pvc(testPvcPrimeName, testVpWorkingNamespace, "", testStorageClassName, "", "", []string{}, nil, "", v1.ReadWriteMany)), + populateFn: populateOperationStartError, + expectedResult: nil, + expectedKeys: []string{pvcPrimeKey}, expectedObjects: &vpObjects{ pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer, vpFinalizer}, dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce), @@ -1103,10 +1097,10 @@ func TestSyncPvcWithProviderImplementation(t *testing.T) { ust(), sc(testStorageClassName, storagev1.VolumeBindingImmediate), }, - pvcPrimeMutator: pvcPrimeMutateError, - populateFn: populateOperationStartError, - expectedResult: fmt.Errorf(testMutatePVCPrimeFailed), - expectedKeys: []string{pvcPrimeKey}, + pvcPrimeCreateFn: pvcPrimeCreateError, + populateFn: populateOperationStartError, + expectedResult: fmt.Errorf(testPvcPrimeCreateFailed), + expectedKeys: []string{pvcPrimeKey}, expectedObjects: &vpObjects{ pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer}, dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce), @@ -1123,10 +1117,10 @@ func TestSyncPvcWithProviderImplementation(t *testing.T) { ust(), sc(testStorageClassName, storagev1.VolumeBindingImmediate), }, - pvcPrimeMutator: pvcPrimeMutatePVCPrimeNil, - populateFn: populateOperationStartError, - expectedResult: fmt.Errorf("pvcPrime must not be nil"), - expectedKeys: []string{pvcPrimeKey}, + pvcPrimeCreateFn: pvcPrimeCreateNil, + populateFn: populateOperationStartError, + expectedResult: fmt.Errorf("pvcPrime must not be nil"), + expectedKeys: []string{pvcPrimeKey}, expectedObjects: &vpObjects{ pvc: pvc(testPvcName, testPvcNamespace, "", testStorageClassName, "", testPvcUid, []string{pvFinalizer}, dsf(testApiGroup, testDatasourceKind, testDataSourceName, testPvcNamespace), "", v1.ReadWriteOnce),