terraform-provider-kubernetes
terraform-provider-kubernetes copied to clipboard
Add support for Kubernetes Pod Disruption Budgets
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
Hi @jcarr-sailthru + @clairefinnie, Thanks for making the big trip from Wellington to visit this repo, and thanks for the PR! I'll give it a spin on some test infrastructure.
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.
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.
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 'selector' is now a TypeList that can have
- match_labels (TypeMap, as selector was before)
- match_expressions (a TypeList with properties: key, operator & value) properties set
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.