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.

Sign up for GitHub

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

Jump to bottom

(aws-eks): spot node termination handler is deployed to all nodes #12368

Closed
dirknilius opened this issue Jan 6, 2021 · 2 comments · Fixed by #12369
Closed

(aws-eks): spot node termination handler is deployed to all nodes #12368

dirknilius opened this issue Jan 6, 2021 · 2 comments · Fixed by #12369
Assignees
@iliapolo
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p1

Comments

@dirknilius
Copy link
Contributor

When adding autoscaling group capacity the aws-node-termination-handler is deployed as Helm chart. There is a nodeSelector applied which should only deploy the DaemonSet to nodes labeled with lifecycle=Ec2Spot. Unfortunately this was not done correctly.

Current implementation:

'nodeSelector.lifecycle': LifecycleLabel.SPOT

When I get the Helm values (helm get values ...) it looks like this:

nodeSelector: {}
nodeSelector.lifecycle: Ec2Spot

Correct implementation:

nodeSelector:{
  lifecycle: LifecycleLabel.SPOT
}

I already created a fix. I will submit a PR.

This is 🐛 Bug Report
@dirknilius dirknilius added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 6, 2021
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Jan 6, 2021
dirknilius pushed a commit to dirknilius/aws-cdk that referenced this issue Jan 6, 2021
fix(aws-eks): fixed the aws-node-termination-handler helm chart node …
ccee9e7 
…selector

fixes aws#12368

The former implementation was using the string format for submitting Helm values in the CLI ("parent.child=value"). This does not work here as JavaScript does not expand the string. We need to define the values is the correct hierarchy.
@dirknilius dirknilius mentioned this issue Jan 6, 2021
Merged
@dirknilius
Copy link
Contributor Author

PR: #12369

@iliapolo iliapolo added effort/small Small work item – less than a day of effort p1 in-progress This issue is being actively worked on. and removed needs-triage This issue or PR still needs to be triaged. labels Jan 10, 2021
@mergify mergify bot closed this as completed in #12369 Jan 14, 2021
mergify bot pushed a commit that referenced this issue Jan 14, 2021
@dirknilius
fix(eks): aws-node-termination-handler incorrectly deployed to on-dem…
05c0b5f 
…and instances as well (#12369)

fixes #12368

The former implementation was using the string format for submitting Helm values in the CLI (`parent.child=value`). This does not work here as JavaScript does not expand the string. We need to define the values is the correct hierarchy.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@iliapolo iliapolo

Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p1
Projects
None yet
Milestone
No milestone
2 participants
@dirknilius @iliapolo