Add PBD and priority class support for Karmada operator #7015

jabellard · 2025-12-13T17:34:06Z

What type of PR is this?

/kind feature

What this PR does / why we need it: Please see this issue: #6973

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

karmada-operator Helm chart: Added support for configuring priority class and pod disruption budget config for Karmada operator deployment

gemini-code-assist · 2025-12-13T17:34:19Z

Summary of Changes

Hello @jabellard, 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 significantly enhances the robustness and operational stability of the Karmada operator by introducing configurable support for Kubernetes Priority Classes and Pod Disruption Budgets (PDBs) via its Helm chart. These additions allow administrators to ensure the operator's critical components are prioritized during scheduling and maintain desired availability levels even during cluster maintenance or scaling events.

Highlights

Priority Class Support: The Karmada operator deployment can now be configured with a priorityClassName, allowing its pods to be scheduled with higher priority on Kubernetes clusters.
Pod Disruption Budget (PDB) Support: A new Pod Disruption Budget resource can be enabled for the Karmada operator, enhancing its availability during voluntary disruptions by ensuring a minimum number of pods remain available.

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.

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. ↩

jabellard · 2025-12-13T17:35:07Z

/cc @RainbowMango @seanlaii

gemini-code-assist

Code Review

This pull request adds support for priorityClassName and PodDisruptionBudget (PDB) to the Karmada operator Helm chart, which is a great feature for enhancing the reliability of operator deployments. The implementation is mostly correct, but I've found a potential issue in the PDB template logic. The current implementation doesn't correctly handle a value of 0 for minAvailable or maxUnavailable, which could lead to generating an invalid PDB resource or one with unintended behavior. I've provided a suggestion to make the template more robust by using toString for checks and adding a fail guard to ensure a valid PDB configuration is always present when enabled.

charts/karmada-operator/templates/karmada-operator-poddisruptionbudget.yaml

codecov-commenter · 2025-12-13T18:03:20Z

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

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.59%. Comparing base (9ba7dc0) to head (d80ef83).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7015      +/-   ##
==========================================
- Coverage   46.60%   46.59%   -0.01%     
==========================================
  Files         699      699              
  Lines       48182    48182              
==========================================
- Hits        22453    22451       -2     
  Misses      24032    24032              
- Partials     1697     1699       +2

