feat: add PodDisruptionBudget support for Karmada control plane compo… #6722
Conversation
…nents - Add PodDisruptionBudgetConfig to CommonSettings API - Implement PDB creation/update/deletion logic for all components - Add validation for PDB configuration (mutual exclusivity, replicas check) - Support both minAvailable and maxUnavailable PDB strategies Signed-off-by: baiyutang <[email protected]>
karmada-bot
added
kind/feature
karmada-bot
added
the
size/XL
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @baiyutang, 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 significantly enhances the high availability of Karmada control plane components by integrating PodDisruptionBudget (PDB) support directly into the Karmada Operator. Users can now declaratively configure PDB strategies through the Karmada CRD, ensuring that critical components remain available during planned disruptions like node maintenance or cluster upgrades. This aligns the operator's capabilities with existing Helm chart functionality, providing a consistent experience for managing high availability.
Highlights
- API Extension: Introduced a new
PodDisruptionBudgetConfigfield within theCommonSettingsAPI, allowing users to specify PDB strategies for Karmada control plane components. - PDB Management Logic: Implemented comprehensive logic for creating, updating, and deleting PodDisruptionBudgets for all Karmada control plane components (e.g., karmada-controller-manager, karmada-scheduler, karmada-apiserver, etcd, webhook, search, metrics-adapter). This logic is centralized in a new
pdbpackage for reusability. - Configuration Validation: Added validation rules for PDB configurations, ensuring mutual exclusivity between
minAvailableandmaxUnavailablefields, and validating thatminAvailabledoes not exceed the component's replica count. - Enhanced Test Coverage: Updated existing unit tests and added new ones to cover the PDB creation, update, and validation flows, ensuring the new functionality behaves as expected.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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
-
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. ↩
There was a problem hiding this comment.
Pull Request Overview
This PR adds PodDisruptionBudget (PDB) support to the Karmada Operator, enabling users to configure high availability guarantees for all Karmada control plane components during planned disruptions. This mirrors the PDB functionality available in Helm charts, providing consistent availability guarantees across installation methods.
- Adds PodDisruptionBudgetConfig to the CommonSettings API with validation
- Implements centralized PDB creation/update/deletion logic for all components
- Updates all component installation flows to include PDB management
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| operator/pkg/apis/operator/v1alpha1/type.go | Adds PodDisruptionBudgetConfig API type with minAvailable/maxUnavailable fields |
| operator/pkg/controlplane/pdb/pdb.go | Implements centralized PDB creation and management logic |
| operator/pkg/util/apiclient/idempotency.go | Adds CreateOrUpdatePodDisruptionBudget helper function |
| operator/pkg/controller/karmada/validating.go | Adds validation for PDB configuration mutual exclusivity and replica checks |
| operator/pkg/constants/constants.go | Adds component label constants for PDB selectors |
| Multiple component files | Updates all components to integrate PDB creation |
| Multiple test files | Updates test expectations to account for additional PDB creation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // getComponentLabels returns the labels for the component | ||
| // These labels must match the labels used in deployment templates | ||
| func getComponentLabels(karmadaName, component string) map[string]string { | ||
| return map[string]string{ | ||
| constants.AppNameLabel: getComponentAppName(component), | ||
| constants.AppInstanceLabel: karmadaName, | ||
| } | ||
| } |
There was a problem hiding this comment.
The function references undefined constants. constants.AppNameLabel and constants.AppInstanceLabel are not defined in the constants file, but they appear to be standard Kubernetes labels like app.kubernetes.io/name and app.kubernetes.io/instance.
Copilot uses AI. Check for mistakes.
| case constants.Etcd: | ||
| return constants.Etcd |
There was a problem hiding this comment.
The function references undefined constants. constants.Etcd is not defined in the constants file. The constant should likely be defined or the function should return a hardcoded string like 'etcd'.
| case constants.Etcd: | |
| return constants.Etcd | |
| case "etcd": | |
| return "etcd" |
Copilot uses AI. Check for mistakes.
| "github.com/karmada-io/karmada/pkg/util/lifted" | ||
| "k8s.io/apimachinery/pkg/util/intstr" |
There was a problem hiding this comment.
[nitpick] The import k8s.io/apimachinery/pkg/util/intstr should be moved up to maintain alphabetical ordering of imports within the same group.
Copilot uses AI. Check for mistakes.
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/util/intstr" | ||
| "k8s.io/apimachinery/pkg/util/validation/field" | ||
| "k8s.io/utils/pointer" |
There was a problem hiding this comment.
The import should be k8s.io/utils/ptr instead of k8s.io/utils/pointer which is deprecated. Also, the function call should be updated from pointer.Int32(3) to ptr.To[int32](3).
| "k8s.io/utils/pointer" | |
| "k8s.io/utils/ptr" |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Code Review
This pull request introduces PodDisruptionBudget (PDB) support for Karmada control plane components, which is a great feature for improving high availability. The implementation is mostly solid, with new APIs, validation, and control plane logic. I've identified a few areas for improvement: a potential bug in the PDB update logic, some test regressions where secret verification was removed, and opportunities to reduce code duplication in validation and component handling logic for better maintainability.
karmada-bot
added
size/XXL
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #6722 +/- ##
==========================================
- Coverage 45.35% 45.30% -0.05%
==========================================
Files 688 689 +1
Lines 57020 57198 +178
==========================================
+ Hits 25862 25916 +54
- Misses 29547 29659 +112
- Partials 1611 1623 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/assign |
|
@baiyutang , thanks for this great work. I'm going to review soon. |
baiyutang
force-pushed
the
feat/support-PDB
branch
from
c645cf5 to
f3cc2ee
Compare
Signed-off-by: baiyutang <[email protected]>
baiyutang
force-pushed
the
feat/support-PDB
branch
from
f3cc2ee to
0ffdfe7
Compare
feat: add PodDisruptionBudget support for Karmada control plane components
What type of PR is this?
/kind feature
/kind api-change
What this PR does / why we need it:
This PR adds PodDisruptionBudget (PDB) support to the Karmada Operator, enabling users to configure high availability guarantees for all Karmada control plane components (karmada-controller-manager, karmada-scheduler, karmada-apiserver, etc.) during planned disruptions.
The feature mirrors the PDB functionality already available in Helm charts, providing consistent high availability guarantees across different installation methods. Users can now declaratively configure PDB strategies through the Karmada CRD, ensuring that control plane components remain available during node maintenance, cluster upgrades, or other planned operations.
Which issue(s) this PR fixes:
Fixes #(issue number)
#6282
Special notes for your reviewer:
**Does this PR introduce a user-fa