fix: add label to update run approval object to distinguish between before and after stage tasks

apis/placement/v1beta1/commons.go

@@ -156,7 +156,10 @@ const (
 	UpdateRunFinalizer = FleetPrefix + "stagedupdaterun-finalizer"
 
 	// TargetUpdateRunLabel indicates the target update run on a staged run related object.
-	TargetUpdateRunLabel = FleetPrefix + "targetupdaterun"
+	TargetUpdateRunLabel = FleetPrefix + "targetUpdateRun"
+
+	// TaskTypeLabel indicates the task type (before-stage or after-stage) on a staged run related object.
+	TaskTypeLabel = FleetPrefix + "taskType"
 
 	// UpdateRunDeleteStageName is the name of delete stage in the staged update run.
 	UpdateRunDeleteStageName = FleetPrefix + "deleteStage"
@@ -167,6 +170,12 @@ const (
 	// TargetUpdatingStageNameLabel indicates the updating stage name on a staged run related object.
 	TargetUpdatingStageNameLabel = FleetPrefix + "targetUpdatingStage"
 
+	// BeforeStageTaskLabelValue is the before stage task label value.
+	BeforeStageTaskLabelValue = "beforeStage"
+
+	// AfterStageTaskLabelValue is the after stage task label value.
+	AfterStageTaskLabelValue = "afterStage"
+
 	// BeforeStageApprovalTaskNameFmt is the format of the before stage approval task name.
 	BeforeStageApprovalTaskNameFmt = "%s-before-%s"
 

apis/placement/v1beta1/stageupdate_types.go

@@ -679,6 +679,7 @@ type ApprovalRequestObjList interface {
 //   - `TargetUpdateRun`: Points to the cluster staged update run that this approval request is for.
 //   - `TargetStage`: The name of the stage that this approval request is for.
 //   - `IsLatestUpdateRunApproval`: Indicates whether this approval request is the latest one related to this update run.
+//   - `TaskTypeLabel`: Indicates whether this approval request is for the before or after stage task.
 type ClusterApprovalRequest struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
@@ -925,6 +926,7 @@ func (s *StagedUpdateStrategyList) GetUpdateStrategyObjs() []UpdateStrategyObj {
 //   - `TargetUpdateRun`: Points to the staged update run that this approval request is for.
 //   - `TargetStage`: The name of the stage that this approval request is for.
 //   - `IsLatestUpdateRunApproval`: Indicates whether this approval request is the latest one related to this update run.
+//   - `TaskTypeLabel`: Indicates whether this approval request is for the before or after stage task.
 type ApprovalRequest struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

config/crd/bases/placement.kubernetes-fleet.io_approvalrequests.yaml

@@ -41,6 +41,7 @@ spec:
             - `TargetUpdateRun`: Points to the staged update run that this approval request is for.
             - `TargetStage`: The name of the stage that this approval request is for.
             - `IsLatestUpdateRunApproval`: Indicates whether this approval request is the latest one related to this update run.
+            - `TaskTypeLabel`: Indicates whether this approval request is for the before or after stage task.
         properties:
           apiVersion:
             description: |-

config/crd/bases/placement.kubernetes-fleet.io_clusterapprovalrequests.yaml

@@ -172,6 +172,7 @@ spec:
             - `TargetUpdateRun`: Points to the cluster staged update run that this approval request is for.
             - `TargetStage`: The name of the stage that this approval request is for.
             - `IsLatestUpdateRunApproval`: Indicates whether this approval request is the latest one related to this update run.
+            - `TaskTypeLabel`: Indicates whether this approval request is for the before or after stage task.
         properties:
           apiVersion:
             description: |-

pkg/controllers/updaterun/execution.go

@@ -83,10 +83,6 @@ func (r *Reconciler) execute(
 	// which would update the lastTransitionTime even though the status hasn't effectively changed.
 	markUpdateRunProgressingIfNotWaitingOrStuck(updateRun)
 	if updatingStageIndex < len(updateRunStatus.StagesStatus) {
-		maxConcurrency, err := calculateMaxConcurrencyValue(updateRunStatus, updatingStageIndex)
-		if err != nil {
-			return false, 0, err
-		}
 		updatingStageStatus = &updateRunStatus.StagesStatus[updatingStageIndex]
 		approved, err := r.checkBeforeStageTasksStatus(ctx, updatingStageIndex, updateRun)
 		if err != nil {
@@ -97,6 +93,10 @@ func (r *Reconciler) execute(
 			markUpdateRunWaiting(updateRun, fmt.Sprintf(condition.UpdateRunWaitingMessageFmt, "before-stage", updatingStageStatus.StageName))
 			return false, stageUpdatingWaitTime, nil
 		}
+		maxConcurrency, err := calculateMaxConcurrencyValue(updateRunStatus, updatingStageIndex)
+		if err != nil {
+			return false, 0, err
+		}
 		waitTime, err = r.executeUpdatingStage(ctx, updateRun, updatingStageIndex, toBeUpdatedBindings, maxConcurrency)
 		// The execution has not finished yet.
 		return false, waitTime, err
@@ -111,17 +111,17 @@ func (r *Reconciler) execute(
 func (r *Reconciler) checkBeforeStageTasksStatus(ctx context.Context, updatingStageIndex int, updateRun placementv1beta1.UpdateRunObj) (bool, error) {
 	updateRunRef := klog.KObj(updateRun)
 	updateRunStatus := updateRun.GetUpdateRunStatus()
-	updatingStageStatus := &updateRunStatus.StagesStatus[updatingStageIndex]
 	updatingStage := &updateRunStatus.UpdateStrategySnapshot.Stages[updatingStageIndex]
 	if updatingStage.BeforeStageTasks == nil {
 		klog.V(2).InfoS("There is no before stage task for this stage", "stage", updatingStage.Name, "updateRun", updateRunRef)
 		return true, nil
 	}
 
+	updatingStageStatus := &updateRunStatus.StagesStatus[updatingStageIndex]
 	for i, task := range updatingStage.BeforeStageTasks {
 		switch task.Type {
 		case placementv1beta1.StageTaskTypeApproval:
-			approved, err := r.handleStageApprovalTask(ctx, &updatingStageStatus.BeforeStageTaskStatus[i], updatingStage, updateRun)
+			approved, err := r.handleStageApprovalTask(ctx, &updatingStageStatus.BeforeStageTaskStatus[i], updatingStage, updateRun, placementv1beta1.BeforeStageTaskLabelValue)
 			if err != nil {
 				return false, err
 			}
@@ -418,7 +418,7 @@ func (r *Reconciler) checkAfterStageTasksStatus(ctx context.Context, updatingSta
 				klog.V(2).InfoS("The after stage wait task has completed", "stage", updatingStage.Name, "updateRun", updateRunRef)
 			}
 		case placementv1beta1.StageTaskTypeApproval:
-			approved, err := r.handleStageApprovalTask(ctx, &updatingStageStatus.AfterStageTaskStatus[i], updatingStage, updateRun)
+			approved, err := r.handleStageApprovalTask(ctx, &updatingStageStatus.AfterStageTaskStatus[i], updatingStage, updateRun, placementv1beta1.AfterStageTaskLabelValue)
 			if err != nil {
 				return false, -1, err
 			}
@@ -440,6 +440,7 @@ func (r *Reconciler) handleStageApprovalTask(
 	stageTaskStatus *placementv1beta1.StageTaskStatus,
 	updatingStage *placementv1beta1.StageConfig,
 	updateRun placementv1beta1.UpdateRunObj,
+	stageTaskType string,
 ) (bool, error) {
 	updateRunRef := klog.KObj(updateRun)
 
@@ -450,7 +451,7 @@ func (r *Reconciler) handleStageApprovalTask(
 	}
 
 	// Check if the approval request has been created.
-	approvalRequest := buildApprovalRequestObject(types.NamespacedName{Name: stageTaskStatus.ApprovalRequestName, Namespace: updateRun.GetNamespace()}, updatingStage.Name, updateRun.GetName())
+	approvalRequest := buildApprovalRequestObject(types.NamespacedName{Name: stageTaskStatus.ApprovalRequestName, Namespace: updateRun.GetNamespace()}, updatingStage.Name, updateRun.GetName(), stageTaskType)
 	requestRef := klog.KObj(approvalRequest)
 	if err := r.Client.Create(ctx, approvalRequest); err != nil {
 		if apierrors.IsAlreadyExists(err) {
@@ -618,9 +619,9 @@ func checkClusterUpdateResult(
 	return false, nil
 }
 
-// buildApprovalRequestObject creates an approval request object for after-stage tasks.
+// buildApprovalRequestObject creates an approval request object for before-stage or after-stage tasks.
 // It returns a ClusterApprovalRequest if namespace is empty, otherwise returns an ApprovalRequest.
-func buildApprovalRequestObject(namespacedName types.NamespacedName, stageName, updateRunName string) placementv1beta1.ApprovalRequestObj {
+func buildApprovalRequestObject(namespacedName types.NamespacedName, stageName, updateRunName, stageTaskType string) placementv1beta1.ApprovalRequestObj {
 	var approvalRequest placementv1beta1.ApprovalRequestObj
 	if namespacedName.Namespace == "" {
 		approvalRequest = &placementv1beta1.ClusterApprovalRequest{
@@ -629,6 +630,7 @@ func buildApprovalRequestObject(namespacedName types.NamespacedName, stageName,
 				Labels: map[string]string{
 					placementv1beta1.TargetUpdatingStageNameLabel:   stageName,
 					placementv1beta1.TargetUpdateRunLabel:           updateRunName,
+					placementv1beta1.TaskTypeLabel:                  stageTaskType,
 					placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 				},
 			},
@@ -645,6 +647,7 @@ func buildApprovalRequestObject(namespacedName types.NamespacedName, stageName,
 				Labels: map[string]string{
 					placementv1beta1.TargetUpdatingStageNameLabel:   stageName,
 					placementv1beta1.TargetUpdateRunLabel:           updateRunName,
+					placementv1beta1.TaskTypeLabel:                  stageTaskType,
 					placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 				},
 			},

pkg/controllers/updaterun/execution_integration_test.go

@@ -173,6 +173,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() {
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   updateRun.Status.StagesStatus[0].StageName,
 						placementv1beta1.TargetUpdateRunLabel:           updateRun.Name,
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.BeforeStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},
@@ -324,6 +325,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() {
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   updateRun.Status.StagesStatus[0].StageName,
 						placementv1beta1.TargetUpdateRunLabel:           updateRun.Name,
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.AfterStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},
@@ -379,6 +381,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() {
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   updateRun.Status.StagesStatus[1].StageName,
 						placementv1beta1.TargetUpdateRunLabel:           updateRun.Name,
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.BeforeStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},
@@ -531,6 +534,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() {
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   updateRun.Status.StagesStatus[1].StageName,
 						placementv1beta1.TargetUpdateRunLabel:           updateRun.Name,
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.AfterStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},
@@ -1085,6 +1089,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() {
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   updateRun.Status.StagesStatus[0].StageName,
 						placementv1beta1.TargetUpdateRunLabel:           updateRun.Name,
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.AfterStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},
@@ -1306,6 +1311,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() {
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   updateRun.Status.StagesStatus[0].StageName,
 						placementv1beta1.TargetUpdateRunLabel:           updateRun.Name,
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.BeforeStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},
@@ -1339,6 +1345,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() {
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   updateRun.Status.StagesStatus[0].StageName,
 						placementv1beta1.TargetUpdateRunLabel:           updateRun.Name,
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.BeforeStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},
@@ -1441,6 +1448,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() {
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   updateRun.Status.StagesStatus[0].StageName,
 						placementv1beta1.TargetUpdateRunLabel:           updateRun.Name,
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.AfterStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},

pkg/controllers/updaterun/execution_test.go

@@ -346,22 +346,25 @@ func TestBuildApprovalRequestObject(t *testing.T) {
 		namespacedName types.NamespacedName
 		stageName      string
 		updateRunName  string
+		stageTaskType  string
 		want           placementv1beta1.ApprovalRequestObj
 	}{
 		{
 			name: "should create ClusterApprovalRequest when namespace is empty",
 			namespacedName: types.NamespacedName{
-				Name:      "test-approval-request",
+				Name:      fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, "test-update-run", "test-stage"),
 				Namespace: "",
 			},
 			stageName:     "test-stage",
 			updateRunName: "test-update-run",
+			stageTaskType: placementv1beta1.BeforeStageTaskLabelValue,
 			want: &placementv1beta1.ClusterApprovalRequest{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: "test-approval-request",
+					Name: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, "test-update-run", "test-stage"),
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   "test-stage",
 						placementv1beta1.TargetUpdateRunLabel:           "test-update-run",
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.BeforeStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},
@@ -374,18 +377,20 @@ func TestBuildApprovalRequestObject(t *testing.T) {
 		{
 			name: "should create namespaced ApprovalRequest when namespace is provided",
 			namespacedName: types.NamespacedName{
-				Name:      "test-approval-request",
+				Name:      fmt.Sprintf(placementv1beta1.AfterStageApprovalTaskNameFmt, "test-update-run", "test-stage"),
 				Namespace: "test-namespace",
 			},
 			stageName:     "test-stage",
 			updateRunName: "test-update-run",
+			stageTaskType: placementv1beta1.AfterStageTaskLabelValue,
 			want: &placementv1beta1.ApprovalRequest{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-approval-request",
+					Name:      fmt.Sprintf(placementv1beta1.AfterStageApprovalTaskNameFmt, "test-update-run", "test-stage"),
 					Namespace: "test-namespace",
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   "test-stage",
 						placementv1beta1.TargetUpdateRunLabel:           "test-update-run",
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.AfterStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},
@@ -399,7 +404,7 @@ func TestBuildApprovalRequestObject(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			got := buildApprovalRequestObject(test.namespacedName, test.stageName, test.updateRunName)
+			got := buildApprovalRequestObject(test.namespacedName, test.stageName, test.updateRunName, test.stageTaskType)
 
 			// Compare the whole objects using cmp.Diff with ignore options
 			if diff := cmp.Diff(test.want, got); diff != "" {
@@ -1035,6 +1040,7 @@ func TestCheckBeforeStageTasksStatus_NegativeCases(t *testing.T) {
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   stageName,
 						placementv1beta1.TargetUpdateRunLabel:           testUpdateRunName,
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.BeforeStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},
@@ -1091,6 +1097,7 @@ func TestCheckBeforeStageTasksStatus_NegativeCases(t *testing.T) {
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   stageName,
 						placementv1beta1.TargetUpdateRunLabel:           testUpdateRunName,
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.BeforeStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},
@@ -1147,6 +1154,7 @@ func TestCheckBeforeStageTasksStatus_NegativeCases(t *testing.T) {
 					Labels: map[string]string{
 						placementv1beta1.TargetUpdatingStageNameLabel:   stageName,
 						placementv1beta1.TargetUpdateRunLabel:           testUpdateRunName,
+						placementv1beta1.TaskTypeLabel:                  placementv1beta1.BeforeStageTaskLabelValue,
 						placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
 					},
 				},