Flag	Coverage Δ
unittests	`46.59% <ø> (-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.

seanlaii

Thanks for the contributions! Left some comments.

seanlaii · 2025-12-13T20:03:34Z

charts/karmada-operator/templates/karmada-operator-deployment.yaml

+      {{- if .Values.operator.priorityClassName }}
+      priorityClassName: {{ .Values.operator.priorityClassName }}


nit:

Suggested change

{{- if .Values.operator.priorityClassName }}

priorityClassName: {{ .Values.operator.priorityClassName }}

{{- with .Values.operator.priorityClassName }}

priorityClassName: {{ . | quote }}

You feel strongly about this? What's the benefit of changing it?

No, it is just more idiomatic. It's fine if you don't want to change it.
Normally, if is for condition check, and with is to check if the value is presented.
But I don't have strong opinion. The current behavior is correct.

seanlaii · 2025-12-13T20:14:15Z

charts/karmada-operator/templates/karmada-operator-poddisruptionbudget.yaml

+    {{- if .Values.commonLabels }}
+    {{- include "common.tplvalues.render" ( dict "value" .Values.commonLabels "context" $ ) | nindent 4 }}
+    {{- end }}


nit:

Suggested change

{{- if .Values.commonLabels }}

{{- include "common.tplvalues.render" ( dict "value" .Values.commonLabels "context" $ ) | nindent 4 }}

{{- end }}

{{- with .Values.commonLabels }}

{{- include "common.tplvalues.render" (dict "value" . "context" $) | nindent 4 }}

{{- end }}

seanlaii · 2025-12-13T20:15:12Z

charts/karmada-operator/templates/karmada-operator-poddisruptionbudget.yaml

+  {{- if .Values.commonAnnotations }}
+  annotations: {{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }}
+  {{- end }}


nit:

Suggested change

{{- if .Values.commonAnnotations }}

annotations: {{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }}

{{- end }}

{{- with .Values.commonAnnotations }}

annotations: {{- include "common.tplvalues.render" ( dict "value" . "context" $ ) | nindent 4 }}

{{- end }}

seanlaii · 2025-12-13T20:26:05Z

charts/karmada-operator/templates/karmada-operator-poddisruptionbudget.yaml

+  namespace: {{ .Release.Namespace }}
+  labels: {{- include "common.labels.standard" . | nindent 4 }}
+    app: {{ include "karmada.operator.fullname" . }}
+    {{- if .Values.commonLabels }}


Maybe we can provide commonLabels and commonAnnotations in values.yaml.

I'm just using the same labels used in the deployment to link it to the pbd.

Although users can use it, we currently don't provide it in https://github.com/karmada-io/karmada/blob/master/charts/karmada-operator/values.yaml.
We can add something like https://github.com/karmada-io/karmada/blob/master/charts/karmada/values.yaml#L12-L18.

This expands the spoce of the PR, which is why I'd prefer to not add add it as part of this work. Would like to keep things as minimal as possible.

Sure, since we also don't have commonAnnotations in Karmada helm chart, I don't feel strongly on this. Feel free to ignore.

seanlaii · 2025-12-13T20:33:09Z

charts/karmada-operator/templates/karmada-operator-poddisruptionbudget.yaml

+metadata:
+  name: {{ include "karmada.operator.fullname" . }}
+  namespace: {{ .Release.Namespace }}
+  labels: {{- include "common.labels.standard" . | nindent 4 }}


Tag standard labels here: https://github.com/bitnami/charts/blob/main/bitnami/common/templates/_labels.tpl for info.

I'm just using the same labels used in the deployment to link it to the pbd.

seanlaii · 2025-12-13T20:46:52Z

charts/karmada-operator/templates/karmada-operator-poddisruptionbudget.yaml

+  selector:
+    matchLabels: {{- include "common.labels.matchLabels" . | nindent 6 }}
+      app: {{ include "karmada.operator.fullname" . }}
+  {{- if .Values.operator.pdb.minAvailable | toString }}


I think we need to use hasKey since null value will be considered as true.
Maybe we can refer to cert-manager.

@seanlaii , thanks for the ref o the cert-manager approach and for thinking about the edge cases, but don't think we need to worry about this one. This would be a falsy value, so that block would not get evaluated.

Considering this case

minAvailable: null maxAvailable: 1

the result will be

selector: matchLabels: minAvailable:

Really? Let me try it.

@seanlaii , thanks for catching this. Will push update.

Just tried. Handling this weird edge case comes at the cost of the code becoming a lot of unreadable and requiring use more advanced stuff. I'm unsure if it's worth the tradeoff. hasKey also will not catch this. You have other ideas?

Yeah, will probably need this

{{- $pdb := .Values.operator.pdb }} {{- if and (hasKey $pdb "minAvailable") (ne $pdb.minAvailable null) (ne $pdb.minAvailable "") }} minAvailable: {{ $pdb.minAvailable }} {{- else if and (hasKey $pdb "maxUnavailable") (ne $pdb.maxUnavailable null) (ne $pdb.maxUnavailable "") }} maxUnavailable: {{ $pdb.maxUnavailable }} {{- end }}

I am OK to keep it to maintain the readability.
Could you add some comment in the values.yaml to inform the users? Thanks!

Will add some instructions on how to have maxUnavailable be the field that takes precedence.

seanlaii · 2025-12-15T00:26:01Z

/assign

seanlaii

LGTM

karmada-bot · 2025-12-15T01:00:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: seanlaii

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:

~~charts/OWNERS~~ [seanlaii]

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

zhzhuang-zju · 2025-12-15T01:39:10Z

Thanks
/assign

zhzhuang-zju · 2025-12-15T08:02:21Z

@jabellard Thanks, I have no further comments. It would be perfect if you could squash your commits into one.

Signed-off-by: Joe Nathan Abellard <[email protected]>

RainbowMango · 2025-12-17T03:56:42Z

@jabellard Thanks, I have no further comments. It would be perfect if you could squash your commits into one.

I rebased and squashed this; nothing has changed since the last review.

/lgtm

karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 13, 2025

karmada-bot requested review from a7i and seanlaii December 13, 2025 17:34

karmada-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 13, 2025

karmada-bot requested a review from RainbowMango December 13, 2025 17:35

gemini-code-assist bot reviewed Dec 13, 2025

View reviewed changes

charts/karmada-operator/templates/karmada-operator-poddisruptionbudget.yaml Outdated Show resolved Hide resolved

seanlaii reviewed Dec 13, 2025

View reviewed changes

karmada-bot assigned seanlaii Dec 15, 2025

seanlaii approved these changes Dec 15, 2025

View reviewed changes

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 15, 2025

karmada-bot assigned zhzhuang-zju Dec 15, 2025

RainbowMango added this to the v1.17 milestone Dec 15, 2025

Add PBD and priority class support for Karmada operator

d80ef83

Signed-off-by: Joe Nathan Abellard <[email protected]>

RainbowMango force-pushed the operator-pbd branch from 66b1b3e to d80ef83 Compare December 17, 2025 03:53

karmada-bot assigned RainbowMango Dec 17, 2025

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 17, 2025

karmada-bot merged commit ed620ac into karmada-io:master Dec 17, 2025
24 checks passed

		{{- if .Values.operator.priorityClassName }}
		priorityClassName: {{ .Values.operator.priorityClassName }}

Add PBD and priority class support for Karmada operator #7015

Add PBD and priority class support for Karmada operator #7015

Conversation

jabellard commented Dec 13, 2025

Uh oh!

gemini-code-assist bot commented Dec 13, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

jabellard commented Dec 13, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

codecov-commenter commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

seanlaii left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanlaii Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanlaii Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanlaii Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanlaii commented Dec 15, 2025

Uh oh!

seanlaii left a comment

Choose a reason for hiding this comment

Uh oh!

karmada-bot commented Dec 15, 2025

Uh oh!

zhzhuang-zju commented Dec 15, 2025

codecov-commenter commented Dec 13, 2025 •

edited

Loading

seanlaii Dec 15, 2025 •

edited

Loading

seanlaii Dec 15, 2025 •

edited

Loading

seanlaii Dec 13, 2025 •

edited

Loading