-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add support for Kubernetes Pod Disruption Budgets #102
base: custom
Are you sure you want to change the base?
Conversation
This resource includes validation to ensure PDB is correctly set to either min_avaliable or max_unavailable and supports both integer and percentages. As this resource cannot be updated in Kubernetes, we only ever create and delete upon changes.
Note: Currently untested due to difficulties executing the test framework.
LAB-358 Add support for Pod Disruption Budgets
Hi @jcarr-sailthru + @clairefinnie, |
Thanks @sl1pm4t - we just saw your new readme comment about the official branch as well, good to see that Hashicorp is giving this provider some love. We'll work to get this PR up to them as well. |
This is primarily to allow us to define selectors with only keys & no values set. This all makes defining PDB more flexible. Main change is that 'selector' is now a TypeList that can have match_labels (TypeMap, as selector was before) OR match_expressions (a TypeList with properties: key, operator & value) Updated relevant flatten & expand functions that map the PDB Spec config & updated relevant tests.
As a warning, I opened a PR on the main repo for adding PodDisruptionBudget based on my own implementation (didn't see this one when I was writing mine). I doubt they'll accept it since it is a beta resource, but doing a code diff between our branches, there are important differences in implementation. In particular, your implementation makes selector required although strictly it seems to be optional. Another difference is missing import support. Mine needs to add the check for min_available and max_unavailable not both being set simultaneously that your implementation has, although I'm not sure if placing that check in the expand is the right place. I have tested it using the acceptance tests. I may open my own PR here once I rebase it against sl1pm4t's fork. |
Improve selector match expression
Hi @mbarrien, Didn't see your PDB work either before submitting this PR. We have made some more changes to this PR with respect to setting selectors. We were unable to target resources with a specific key set, not caring about the value of that key, just looking for the presence of a key in labels. So we added the ability to use match expressions or match labels for selectors. Main change is
We think this new structure matches better with how they are implementing this in the latest upstream code, for some other resources . Totally agree with you that the validation does not sit nicely inside the expand functions. I could add a separate validation method as you did. |
hi @sl1pm4t ,
Greetings all the way from Wellington! Big fans of this fork, thanks for your work here.
@clairefinnie has added support for Pod Disruption Budgets to our fork of your fork (fork2?) and it would be awesome to get this merged up into your repo.
We have validated the creation and deletion of the Pod Disruption Budgets, but we struggled to get a working environment to validate the tests that we wrote, so the tests themselves are untested.
regards,
Jethro