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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: introduce filter for flattenPodSpec to control tolerations #1012

Closed
wants to merge 1 commit into from

Conversation

elodani
Copy link

@elodani elodani commented Sep 19, 2020

Description

If you start a pod natively, k8s may add tolerations automatically that
you did not define. There is already a filter in the flattenPodSpec that
filters these out, but if you start it as part of a
deployment/replicaset/daemonset, you might want to use similar
tolerations and don't want to get in a perpetual diff (where state
refresh ignores your toleration in deployments just like it would with
native pods).

This is not a silver bullet, but unblocked me at my work and will possibly unblock many others, as these taint tolerations are used by modules in the registry including https://registry.terraform.io/modules/spotinst/ocean-controller/spotinst/0.5.0 which connects spot.io managed workers to EKS.

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=TestFlattenTolerationsNative'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test "/home/elodani/playground/terraform-provider-kubernetes/kubernetes" -v -run=TestFlattenTolerationsNative -timeout 120m
=== RUN   TestFlattenTolerationsNative
2020/09/19 09:52:25 [INFO] ignoring toleration with key: node.kubernetes.io/not-ready
--- PASS: TestFlattenTolerationsNative (0.00s)
PASS
ok  	github.com/hashicorp/terraform-provider-kubernetes/kubernetes	0.037s
$ make testacc TESTARGS='-run=TestFlattenTolerationsNonNative'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test "/home/elodani/playground/terraform-provider-kubernetes/kubernetes" -v -run=TestFlattenTolerationsNonNative -timeout 120m
=== RUN   TestFlattenTolerationsNonNative
--- PASS: TestFlattenTolerationsNonNative (0.00s)
PASS
ok  	github.com/hashicorp/terraform-provider-kubernetes/kubernetes	0.043s

Release Note

Release note for CHANGELOG:

`node.kubernetes.io/` prefixed tolerations in pod templates of daemonset/replicaset/deployments are no longer filtered out from the state

References

fixes issue: #955
slightly related: #978

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

If you start a pod natively, k8s may add tolerations automatically that
you did not define. There is already a filter in the flattenPodSpec that
filters these out, but if you start it as part of a
deployment/replicaset/daemonset, you might want to use similar
tolerations and don't want to get in a perpetual diff (where state
refresh ignores your toleration in deployments just like it would with
native pods).

This is not a silver bullet, but unblocked me at my work, in the future
maybe a more robust solution (e.g. there is not tellign that you will
use this prefix of tolerations in natively started pods, making the
problem I have with deployments appear there as well)
@hashicorp-cla
Copy link

hashicorp-cla commented Nov 22, 2020

CLA assistant check
All committers have signed the CLA.

Base automatically changed from master to main March 23, 2021 15:53
@simonweil
Copy link

Any chance to get this merged?
The related bug is soooo annoying 馃槙

@alexppg
Copy link

alexppg commented Apr 8, 2021

Yes please, let this merge happen if there's no problem.

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.

None yet

4 participants