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.

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

feat(helm): add PodDisruptionBudget api version helm chart helpers #1865

Merged
merged 3 commits into from
Nov 23, 2021

Conversation

pmalek-sumo
Copy link
Contributor

@pmalek-sumo pmalek-sumo commented Nov 3, 2021

The idea behind this change is to support newer version of PodDisruptionBudget (policy/v1beta1/PodDisruptionBudget vs policy/v1/PodDisruptionBudget)
and to get rid of deprecation warnings when installing our chart:

$ helm upgrade --install collection sumologic/sumologic
Release "collection" does not exist. Installing it now.
...
W1118 17:09:13.651675 2835438 warnings.go:70] policy/v1beta1 PodDisruptionBudget is deprecated in v1.21+, unavailable in v1.25+; use policy/v1 PodDisruptionBudget
W1118 17:09:13.654276 2835438 warnings.go:70] policy/v1beta1 PodDisruptionBudget is deprecated in v1.21+, unavailable in v1.25+; use policy/v1 PodDisruptionBudget
...
W1118 17:09:14.026749 2835438 warnings.go:70] policy/v1beta1 PodDisruptionBudget is deprecated in v1.21+, unavailable in v1.25+; use policy/v1 PodDisruptionBudget
W1118 17:09:14.028299 2835438 warnings.go:70] policy/v1beta1 PodDisruptionBudget is deprecated in v1.21+, unavailable in v1.25+; use policy/v1 PodDisruptionBudget
NAME: collection
LAST DEPLOYED: Thu Nov 18 17:09:12 2021
NAMESPACE: sumologic
STATUS: deployed
REVISION: 1
NOTES:
Thank you for installing sumologic.

@pmalek-sumo pmalek-sumo self-assigned this Nov 3, 2021
@pmalek-sumo pmalek-sumo requested a review from a team as a code owner November 3, 2021 10:53
@pmalek-sumo
Copy link
Contributor Author

This will require some more work in tests since we need to pass in api versions to helm template invocation like so:

helm template ... --api-versions policy/v1/PodDisruptionBudget ... 

but we don't want to hardcode those and we'd like to make this configurable per test.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request contains invalid labels. Please remove all of the following labels: ['do-not-merge/hold']

@pmalek-sumo pmalek-sumo force-pushed the add-api-version-helpers-for-pod-disruption-budget branch from 26c0d4b to fa647f7 Compare November 9, 2021 18:23
@github-actions github-actions bot added the documentation documentation label Nov 9, 2021
@pmalek-sumo pmalek-sumo force-pushed the add-api-version-helpers-for-pod-disruption-budget branch 4 times, most recently from 4acbad3 to 924dd22 Compare November 9, 2021 18:46
@github-actions github-actions bot added the ci label Nov 9, 2021
@pmalek-sumo pmalek-sumo force-pushed the add-api-version-helpers-for-pod-disruption-budget branch from 924dd22 to e8aeda7 Compare November 9, 2021 18:48
@pmalek-sumo pmalek-sumo force-pushed the add-api-version-helpers-for-pod-disruption-budget branch from e8aeda7 to cfeac0f Compare November 9, 2021 18:53
@github-actions github-actions bot removed the ci label Nov 9, 2021
@perk-sumo perk-sumo added this to the v2.3 milestone Nov 23, 2021
Copy link
Contributor

@perk-sumo perk-sumo left a comment

Choose a reason for hiding this comment

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

Cool, the end state is OK, thanks for working on that!

But 😅
I don't like the commit history here, please either fix the history or squash the PR before merge.

@pmalek-sumo pmalek-sumo merged commit f49f290 into main Nov 23, 2021
@pmalek-sumo pmalek-sumo deleted the add-api-version-helpers-for-pod-disruption-budget branch November 23, 2021 15:11
@pmalek-sumo
Copy link
Contributor Author

please either fix the history or squash the PR before merge.

squashed :) Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants