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

Allow datacenter and node class filtering #535

Merged
merged 6 commits into from
Nov 15, 2021
Merged

Conversation

lgfa29
Copy link
Contributor

@lgfa29 lgfa29 commented Nov 12, 2021

The initial implementation for the node filtering would only take either node class or datacenter as the filter input. But as explained in #531, there are valid use cases where defining both is required in order to properly match Nomad clients with cloud instances.

This PR creates a new ClusterNodePoolIdentifier called combinedClusterPoolIdentifier that can be used to merge other identifiers, applying a simple and or or logic.

When both node_class and datacenter are present in a scaling policy, the Autoscaler will use this combined identifier to merge the existing NewNodeDatacenterPoolIdentifier and NewNodeDatacenterPoolIdentifier logic with the and operator.

In the future we may add an option to use the or operator, but it's not clear to me if there is any real benefit since it would increase the node pool instead of reducing it. Regardless, the capability is there if needed.

Closes #531

Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

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

Great addition!

Minor comment inline but certainly not a blocker and I expect the answer is future proofing.

policy/policy.go Outdated Show resolved Hide resolved
sdk/helper/scaleutils/nodepool/combined.go Outdated Show resolved Hide resolved
@lgfa29 lgfa29 force-pushed the aws-filter-dc-node-class branch from dcd7809 to 9e78e5a Compare November 15, 2021 16:53
@lgfa29 lgfa29 merged commit b0e2119 into main Nov 15, 2021
@lgfa29 lgfa29 deleted the aws-filter-dc-node-class branch November 15, 2021 16:56
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.

aws-asg: allow filtering by node_class and datacenter
2 participants