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

add provisioner taints to nodes #699

Merged
merged 2 commits into from
Sep 23, 2021
Merged

add provisioner taints to nodes #699

merged 2 commits into from
Sep 23, 2021

Conversation

bwagner5
Copy link
Contributor

1. Issue, if available:
N/A

2. Description of changes:
Taints configured on the provisioner were not being added to nodes provisioned by the provisioner. Now they are being added.

3. Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: link to issue
  • No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@netlify
Copy link

netlify bot commented Sep 23, 2021

✔️ Deploy Preview for karpenter-docs-prod canceled.

🔨 Explore the source changes: 4ad7a91

🔍 Inspect the deploy log: https://app.netlify.com/sites/karpenter-docs-prod/deploys/614cc51b4378f70007b1b17b

Copy link
Contributor

@ellistarn ellistarn left a comment

Choose a reason for hiding this comment

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

Bad merge? I'm adding tests for this in my node affinity PR.

ellistarn
ellistarn previously approved these changes Sep 23, 2021
@bwagner5 bwagner5 dismissed stale reviews from JacobGabrielson and ellistarn via 4ad7a91 September 23, 2021 18:19
@bwagner5
Copy link
Contributor Author

Just to add more context to this:

  1. Taints are ideally added in two places, once in the allocation controller and once in the cloudprovider's node bootstrap script (userdata for aws). If you're using a custom LaunchTemplate, then the taints don't get added to userdata, and the allocation controller taints were not being added (this PR fixes that).

  2. The tests passed because the cloudprovider mock was adding taints. I'm assuming this was added to the mock because userdata is expected to run to add the taints, but like mentioned in 1, we can't necessarily rely on that if a custom launch template is used.

@bwagner5 bwagner5 merged commit bcc0bbe into aws:main Sep 23, 2021
@bwagner5 bwagner5 deleted the fix-taints branch September 23, 2021 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants