stop populating ClusterResourceBinding with WorkloadAffinity fields

[zhzhuang-zju]

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
stop populating ClusterResourceBinding with WorkloadAffinity fields

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

[gemini-code-assist]

Summary of Changes

Hello @zhzhuang-zju, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request performs a cleanup by removing the population of WorkloadAffinity related fields within ClusterResourceBinding objects. This change streamlines the binding creation and application process by eliminating deprecated or unused logic, ensuring that the system no longer attempts to process or store workload affinity information in these bindings.

Highlights

• Removal of WorkloadAffinity field population: The logic responsible for populating the WorkloadAffinityGroups field in ClusterResourceBinding objects has been removed from the ApplyClusterPolicy and BuildClusterResourceBinding functions.
• Cleanup of related test cases: Associated test cases and feature gate settings for WorkloadAffinity in detector_test.go have been removed, reflecting the deprecation of this functionality.

Changelog

• pkg/detector/detector.go
  • Removed the assignment of bindingCopy.Spec.WorkloadAffinityGroups in ApplyClusterPolicy.
  • Eliminated the conditional block that populated binding.Spec.WorkloadAffinityGroups based on the WorkloadAffinity feature gate in BuildClusterResourceBinding.
• pkg/detector/detector_test.go
  • Removed the enableWorkloadAffinity field from the TestApplyClusterPolicy test case struct.
  • Deleted a specific test case that validated WorkloadAffinity behavior.
  • Removed the feature gate setting for WorkloadAffinity during test execution.
  • Removed calls to verifyWorkloadAffinity in test assertions.

Activity

• No human activity has been recorded on this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[Copilot]

Pull request overview

This PR removes the WorkloadAffinity feature support specifically for ClusterResourceBinding resources, while keeping it for ResourceBinding resources. The changes align with the PR title "stop populating ClusterResourceBinding with WorkloadAffinity fields."

Changes:

• Removed WorkloadAffinityGroups population from BuildClusterResourceBinding function
• Removed WorkloadAffinityGroups preservation logic in ApplyClusterPolicy for cluster-scoped resources
• Removed corresponding test cases and verification logic for ClusterResourceBinding

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
pkg/detector/detector.go	Removed WorkloadAffinityGroups population and preservation logic from ClusterResourceBinding creation and update paths
pkg/detector/detector_test.go	Removed test case for WorkloadAffinity with ClusterResourceBinding, removed verifyWorkloadAffinity calls for ClusterResourceBinding tests, and removed feature gate setup

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gemini-code-assist]

Code Review

This pull request performs a cleanup by stopping the population of WorkloadAffinityGroups in ClusterResourceBinding. This is a logical change as WorkloadAffinity is namespace-scoped and ClusterResourceBinding is for cluster-scoped resources. The implementation changes in pkg/detector/detector.go are correct. However, the test changes in pkg/detector/detector_test.go seem to have removed some valid test coverage for ResourceBindings created from a ClusterPropagationPolicy. I've added a comment with a suggestion to restore the relevant test code to prevent future regressions.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.04%. Comparing base (c22fe72) to head (4c9035d).

Files with missing lines	Patch %	Lines
pkg/scheduler/framework/testing/mock_interface.go	0.00%	4 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7195      +/-   ##
==========================================
- Coverage   42.05%   42.04%   -0.01%     
==========================================
  Files         874      874              
  Lines       53473    53470       -3     
==========================================
- Hits        22488    22484       -4     
- Misses      29297    29298       +1     
  Partials     1688     1688              

Flag	Coverage Δ
unittests	42.04% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[RainbowMango]

Shall we add some notes somewhere to explain why ClusterResourceBinding does not need this?

[zhzhuang-zju]

How about：

$ git diff pkg/apis/work/v1alpha2/binding_types.go
@@ -179,6 +179,7 @@ type ResourceBindingSpec struct {
        // used to keep workloads with the same affinity group co-located or those with the same
        // anti-affinity group separated across clusters. Populated by controllers, the scheduler
        // consumes it for decisions.
+       // Since workload is a namespace-scoped resource, workload affinity only applies to ResourceBinding. Therefore, the WorkloadAffinityGroups field in ClusterResourceBinding will no
t be set and will not be consumed by the scheduler.
        // +optional
        WorkloadAffinityGroups *WorkloadAffinityGroups `json:"workloadAffinityGroups,omitempty"`
 }

[RainbowMango]

I'm fine with the approach that documents it on the API, just a small format suggestion:

diff --git a/pkg/apis/work/v1alpha2/binding_types.go b/pkg/apis/work/v1alpha2/binding_types.go
index cd46d0216..126630e8b 100644
--- a/pkg/apis/work/v1alpha2/binding_types.go
+++ b/pkg/apis/work/v1alpha2/binding_types.go
@@ -179,6 +179,9 @@ type ResourceBindingSpec struct {
        // used to keep workloads with the same affinity group co-located or those with the same
        // anti-affinity group separated across clusters. Populated by controllers, the scheduler
        // consumes it for decisions.
+       // Note: Since workloads are namespace-scoped resources, workload affinity only applies to
+       // ResourceBinding. Therefore, the WorkloadAffinityGroups field in ClusterResourceBinding
+       // will not be set and will not be consumed by the scheduler.
        // +optional
        WorkloadAffinityGroups *WorkloadAffinityGroups `json:"workloadAffinityGroups,omitempty"`
 }

[RainbowMango]

/approve

@mszacillo Do you want to take another look?

[RainbowMango]

/lgtm
@mszacillo is on a business trip this week.

[karmada-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [RainbowMango]
• pkg/apis/OWNERS [RainbowMango]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment