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

scaleutils: add datacenter nodepool implementation #468

Merged
merged 7 commits into from
Apr 20, 2021
Merged

Conversation

jrasell
Copy link
Member

@jrasell jrasell commented Apr 19, 2021

closes #459

When understanding the nodes which form a scalable pool of resource, the Nomad Autoscaler uses the nodepool interface which provides functionality to understand node grouping. This change adds a new implementation, datacenter, which allows nodes to be group according to their datacenter configuration option.

The cluster policy validation has been updated to account for this change. Particuary the node_class and datacenter options are mutually exclusive. The new validation functions use the helper/error pkg to create better formatted errors. Work to update the entire error chain will be conducted in a follow up PR.

The Nomad project site will need updating to document the new config option. It may also prove valuable to add a demo, or at least commented out examples of using datacenter rather than node_class.

Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

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

LGTM!

sdk/helper/scaleutils/nodepool/datacenter.go Outdated Show resolved Hide resolved
policy/policy.go Show resolved Hide resolved
@lgfa29 lgfa29 modified the milestones: 0.4.0, 0.3.3 Apr 20, 2021
@jrasell jrasell merged commit 7f02712 into main Apr 20, 2021
@jrasell jrasell deleted the f-gh-459-dc-pool-id branch April 20, 2021 07:06
jrasell added a commit that referenced this pull request Apr 20, 2021
jrasell added a commit that referenced this pull request Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nodepool: add Datacenter nodepool configuration option
2 participants