Skip to content

Conversation

whosefriendA
Copy link

@whosefriendA whosefriendA commented Sep 24, 2025

What type of PR is this?
/kind api-change

What this PR does / why we need it:

Which issue(s) this PR fixes:
part of #6746

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

API Change: Added four new fields to the GracefulEvictionPolicy for advanced rate control:
- `resourceEvictionRate`: default eviction rate when the system is healthy.
- `secondaryResourceEvictionRate`: fallback rate when too many clusters are unhealthy.
- `unhealthyClusterThreshold`: threshold ratio of unhealthy clusters triggering downgrade.
- `largeClusterNumThreshold`: cluster number threshold to adjust eviction behavior in large-scale instances.

@karmada-bot karmada-bot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Sep 24, 2025
@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign lonelycz for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

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

Copy link

Summary of Changes

Hello @whosefriendA, I'm Gemini Code Assist1! 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 enhances the failover mechanism within the controller manager by introducing dynamic rate limiting for resource eviction. It provides new configuration parameters that allow administrators to define primary and secondary eviction rates, as well as thresholds for cluster health and scale. These parameters enable the system to intelligently adjust eviction speeds based on the current state of the Karmada instance, aiming to prevent aggressive evictions during periods of instability or in large-scale deployments, thereby improving overall system resilience.

Highlights

  • Dynamic Resource Eviction Rate Limiting: Introduces new configuration options to dynamically adjust the rate at which resources are evicted during failover scenarios, enhancing system stability.
  • Configurable Eviction Rates: Adds ResourceEvictionRate and SecondaryResourceEvictionRate to define default and reduced eviction speeds, respectively.
  • Health-Based Rate Adjustment: Implements UnhealthyClusterThreshold to automatically switch to the secondary eviction rate when a significant portion of clusters are unhealthy.
  • Scale-Aware Eviction Behavior: Adds LargeClusterNumThreshold to modify eviction strategies for large-scale Karmada instances, potentially halting eviction for smaller, unhealthy instances.
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.

@karmada-bot karmada-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 24, 2025
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces new configuration options to FailoverOptions for dynamic rate limiting of evictions. Note that the PR title mentions EvictionQueueOptions, but the changes are made to FailoverOptions. The changes add new fields to the struct and corresponding command-line flags. My review focuses on the correctness and clarity of these new API additions.

I've identified a high-severity issue regarding the lack of validation for the new options, which could lead to unexpected behavior if invalid values are provided. I've also suggested a medium-severity improvement to the help text of one of the new flags for better user clarity. Overall, the changes are a good step towards implementing dynamic eviction rate limiting, but the validation aspect needs to be addressed before this can be considered complete.

@codecov-commenter
Copy link

codecov-commenter commented Sep 24, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 9.67742% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.82%. Comparing base (64428fa) to head (88023ac).

Files with missing lines Patch % Lines
cmd/controller-manager/app/options/failover.go 9.67% 24 Missing and 4 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6777      +/-   ##
==========================================
- Coverage   45.84%   45.82%   -0.03%     
==========================================
  Files         690      690              
  Lines       57300    57330      +30     
==========================================
+ Hits        26271    26273       +2     
- Misses      29399    29423      +24     
- Partials     1630     1634       +4     
Flag Coverage Δ
unittests 45.82% <9.67%> (-0.03%) ⬇️

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.

@XiShanYongYe-Chang
Copy link
Member

  1. The title appears incomplete; it can be adjusted.
  2. It can link the umbrella issue in Which issue(s) this PR fixes. like this: part of #xxxx
  3. Fill in the release note as hint.

Other PRs can also be adjusted in this way.

@whosefriendA whosefriendA changed the title api(failover): Add EvictionQueueOptions to configure dynamic rate lim… api(failover): Add EvictionQueueOptions to configure dynamic rate limit Sep 26, 2025
@karmada-bot karmada-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 26, 2025
@XiShanYongYe-Chang
Copy link
Member

  1. The title appears incomplete; it can be adjusted.
  2. It can link the umbrella issue in Which issue(s) this PR fixes. like this: part of #xxxx
  3. Fill in the release note as hint.

Other PRs can also be adjusted in this way.

Hi @whosefriendA, can you help fix it and squash the commit into one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants