Skip to content

Commit e63165c

Browse files
britaniarbritaniar
committed
reformat and add label to distinguish between approval object tasks
Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
1 parent 2e50010 commit e63165c

File tree

9 files changed

+54
-13
lines changed

9 files changed

+54
-13
lines changed

‎apis/placement/v1beta1/commons.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,9 @@ const (
158158
// TargetUpdateRunLabel indicates the target update run on a staged run related object.
159159
TargetUpdateRunLabel = FleetPrefix + "targetupdaterun"
160160

161+
// TargetStageTaskLabel indicates the target stage task on a staged run related object.
162+
TargetStageTaskLabel = FleetPrefix + "targetstagetask"
163+
161164
// UpdateRunDeleteStageName is the name of delete stage in the staged update run.
162165
UpdateRunDeleteStageName = FleetPrefix + "deleteStage"
163166

‎apis/placement/v1beta1/stageupdate_types.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,7 @@ type ApprovalRequestObjList interface {
679679
// - `TargetUpdateRun`: Points to the cluster staged update run that this approval request is for.
680680
// - `TargetStage`: The name of the stage that this approval request is for.
681681
// - `IsLatestUpdateRunApproval`: Indicates whether this approval request is the latest one related to this update run.
682+
// - `TargetStageTask`: Indicates whether this approval request is for the before or after stage task.
682683
type ClusterApprovalRequest struct {
683684
metav1.TypeMeta `json:",inline"`
684685
metav1.ObjectMeta `json:"metadata,omitempty"`
@@ -925,6 +926,7 @@ func (s *StagedUpdateStrategyList) GetUpdateStrategyObjs() []UpdateStrategyObj {
925926
// - `TargetUpdateRun`: Points to the staged update run that this approval request is for.
926927
// - `TargetStage`: The name of the stage that this approval request is for.
927928
// - `IsLatestUpdateRunApproval`: Indicates whether this approval request is the latest one related to this update run.
929+
// - `TargetStageTask`: Indicates whether this approval request is for the before or after stage task.
928930
type ApprovalRequest struct {
929931
metav1.TypeMeta `json:",inline"`
930932
metav1.ObjectMeta `json:"metadata,omitempty"`

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ spec:
4141
- `TargetUpdateRun`: Points to the staged update run that this approval request is for.
4242
- `TargetStage`: The name of the stage that this approval request is for.
4343
- `IsLatestUpdateRunApproval`: Indicates whether this approval request is the latest one related to this update run.
44+
- `TargetStageTask`: Indicates whether this approval request is for the before or after stage task.
4445
properties:
4546
apiVersion:
4647
description: |-

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ spec:
172172
- `TargetUpdateRun`: Points to the cluster staged update run that this approval request is for.
173173
- `TargetStage`: The name of the stage that this approval request is for.
174174
- `IsLatestUpdateRunApproval`: Indicates whether this approval request is the latest one related to this update run.
175+
- `TargetStageTask`: Indicates whether this approval request is for the before or after stage task.
175176
properties:
176177
apiVersion:
177178
description: |-

‎pkg/controllers/updaterun/execution.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,6 @@ func (r *Reconciler) execute(
8383
// which would update the lastTransitionTime even though the status hasn't effectively changed.
8484
markUpdateRunProgressingIfNotWaitingOrStuck(updateRun)
8585
if updatingStageIndex < len(updateRunStatus.StagesStatus) {
86-
maxConcurrency, err := calculateMaxConcurrencyValue(updateRunStatus, updatingStageIndex)
87-
if err != nil {
88-
return false, 0, err
89-
}
9086
updatingStageStatus = &updateRunStatus.StagesStatus[updatingStageIndex]
9187
approved, err := r.checkBeforeStageTasksStatus(ctx, updatingStageIndex, updateRun)
9288
if err != nil {
@@ -97,6 +93,10 @@ func (r *Reconciler) execute(
9793
markUpdateRunWaiting(updateRun, fmt.Sprintf(condition.UpdateRunWaitingMessageFmt, "before-stage", updatingStageStatus.StageName))
9894
return false, stageUpdatingWaitTime, nil
9995
}
96+
maxConcurrency, err := calculateMaxConcurrencyValue(updateRunStatus, updatingStageIndex)
97+
if err != nil {
98+
return false, 0, err
99+
}
100100
waitTime, err = r.executeUpdatingStage(ctx, updateRun, updatingStageIndex, toBeUpdatedBindings, maxConcurrency)
101101
// The execution has not finished yet.
102102
return false, waitTime, err
@@ -111,13 +111,13 @@ func (r *Reconciler) execute(
111111
func (r *Reconciler) checkBeforeStageTasksStatus(ctx context.Context, updatingStageIndex int, updateRun placementv1beta1.UpdateRunObj) (bool, error) {
112112
updateRunRef := klog.KObj(updateRun)
113113
updateRunStatus := updateRun.GetUpdateRunStatus()
114-
updatingStageStatus := &updateRunStatus.StagesStatus[updatingStageIndex]
115114
updatingStage := &updateRunStatus.UpdateStrategySnapshot.Stages[updatingStageIndex]
116115
if updatingStage.BeforeStageTasks == nil {
117116
klog.V(2).InfoS("There is no before stage task for this stage", "stage", updatingStage.Name, "updateRun", updateRunRef)
118117
return true, nil
119118
}
120119

120+
updatingStageStatus := &updateRunStatus.StagesStatus[updatingStageIndex]
121121
for i, task := range updatingStage.BeforeStageTasks {
122122
switch task.Type {
123123
case placementv1beta1.StageTaskTypeApproval:
@@ -604,9 +604,13 @@ func checkClusterUpdateResult(
604604
return false, nil
605605
}
606606

607-
// buildApprovalRequestObject creates an approval request object for after-stage tasks.
607+
// buildApprovalRequestObject creates an approval request object for before-stage or after-stage tasks.
608608
// It returns a ClusterApprovalRequest if namespace is empty, otherwise returns an ApprovalRequest.
609609
func buildApprovalRequestObject(namespacedName types.NamespacedName, stageName, updateRunName string) placementv1beta1.ApprovalRequestObj {
610+
stageTask := "before"
611+
if strings.Contains(namespacedName.Name, "after") {
612+
stageTask = "after"
613+
}
610614
var approvalRequest placementv1beta1.ApprovalRequestObj
611615
if namespacedName.Namespace == "" {
612616
approvalRequest = &placementv1beta1.ClusterApprovalRequest{
@@ -615,6 +619,7 @@ func buildApprovalRequestObject(namespacedName types.NamespacedName, stageName,
615619
Labels: map[string]string{
616620
placementv1beta1.TargetUpdatingStageNameLabel: stageName,
617621
placementv1beta1.TargetUpdateRunLabel: updateRunName,
622+
placementv1beta1.TargetStageTaskLabel: stageTask,
618623
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
619624
},
620625
},
@@ -631,6 +636,7 @@ func buildApprovalRequestObject(namespacedName types.NamespacedName, stageName,
631636
Labels: map[string]string{
632637
placementv1beta1.TargetUpdatingStageNameLabel: stageName,
633638
placementv1beta1.TargetUpdateRunLabel: updateRunName,
639+
placementv1beta1.TargetStageTaskLabel: stageTask,
634640
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
635641
},
636642
},

‎pkg/controllers/updaterun/execution_integration_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() {
172172
Labels: map[string]string{
173173
placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[0].StageName,
174174
placementv1beta1.TargetUpdateRunLabel: updateRun.Name,
175+
placementv1beta1.TargetStageTaskLabel: "before",
175176
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
176177
},
177178
},
@@ -323,6 +324,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() {
323324
Labels: map[string]string{
324325
placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[0].StageName,
325326
placementv1beta1.TargetUpdateRunLabel: updateRun.Name,
327+
placementv1beta1.TargetStageTaskLabel: "after",
326328
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
327329
},
328330
},
@@ -378,6 +380,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() {
378380
Labels: map[string]string{
379381
placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[1].StageName,
380382
placementv1beta1.TargetUpdateRunLabel: updateRun.Name,
383+
placementv1beta1.TargetStageTaskLabel: "before",
381384
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
382385
},
383386
},
@@ -530,6 +533,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() {
530533
Labels: map[string]string{
531534
placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[1].StageName,
532535
placementv1beta1.TargetUpdateRunLabel: updateRun.Name,
536+
placementv1beta1.TargetStageTaskLabel: "after",
533537
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
534538
},
535539
},
@@ -1084,6 +1088,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() {
10841088
Labels: map[string]string{
10851089
placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[0].StageName,
10861090
placementv1beta1.TargetUpdateRunLabel: updateRun.Name,
1091+
placementv1beta1.TargetStageTaskLabel: "after",
10871092
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
10881093
},
10891094
},
@@ -1305,6 +1310,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() {
13051310
Labels: map[string]string{
13061311
placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[0].StageName,
13071312
placementv1beta1.TargetUpdateRunLabel: updateRun.Name,
1313+
placementv1beta1.TargetStageTaskLabel: "before",
13081314
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
13091315
},
13101316
},
@@ -1338,6 +1344,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() {
13381344
Labels: map[string]string{
13391345
placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[0].StageName,
13401346
placementv1beta1.TargetUpdateRunLabel: updateRun.Name,
1347+
placementv1beta1.TargetStageTaskLabel: "before",
13411348
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
13421349
},
13431350
},
@@ -1440,6 +1447,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() {
14401447
Labels: map[string]string{
14411448
placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[0].StageName,
14421449
placementv1beta1.TargetUpdateRunLabel: updateRun.Name,
1450+
placementv1beta1.TargetStageTaskLabel: "after",
14431451
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
14441452
},
14451453
},

‎pkg/controllers/updaterun/execution_test.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -351,17 +351,18 @@ func TestBuildApprovalRequestObject(t *testing.T) {
351351
{
352352
name: "should create ClusterApprovalRequest when namespace is empty",
353353
namespacedName: types.NamespacedName{
354-
Name: "test-approval-request",
354+
Name: "test-before-approval-request",
355355
Namespace: "",
356356
},
357357
stageName: "test-stage",
358358
updateRunName: "test-update-run",
359359
want: &placementv1beta1.ClusterApprovalRequest{
360360
ObjectMeta: metav1.ObjectMeta{
361-
Name: "test-approval-request",
361+
Name: "test-before-approval-request",
362362
Labels: map[string]string{
363363
placementv1beta1.TargetUpdatingStageNameLabel: "test-stage",
364364
placementv1beta1.TargetUpdateRunLabel: "test-update-run",
365+
placementv1beta1.TargetStageTaskLabel: "before",
365366
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
366367
},
367368
},
@@ -374,18 +375,19 @@ func TestBuildApprovalRequestObject(t *testing.T) {
374375
{
375376
name: "should create namespaced ApprovalRequest when namespace is provided",
376377
namespacedName: types.NamespacedName{
377-
Name: "test-approval-request",
378+
Name: "test-after-approval-request",
378379
Namespace: "test-namespace",
379380
},
380381
stageName: "test-stage",
381382
updateRunName: "test-update-run",
382383
want: &placementv1beta1.ApprovalRequest{
383384
ObjectMeta: metav1.ObjectMeta{
384-
Name: "test-approval-request",
385+
Name: "test-after-approval-request",
385386
Namespace: "test-namespace",
386387
Labels: map[string]string{
387388
placementv1beta1.TargetUpdatingStageNameLabel: "test-stage",
388389
placementv1beta1.TargetUpdateRunLabel: "test-update-run",
390+
placementv1beta1.TargetStageTaskLabel: "after",
389391
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
390392
},
391393
},
@@ -948,6 +950,7 @@ func TestCalculateMaxConcurrencyValue(t *testing.T) {
948950
func TestCheckBeforeStageTasksStatus_NegativeCases(t *testing.T) {
949951
stageName := "stage-0"
950952
testUpdateRunName = "test-update-run"
953+
stageTask := "before"
951954
approvalRequestName := fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName)
952955
tests := []struct {
953956
name string
@@ -1035,6 +1038,7 @@ func TestCheckBeforeStageTasksStatus_NegativeCases(t *testing.T) {
10351038
Labels: map[string]string{
10361039
placementv1beta1.TargetUpdatingStageNameLabel: stageName,
10371040
placementv1beta1.TargetUpdateRunLabel: testUpdateRunName,
1041+
placementv1beta1.TargetStageTaskLabel: stageTask,
10381042
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
10391043
},
10401044
},
@@ -1091,6 +1095,7 @@ func TestCheckBeforeStageTasksStatus_NegativeCases(t *testing.T) {
10911095
Labels: map[string]string{
10921096
placementv1beta1.TargetUpdatingStageNameLabel: stageName,
10931097
placementv1beta1.TargetUpdateRunLabel: testUpdateRunName,
1098+
placementv1beta1.TargetStageTaskLabel: stageTask,
10941099
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
10951100
},
10961101
},
@@ -1147,6 +1152,7 @@ func TestCheckBeforeStageTasksStatus_NegativeCases(t *testing.T) {
11471152
Labels: map[string]string{
11481153
placementv1beta1.TargetUpdatingStageNameLabel: stageName,
11491154
placementv1beta1.TargetUpdateRunLabel: testUpdateRunName,
1155+
placementv1beta1.TargetStageTaskLabel: stageTask,
11501156
placementv1beta1.IsLatestUpdateRunApprovalLabel: "true",
11511157
},
11521158
},

‎test/e2e/cluster_staged_updaterun_test.go‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package e2e
1919
import (
2020
"fmt"
2121
"os/exec"
22+
"strings"
2223
"time"
2324

2425
. "github.com/onsi/ginkgo/v2"
@@ -1332,6 +1333,7 @@ var _ = Describe("test CRP rollout with staged update run", func() {
13321333
if err := hubClient.List(ctx, appReqList, client.MatchingLabels{
13331334
placementv1beta1.TargetUpdatingStageNameLabel: envCanary,
13341335
placementv1beta1.TargetUpdateRunLabel: updateRunName,
1336+
placementv1beta1.TargetStageTaskLabel: "after",
13351337
}); err != nil {
13361338
return fmt.Errorf("failed to list approval requests: %w", err)
13371339
}
@@ -1344,11 +1346,11 @@ var _ = Describe("test CRP rollout with staged update run", func() {
13441346
})
13451347

13461348
It("Should approve after-stage cluster approval request using kubectl-fleet approve plugin for canary stage", func() {
1347-
approveClusterApprovalRequest(envCanary, updateRunName)
1349+
approveClusterApprovalRequest(envCanary, updateRunName, "after")
13481350
})
13491351

13501352
It("Should approve before-stage cluster approval request using kubectl-fleet approve plugin for prod stage", func() {
1351-
approveClusterApprovalRequest(envProd, updateRunName)
1353+
approveClusterApprovalRequest(envProd, updateRunName, "before")
13521354
})
13531355

13541356
It("Should complete the staged update run after approval", func() {
@@ -2086,12 +2088,21 @@ func createClusterStagedUpdateRunSucceedWithNoResourceSnapshotIndex(updateRunNam
20862088
Expect(hubClient.Create(ctx, updateRun)).To(Succeed(), "Failed to create ClusterStagedUpdateRun %s", updateRunName)
20872089
}
20882090

2091+
func getStageTask(approvalRequestNameFmt string) string {
2092+
if strings.Contains(approvalRequestNameFmt, "after") {
2093+
return "after"
2094+
}
2095+
return "before"
2096+
}
2097+
20892098
func validateAndApproveClusterApprovalRequests(updateRunName, stageName, approvalRequestNameFmt string) {
2099+
stageTask := getStageTask(approvalRequestNameFmt)
20902100
Eventually(func() error {
20912101
appReqList := &placementv1beta1.ClusterApprovalRequestList{}
20922102
if err := hubClient.List(ctx, appReqList, client.MatchingLabels{
20932103
placementv1beta1.TargetUpdatingStageNameLabel: stageName,
20942104
placementv1beta1.TargetUpdateRunLabel: updateRunName,
2105+
placementv1beta1.TargetStageTaskLabel: stageTask,
20952106
}); err != nil {
20962107
return fmt.Errorf("failed to list approval requests: %w", err)
20972108
}
@@ -2122,7 +2133,7 @@ func updateConfigMapSucceed(newConfigMap *corev1.ConfigMap) {
21222133
Expect(hubClient.Update(ctx, cm)).To(Succeed(), "Failed to update configmap %s in namespace %s", newConfigMap.Name, newConfigMap.Namespace)
21232134
}
21242135

2125-
func approveClusterApprovalRequest(stageName, updateRunName string) {
2136+
func approveClusterApprovalRequest(stageName, updateRunName, stageTask string) {
21262137
var approvalRequestName string
21272138

21282139
// Get the cluster approval request name.
@@ -2131,6 +2142,7 @@ func approveClusterApprovalRequest(stageName, updateRunName string) {
21312142
if err := hubClient.List(ctx, appReqList, client.MatchingLabels{
21322143
placementv1beta1.TargetUpdatingStageNameLabel: stageName,
21332144
placementv1beta1.TargetUpdateRunLabel: updateRunName,
2145+
placementv1beta1.TargetStageTaskLabel: stageTask,
21342146
}); err != nil {
21352147
return fmt.Errorf("failed to list approval requests: %w", err)
21362148
}

‎test/e2e/staged_updaterun_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,11 +1651,13 @@ func createStagedUpdateRunSucceedWithNoResourceSnapshotIndex(updateRunName, name
16511651
}
16521652

16531653
func validateAndApproveNamespacedApprovalRequests(updateRunName, namespace, stageName, approvalRequestNameFmt string) {
1654+
stageTask := getStageTask(approvalRequestNameFmt)
16541655
Eventually(func() error {
16551656
appReqList := &placementv1beta1.ApprovalRequestList{}
16561657
if err := hubClient.List(ctx, appReqList, client.InNamespace(namespace), client.MatchingLabels{
16571658
placementv1beta1.TargetUpdatingStageNameLabel: stageName,
16581659
placementv1beta1.TargetUpdateRunLabel: updateRunName,
1660+
placementv1beta1.TargetStageTaskLabel: stageTask,
16591661
}); err != nil {
16601662
return fmt.Errorf("failed to list approval requests: %w", err)
16611663
}

0 commit comments

Comments
 (0)