feat: sync the latest round of changes from the internal fork

[michaelawyu]

Description of your changes

This PR sync'd the latest round of changes from the internal fork.

I have:

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

How has this code been tested

N/A

Special notes for your reviewer

N/A

[kaito-pr-agent]

Title

(Describe updated until commit b432a80)

Enhance fleet resource handling and testing

---

Description

• Added new functionality to handle partial failures during manifest application.

• Created new test cases to cover edge scenarios involving decoding errors.

• Implemented helper functions to manage cluster eviction and resource management.

• Enhanced logging and error handling across various components.

---

Changes walkthrough 📝

	Relevant files
Enhancement	8 files execution_integration_test.go Improve update run execution test cases                                    +17/-13  status.go Enhance status update logic with detailed logs                      +48/-3    execution.go Add progress and success messages to update run conditions +24/-0    placement_negative_cases_test.go Introduce negative test cases for manifest decoding errors +220/-0  fleetresourcehandler_webhook.go Allow specific operations based on namespace prefixes        +19/-17  eviction.go Implement utility functions to check eviction states          +42/-0    drain.go Add functionality to drain member clusters                              +205/-0  uncordon.go Add functionality to uncordon member clusters                        +49/-0
Documentation	2 files README.md Document how to build and use the draincluster tool            +77/-0    README.md Document how to build and use the uncordoncluster tool      +70/-0
Additional files	31 files ci.yml +1/-1      code-lint.yml +1/-1      codespell.yml +1/-1      trivy.yml +1/-1      upgrade.yml +1/-1      .golangci.yml +1/-1      hub-agent.Dockerfile +1/-1      member-agent.Dockerfile +1/-1      refresh-token.Dockerfile +1/-1      go.mod +1/-1      controller.go +3/-33    controller.go +15/-0    controller_integration_test.go +23/-0    validation_integration_test.go +4/-3      apply.go +9/-0      availability_tracker.go +10/-0    controller_integration_test.go +243/-0  controller_test.go +14/-0    preprocess.go +6/-3      process.go +11/-1    common.go +15/-9    eviction_test.go +153/-0  fleetresourcehandler_webhook_test.go +58/-5    membercluster_validating_webhook.go +28/-4    webhook.go +1/-0      actuals_test.go +11/-16  fleet_guard_rail_test.go +21/-7    join_and_leave_test.go +84/-2    main.go +78/-0    main.go +59/-0    common.go +46/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[kaito-pr-agent]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Redundant Condition Updates The code updates the progressing condition multiple times unnecessarily. It should be updated once at the beginning of the function and then only when necessary. meta.SetStatusCondition(&updateRun.Status.Conditions, metav1.Condition{ Type: string(placementv1beta1.StagedUpdateRunConditionProgressing), Status: metav1.ConditionFalse, ObservedGeneration: updateRun.Generation, Reason: condition.UpdateRunSucceededReason, Message: "All stages are completed", }) Missing Finalizer Removal The code adds a finalizer but does not remove it when the update fails. This could lead to orphaned resources. controllerutil.AddFinalizer(updateRun, placementv1beta1.ClusterStagedUpdateRunFinalizer) return r.Update(ctx, updateRun, client.FieldOwner(utils.UpdateRunControllerFieldManagerName))

[kaito-pr-agent]

PR Code Suggestions ✨

Latest suggestions up to b432a80
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Extract condition generation logic for better readability Improve readability by extracting the condition generation logic into a separate function. pkg/controllers/updaterun/execution_integration_test.go [272-273] -wantStatus.StagesStatus[0].Conditions[0] = generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing) // The progressing condition now becomes false with waiting reason. +func generateFalseProgressingCondition(updateRun *updateRun, conditionType placementv1beta1.ConditionType, succeeded bool) metav1.Condition { + return metav1.Condition{ + Type: conditionType, + Status: metav1.ConditionFalse, + Reason: condition.StageUpdatingWaitingReason, + Message: fmt.Sprintf("Condition %s is false with reason %s", conditionType, condition.StageUpdatingWaitingReason), + } +} +wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, true) + Suggestion importance[1-10]: 7 __ Why: The suggestion improves readability by encapsulating the condition generation logic into a separate function, making the code easier to understand and maintain.	Medium
	Simplified log message Simplify the log message by removing redundant information. pkg/controllers/workapplier/availability_tracker.go [56-64] -if bundle.gvr != nil && bundle.manifestObj != nil { - klog.V(2).InfoS("The manifest object is not applied yet, skipping the availability check", - "GVR", *bundle.gvr, "manifestObj", klog.KObj(bundle.manifestObj), "inMemberClusterObj", klog.KObj(bundle.inMemberClusterObj), "work", workRef) -} else { - klog.V(2).InfoS("The manifest object is not applied yet, skipping the availability check", - "ordinal", pieces, "work", workRef) -} +klog.V(2).InfoS("Skipping availability check due to unapplied manifest object", + "GVR", *bundle.gvr, "manifestObj", klog.KObj(bundle.manifestObj), "ordinal", pieces, "work", workRef) Suggestion importance[1-10]: 7 __ Why: The suggestion simplifies the log message by consolidating redundant information, making it more concise and easier to read without losing important details.	Medium
	Improve condition message accuracy Ensure that the condition message accurately reflects the condition status. pkg/controllers/updaterun/execution_integration_test.go [273-274] -wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing) +wantStatus.StagesStatus[0].Conditions[0] = metav1.Condition{ + Type: placementv1beta1.StageUpdatingConditionProgressing, + Status: metav1.ConditionFalse, + Reason: condition.StageUpdatingWaitingReason, + Message: "Condition is false due to waiting reason", +} Suggestion importance[1-10]: 6 __ Why: The suggestion ensures that the condition message accurately reflects the condition status, improving the clarity and usefulness of the logs.	Low
	Simplify condition initialization Simplify the condition initialization by removing unnecessary parameters. pkg/controllers/updaterun/execution_integration_test.go [273-274] -wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, true) +wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing) Suggestion importance[1-10]: 5 __ Why: While the suggestion simplifies the condition initialization, it removes necessary parameters, potentially leading to less flexible and harder-to-maintain code.	Low
	Remove redundant message field Remove redundant message field pkg/controllers/updaterun/execution.go [394-395] meta.SetStatusCondition(&appReq.Status.Conditions, metav1.Condition{ Type: string(placementv1beta1.ApprovalRequestApprovalAcceptedReason), Status: metav1.ConditionTrue, ObservedGeneration: appReq.Generation, Reason: condition.ApprovalRequestApprovalAcceptedReason, - Message: "The approval request has been approved and cannot be reverted", }) Suggestion importance[1-10]: 2 __ Why: The message field is already set in the Type and Reason fields, making the message redundant.	Low

