-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
CSI: implement support for topology #12129
Conversation
I'm not sure what's going on in https://app.circleci.com/pipelines/github/hashicorp/nomad/20207/workflows/694b6571-3edf-44f1-bcff-5ad97409f63b/jobs/207214 but that looks like boltdd stuff and unrelated to this PR. Will investigate on Monday. |
I've opened #12143 to workaround the test issue for now, and re-run here successfully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT, nice to see this implemented, and the sample CLI outputs were very helpful.
Just missing a CHANGELOG entry 👍
// requisite topologies." | ||
if len(vol.Topologies) > 0 { | ||
var ok bool | ||
for _, requiredTopo := range vol.Topologies { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checking my understanding of this feature: at this point the volume is already registered, so we don't need to worry about preferred
vs. required
. The plugin that handled the registration resolved the supported topologies and we stored them in the state store, so any match here would match the user's request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly!
(Which is really good for us, because otherwise we'd have had to implement a new scoring iterator too 😀 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! just trivial things
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #7669
Fixes #10891
Fixes #11778
Allows the user to create CSI volumes within specific locations using opaque tags ("segments") specific to the storage provider. Update the feasibility checker for CSI volumes so that allocations are only placed on nodes that meet the topology requirements.
Smoke test with an AWS EBS volume created in zone us-east-1a but where all the node plugins are running in us-east-1b:
volume.hcl
nomad plugin status -verbose
nomad volume status ebs-vol[0]
And then switching to a volume in the right AZ works just fine, as expected.