Merge pull request #435 from raghavendra-talur/rtalur-backport-1738-1739

BUG DFBUGS-997: backport 1738 and 1739

.golangci.yaml

@@ -47,7 +47,6 @@ linters-settings:
       - name: error-strings
       - name: error-naming
       - name: exported
-      - name: if-return
       - name: increment-decrement
       - name: var-naming
       - name: var-declaration

internal/controller/volumereplicationgroup_controller.go

@@ -547,9 +547,10 @@ func (v *VRGInstance) processVRG() ctrl.Result {
 		}
 	}
 
-	if err := RecipeElementsGet(
-		v.ctx, v.reconciler.Client, *v.instance, *v.ramenConfig, v.log, &v.recipeElements,
-	); err != nil {
+	var err error
+
+	v.recipeElements, err = RecipeElementsGet(v.ctx, v.reconciler.Client, *v.instance, *v.ramenConfig, v.log)
+	if err != nil {
 		return v.invalid(err, "Failed to get recipe", false)
 	}
 

internal/controller/vrg_kubeobjects.go

@@ -820,6 +820,12 @@ func getCaptureGroups(recipe Recipe.Recipe) ([]kubeobjects.CaptureSpec, error) {
 		for resourceType, resourceName := range resource {
 			captureInstance, err := getResourceAndConvertToCaptureGroup(recipe, resourceType, resourceName)
 			if err != nil {
+				if errors.Is(err, ErrVolumeCaptureNotSupported) {
+					// we only use the volumes group for determining the label selector
+					// ignore it in the capture sequence
+					continue
+				}
+
 				return resources, err
 			}
 
@@ -852,6 +858,12 @@ func getRecoverGroups(recipe Recipe.Recipe) ([]kubeobjects.RecoverSpec, error) {
 		for resourceType, resourceName := range resource {
 			captureInstance, err := getResourceAndConvertToRecoverGroup(recipe, resourceType, resourceName)
 			if err != nil {
+				if errors.Is(err, ErrVolumeRecoverNotSupported) {
+					// we only use the volumes group for determining the label selector
+					// ignore it in the capture sequence
+					continue
+				}
+
 				return resources, err
 			}
 
@@ -862,6 +874,11 @@ func getRecoverGroups(recipe Recipe.Recipe) ([]kubeobjects.RecoverSpec, error) {
 	return resources, nil
 }
 
+var (
+	ErrVolumeCaptureNotSupported = errors.New("volume capture not supported")
+	ErrVolumeRecoverNotSupported = errors.New("volume recover not supported")
+)
+
 func getResourceAndConvertToCaptureGroup(
 	recipe Recipe.Recipe, resourceType, name string) (*kubeobjects.CaptureSpec, error,
 ) {
@@ -873,6 +890,10 @@ func getResourceAndConvertToCaptureGroup(
 			}
 		}
 
+		if name == recipe.Spec.Volumes.Name {
+			return nil, ErrVolumeCaptureNotSupported
+		}
+
 		return nil, k8serrors.NewNotFound(schema.GroupResource{Resource: "Recipe.Spec.Group.Name"}, name)
 	}
 
@@ -904,6 +925,10 @@ func getResourceAndConvertToRecoverGroup(
 			}
 		}
 
+		if name == recipe.Spec.Volumes.Name {
+			return nil, ErrVolumeRecoverNotSupported
+		}
+
 		return nil, k8serrors.NewNotFound(schema.GroupResource{Resource: "Recipe.Spec.Group.Name"}, name)
 	}
 

internal/controller/vrg_pvc_selector_test.go

@@ -41,10 +41,12 @@ var _ = Describe("VolumeReplicationGroupPVCSelector", func() {
 		testCtx, cancel = context.WithCancel(context.TODO())
 		Expect(k8sClient).NotTo(BeNil())
 		vrgTestNamespace = createUniqueNamespace(testCtx)
+		ramenConfig.RamenOpsNamespace = vrgTestNamespace
 	})
 
 	AfterEach(func() {
 		Expect(k8sClient.Delete(testCtx, testNamespace)).To(Succeed())
+		ramenConfig.RamenOpsNamespace = ""
 
 		cancel()
 	})
@@ -164,8 +166,9 @@ func getBaseVRG(namespace string) *ramen.VolumeReplicationGroup {
 			Async: &ramen.VRGAsyncSpec{
 				SchedulingInterval: "5m",
 			},
-			ReplicationState: ramen.Primary,
-			S3Profiles:       []string{"dummy-s3-profile"},
+			ReplicationState:    ramen.Primary,
+			S3Profiles:          []string{"dummy-s3-profile"},
+			ProtectedNamespaces: &[]string{namespace},
 		},
 	}
 }
