Skip to content

fix(argo-events): render maxUnavailable 0 correctly in PodDisruptionBudget - #4115

Open
imansabet wants to merge 4 commits into
argoproj:mainfrom
imansabet:fix/argo-events-pdb-max-unavailable
Open

imansabet wants to merge 4 commits into
argoproj:mainfrom
imansabet:fix/argo-events-pdb-max-unavailable

Conversation

@imansabet

@imansabet imansabet commented Oct 1, 2026 •

Copy link
Copy Markdown

Fixes #4092

The controller and admission webhook PodDisruptionBudget templates used a plain truthiness check on minAvailable / maxUnavailable. Since 0 is falsy in Go templates, maxUnavailable: 0 fell through to the default branch and rendered minAvailable: 0, which does not block voluntary evictions.

This switches both templates to not (kindIs "invalid" ...), the same pattern already used in the argo-workflows PDB templates, so an explicit 0 is honored while unset values keep the current default.

Tested with helm template for the controller and the webhook:

  • maxUnavailable=0 renders maxUnavailable: 0 (was minAvailable: 0)
  • minAvailable=1 renders minAvailable: 1
  • nothing set renders minAvailable: 0 (unchanged)
  • maxUnavailable=1 renders maxUnavailable: 1

I also compared the full rendered output before and after the change. The only differences are the chart version label, the config checksum (derived from a ConfigMap that carries the chart version label), and the two PDBs.

Checklist:

  • I have bumped the chart version according to versioning
  • I have updated the documentation according to documentation
  • I have updated the chart changelog with all the changes that come with this pull request according to changelog.
  • I wrote this PR with the help of a LLM but I fully vetted the work before raising this PR for review
  • Any new values are backwards compatible and/or have sensible default.
  • I have signed off all my commits as required by DCO.
  • I have created a separate pull request for each chart according to pull requests
  • My build is green (troubleshooting builds).

…udget

Signed-off-by: iman <dev.imansabet@gmail.com>
@imansabet
imansabet marked this pull request as ready for review October 1, 2026 09:38

@tico24 tico24 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's already #4093 open for #4092, and I've asked for this same kindIs "invalid" change on it. I'd like to give that author the chance to push it first, so I'm holding off on this one for now.

@imansabet

Copy link
Copy Markdown
Author

There's already #4093 open for #4092, and I've asked for this same kindIs "invalid" change on it. I'd like to give that author the chance to push it first, so I'm holding off on this one for now.

thanks for letting me know. I missed #4093 when I started on this.

tico24
tico24 previously approved these changes Oct 28, 2026
@tico24
tico24 enabled auto-merge (squash) October 28, 2026 08:59
tico24 and others added 2 commits October 29, 2026 07:10
argo-helm now bumps chart versions after merge (argoproj#4176). This puts the version and artifacthub.io/changes in Chart.yaml back to main's, and moves this PR's changelog entry into charts/argo-events/changes/render-maxunavailable-0-correctly-in-poddisruptionbudget.yaml with the same bump level.

Signed-off-by: Tim Collins <tim@thecollins.team>

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[argo-events] maxUnavailable: 0 renders an unprotected PDB

2 participants