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.

Sign up for GitHub

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 constraint spec #40

Merged
merged 1 commit into from
Jun 13, 2024
Merged

add constraint spec #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 38 additions & 10 deletions plugins/nf-nomad/src/main/nextflow/nomad/NomadConfig.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package nextflow.nomad
import groovy.transform.CompileStatic
import groovy.util.logging.Slf4j
import nextflow.nomad.config.AffinitySpec
import nextflow.nomad.config.ConstraintSpec
import nextflow.nomad.config.VolumeSpec

/**
Expand Down Expand Up @@ -61,6 +62,7 @@ class NomadConfig {
final String dockerVolume
final VolumeSpec volumeSpec
final AffinitySpec affinitySpec
final ConstraintSpec constraintSpec

NomadJobOpts(Map nomadJobOpts){
deleteOnCompletion = nomadJobOpts.containsKey("deleteOnCompletion") ?
Expand All @@ -78,25 +80,51 @@ class NomadConfig {
if( dockerVolume ){
log.info "dockerVolume config will be deprecated, use volume type:'docker' name:'name' instead"
}

this.volumeSpec = parseVolume(nomadJobOpts)
this.affinitySpec = parseAffinity(nomadJobOpts)
this.constraintSpec = parseConstraint(nomadJobOpts)
}

VolumeSpec parseVolume(Map nomadJobOpts){
if( nomadJobOpts.volume && nomadJobOpts.volume instanceof Closure){
this.volumeSpec = new VolumeSpec()
def volumeSpec = new VolumeSpec()
def closure = (nomadJobOpts.volume as Closure)
def clone = closure.rehydrate(this.volumeSpec, closure.owner, closure.thisObject)
def clone = closure.rehydrate(volumeSpec, closure.owner, closure.thisObject)
clone.resolveStrategy = Closure.DELEGATE_FIRST
clone()
this.volumeSpec.validate()
volumeSpec.validate()
volumeSpec
}else{
volumeSpec = null
null
}
if( nomadJobOpts.affinity && nomadJobOpts.affinity instanceof Closure){
this.affinitySpec = new AffinitySpec()
}

AffinitySpec parseAffinity(Map nomadJobOpts) {
if (nomadJobOpts.affinity && nomadJobOpts.affinity instanceof Closure) {
def affinitySpec = new AffinitySpec()
def closure = (nomadJobOpts.affinity as Closure)
def clone = closure.rehydrate(this.affinitySpec, closure.owner, closure.thisObject)
def clone = closure.rehydrate(affinitySpec, closure.owner, closure.thisObject)
clone.resolveStrategy = Closure.DELEGATE_FIRST
clone()
this.affinitySpec.validate()
}else{
affinitySpec = null
affinitySpec.validate()
affinitySpec
} else {
null
}
}

ConstraintSpec parseConstraint(Map nomadJobOpts){
if (nomadJobOpts.constraint && nomadJobOpts.constraint instanceof Closure) {
def constraintSpec = new ConstraintSpec()
def closure = (nomadJobOpts.constraint as Closure)
def clone = closure.rehydrate(constraintSpec, closure.owner, closure.thisObject)
clone.resolveStrategy = Closure.DELEGATE_FIRST
clone()
constraintSpec.validate()
constraintSpec
} else {
null
}
}
}
Expand Down
38 changes: 38 additions & 0 deletions plugins/nf-nomad/src/main/nextflow/nomad/config/ConstraintSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package nextflow.nomad.config

class ConstraintSpec {

private String attribute
private String operator
private String value

String getOperator(){
return operator
}

String getAttribute() {
return attribute
}

String getValue() {
return value
}

ConstraintSpec attribute(String attribute){
this.attribute=attribute
this
}

ConstraintSpec operator(String operator){
this.operator = operator
this
}

ConstraintSpec value(String value){
this.value = value
this
}

void validate(){
}
}
16 changes: 16 additions & 0 deletions plugins/nf-nomad/src/main/nextflow/nomad/executor/NomadService.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import io.nomadproject.client.ApiClient
import io.nomadproject.client.api.JobsApi
import io.nomadproject.client.model.Affinity
import io.nomadproject.client.model.AllocationListStub
import io.nomadproject.client.model.Constraint
import io.nomadproject.client.model.Job
import io.nomadproject.client.model.JobRegisterRequest
import io.nomadproject.client.model.JobRegisterResponse
Expand Down Expand Up @@ -198,6 +199,21 @@ class NomadService implements Closeable{
}
taskDef.affinities([affinity])
}

if( config.jobOpts.constraintSpec ){
def constraint = new Constraint()
if(config.jobOpts.constraintSpec.attribute){
constraint.ltarget(config.jobOpts.constraintSpec.attribute)
}

constraint.operand(config.jobOpts.constraintSpec.operator ?: "=")

if(config.jobOpts.constraintSpec.value){
constraint.rtarget(config.jobOpts.constraintSpec.value)
}
taskDef.constraints([constraint])
}

taskDef
}

Expand Down
17 changes: 17 additions & 0 deletions plugins/nf-nomad/src/test/nextflow/nomad/NomadConfigSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,21 @@ class NomadConfigSpec extends Specification {
config.jobOpts.affinitySpec.getValue() == '3'
config.jobOpts.affinitySpec.getWeight() == 50
}

void "should instantiate a constraint spec if specified"() {
when:
def config = new NomadConfig([
jobs: [constraint : {
attribute '${meta.my_custom_value}'
operator ">"
value "3"
}]
Comment on lines +187 to +192
Copy link
Member

Choose a reason for hiding this comment

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

For now, a direct mapping is great since it works with mental model of Nomad users as per the job definitions https://developer.hashicorp.com/nomad/docs/job-specification/constraint

Though for future, maybe this is something that maybe we can parse in a more user-friendly manner using a small DSL, which can have the added benefit that we can add more Nextflow level checks for the constraints being assigned correctly. What do you tihnk @jagedn , something worth tracking ?

Otherwise, in failing cases, Nomad layer would give an error code/message to the NF layer and we'd have to parse that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

totally agree

as reminder, we need also to work in the process configuration as current constraint and affinity is for all tasks

])

then:
config.jobOpts.constraintSpec
config.jobOpts.constraintSpec.getAttribute() == '${meta.my_custom_value}'
config.jobOpts.constraintSpec.getOperator() == '>'
config.jobOpts.constraintSpec.getValue() == '3'
}
}
Loading
Loading