@@ -200,12 +203,13 @@ func getVRGDefinitionWithKubeObjectProtection(hasPVCSelectorLabels bool, namespa
 	return vrg
 }
 
-func getTestHook() *Recipe.Hook {
+func getTestHook(testNamespace string) *Recipe.Hook {
 	duration := 30
 
 	return &Recipe.Hook{
-		Name: "hook-single",
-		Type: "exec",
+		Name:      "hook-single",
+		Type:      "exec",
+		Namespace: testNamespace,
 		LabelSelector: &metav1.LabelSelector{
 			MatchLabels: map[string]string{
 				"myapp": "testapp",
@@ -267,7 +271,7 @@ func getRecipeDefinition(namespace string) *Recipe.Recipe {
 		Spec: Recipe.RecipeSpec{
 			Groups:  []*Recipe.Group{getTestGroup()},
 			Volumes: getTestVolumeGroup(),
-			Hooks:   []*Recipe.Hook{getTestHook()},
+			Hooks:   []*Recipe.Hook{getTestHook(namespace)},
 			Workflows: []*Recipe.Workflow{
 				{
 					Name: "backup",
@@ -279,7 +283,7 @@ func getRecipeDefinition(namespace string) *Recipe.Recipe {
 							"group": "test-group",
 						},
 						{
-							"hook": "test-hook",
+							"hook": "hook-single/checkpoint",
 						},
 					},
 				},

internal/controller/vrg_recipe.go

@@ -79,44 +79,35 @@ func GetPVCSelector(ctx context.Context, reader client.Reader, vrg ramen.VolumeR
 	ramenConfig ramen.RamenConfig,
 	log logr.Logger,
 ) (PvcSelector, error) {
-	var recipeElements RecipeElements
+	recipeElements, err := RecipeElementsGet(ctx, reader, vrg, ramenConfig, log)
+	if err != nil {
+		return PvcSelector{}, err
+	}
 
-	return recipeElements.PvcSelector, recipeVolumesAndOptionallyWorkflowsGet(
-		ctx, reader, vrg, ramenConfig, log, &recipeElements,
-		func(recipe.Recipe, *RecipeElements, ramen.VolumeReplicationGroup, ramen.RamenConfig) error {
-			return nil
-		},
-	)
+	return recipeElements.PvcSelector, nil
 }
 
 func RecipeElementsGet(ctx context.Context, reader client.Reader, vrg ramen.VolumeReplicationGroup,
-	ramenConfig ramen.RamenConfig, log logr.Logger, recipeElements *RecipeElements,
-) error {
-	return recipeVolumesAndOptionallyWorkflowsGet(ctx, reader, vrg, ramenConfig, log, recipeElements,
-		recipeWorkflowsGet,
-	)
-}
+	ramenConfig ramen.RamenConfig, log logr.Logger,
+) (RecipeElements, error) {
+	var recipeElements RecipeElements
 
-func recipeVolumesAndOptionallyWorkflowsGet(ctx context.Context, reader client.Reader, vrg ramen.VolumeReplicationGroup,
-	ramenConfig ramen.RamenConfig, log logr.Logger, recipeElements *RecipeElements,
-	workflowsGet func(recipe.Recipe, *RecipeElements, ramen.VolumeReplicationGroup, ramen.RamenConfig) error,
-) error {
 	if vrg.Spec.KubeObjectProtection == nil {
-		*recipeElements = RecipeElements{
+		recipeElements = RecipeElements{
 			PvcSelector: getPVCSelector(vrg, ramenConfig, nil, nil),
 		}
 
-		return nil
+		return recipeElements, nil
 	}
 
 	if vrg.Spec.KubeObjectProtection.RecipeRef == nil {
-		*recipeElements = RecipeElements{
+		recipeElements = RecipeElements{
 			PvcSelector:     getPVCSelector(vrg, ramenConfig, nil, nil),
 			CaptureWorkflow: captureWorkflowDefault(vrg, ramenConfig),
 			RecoverWorkflow: recoverWorkflowDefault(vrg, ramenConfig),
 		}
 
-		return nil
+		return recipeElements, nil
 	}
 
 	recipeNamespacedName := types.NamespacedName{
@@ -126,11 +117,11 @@ func recipeVolumesAndOptionallyWorkflowsGet(ctx context.Context, reader client.R
 
 	recipe := recipe.Recipe{}
 	if err := reader.Get(ctx, recipeNamespacedName, &recipe); err != nil {
-		return fmt.Errorf("recipe %v get error: %w", recipeNamespacedName.String(), err)
+		return recipeElements, fmt.Errorf("recipe %v get error: %w", recipeNamespacedName.String(), err)
 	}
 
 	if err := RecipeParametersExpand(&recipe, vrg.Spec.KubeObjectProtection.RecipeParameters, log); err != nil {
-		return err
+		return recipeElements, fmt.Errorf("recipe %v parameters expansion error: %w", recipeNamespacedName.String(), err)
 	}
 
 	var selector PvcSelector
@@ -141,15 +132,19 @@ func recipeVolumesAndOptionallyWorkflowsGet(ctx context.Context, reader client.R
 			recipe.Spec.Volumes.LabelSelector)
 	}
 
-	*recipeElements = RecipeElements{
+	recipeElements = RecipeElements{
 		PvcSelector: selector,
 	}
 
-	if err := workflowsGet(recipe, recipeElements, vrg, ramenConfig); err != nil {
-		return err
+	if err := recipeWorkflowsGet(recipe, &recipeElements, vrg, ramenConfig); err != nil {
+		return recipeElements, fmt.Errorf("recipe %v workflows get error: %w", recipeNamespacedName.String(), err)
+	}
+
+	if err := recipeNamespacesValidate(recipeElements, vrg, ramenConfig); err != nil {
+		return recipeElements, fmt.Errorf("recipe %v namespaces validation error: %w", recipeNamespacedName.String(), err)
 	}
 
-	return recipeNamespacesValidate(*recipeElements, vrg, ramenConfig)
+	return recipeElements, nil
 }
 
 func RecipeParametersExpand(recipe *recipe.Recipe, parameters map[string][]string,

internal/controller/vrg_volsync.go

@@ -554,23 +554,39 @@ func (v *VRGInstance) disownPVCs() error {
 	return nil
 }
 
-// cleanupResources this function deleted all PS, PD and VolumeSnapshots from its owner (VRG)
+// cleanupResources this function deleted all RS, RD and VolumeSnapshots from its owner (VRG)
 func (v *VRGInstance) cleanupResources() error {
 	for idx := range v.volSyncPVCs {
 		pvc := &v.volSyncPVCs[idx]
 
-		if err := v.volSyncHandler.DeleteRS(pvc.Name, pvc.Namespace); err != nil {
+		if err := v.doCleanupResources(pvc.Name, pvc.Namespace); err != nil {
 			return err
 		}
+	}
 
-		if err := v.volSyncHandler.DeleteRD(pvc.Name, pvc.Namespace); err != nil {
-			return err
-		}
+	for idx := range v.instance.Spec.VolSync.RDSpec {
+		protectedPVC := v.instance.Spec.VolSync.RDSpec[idx].ProtectedPVC
 
-		if err := v.volSyncHandler.DeleteSnapshots(pvc.Namespace); err != nil {
+		if err := v.doCleanupResources(protectedPVC.Name, protectedPVC.Namespace); err != nil {
 			return err
 		}
 	}
 
 	return nil
 }
+
+func (v *VRGInstance) doCleanupResources(name, namespace string) error {
+	if err := v.volSyncHandler.DeleteRS(name, namespace); err != nil {
+		return err
+	}
+
+	if err := v.volSyncHandler.DeleteRD(name, namespace); err != nil {
+		return err
+	}
+
+	if err := v.volSyncHandler.DeleteSnapshots(namespace); err != nil {
+		return err
+	}
+
+	return nil
+}