---

Previous suggestions

Suggestions up to commit 1443a67

Category	Suggestion	Impact
General	Fix inconsistent condition reason Ensure consistency in handling progressing conditions. pkg/controllers/updaterun/controller_integration_test.go [572] case placementv1beta1.StageUpdatingConditionProgressing: - reason = condition.StageUpdatingWaitingReason + reason = condition.StageUpdatingProgressingReason Suggestion importance[1-10]: 7 __ Why: The suggestion improves consistency in handling progressing conditions by using a more appropriate reason.	Medium
	Set correct reason when marking progressing condition as false Ensure the condition reason is set correctly when marking the progressing condition as false. pkg/controllers/updaterun/execution_integration_test.go [272] -wantStatus.StagesStatus[0].Conditions[0] = generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing) // The progressing condition now becomes false with waiting reason. +wantStatus.StagesStatus[0].Conditions[0] = generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, "waiting") // The progressing condition now becomes false with waiting reason. Suggestion importance[1-10]: 6 __ Why: The existing code lacks a reason for the condition, which could lead to confusion about the state.	Low
	Set correct reason when marking approval request as accepted Ensure the condition reason is set correctly when marking the approval request as accepted. pkg/controllers/updaterun/execution.go [391] -meta.SetStatusCondition(&appReq.Status.Conditions, cond) +meta.SetStatusCondition(&appReq.Status.Conditions, metav1.Condition{ + Type: condition.ApprovalRequestApprovalAcceptedReason, + Status: metav1.ConditionTrue, + ObservedGeneration: appReq.Generation, + Reason: condition.ApprovalRequestApprovalAcceptedReason, + Message: "The approval request has been approved and cannot be reverted", +}) Suggestion importance[1-10]: 6 __ Why: The existing code lacks a reason for the condition, which could lead to confusion about the state.	Low
	Set correct reason when marking stage updating as succeeded Ensure the condition reason is set correctly when marking the stage updating as succeeded. pkg/controllers/updaterun/execution.go [489] meta.SetStatusCondition(&stageUpdatingStatus.Conditions, metav1.Condition{ Type: string(placementv1beta1.StageUpdatingConditionProgressing), Status: metav1.ConditionFalse, ObservedGeneration: generation, Reason: condition.StageUpdatingSucceededReason, + Message: "All clusters in the stage are updated and after-stage tasks are completed", }) Suggestion importance[1-10]: 6 __ Why: The existing code lacks a reason for the condition, which could lead to confusion about the state.	Low
	Correct condition status Simplify condition update logic. pkg/controllers/updaterun/validation_integration_test.go [442] -started.Conditions[1] = generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, false) +started.Conditions[1] = generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, true) Suggestion importance[1-10]: 5 __ Why: The suggestion simplifies the condition update logic but incorrectly sets the condition status to true instead of false.	Low

[Copilot]

Copilot reviewed 37 out of 41 changed files in this pull request and generated 1 comment.

Files not reviewed (4)

• docker/hub-agent.Dockerfile: Language not supported
• docker/member-agent.Dockerfile: Language not supported
• docker/refresh-token.Dockerfile: Language not supported
• go.mod: Language not supported

[Copilot]

Ensure that the Conditions array is pre-populated before directly assigning to an index to prevent potential out-of-range errors if the array is empty.

Suggested change

	wantStatus.StagesStatus[0].Clusters[4].Conditions = append(wantStatus.StagesStatus[0].Clusters[4].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded))
	wantStatus.StagesStatus[0].Clusters[4].Conditions = append(wantStatus.StagesStatus[0].Clusters[4].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded))
	if wantStatus.StagesStatus[0].Conditions == nil || len(wantStatus.StagesStatus[0].Conditions) < 1 {
	wantStatus.StagesStatus[0].Conditions = make([]metav1.Condition, 1)
	}