Skip to content

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

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

Open
wants to merge 16 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.

This PR will resolve #2662

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.

@xinkechen-evernorth xinkechen-evernorth requested a review from a team as a code owner December 2, 2024 14:46
@xinkechen-evernorth
Copy link
Author

@jrhouston @BBBmau

Apologies for the mentions, but is there anything additional I need to do help move this PR along?

@xinkechen-evernorth
Copy link
Author

@jrhouston @BBBmau @alexsomesan @iBrandyJackson

Just wanted to follow up, is there anything I can to help move this pull request along? Would love to get this bug resolved so that the K8S Provider has parity with what you can do with kubectl taint.

@xinkechen-evernorth
Copy link
Author

xinkechen-evernorth commented May 12, 2025

@alexsomesan @arybolovlev @jrhouston

Apologies for the direct ping, but is there any way I can help move this bugfix along? I would love to be able to get this fix in and have the provider have parity with the Kubernetes API/kubectl.

I also want to to highlight as well besides the deviation away from the Kubernetes docs/API that a major distribution of Kubernetes, OpenShift/OKD, also directly instructs their users to setup certain dedicated "infrastructure" nodes with the following tolerations in order to correctly schedule workloads on said nodes.

    tolerations:
    - effect: NoSchedule
      key: node-role.kubernetes.io/infra
      value: reserved
    - effect: NoExecute
      key: node-role.kubernetes.io/infra
      value: reserved

For users of OpenShift or OKD that are using Terraform to manage physical compute for their cluster, i.e. what RH refers to a user provisioned infrastructure install method, the lack of ability to define these taints means they are not able to completely follow the documentation to create infrastructure nodes as described by the docs and support.

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.

Resource kubernetes_node_taint incorrectly denies the creation of multiple taints that share the same key but different effects
1 participant