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

add spread feature #88

Merged
merged 1 commit into from
Sep 16, 2024
Merged

add spread feature #88

merged 1 commit into from
Sep 16, 2024

Conversation

jagedn
Copy link
Collaborator

@jagedn jagedn commented Sep 14, 2024

this is a WIP implementation for spread using a "map" syntax, i.e.

jobs {
        spreads = {
            spread = name:'node.datacenter', weight: 50, targets: ['a': 20]
        }
    }

Attached you can see how the spread is sent to cluster in the Job defintion

imagen

Signed-off-by: Jorge Aguilera <[email protected]>
@jagedn jagedn self-assigned this Sep 14, 2024
@jagedn jagedn requested a review from abhi18av September 14, 2024 11:44
@jagedn jagedn marked this pull request as draft September 14, 2024 11:44
@jagedn jagedn requested a review from matthdsm September 14, 2024 11:44
@abhi18av abhi18av linked an issue Sep 15, 2024 that may be closed by this pull request
@jagedn jagedn marked this pull request as ready for review September 16, 2024 06:20
@jagedn
Copy link
Collaborator Author

jagedn commented Sep 16, 2024

attached and example with the -profile localnomad so process directives is used also

imagen

Copy link
Member

@abhi18av abhi18av left a comment

Choose a reason for hiding this comment

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

Hi @jagedn ,

Thank you for taking this forward! I found a couple of neat design tricks in the code. Though I do confess that at the moment, I am unable to really find a use-case within the sun-nomadlab directly and therefore what validations exactly we need to add.

At some point, we need the DSL to dictate the legal/illegal values even if the underlying nomad layer allows a specific ability. I'm just not sure what aspect we'd need to put some gates upon.

Overall, to me this looks good to merge for now and we can continue with the v0.2.1 release so that @matthdsm and @tomiles can test this further.

@abhi18av abhi18av merged commit e8f28ab into master Sep 16, 2024
2 checks passed
@abhi18av abhi18av deleted the add-spread branch September 16, 2024 14:44
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.

Feature: add support for spread JobOpt
2 participants