Skip to content

feat: Added support for unhealthyPodEvictionPolicy to PDBs#7537

Closed
peterabarr wants to merge 1 commit intocert-manager:masterfrom
peterabarr:unhealthyPodEvictionPolicy
Closed

feat: Added support for unhealthyPodEvictionPolicy to PDBs#7537
peterabarr wants to merge 1 commit intocert-manager:masterfrom
peterabarr:unhealthyPodEvictionPolicy

Conversation

@peterabarr
Copy link
Copy Markdown

@peterabarr peterabarr commented Jan 30, 2025 •
edited
Loading

Pull Request Motivation

I opened #7275 as I would like to be able to set this policy to the Cert Manager PDBs used in my environments.

Kind

/kind

Release Note

Added `unhealthyPodEvictionPolicy` to all PDBs which can be set to `IfHealthyBudget` and `AlwaysAllow`.

@peterabarr
feat: Added support for unhealthyPodEvictionPolicy to PDBs
2b03ddc 
template

Signed-off-by: peterabarr <90917602+peterabarr@users.noreply.github.com>
@cert-manager-prow
Copy link
Copy Markdown
Contributor

cert-manager-prow bot commented Jan 30, 2025

@peterabarr: The label(s) kind/release, kind/note cannot be applied, because the repository doesn't have them.

Details

In response to this:

Pull Request Motivation

Kind

/kind

Release Note

NONE

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@cert-manager-prow cert-manager-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. area/deploy Indicates a PR modifies deployment configuration labels Jan 30, 2025
@cert-manager-prow
Copy link
Copy Markdown
Contributor

cert-manager-prow bot commented Jan 30, 2025

[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 sgtcodfish for approval. For more information see the Kubernetes Code Review Process.

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

Details 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

@cert-manager-prow cert-manager-prow bot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 30, 2025
@cert-manager-prow
Copy link
Copy Markdown
Contributor

cert-manager-prow bot commented Jan 30, 2025

Hi @peterabarr. Thanks for your PR.

I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@cert-manager-prow cert-manager-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jan 30, 2025
@peterabarr
Copy link
Copy Markdown
Author

peterabarr commented Jan 30, 2025

/kind feature

@cert-manager-prow cert-manager-prow bot added kind/feature Categorizes issue or PR as related to a new feature. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Jan 30, 2025
@peterabarr peterabarr marked this pull request as ready for review January 30, 2025 15:34
@cert-manager-prow cert-manager-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 30, 2025
@cert-manager-bot
Copy link
Copy Markdown
Contributor

cert-manager-bot commented Apr 30, 2025

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
/lifecycle stale

@cert-manager-prow cert-manager-prow bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 30, 2025
erikgb
erikgb reviewed Apr 30, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@erikgb erikgb left a comment

Choose a reason for hiding this comment

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

/remove-lifecycle stale
/ok-to-test

This looks useful. Sorry for the delayed review.

deploy/charts/cert-manager/templates/cainjector-poddisruptionbudget.yaml
{{- if hasKey .Values.cainjector.podDisruptionBudget "maxUnavailable" }}
maxUnavailable: {{ .Values.cainjector.podDisruptionBudget.maxUnavailable }}
{{- end }}
{{- if (semverCompare ">= 1.27-0" .Capabilities.KubeVersion.Version) }}
Copy link
Copy Markdown
Member

@erikgb erikgb Apr 30, 2025

Choose a reason for hiding this comment

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

I think this conditional can be removed. Even on Kubernetes 1.27 this requires enabling a feature gate. The feature gate is only enabled by default from K8s 1.31, ref. https://kubernetes.io/docs/tasks/run-application/configure-pdb/#unhealthy-pod-eviction-policy

Same in other places.

@cert-manager-prow cert-manager-prow bot added ok-to-test and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 30, 2025
@cert-manager-prow
Copy link
Copy Markdown
Contributor

cert-manager-prow bot commented Apr 30, 2025

@peterabarr: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cert-manager-master-make-verify 2b03ddc link true /test pull-cert-manager-master-make-verify
pull-cert-manager-master-e2e-v1-32 2b03ddc link true /test pull-cert-manager-master-e2e-v1-32

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@cert-manager-bot
Copy link
Copy Markdown
Contributor

cert-manager-bot commented May 1, 2025

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
/lifecycle stale

@cert-manager-prow cert-manager-prow bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 1, 2025
@erikgb
Copy link
Copy Markdown
Member

erikgb commented May 1, 2025

/remove-lifecycle stale

@cert-manager-prow cert-manager-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 1, 2025
@erikgb erikgb mentioned this pull request May 2, 2025
Merged
@erikgb
Copy link
Copy Markdown
Member

erikgb commented May 3, 2025

@peterabarr, are you able to finalize this PR? We just got a similar PR from @jcpunk suggesting a similar change: #7728. But I would love to merge this older PR if you can fix the review feedback and make CI green. Please let us know!

@cert-manager-bot
Copy link
Copy Markdown
Contributor

cert-manager-bot commented Nov 1, 2025

Issues go stale after 6 months of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues remain open for an additional 3 months of inactivity and then close.
If this issue is safe to close now please do so with /close.
/lifecycle stale

@cert-manager-prow cert-manager-prow bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 1, 2025
@erikgb
Copy link
Copy Markdown
Member

erikgb commented Nov 1, 2025

As we haven't got any feedback from the author of this PR, I am going to close it and try to get #7728 merged instead.

/close

@cert-manager-prow cert-manager-prow bot closed this Nov 1, 2025
@cert-manager-prow
Copy link
Copy Markdown
Contributor

cert-manager-prow bot commented Nov 1, 2025

@erikgb: Closed this PR.

Details

In response to this:

As we haven't got any feedback from the author of this PR, I am going to close it and try to get #7728 merged instead.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

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

Reviewers

@erikgb erikgb erikgb left review comments

Assignees

No one assigned

Labels

area/deploy Indicates a PR modifies deployment configuration dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. ok-to-test release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@peterabarr @cert-manager-bot @erikgb