Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[bitnami/external-dns] PDB fixes #26292

Merged
merged 15 commits into from
May 22, 2024
Merged

[bitnami/external-dns] PDB fixes #26292

merged 15 commits into from
May 22, 2024
+12 −8

Conversation

fmulero
Copy link
Collaborator

@fmulero fmulero commented May 21, 2024 •
edited
Loading

Description of the change

Remove default value for pdb.minAvailable and set maxUnavailableto 1 in PodDisruptionBudgets if none is set.

Benefits

Maintenance to infrastructure is allowed with chart defaults.

Possible drawbacks

Some users could expect old value in minAvailable. In that case the value should be set explicitly.

Applicable issues

Additional information

Follow up #25933

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

Sorry, something went wrong.

@fmulero
[bitnami/external-dns] PDB fixes

Verified

This commit was signed with the committer’s verified signature.
fmulero Fran Mulero
GPG key ID: 017C5A37D75CC19B
Verified
Learn about vigilant mode
Loading
Loading status checks…
9971978 
Signed-off-by: Fran Mulero <fmulero@vmware.com>
@bitnami-bot bitnami-bot added the verify Execute verification workflow for these changes label May 21, 2024
@github-actions github-actions bot requested a review from jotamartos May 21, 2024 14:11
Bitnami Containers and others added 10 commits May 21, 2024 14:13
Update CHANGELOG.md
9b9e4f9 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
Update README.md with readme-generator-for-helm
Loading
Loading status checks…
0a2d75a 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@fmulero
Set pdb.maxUnavailable instead of pdb.minAvailable by default

Verified

This commit was signed with the committer’s verified signature.
fmulero Fran Mulero
GPG key ID: 017C5A37D75CC19B
Verified
Learn about vigilant mode
cb0de48 
Signed-off-by: Fran Mulero <fmulero@vmware.com>
@fmulero
Merge branch 'bugfix/external-dns-pdb-2' of github.com:fmulero/charts…

Verified

This commit was signed with the committer’s verified signature.
fmulero Fran Mulero
GPG key ID: 017C5A37D75CC19B
Verified
Learn about vigilant mode
Loading
Loading status checks…
5c7ceca 
… into bugfix/external-dns-pdb-2
Update CHANGELOG.md
72a864a 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
Update README.md with readme-generator-for-helm
Loading
Loading status checks…
fa9e191 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@fmulero
Merge remote-tracking branch 'upstream/main' into bugfix/external-dns…

Verified

This commit was signed with the committer’s verified signature.
fmulero Fran Mulero
GPG key ID: 017C5A37D75CC19B
Verified
Learn about vigilant mode
1bce5c3 
…-pdb-2

Signed-off-by: Fran Mulero <fmulero@vmware.com>
@fmulero
Merge branch 'bugfix/external-dns-pdb-2' of github.com:fmulero/charts…

Verified

This commit was signed with the committer’s verified signature.
fmulero Fran Mulero
GPG key ID: 017C5A37D75CC19B
Verified
Learn about vigilant mode
Loading
Loading status checks…
44510bb 
… into bugfix/external-dns-pdb-2
@fmulero
Fix CHANGELOG.md

Verified

This commit was signed with the committer’s verified signature.
fmulero Fran Mulero
GPG key ID: 017C5A37D75CC19B
Verified
Learn about vigilant mode
Loading
Loading status checks…
8b6b6ea 
Signed-off-by: Fran Mulero <fmulero@vmware.com>
Update CHANGELOG.md
Loading
Loading status checks…
3d38071 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@fmulero fmulero marked this pull request as ready for review May 21, 2024 15:56
@fmulero fmulero requested review from javsalgar and juan131 May 21, 2024 16:06
juan131
juan131 requested changes May 22, 2024
View reviewed changes
Copy link
Contributor

@juan131 juan131 left a comment

Choose a reason for hiding this comment

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

We should keep 1 replica by default and set pdb.maxUnavailable instead.

pdb:
  minAvailable: ""
  maxUnavailable: 1

@fmulero
Copy link
Collaborator Author

fmulero commented May 22, 2024

We should keep 1 replica by default and set pdb.maxUnavailable instead.

pdb:
  minAvailable: ""
  maxUnavailable: 1

