Skip to content

Conversation

@britaniar
Copy link
Collaborator

@britaniar britaniar commented Nov 12, 2025

Description of your changes

I have:

  • Added a check within update run execution to check for before stage tasks.

  • Returned unexpected error for TimedWait tasks as a before stage task, otherwise continue with approval task.

  • Check the different states of approval tasks.

  • Update the names of the approval requests to differentiate between before and after tasks.

  • Update current integration and e2e tests to include before stage tasks

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

  • Integration Test
  • E2E

Special notes for your reviewer

@britaniar britaniar changed the title add beforeStageTasks to stagedUpdateRun feat: add beforeStageTasks to stagedUpdateRun Nov 12, 2025
@codecov
Copy link

codecov bot commented Nov 13, 2025

Codecov Report

❌ Patch coverage is 84.42623% with 19 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/controllers/updaterun/execution.go 85.00% 11 Missing and 4 partials ⚠️
pkg/controllers/updaterun/initialization.go 78.94% 3 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@britaniar britaniar marked this pull request as ready for review November 18, 2025 02:30
@britaniar britaniar requested review from Arvindthiru and jwtty and removed request for jwtty November 18, 2025 19:35
@britaniar britaniar requested review from Arvindthiru and jwtty and removed request for jwtty November 18, 2025 22:04
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
}
updatingStage := &updateRunStatus.StagesStatus[updatingStageIndex]
approved, beforeStageTaskErr := r.checkBeforeStageTasksStatus(ctx, updatingStageIndex, updateRun)
if beforeStageTaskErr != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this err is Aborted, we need to mark stage to be failed: https://github.com/kubefleet-dev/kubefleet/blob/main/pkg/controllers/updaterun/execution.go#L79C3-L82C4. Since multiple such checks exist in this code, please create a defer function.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found a potential bug: calculateMaxConcurrencyValue should return errIsAborted. cc @Arvindthiru please confirm and submit a separate fix PR.

@kubefleet-dev kubefleet-dev deleted a comment from jwtty Nov 25, 2025
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants