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

filter non-ASG nodes by tag #654

Closed
universam1 opened this issue Jun 17, 2022 · 7 comments
Closed

filter non-ASG nodes by tag #654

universam1 opened this issue Jun 17, 2022 · 7 comments
Assignees
Labels
Priority: Critical This issue will be seen by all users Type: Bug Something isn't working

Comments

@universam1
Copy link
Contributor

universam1 commented Jun 17, 2022

Describe the feature

Karpenter nodes are not bound to a Autoscaling Group. Filter on tags via check-asg-tag-before-draining is scoped to ASG only, so it is not possible to filter nodes such nodes based on tags.

if m.CheckIfManaged {
if nodeInfo.AsgName == "" {
// If ASG tags are not propagated we might need to use the API
// to retrieve the ASG name
nodeInfo.AsgName, err = m.retrieveAutoScalingGroupName(nodeInfo.InstanceID)
if err != nil {
return nil, fmt.Errorf("unable to retrieve AutoScaling group: %w", err)
}
}
if nodeInfo.Tags[m.ManagedAsgTag] == "" {

Is the feature request related to a problem?

AFAIK pre - filtering the Spot ITN Events is not possible because EC2 Spot Instance Interruption Warning do not contain tags of the instance where we could filter the rule on

I have several clusters in one account and thus I am missing an option in NTH to filter for cluster respective nodes only.

Describe alternatives you've considered

none

@jillmon jillmon added the Type: Question All types of questions to/from customers label Jun 22, 2022
@AustinSiu
Copy link
Contributor

Hi @universam1! Can you help confirm the ask? It sounds like you want the ability for NTH to ignore nodes that are not associated with an ASG, using the same ManagedAsgTag we use today for nodes in ASGs (name might need to be revisited...).

I have to bound NTH to the cluster respective nodes only.

Can you help clarify what you mean, assuming this is your current workaround to the issue?

@universam1
Copy link
Contributor Author

universam1 commented Jun 27, 2022

@AustinSiu Thank you for asking - actually the opposite 😄

I want NTH to manage non-ASG nodes that are created by Karpenter. Plus I need to filter based on tags to identify the cluster-nodes.
However, ManagedAsgTag filter is not applicable for non-ASG nodes, see above.

assuming this is your current workaround to the issue?

Sorry if I was unclear, I have no workaround for this problem.

@AustinSiu AustinSiu added Type: Enhancement New feature or request and removed Type: Question All types of questions to/from customers labels Jun 29, 2022
@bwagner5
Copy link
Contributor

👋 @universam1 you are right and this is a regression. We used to have an AssumeASGTagPropagation configuration that would result in no ASG lookup, but that was removed recently. We should add that back.

37e9899#r78210273

@bwagner5 bwagner5 added Priority: Critical This issue will be seen by all users Type: Bug Something isn't working and removed Type: Enhancement New feature or request labels Jul 11, 2022
@pangorgo
Copy link

I've just encountered a similar issue. I just wanted to post a potential workaround to tag your instance with aws:autoscaling:groupName, but I forgot aws: prefix is reserved and the user can't assign such a tag.

We are looking forward to the solution. @bwagner5 not sure what Priority:Critical means. Can you shed some light on a possible timeframe for a resolution? Not sure if we should switch to IMDS or just wait for the fix, so any estimation (days, weeks, months?) would be very useful.

@bwagner5
Copy link
Contributor

@pangorgo likely a week or so to get the change in and tested. A current workaround is to rollback to v1.16.2

@pangorgo
Copy link

Thanks @bwagner5 I've just tested it with version v1.16.2 and it works 🎉
I just had to add ASSUME_ASG_TAG_PROPAGATION=true

@snay2 snay2 added the Pending-Release Pending an NTH or eks-charts release label Aug 18, 2022
@snay2
Copy link
Contributor

snay2 commented Aug 18, 2022

This has been released in NTH app version v1.17.0. Helm chart version v0.19.0.

@snay2 snay2 closed this as completed Aug 18, 2022
@snay2 snay2 removed the Pending-Release Pending an NTH or eks-charts release label Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Critical This issue will be seen by all users Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants