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

Match specific tolerations to prevent Diffs #978

Merged
merged 2 commits into from
Apr 29, 2021

Conversation

DrFaust92
Copy link
Contributor

Description

Acceptance tests

  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch? (If so, please include the test log in a gist)
--- PASS: TestAccKubernetesDeployment_with_tolerations (54.41s)
--- PASS: TestAccKubernetesDeployment_with_tolerations_unset_toleration_seconds (11.52s)

--- PASS: TestAccKubernetesDaemonSet_with_tolerations (5.58s)
--- PASS: TestAccKubernetesDaemonSet_with_tolerations_unset_toleration_seconds (6.17s)

References

Closes #955

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

@DrFaust92
Copy link
Contributor Author

rebased

Base automatically changed from master to main March 23, 2021 15:53
@dak1n1 dak1n1 added the acknowledged Issue has undergone initial review and is in our work queue. label Apr 27, 2021
@dak1n1 dak1n1 self-assigned this Apr 27, 2021
Copy link
Contributor

@dak1n1 dak1n1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! I did some manual testing and ran the full set of acceptance tests on it, and everything passed. I just left one suggestion.

Comment on lines 15 to 21
"node.kubernetes.io/not-ready": "",
"node.kubernetes.io/unreachable": "",
"node.kubernetes.io/out-of-disk": "",
"node.kubernetes.io/memory-pressure": "",
"node.kubernetes.io/disk-pressure": "",
"node.kubernetes.io/network-unavailable": "",
"node.kubernetes.io/unschedulable": "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of hard-coding these names, how would you feel about pulling the values from the Kubernetes API library? They're defined as constants here:

https://github.com/kubernetes/kubernetes/blob/ea0764452222146c47ec826977f49d7001b0ea8c/staging/src/k8s.io/api/core/v1/well_known_taints.go#L22

For example:

	v1.TaintNodeNotReady:           "",
	v1.TaintNodeUnreachable:        "",

Then any future naming changes will be pulled in automatically through that library. I also noticed that file contains one that was missing from the PR: TaintNodePIDPressure. (Probably because it wasn't listed on their docs page).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, will change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Copy link
Contributor

@dak1n1 dak1n1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution!

@dak1n1 dak1n1 merged commit 637fb9c into hashicorp:main Apr 29, 2021
@DrFaust92 DrFaust92 deleted the tolerations branch April 29, 2021 19:31
@ghost
Copy link

ghost commented May 30, 2021

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators May 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
acknowledged Issue has undergone initial review and is in our work queue. size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

My toleration is not saved to the tfstate at the end of both successful or failed terraform apply
2 participants