feat: add beforeStageTasks to stagedUpdateRun

apis/placement/v1beta1/commons.go

@@ -167,8 +167,11 @@ const (
 	// TargetUpdatingStageNameLabel indicates the updating stage name on a staged run related object.
 	TargetUpdatingStageNameLabel = FleetPrefix + "targetUpdatingStage"
 
-	// ApprovalTaskNameFmt is the format of the approval task name.
-	ApprovalTaskNameFmt = "%s-%s"
+	// BeforeStageApprovalTaskNameFmt is the format of the before stage approval task name.
+	BeforeStageApprovalTaskNameFmt = "%s-before-%s"
+
+	// AfterStageApprovalTaskNameFmt is the format of the after stage approval task name.
+	AfterStageApprovalTaskNameFmt = "%s-after-%s"
 )
 
 var (

pkg/controllers/updaterun/controller_integration_test.go

@@ -543,17 +543,18 @@ func generateTestClusterStagedUpdateStrategy() *placementv1beta1.ClusterStagedUp
 	}
 }
 
-func generateTestClusterStagedUpdateStrategyWithSingleStage(afterStageTasks []placementv1beta1.StageTask) *placementv1beta1.ClusterStagedUpdateStrategy {
+func generateTestClusterStagedUpdateStrategyWithSingleStage(afterStageTasks, beforeStageTasks []placementv1beta1.StageTask) *placementv1beta1.ClusterStagedUpdateStrategy {
 	return &placementv1beta1.ClusterStagedUpdateStrategy{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: testUpdateStrategyName,
 		},
 		Spec: placementv1beta1.UpdateStrategySpec{
 			Stages: []placementv1beta1.StageConfig{
 				{
-					Name:            "stage1",
-					LabelSelector:   &metav1.LabelSelector{}, // Select all clusters.
-					AfterStageTasks: afterStageTasks,
+					Name:             "stage1",
+					LabelSelector:    &metav1.LabelSelector{}, // Select all clusters.
+					AfterStageTasks:  afterStageTasks,
+					BeforeStageTasks: beforeStageTasks,
 				},
 			},
 		},
@@ -724,9 +725,9 @@ func generateTrueCondition(obj client.Object, condType any) metav1.Condition {
 		case placementv1beta1.StageTaskConditionWaitTimeElapsed:
 			reason = condition.AfterStageTaskWaitTimeElapsedReason
 		case placementv1beta1.StageTaskConditionApprovalRequestCreated:
-			reason = condition.AfterStageTaskApprovalRequestCreatedReason
+			reason = condition.StageTaskApprovalRequestCreatedReason
 		case placementv1beta1.StageTaskConditionApprovalRequestApproved:
-			reason = condition.AfterStageTaskApprovalRequestApprovedReason
+			reason = condition.StageTaskApprovalRequestApprovedReason
 		}
 		typeStr = string(cond)
 	case placementv1beta1.ApprovalRequestConditionType:

pkg/controllers/updaterun/execution_test.go

@@ -19,6 +19,7 @@ package updaterun
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strings"
 	"testing"
 	"time"
@@ -943,3 +944,241 @@ func TestCalculateMaxConcurrencyValue(t *testing.T) {
 		})
 	}
 }
