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

kubernetes_node_taint: fix warning when node is missing

Open partcyborg opened this issue 1 year ago • 1 comments

Description

Move the check for errors.IsNotFound to the correct err so nodes that have been turned down do not cause existing resources to error.

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'
==> Checking that code complies with gofmt requirements...
go vet ./...
TF_ACC=1 go test "/home/mwilder/src/go/src/github.com/partcyborg/terraform-provider-kubernetes/kubernetes" -v -vet=off -run=TestAccKubernetesResourceNodeTaint -parallel 8 -timeout 3h
=== RUN   TestAccKubernetesResourceNodeTaint_basic
--- PASS: TestAccKubernetesResourceNodeTaint_basic (8.82s)
=== RUN   TestAccKubernetesResourceNodeTaint_MultipleBasic
--- PASS: TestAccKubernetesResourceNodeTaint_MultipleBasic (22.61s)
PASS
ok  	github.com/hashicorp/terraform-provider-kubernetes/kubernetes	31.460s

There does not appear to be a way to test this with the existing test infrastructure, as we would need a state file that contains a node that doesn't exist in the context used.

As an alternative, I tested this change manually with existing terraform state using a dev_override. While it fails with the upstream provider, with the dev version it warns as expected:

╷
│ Warning: Node has been deleted
│
│   with module.retired_taints.kubernetes_node_taint.this["ip-10-252-229-82.ca-central-1.compute.internal"],
│   on ../../../../../tf_modules/k8s/node_taints/main.tf line 11, in resource "kubernetes_node_taint" "this":
│   11: resource "kubernetes_node_taint" "this" {
│
│ The underlying node "ip-10-252-229-82.ca-central-1.compute.internal" has
│ been deleted. You should remove it from your configuration.
│
│ (and 11 more similar warnings elsewhere)

Release Note

Release note for CHANGELOG:

`resource/kubernetes_node_taint`: Fix the error check for nonexistant nodes so that terraform does not fail if there is a taint in the state file for a node that has been deleted.

References

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

partcyborg avatar Jan 13 '24 02:01 partcyborg

Friendly ping @jrhouston @alexsomesan @arybolovlev.

Apologies for messing up the error check before. This is the last blocker keeping us from migrating off of from our custom provider fork sdb-cloud-ops/kubernetes to the official upstream one, so I would love to get it checked in.

Thanks in advance!

partcyborg avatar Feb 14 '24 18:02 partcyborg