Add hook tests for BackupBatch (#1014)

Tasks:
- [x] PreBackup Hook
- [x] Post Backup Hooks

Test Types:
- [x] HTTP probe (covers HTTPGetAction, HTTPPostAction, and TCPSocketAction).
- [x] ExecAction
- [x] Different Situations
	- [x] Global preBackup hook fail ( should not execute member's hook and backup process)
	- [x] Global postBackup hook fail ( should fail BackupSession but backup data will be present).
	- [x] One of the target fail ( should execute global postBackup hook even if a target fail).

pkg/backup/backupsession.go

@@ -268,7 +268,6 @@ func (c *BackupSessionController) backup(invoker apis.Invoker, targetInfo apis.T
 
 	if c.InvokerType == api_v1beta1.ResourceKindBackupBatch {
 		c.SetupOpt.Path = fmt.Sprintf("%s/%s/%s", c.SetupOpt.Path, strings.ToLower(c.BackupTargetKind), c.BackupTargetName)
-
 	}
 
 	// apply nice, ionice settings from env

pkg/cmds/run_hook.go

@@ -117,95 +117,56 @@ func NewCmdRunHook() *cobra.Command {
 }
 
 func (opt *hookOptions) executeHook() error {
-	var backupConfig *v1beta1.BackupConfiguration
-	var restoreSession *v1beta1.RestoreSession
-	var err error
+	var hook interface{}
+	var executorPodName string
 
-	// For backup hooks, BackupSession name will be provided. We will read the hooks from the underlying BackupConfiguration.
 	if opt.backupSessionName != "" {
-		backupSession, err := opt.stashClient.StashV1beta1().BackupSessions(opt.namespace).Get(opt.backupSessionName, metav1.GetOptions{})
+		// For backup hooks, BackupSession name will be provided. We will read the hooks from the underlying backup invoker.
+		invoker, err := apis.ExtractBackupInvokerInfo(opt.stashClient, opt.invokerType, opt.invokerName, opt.namespace)
 		if err != nil {
 			return err
 		}
-		if backupSession.Spec.Invoker.Kind != v1beta1.ResourceKindBackupConfiguration {
-			return fmt.Errorf("backup hook for invoker kind: %s is not supported yet", backupSession.Spec.Invoker.Kind)
+		// We need to extract the hook only for the current target
+		for _, targetInfo := range invoker.TargetsInfo {
+			if targetInfo.Target != nil && targetInfo.Target.Ref.Kind == opt.targetKind && targetInfo.Target.Ref.Name == opt.targetName {
+				hook = targetInfo.Hooks
+				executorPodName, err = opt.getHookExecutorPodName(targetInfo.Target.Ref)
+				if err != nil {
+					return err
+				}
+				break
+			}
 		}
-		backupConfig, err = opt.stashClient.StashV1beta1().BackupConfigurations(opt.namespace).Get(backupSession.Spec.Invoker.Name, metav1.GetOptions{})
+	} else if opt.restoreSessionName != "" {
+		// For restore hooks, RestoreSession name will be provided. We will read the hooks from the RestoreSession.
+		restoreSession, err := opt.stashClient.StashV1beta1().RestoreSessions(opt.namespace).Get(opt.restoreSessionName, metav1.GetOptions{})
 		if err != nil {
 			return err
 		}
-	}
-
-	// For restore hooks, RestoreSession name will be provided. We will read the hooks from the RestoreSession.
-	if opt.restoreSessionName != "" {
-		restoreSession, err = opt.stashClient.StashV1beta1().RestoreSessions(opt.namespace).Get(opt.restoreSessionName, metav1.GetOptions{})
-		if err != nil {
-			return err
+		hook = restoreSession.Spec.Hooks
+		if restoreSession.Spec.Target != nil {
+			executorPodName, err = opt.getHookExecutorPodName(restoreSession.Spec.Target.Ref)
+			if err != nil {
+				return err
+			}
+		} else {
+			executorPodName = os.Getenv(apis.KeyPodName)
 		}
+	} else {
+		return fmt.Errorf("can not execute hooks. Reason: Respective BackupSession or RestoreSession has not been specified")
 	}
 
-	// Extract the hooks from the BackupConfiguration or RestoreSession
-	hook, err := opt.getHook(backupConfig, restoreSession)
-	if err != nil {
-		return err
-	}
-
-	// Now, determine the pod where the hook will execute
-	podName, err := opt.getPodName(backupConfig, restoreSession)
-	if err != nil {
-		return err
-	}
 	// Execute the hooks
-	return util.ExecuteHook(opt.config, hook, opt.hookType, podName, opt.namespace)
+	return util.ExecuteHook(opt.config, hook, opt.hookType, executorPodName, opt.namespace)
 }
 
-func (opt *hookOptions) getHook(backupConfig *v1beta1.BackupConfiguration, restoreSession *v1beta1.RestoreSession) (interface{}, error) {
-	switch opt.hookType {
-	case apis.PreBackupHook:
-		if backupConfig != nil && backupConfig.Spec.Hooks != nil && backupConfig.Spec.Hooks.PreBackup != nil {
-			return backupConfig.Spec.Hooks, nil
-		} else {
-			return nil, fmt.Errorf("no %s hook found in BackupConfiguration %s/%s", opt.hookType, opt.namespace, opt.backupSessionName)
-		}
-	case apis.PostBackupHook:
-		if backupConfig != nil && backupConfig.Spec.Hooks != nil && backupConfig.Spec.Hooks.PostBackup != nil {
-			return backupConfig.Spec.Hooks, nil
-		} else {
-			return nil, fmt.Errorf("no %s hook found in BackupConfiguration %s/%s", opt.hookType, opt.namespace, opt.backupSessionName)
-		}
-	case apis.PreRestoreHook:
-		if restoreSession != nil && restoreSession.Spec.Hooks != nil && restoreSession.Spec.Hooks.PreRestore != nil {
-			return restoreSession.Spec.Hooks, nil
-		} else {
-			return nil, fmt.Errorf("no %s hook found in RestoreSession %s/%s", opt.hookType, opt.namespace, opt.restoreSessionName)
-		}
-	case apis.PostRestoreHook:
-		if restoreSession != nil && restoreSession.Spec.Hooks != nil && restoreSession.Spec.Hooks.PostRestore != nil {
-			return restoreSession.Spec.Hooks, nil
-		} else {
-			return nil, fmt.Errorf("no %s hook found in RestoreSession %s/%s", opt.hookType, opt.namespace, opt.restoreSessionName)
-		}
-	default:
-		return nil, fmt.Errorf("unknown hook type: %s", opt.hookType)
-	}
-}
-
-func (opt *hookOptions) getPodName(backupConfig *v1beta1.BackupConfiguration, restoreSession *v1beta1.RestoreSession) (string, error) {
-	var targetRef v1beta1.TargetRef
-	// only one of backupConfig or restoreSession will be not nil
-	if backupConfig != nil && backupConfig.Spec.Target != nil {
-		targetRef = backupConfig.Spec.Target.Ref
-	} else if restoreSession != nil && restoreSession.Spec.Target != nil {
-		targetRef = restoreSession.Spec.Target.Ref
-	} else {
-		return "", fmt.Errorf("invalid target. target can't be nil for executing hook in Function-Task model")
-	}
-
+func (opt *hookOptions) getHookExecutorPodName(targetRef v1beta1.TargetRef) (string, error) {
 	switch targetRef.Kind {
 	case apis.KindAppBinding:
 		// For AppBinding, we will execute the hooks in the respective app pod
 		return opt.getAppPodName(targetRef.Name)
 	default:
+		// For other types of target, hook will be executed where this process is running.
 		return os.Getenv(apis.KeyPodName), nil
 	}
 }

pkg/controller/backup_session.go

@@ -132,7 +132,7 @@ func (c *StashController) applyBackupSessionReconciliationLogic(backupSession *a
 	// if backup process completed ( Failed or Succeeded), execute postBackup hook
 	if (phase == api_v1beta1.BackupSessionFailed || phase == api_v1beta1.BackupSessionSucceeded) &&
 		invoker.Hooks != nil && invoker.Hooks.PostBackup != nil {
-		err = util.ExecuteHook(c.clientConfig, invoker.Hooks.PostBackup, apis.PostBackupHook, os.Getenv("MY_POD_NAME"), os.Getenv("MY_POD_NAMESPACE"))
+		err = util.ExecuteHook(c.clientConfig, invoker.Hooks, apis.PostBackupHook, os.Getenv("MY_POD_NAME"), os.Getenv("MY_POD_NAMESPACE"))
 		if err != nil {
 			return c.setBackupSessionFailed(invoker, backupSession, err)
 		}
@@ -165,7 +165,7 @@ func (c *StashController) applyBackupSessionReconciliationLogic(backupSession *a
 
 	// if preBackup hook exist, then execute preBackupHook
 	if invoker.Hooks != nil && invoker.Hooks.PreBackup != nil {
-		err = util.ExecuteHook(c.clientConfig, invoker.Hooks.PreBackup, apis.PreBackupHook, os.Getenv("MY_POD_NAME"), os.Getenv("MY_POD_NAMESPACE"))
+		err = util.ExecuteHook(c.clientConfig, invoker.Hooks, apis.PreBackupHook, os.Getenv("MY_POD_NAME"), os.Getenv("MY_POD_NAMESPACE"))
 		if err != nil {
 			return c.setBackupSessionFailed(invoker, backupSession, err)
 		}
@@ -176,7 +176,8 @@ func (c *StashController) applyBackupSessionReconciliationLogic(backupSession *a
 			// skip if backup model is sidecar.
 			// for sidecar model controller inside sidecar will take care of it.
 			if invoker.Driver != api_v1beta1.VolumeSnapshotter && util.BackupModel(targetInfo.Target.Ref.Kind) == apis.ModelSidecar {
-				log.Infof("Skipping processing BackupSession %s/%s. Reason: Backup model is sidecar. Controller inside sidecar will take care of it.", backupSession.Namespace, backupSession.Name)
+				log.Infof("Skipping processing BackupSession %s/%s for target %s %s/%s. Reason: Backup model is sidecar."+
+					"Controller inside sidecar will take care of it.", backupSession.Namespace, backupSession.Name, targetInfo.Target.Ref.Kind, backupSession.Namespace, targetInfo.Target.Ref.Name)
 			}
 
 			// if VolumeSnapshotter driver is used then ensure VolumeSnapshotter job and return
@@ -197,13 +198,14 @@ func (c *StashController) applyBackupSessionReconciliationLogic(backupSession *a
 			}
 
 			// Set BackupSession phase "Running"
-			backupSession, err = c.setBackupSessionRunning(targetInfo.Target, invoker.Driver, backupSession)
+			backupSession, err = c.setTargetPhaseRunning(targetInfo.Target, invoker.Driver, backupSession)
 			if err != nil {
 				return err
 			}
 		}
 	}
-	return nil
+	_, err = c.setBackupSessionRunning(backupSession)
+	return err
 }
 
 func (c *StashController) ensureBackupJob(invoker apis.Invoker, targetInfo apis.TargetInfo, backupSession *api_v1beta1.BackupSession, index int) error {
@@ -263,7 +265,7 @@ func (c *StashController) ensureBackupJob(invoker apis.Invoker, targetInfo apis.
 	}
 
 	if backupSession.Spec.Invoker.Kind == api_v1beta1.ResourceKindBackupBatch {
-		repoInputs[apis.RepositoryPrefix] = fmt.Sprintf("%s/%s/%s", repoInputs[apis.RepositoryPrefix], targetInfo.Target.Ref.Kind, targetInfo.Target.Ref.Name)
+		repoInputs[apis.RepositoryPrefix] = fmt.Sprintf("%s/%s/%s", repoInputs[apis.RepositoryPrefix], strings.ToLower(targetInfo.Target.Ref.Kind), targetInfo.Target.Ref.Name)
 	}
 
 	bcInputs, err := c.inputsForBackupConfig(invoker, targetInfo)
@@ -448,17 +450,16 @@ func (c *StashController) setBackupSessionSkipped(backupSession *api_v1beta1.Bac
 	return err
 }
 
-func (c *StashController) setBackupSessionRunning(target *api_v1beta1.BackupTarget, driver api_v1beta1.Snapshotter, backupSession *api_v1beta1.BackupSession) (*api_v1beta1.BackupSession, error) {
+func (c *StashController) setTargetPhaseRunning(target *api_v1beta1.BackupTarget, driver api_v1beta1.Snapshotter, backupSession *api_v1beta1.BackupSession) (*api_v1beta1.BackupSession, error) {
 	// find out the total number of hosts in target that will be backed up in this backup session
 	totalHosts, err := c.getTotalHosts(target, backupSession.Namespace, driver)
 	if err != nil {
 		return nil, err
 	}
-	// set BackupSession phase to "Running"
+	// set target phase to "Running"
 	backupSession, err = stash_util.UpdateBackupSessionStatus(c.stashClient.StashV1beta1(), backupSession, func(in *api_v1beta1.BackupSessionStatus) *api_v1beta1.BackupSessionStatus {
-		in.Phase = api_v1beta1.BackupSessionRunning
 		if target != nil {
-			in.Targets = append(backupSession.Status.Targets, api_v1beta1.Target{
+			in.Targets = upsertTargetStatsEntry(backupSession.Status.Targets, api_v1beta1.Target{
 				TotalHosts: totalHosts,
 				Ref: api_v1beta1.TargetRef{
 					Name: target.Ref.Name,
@@ -469,6 +470,15 @@ func (c *StashController) setBackupSessionRunning(target *api_v1beta1.BackupTarg
 		}
 		return in
 	})
+	return backupSession, err
+}
+
+func (c *StashController) setBackupSessionRunning(backupSession *api_v1beta1.BackupSession) (*api_v1beta1.BackupSession, error) {
+	// set BackupSession phase to "Running"
+	backupSession, err := stash_util.UpdateBackupSessionStatus(c.stashClient.StashV1beta1(), backupSession, func(in *api_v1beta1.BackupSessionStatus) *api_v1beta1.BackupSessionStatus {
+		in.Phase = api_v1beta1.BackupSessionRunning
+		return in
+	})
 	if err != nil {
 		return nil, err
 	}
@@ -482,7 +492,6 @@ func (c *StashController) setBackupSessionRunning(target *api_v1beta1.BackupTarg
 		eventer.EventReasonBackupSessionRunning,
 		fmt.Sprintf("Backup job has been created succesfully/sidecar is watching the BackupSession."),
 	)
-
 	return backupSession, err
 }
 
@@ -637,3 +646,16 @@ func (c *StashController) cleanupBackupHistory(backupInvokerRef api_v1beta1.Back
 	}
 	return nil
 }
+
+func upsertTargetStatsEntry(targetStats []api_v1beta1.Target, newEntry api_v1beta1.Target) []api_v1beta1.Target {
+	// already exist, then just update
+	for i := range targetStats {
+		if targetStats[i].Ref.Kind == newEntry.Ref.Kind && targetStats[i].Ref.Name == newEntry.Ref.Name {
+			targetStats[i] = newEntry
+			return targetStats
+		}
+	}
+	// target entry does not exist. add new entry
+	targetStats = append(targetStats, newEntry)
+	return targetStats
+}

test/e2e/framework/service.go

@@ -27,15 +27,15 @@ const (
 	TEST_HEADLESS_SERVICE = "headless"
 )
 
-func (fi *Invocation) HeadlessService(label string) core.Service {
+func (fi *Invocation) HeadlessService(name string) core.Service {
 	return core.Service{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      TEST_HEADLESS_SERVICE,
+			Name:      name,
 			Namespace: fi.namespace,
 		},
 		Spec: core.ServiceSpec{
 			Selector: map[string]string{
-				"app": label,
+				"app": name,
 			},
 			ClusterIP: core.ClusterIPNone,
 			Ports: []core.ServicePort{

test/e2e/framework/statefulset.go

@@ -53,7 +53,7 @@ func (fi *Invocation) StatefulSet(name, pvcName, volName string) apps.StatefulSe
 			},
 			Replicas:    types.Int32P(1),
 			Template:    fi.PodTemplate(labels, pvcName, volName),
-			ServiceName: TEST_HEADLESS_SERVICE,
+			ServiceName: name,
 			UpdateStrategy: apps.StatefulSetUpdateStrategy{
 				Type: apps.RollingUpdateStatefulSetStrategyType,
 			},
@@ -77,7 +77,7 @@ func (fi *Invocation) StatefulSetForV1beta1API(name, volName string, replica int
 				MatchLabels: labels,
 			},
 			Replicas:    &replica,
-			ServiceName: TEST_HEADLESS_SERVICE,
+			ServiceName: name,
 			Template: core.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: labels,
@@ -228,8 +228,8 @@ func (fi *Invocation) DeployStatefulSet(name string, replica int32, volName stri
 	return createdss, err
 }
 
-func (fi *Invocation) DeployStatefulSetWithProbeClient() (*apps.StatefulSet, error) {
-	name := fmt.Sprintf("%s-%s", ProberDemoPodPrefix, fi.app)
+func (fi *Invocation) DeployStatefulSetWithProbeClient(name string) (*apps.StatefulSet, error) {
+	name = fmt.Sprintf("%s-%s", name, fi.app)
 	svc, err := fi.CreateService(fi.HeadlessService(name))
 	if err != nil {
 		return nil, err
@@ -251,7 +251,7 @@ func (fi *Invocation) DeployStatefulSetWithProbeClient() (*apps.StatefulSet, err
 			Selector: &metav1.LabelSelector{
 				MatchLabels: labels,
 			},
-			ServiceName: TEST_HEADLESS_SERVICE,
+			ServiceName: name,
 			Template: core.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: labels,

test/e2e/framework/util.go

@@ -1005,3 +1005,23 @@ func isDebugTarget(containers []core.Container) (bool, []string) {
 	}
 	return false, nil
 }
+
+func (fi *Invocation) HookFailed(involvedObjectKind string, involvedObjectMeta metav1.ObjectMeta, probeType string) (bool, error) {
+	fieldSelector := fields.SelectorFromSet(fields.Set{
+		"involvedObject.kind":      involvedObjectKind,
+		"involvedObject.name":      involvedObjectMeta.Name,
+		"involvedObject.namespace": involvedObjectMeta.Namespace,
+		"type":                     core.EventTypeWarning,
+	})
+	events, err := fi.KubeClient.CoreV1().Events(fi.namespace).List(metav1.ListOptions{FieldSelector: fieldSelector.String()})
+	Expect(err).NotTo(HaveOccurred())
+
+	hasHookFailureEvent := false
+	for _, e := range events.Items {
+		if strings.Contains(e.Message, fmt.Sprintf("failed to execute %q probe.", probeType)) {
+			hasHookFailureEvent = true
+			break
+		}
+	}
+	return hasHookFailureEvent, nil
+}