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

Fix kubernetes_node_taint so that taints are unique over key and effect and not just key #2611

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

xinkechen-evernorth
Copy link

@xinkechen-evernorth xinkechen-evernorth commented Oct 30, 2024

Description

This PR seeks to fix the kubernetes_node_taint resource so that it's possible to set multiple effects for a given key. This is motivated by the fact that with kubectl it is possible to do the following:

❯ kubectl taint nodes dedicated-node.example.com key1=value:NoSchedule key1=value:NoExecute
node/dedicated-node.example.com tainted
❯ kubectl get node -o jsonpath='{.spec.taints}' dedicated-node.example.com
[{"effect":"NoSchedule","key":"key1","value":"value"},{"effect":"NoExecute","key":"key1","value":"value"}

A user attempting to express the same set of taints kubernetes_node_taint erroneously fails with the message that taints unique over key only.

Acceptance tests

  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch?

Output from acceptance testing:

❯ make testacc TESTARGS="-run ^TestAccKubernetesResourceNodeTaint"
...
=== RUN   TestAccKubernetesResourceNodeTaint_basic
--- PASS: TestAccKubernetesResourceNodeTaint_basic (8.44s)
=== RUN   TestAccKubernetesResourceNodeTaint_MultipleBasic
--- PASS: TestAccKubernetesResourceNodeTaint_MultipleBasic (21.67s)
=== RUN   TestAccKubernetesResourceNodeTaint_MultipleSameKeyDifferentEffect
--- PASS: TestAccKubernetesResourceNodeTaint_MultipleSameKeyDifferentEffect (8.80s)
=== RUN   TestAccKubernetesResourceNodeTaint_DuplicateTaintKeyAndEffectExpectError
--- PASS: TestAccKubernetesResourceNodeTaint_DuplicateTaintKeyAndEffectExpectError (1.86s)
PASS
ok  	github.com/hashicorp/terraform-provider-kubernetes/kubernetes	40.998s

Release Note

Release note for CHANGELOG:

`resource/kubernetes_node_taint`: Fix taint uniqueness check so it is enforced over key and effect and not solely key alone.

References

Documented examples of using the same key with a different effect

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Copy link

hashicorp-cla-app bot commented Oct 30, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

@xinkechen-evernorth xinkechen-evernorth marked this pull request as ready for review October 31, 2024 03:45
@xinkechen-evernorth xinkechen-evernorth requested a review from a team as a code owner October 31, 2024 03:45
@xinkechen-evernorth xinkechen-evernorth changed the title Fix kubernetes_node_taint that taints are unique over key and effect and not just key Fix kubernetes_node_taint so that taints are unique over key and effect and not just key Oct 31, 2024
@xinkechen-evernorth
Copy link
Author

xinkechen-evernorth commented Nov 6, 2024

Apologies for the ask since I know there is a sizeable queue, but is there an estimate on when an initial review might happen?

I want to make sure this PR is in a good spot before my personal availability becomes tied up for the next couple weeks and not have any requested changes or comments languish out here.

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

Successfully merging this pull request may close these issues.

1 participant