Affinity struct, API and parsing #4512
Conversation
|
Thats really good stuff @preetapan - thank you! |
|
Known TODOs
|
command/agent/job_endpoint.go
Outdated
| @@ -892,3 +919,10 @@ func ApiConstraintToStructs(c1 *api.Constraint, c2 *structs.Constraint) { | |||
| c2.RTarget = c1.RTarget | |||
| c2.Operand = c1.Operand | |||
| } | |||
|
|
|||
| func ApiAffinityToStructs(a1 *api.Affinity, a2 *structs.Affinity) { | |||
There was a problem hiding this comment.
Why not have this return the structs affinity?
There was a problem hiding this comment.
I followed the pattern in the rest of this file, like we do for constraint above, but happy to fix this one and the upcoming equivalent method for spread.
jobspec/test-fixtures/basic.hcl
Outdated
| weight = 100 | ||
| } | ||
|
|
||
|
|
| @@ -228,6 +228,17 @@ func (tg *TaskGroup) Diff(other *TaskGroup, contextual bool) (*TaskGroupDiff, er | |||
| diff.Objects = append(diff.Objects, conDiff...) | |||
| } | |||
|
|
|||
| // Affinities diff | |||
| affinitiesDiff := primitiveObjectSetDiff( | |||
nomad/structs/structs.go
Outdated
| func (a *Affinity) Equal(o *Affinity) bool { | ||
| return a.LTarget == o.LTarget && | ||
| a.RTarget == o.RTarget && | ||
| a.Operand == o.Operand |
|
|
||
| // Ensure that weight is between -100 and 100, and not zero | ||
| if a.Weight == 0 { | ||
| mErr.Errors = append(mErr.Errors, fmt.Errorf("Affinity weight cannot be zero")) |
There was a problem hiding this comment.
Worth thinking about. It may be nice to be able to drop the affinity by just changing its weight to zero instead of having to comment out the whole stanza.
There was a problem hiding this comment.
Unless that's documented clearly, seems like it might lead to confusion as well with affinities being dropped silently. Is this a common pattern we use elsewhere for disabling things?
nomad/structs/structs.go
Outdated
| @@ -5251,6 +5287,89 @@ func (c *Constraint) Validate() error { | |||
| return mErr.ErrorOrNil() | |||
| } | |||
|
|
|||
| const ( | |||
| AffinitySetContainsAll = "set_contains_all" | |||
There was a problem hiding this comment.
Why introduce a new set_contains_all? Why can't we reuse ConstraintSetContains
There was a problem hiding this comment.
per our discussion both set_contains and set_contains_all` are accepted as equivalent operators.
There was a problem hiding this comment.
Should we rename to be ConstraintSetContainsAll/Any?
| Weight float64 // Weight applied to nodes that match the affinity. Can be negative | ||
| } | ||
|
|
||
| func NewAffinity(LTarget string, Operand string, RTarget string, Weight float64) *Affinity { |
There was a problem hiding this comment.
No need to change; just fyi: you can skip repeating the type if it's the same between arguments:
func NewAffinity(LTarget, Operand, RTarget string, Weight float64) *Affinity {
api/tasks_test.go
Outdated
|
|
||
| // Add an affinity to the task | ||
| out := task.AddAffinity(NewAffinity("kernel.version", "=", "4.6", 100)) | ||
| if n := len(task.Affinities); n != 1 { |
There was a problem hiding this comment.
require.Len is handy for this https://godoc.org/github.com/stretchr/testify/require#Len
nomad/structs/structs.go
Outdated
| @@ -2167,6 +2172,18 @@ func (j *Job) Validate() error { | |||
| mErr.Errors = append(mErr.Errors, outer) | |||
| } | |||
| } | |||
| if j.Type == JobTypeSystem { | |||
| if j.Affinities != nil { | |||
| mErr.Errors = append(mErr.Errors, fmt.Errorf("System jobs should not have an affinity stanza")) | |||
There was a problem hiding this comment.
should not have -> may not include
nomad/structs/structs.go
Outdated
| @@ -5251,6 +5287,89 @@ func (c *Constraint) Validate() error { | |||
| return mErr.ErrorOrNil() | |||
| } | |||
|
|
|||
| const ( | |||
| AffinitySetContainsAll = "set_contains_all" | |||
There was a problem hiding this comment.
Should we rename to be ConstraintSetContainsAll/Any?
|
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. |
This PR adds new struct for modelling affinities, which can be specified at the job/taskgroup/task level similar to constraints.
This also includes the API layer and parsing for affinities.