With those defaults, if any user tries to install the chart setting only `pdb.minAvailable=1' this error will be faced:

$ helm install test bitnami/external-dns --set pdb.minAvailable=1
Error: INSTALLATION FAILED: PodDisruptionBudget.policy "test-external-dns" is invalid: spec: Invalid value: policy.PodDisruptionBudgetSpec{MinAvailable:(*intstr.IntOrString)(0xc00d46e580), Selector:(*v1.LabelSelector)(0xc00d46e5a0), MaxUnavailable:(*intstr.IntOrString)(0xc00d46e560), UnhealthyPodEvictionPolicy:(*policy.UnhealthyPodEvictionPolicyType)(nil)}: minAvailable and maxUnavailable cannot be both set

Users should unset pdb.maxUnavailable explicitly.

With the current approach that is not needed. Users settings take precedence but if pdb.maxUnavailable and pdb.minAvailable are not set , maxUnavailable will be set to 1 by default in the PodDisruptionBudget.

fmulero and others added 2 commits May 22, 2024 12:28
@fmulero
Add documentation for pdb.maxUnavailable value

Verified

This commit was signed with the committer’s verified signature.
fmulero Fran Mulero
GPG key ID: 017C5A37D75CC19B
Verified
Learn about vigilant mode
Loading
Loading status checks…
4f2a8da 
Signed-off-by: Fran Mulero <fmulero@vmware.com>
Update CHANGELOG.md
Loading
Loading status checks…
e036ebe 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@fmulero @juan131
Update bitnami/external-dns/values.yaml

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
Loading
Loading status checks…
a844984 
Co-authored-by: Juan Ariza Toledano <jariza@vmware.com>
Signed-off-by: Fran Mulero <francisco-jose.mulero@broadcom.com>
juan131
juan131 previously approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@juan131 juan131 left a comment

Choose a reason for hiding this comment

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

LGTM

@fmulero fmulero enabled auto-merge (squash) May 22, 2024 11:07
Update README.md with readme-generator-for-helm
Loading
Loading status checks…
0d9b5d0 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@fmulero fmulero disabled auto-merge May 22, 2024 11:47
@fmulero fmulero merged commit 5e9d5eb into bitnami:main May 22, 2024
9 checks passed
@fmulero fmulero deleted the bugfix/external-dns-pdb-2 branch May 22, 2024 11:48
@fmulero fmulero mentioned this pull request May 22, 2024
Closed
fmulero added a commit to fmulero/charts that referenced this pull request May 22, 2024
@fmulero @juan131
[bitnami/external-dns] PDB fixes (bitnami#26292)

Verified

This commit was signed with the committer’s verified signature.
fmulero Fran Mulero
GPG key ID: 017C5A37D75CC19B
Verified
Learn about vigilant mode
487d7aa 
* [bitnami/external-dns] PDB fixes

Signed-off-by: Fran Mulero <fmulero@vmware.com>

* Update CHANGELOG.md

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* Update README.md with readme-generator-for-helm

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* Set pdb.maxUnavailable instead of pdb.minAvailable by default

Signed-off-by: Fran Mulero <fmulero@vmware.com>

* Update CHANGELOG.md

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* Update README.md with readme-generator-for-helm

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* Fix CHANGELOG.md

Signed-off-by: Fran Mulero <fmulero@vmware.com>

* Update CHANGELOG.md

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* Add documentation for pdb.maxUnavailable value

Signed-off-by: Fran Mulero <fmulero@vmware.com>

* Update CHANGELOG.md

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

* Update bitnami/external-dns/values.yaml

Co-authored-by: Juan Ariza Toledano <jariza@vmware.com>
Signed-off-by: Fran Mulero <francisco-jose.mulero@broadcom.com>

* Update README.md with readme-generator-for-helm

Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>

---------

Signed-off-by: Fran Mulero <fmulero@vmware.com>
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
Signed-off-by: Fran Mulero <francisco-jose.mulero@broadcom.com>
Co-authored-by: Bitnami Containers <bitnami-bot@vmware.com>
Co-authored-by: Juan Ariza Toledano <jariza@vmware.com>
Signed-off-by: Fran Mulero <fmulero@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juan131 juan131

@jotamartos jotamartos

@javsalgar javsalgar

Assignees

@javsalgar javsalgar

@juan131 juan131

@jotamartos jotamartos

Labels
bitnami external-dns solved verify Execute verification workflow for these changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[external-dns] Default PDB is incompatible with default replica count
5 participants
@fmulero @juan131 @javsalgar @jotamartos @bitnami-bot