From f907c42ba541998727cbcebf07f8e95774a8d045 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Tue, 24 Jul 2018 10:37:13 -0500 Subject: [PATCH] More review comments --- api/tasks_test.go | 6 +++--- jobspec/parse.go | 8 ++++---- nomad/structs/structs.go | 15 ++++++--------- nomad/structs/structs_test.go | 2 +- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/api/tasks_test.go b/api/tasks_test.go index 6ccfe7489..7c23c577c 100644 --- a/api/tasks_test.go +++ b/api/tasks_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/nomad/structs" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestTaskGroup_NewTaskGroup(t *testing.T) { @@ -274,9 +275,8 @@ func TestTask_AddAffinity(t *testing.T) { // Add an affinity to the task out := task.AddAffinity(NewAffinity("kernel.version", "=", "4.6", 100)) - if n := len(task.Affinities); n != 1 { - t.Fatalf("expected 1 affinity, got: %d", n) - } + require := require.New(t) + require.Len(out, 1) // Check that the task was returned if out != task { diff --git a/jobspec/parse.go b/jobspec/parse.go index 464da857a..573a05f27 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -637,15 +637,15 @@ func parseAffinities(result *[]*api.Affinity, list *ast.ObjectList) error { // If "set_contains_any" is provided, set the operand // to "set_contains_any" and the value to the "RTarget" - if affinity, ok := m[structs.AffinitySetContainsAny]; ok { - m["Operand"] = structs.AffinitySetContainsAny + if affinity, ok := m[structs.ConstraintSetContaintsAny]; ok { + m["Operand"] = structs.ConstraintSetContaintsAny m["RTarget"] = affinity } // If "set_contains_all" is provided, set the operand // to "set_contains_all" and the value to the "RTarget" - if affinity, ok := m[structs.AffinitySetContainsAll]; ok { - m["Operand"] = structs.AffinitySetContainsAll + if affinity, ok := m[structs.ConstraintSetContainsAll]; ok { + m["Operand"] = structs.ConstraintSetContainsAll m["RTarget"] = affinity } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 77f6e1b4d..b7fbb3831 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -2174,7 +2174,7 @@ func (j *Job) Validate() error { } if j.Type == JobTypeSystem { if j.Affinities != nil { - mErr.Errors = append(mErr.Errors, fmt.Errorf("System jobs should not have an affinity stanza")) + mErr.Errors = append(mErr.Errors, fmt.Errorf("System jobs may not have an affinity stanza")) } } else { for idx, affinity := range j.Affinities { @@ -3431,7 +3431,7 @@ func (tg *TaskGroup) Validate(j *Job) error { } if j.Type == JobTypeSystem { if tg.Affinities != nil { - mErr.Errors = append(mErr.Errors, fmt.Errorf("System jobs should not have an affinity stanza")) + mErr.Errors = append(mErr.Errors, fmt.Errorf("System jobs may not have an affinity stanza")) } } else { for idx, affinity := range tg.Affinities { @@ -4230,7 +4230,7 @@ func (t *Task) Validate(ephemeralDisk *EphemeralDisk, jobType string) error { if jobType == JobTypeSystem { if t.Affinities != nil { - mErr.Errors = append(mErr.Errors, fmt.Errorf("System jobs should not have an affinity stanza")) + mErr.Errors = append(mErr.Errors, fmt.Errorf("System jobs may not have an affinity stanza")) } } else { for idx, affinity := range t.Affinities { @@ -5217,6 +5217,8 @@ const ( ConstraintRegex = "regexp" ConstraintVersion = "version" ConstraintSetContains = "set_contains" + ConstraintSetContainsAll = "set_contains_all" + ConstraintSetContaintsAny = "set_contains_any" ) // Constraints are used to restrict placement options. @@ -5303,11 +5305,6 @@ func (c *Constraint) Validate() error { return mErr.ErrorOrNil() } -const ( - AffinitySetContainsAll = "set_contains_all" - AffinitySetContainsAny = "set_contains_any" -) - // Affinity is used to score placement options based on a weight type Affinity struct { LTarget string // Left-hand target @@ -5350,7 +5347,7 @@ func (a *Affinity) Validate() error { // Perform additional validation based on operand switch a.Operand { - case AffinitySetContainsAll, AffinitySetContainsAny, ConstraintSetContains: + case ConstraintSetContainsAll, ConstraintSetContaintsAny, ConstraintSetContains: if a.RTarget == "" { mErr.Errors = append(mErr.Errors, fmt.Errorf("Set contains operators require an RTarget")) } diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 60ef60349..f8a97a3bc 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -404,7 +404,7 @@ func TestJob_SystemJob_Validate(t *testing.T) { }} err = j.Validate() require.NotNil(t, err) - require.Contains(t, err.Error(), "System jobs should not have an affinity stanza") + require.Contains(t, err.Error(), "System jobs may not have an affinity stanza") } func TestJob_VaultPolicies(t *testing.T) {