+
+func TestCheckBeforeStageTasksStatus_NegativeCases(t *testing.T) {
+	stageName := "stage-0"
+	testUpdateRunName = "test-update-run"
+	tests := []struct {
+		name            string
+		stageIndex      int
+		updateRun       *placementv1beta1.ClusterStagedUpdateRun
+		approvalRequest *placementv1beta1.ClusterApprovalRequest
+		wantError       bool
+	}{
+		// Negative test cases only
+		{
+			name:       "should return err if before stage task is TimedWait",
+			stageIndex: 0,
+			updateRun: &placementv1beta1.ClusterStagedUpdateRun{
+				Status: placementv1beta1.UpdateRunStatus{
+					UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{
+						Stages: []placementv1beta1.StageConfig{
+							{
+								Name: stageName,
+								BeforeStageTasks: []placementv1beta1.StageTask{
+									{
+										Type: placementv1beta1.StageTaskTypeTimedWait,
+									},
+								},
+							},
+						},
+					},
+					StagesStatus: []placementv1beta1.StageUpdatingStatus{
+						{
+							StageName: stageName,
+							BeforeStageTaskStatus: []placementv1beta1.StageTaskStatus{
+								{
+									Type: placementv1beta1.StageTaskTypeTimedWait,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name:       "should return err if Approval request has wrong target stage in spec",
+			stageIndex: 0,
+			updateRun: &placementv1beta1.ClusterStagedUpdateRun{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: testUpdateRunName,
+				},
+				Status: placementv1beta1.UpdateRunStatus{
+					UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{
+						Stages: []placementv1beta1.StageConfig{
+							{
+								Name: stageName,
+								BeforeStageTasks: []placementv1beta1.StageTask{
+									{
+										Type: placementv1beta1.StageTaskTypeApproval,
+									},
+								},
+							},
+						},
+					},
+					StagesStatus: []placementv1beta1.StageUpdatingStatus{
+						{
+							StageName: stageName,
+							BeforeStageTaskStatus: []placementv1beta1.StageTaskStatus{
+								{
+									Type:                placementv1beta1.StageTaskTypeApproval,
+									ApprovalRequestName: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName),
+									Conditions: []metav1.Condition{
+										{
+											Type:   string(placementv1beta1.StageTaskConditionApprovalRequestCreated),
+											Status: metav1.ConditionTrue,
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			approvalRequest: &placementv1beta1.ClusterApprovalRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName),
+					Labels: map[string]string{
+						placementv1beta1.TargetUpdatingStageNameLabel:   stageName,
+						placementv1beta1.TargetUpdateRunLabel:           testUpdateRunName,
+						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
+					},
+				},
+				Spec: placementv1beta1.ApprovalRequestSpec{
+					TargetUpdateRun: testUpdateRunName,
+					TargetStage:     "stage-1",
+				},
+			},
+		},
+		{
+			name:       "should return err if Approval request has wrong target update run in spec",
+			stageIndex: 0,
+			updateRun: &placementv1beta1.ClusterStagedUpdateRun{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: testUpdateRunName,
+				},
+				Status: placementv1beta1.UpdateRunStatus{
+					UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{
+						Stages: []placementv1beta1.StageConfig{
+							{
+								Name: stageName,
+								BeforeStageTasks: []placementv1beta1.StageTask{
+									{
+										Type: placementv1beta1.StageTaskTypeApproval,
+									},
+								},
+							},
+						},
+					},
+					StagesStatus: []placementv1beta1.StageUpdatingStatus{
+						{
+							StageName: stageName,
+							BeforeStageTaskStatus: []placementv1beta1.StageTaskStatus{
+								{
+									Type:                placementv1beta1.StageTaskTypeApproval,
+									ApprovalRequestName: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName),
+									Conditions: []metav1.Condition{
+										{
+											Type:   string(placementv1beta1.StageTaskConditionApprovalRequestCreated),
+											Status: metav1.ConditionTrue,
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			approvalRequest: &placementv1beta1.ClusterApprovalRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName),
+					Labels: map[string]string{
+						placementv1beta1.TargetUpdatingStageNameLabel:   stageName,
+						placementv1beta1.TargetUpdateRunLabel:           testUpdateRunName,
+						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
+					},
+				},
+				Spec: placementv1beta1.ApprovalRequestSpec{
+					TargetUpdateRun: "wrong-update-run",
+					TargetStage:     stageName,
+				},
+			},
+		},
+		{
+			name:       "should return err if cannot update Approval request that is approved as accepted",
+			stageIndex: 0,
+			updateRun: &placementv1beta1.ClusterStagedUpdateRun{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: testUpdateRunName,
+				},
+				Status: placementv1beta1.UpdateRunStatus{
+					UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{
+						Stages: []placementv1beta1.StageConfig{
+							{
+								Name: stageName,
+								BeforeStageTasks: []placementv1beta1.StageTask{
+									{
+										Type: placementv1beta1.StageTaskTypeApproval,
+									},
+								},
+							},
+						},
+					},
+					StagesStatus: []placementv1beta1.StageUpdatingStatus{
+						{
+							StageName: stageName,
+							BeforeStageTaskStatus: []placementv1beta1.StageTaskStatus{
+								{
+									Type:                placementv1beta1.StageTaskTypeApproval,
+									ApprovalRequestName: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName),
+									Conditions: []metav1.Condition{
+										{
+											Type:   string(placementv1beta1.StageTaskConditionApprovalRequestCreated),
+											Status: metav1.ConditionTrue,
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			approvalRequest: &placementv1beta1.ClusterApprovalRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName),
+					Labels: map[string]string{
+						placementv1beta1.TargetUpdatingStageNameLabel:   stageName,
+						placementv1beta1.TargetUpdateRunLabel:           testUpdateRunName,
+						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
+					},
+				},
+				Spec: placementv1beta1.ApprovalRequestSpec{
+					TargetUpdateRun: testUpdateRunName,
+					TargetStage:     stageName,
+				},
+				Status: placementv1beta1.ApprovalRequestStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   string(placementv1beta1.ApprovalRequestConditionApproved),
+							Status: metav1.ConditionTrue,
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			objects := []client.Object{tt.updateRun}
+			if tt.approvalRequest != nil {
+				objects = append(objects, tt.approvalRequest)
+			}
+			objectsWithStatus := []client.Object{tt.updateRun}
+			scheme := runtime.NewScheme()
+			_ = placementv1beta1.AddToScheme(scheme)
+			fakeClient := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithObjects(objects...).
+				WithStatusSubresource(objectsWithStatus...).
+				Build()
+			r := Reconciler{
+				Client: fakeClient,
+			}
+			ctx := context.Background()
+			_, err := r.checkBeforeStageTasksStatus(ctx, tt.stageIndex, tt.updateRun)
+			if err == nil {
+				t.Fatalf("checkBeforeStageTasksStatus() expected error but got nil")
+			}
+		})
+	}
+}

pkg/controllers/updaterun/initialization.go

@@ -344,6 +344,12 @@ func (r *Reconciler) computeRunStageStatus(
 
 	// Apply the label selectors from the UpdateStrategy to filter the clusters.
 	for _, stage := range updateRunStatus.UpdateStrategySnapshot.Stages {
+		if err := validateBeforeStageTask(stage.BeforeStageTasks); err != nil {
+			klog.ErrorS(err, "Failed to validate the before stage tasks", "updateStrategy", strategyKey, "stageName", stage.Name, "updateRun", updateRunRef)
+			// no more retries here.
+			invalidBeforeStageErr := controller.NewUserError(fmt.Errorf("the before stage tasks are invalid, updateStrategy: `%s`, stage: %s, err: %s", strategyKey, stage.Name, err.Error()))
+			return fmt.Errorf("%w: %s", errInitializedFailed, invalidBeforeStageErr.Error())
+		}
 		if err := validateAfterStageTask(stage.AfterStageTasks); err != nil {
 			klog.ErrorS(err, "Failed to validate the after stage tasks", "updateStrategy", strategyKey, "stageName", stage.Name, "updateRun", updateRunRef)
 			// no more retries here.
@@ -418,12 +424,20 @@ func (r *Reconciler) computeRunStageStatus(
 			curStageUpdatingStatus.Clusters[i].ClusterName = cluster.Name
 		}
 
+		// Create the before stage tasks.
+		curStageUpdatingStatus.BeforeStageTaskStatus = make([]placementv1beta1.StageTaskStatus, len(stage.BeforeStageTasks))
+		for i, task := range stage.BeforeStageTasks {
+			curStageUpdatingStatus.BeforeStageTaskStatus[i].Type = task.Type
+			if task.Type == placementv1beta1.StageTaskTypeApproval {
+				curStageUpdatingStatus.BeforeStageTaskStatus[i].ApprovalRequestName = fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, updateRun.GetName(), stage.Name)
+			}
+		}
 		// Create the after stage tasks.
 		curStageUpdatingStatus.AfterStageTaskStatus = make([]placementv1beta1.StageTaskStatus, len(stage.AfterStageTasks))
 		for i, task := range stage.AfterStageTasks {
 			curStageUpdatingStatus.AfterStageTaskStatus[i].Type = task.Type
 			if task.Type == placementv1beta1.StageTaskTypeApproval {
-				curStageUpdatingStatus.AfterStageTaskStatus[i].ApprovalRequestName = fmt.Sprintf(placementv1beta1.ApprovalTaskNameFmt, updateRun.GetName(), stage.Name)
+				curStageUpdatingStatus.AfterStageTaskStatus[i].ApprovalRequestName = fmt.Sprintf(placementv1beta1.AfterStageApprovalTaskNameFmt, updateRun.GetName(), stage.Name)
 			}
 		}
 		stagesStatus = append(stagesStatus, curStageUpdatingStatus)
@@ -448,8 +462,25 @@ func (r *Reconciler) computeRunStageStatus(
 	return nil
 }
 
-// validateAfterStageTask valides the afterStageTasks in the stage defined in the UpdateStrategy.
-// The error returned from this function is not retryable.
+// validateBeforeStageTask validates the beforeStageTasks in the stage defined in the UpdateStrategy.
+// The error returned from this function is not retriable.
+func validateBeforeStageTask(tasks []placementv1beta1.StageTask) error {
+	if len(tasks) > 1 {
+		return fmt.Errorf("beforeStageTasks can have at most one task")
+	}
+	for i, task := range tasks {
+		if task.Type == placementv1beta1.StageTaskTypeTimedWait {
+			return fmt.Errorf("task %d of type TimedWait is not allowed in beforeStageTasks", i)
+		}
+		if task.Type == placementv1beta1.StageTaskTypeApproval && task.WaitTime != nil {
+			return fmt.Errorf("task %d of type Approval cannot have wait duration set", i)
+		}
+	}
+	return nil
+}
+
+// validateAfterStageTask validates the afterStageTasks in the stage defined in the UpdateStrategy.
+// The error returned from this function is not retriable.
 func validateAfterStageTask(tasks []placementv1beta1.StageTask) error {
 	if len(tasks) == 2 && tasks[0].Type == tasks[1].Type {
 		return fmt.Errorf("afterStageTasks cannot have two tasks of the same type: %s", tasks[0].Type)