terraform-provider-kubernetes icon indicating copy to clipboard operation
terraform-provider-kubernetes copied to clipboard

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

Open xinkechen-evernorth opened this issue 1 year ago • 6 comments

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

  • [x] Have you added an acceptance test for the functionality being added?
  • [x] 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

  • Kubernetes PR that made taints unique by key and effect: https://github.com/kubernetes/kubernetes/pull/30590
  • Kubernetes Documentation with example use case using the same key but different effect: https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/#example-use-cases

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

xinkechen-evernorth avatar Oct 30 '24 02:10 xinkechen-evernorth

CLA assistant check
All committers have signed the CLA.

hashicorp-cla-app[bot] avatar Oct 30 '24 02:10 hashicorp-cla-app[bot]

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.

hashicorp-cla-app[bot] avatar Oct 30 '24 02:10 hashicorp-cla-app[bot]

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 avatar Nov 06 '24 17:11 xinkechen-evernorth

@jrhouston @BBBmau

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

xinkechen-evernorth avatar Dec 02 '24 14:12 xinkechen-evernorth

@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 avatar Dec 12 '24 15:12 xinkechen-evernorth

@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.

xinkechen-evernorth avatar May 12 '25 16:05 xinkechen-evernorth