Respond to comments

nomad/structs/node_class.go

@@ -84,15 +84,10 @@ func constraintTargetEscapes(target string) bool {
 	switch {
 	case strings.HasPrefix(target, "$node.unique."):
 		return true
-
+	case strings.HasPrefix(target, "$attr.unique."):
-	case strings.HasPrefix(target, "$attr."):
+		return true
-		attr := strings.TrimPrefix(target, "$attr.")
+	case strings.HasPrefix(target, "$meta.unique."):
-		return strings.HasPrefix(attr, NodeUniqueNamespace)
+		return true
-
-	case strings.HasPrefix(target, "$meta."):
-		meta := strings.TrimPrefix(target, "$meta.")
-		return strings.HasPrefix(meta, NodeUniqueNamespace)
-
 	default:
 		return false
 	}

nomad/structs/structs.go

@@ -496,9 +496,6 @@ type Node struct {
 	// client. This is opaque to Nomad.
 	Meta map[string]string
 
-	// UniqueAttributes are attributes that uniquely identify a node.
-	UniqueAttributes map[string]struct{}
-
 	// NodeClass is an opaque identifier used to group nodes
 	// together for the purpose of determining scheduling pressure.
 	NodeClass string

scheduler/context.go

@@ -138,41 +138,51 @@ func (e *EvalContext) Eligibility() *EvalEligibility {
 	return e.eligibility
 }
 
-type ComputedClassEligibility byte
+type ComputedClassFeasibility byte
 
 const (
-	// The EvalComputedClass enums denote the eligibility of the computed class
+	// EvalComputedClassUnknown is the initial state until the eligibility has
-	// for the evaluation.
+	// been explicitely marked to eligible/ineligible or escaped.
-	EvalComputedClassUnknown ComputedClassEligibility = iota
+	EvalComputedClassUnknown ComputedClassFeasibility = iota
+
+	// EvalComputedClassIneligible is used to mark the computed class as
+	// ineligible for the evaluation.
 	EvalComputedClassIneligible
+
+	// EvalComputedClassIneligible is used to mark the computed class as
+	// eligible for the evaluation.
 	EvalComputedClassEligible
+
+	// EvalComputedClassEscaped signals that computed class can not determine
+	// eligibility because a constraint exists that is not captured by computed
+	// node classes.
 	EvalComputedClassEscaped
 )
 
 // EvalEligibility tracks eligibility of nodes by computed node class over the
 // course of an evaluation.
 type EvalEligibility struct {
-	// Job tracks the eligibility at the job level per computed node class.
+	// job tracks the eligibility at the job level per computed node class.
-	Job map[uint64]ComputedClassEligibility
+	job map[uint64]ComputedClassFeasibility
 
-	// JobEscapedConstraints tracks escaped constraints at the job level.
+	// jobEscapedConstraints tracks escaped constraints at the job level.
-	JobEscapedConstraints []*structs.Constraint
+	jobEscapedConstraints []*structs.Constraint
 
-	// TaskGroups tracks the eligibility at the task group level per computed
+	// taskGroups tracks the eligibility at the task group level per computed
 	// node class.
-	TaskGroups map[string]map[uint64]ComputedClassEligibility
+	taskGroups map[string]map[uint64]ComputedClassFeasibility
 
-	// TgEscapedConstraints is a map of task groups to a set of constraints that
+	// tgEscapedConstraints is a map of task groups to a set of constraints that
 	// have escaped.
-	TgEscapedConstraints map[string][]*structs.Constraint
+	tgEscapedConstraints map[string][]*structs.Constraint
 }
 
 // NewEvalEligibility returns an eligibility tracker for the context of an evaluation.
 func NewEvalEligibility() *EvalEligibility {
 	return &EvalEligibility{
-		Job:                  make(map[uint64]ComputedClassEligibility),
+		job:                  make(map[uint64]ComputedClassFeasibility),
-		TaskGroups:           make(map[string]map[uint64]ComputedClassEligibility),
+		taskGroups:           make(map[string]map[uint64]ComputedClassFeasibility),
-		TgEscapedConstraints: make(map[string][]*structs.Constraint),
+		tgEscapedConstraints: make(map[string][]*structs.Constraint),
 	}
 }
 
@@ -180,7 +190,7 @@ func NewEvalEligibility() *EvalEligibility {
 // at the job and task group level.
 func (e *EvalEligibility) SetJob(job *structs.Job) {
 	// Determine the escaped constraints for the job.
-	e.JobEscapedConstraints = structs.EscapedConstraints(job.Constraints)
+	e.jobEscapedConstraints = structs.EscapedConstraints(job.Constraints)
 
 	// Determine the escaped constraints per task group.
 	for _, tg := range job.TaskGroups {
@@ -189,17 +199,20 @@ func (e *EvalEligibility) SetJob(job *structs.Job) {
 			constraints = append(constraints, task.Constraints...)
 		}
 
-		e.TgEscapedConstraints[tg.Name] = structs.EscapedConstraints(constraints)
+		e.tgEscapedConstraints[tg.Name] = structs.EscapedConstraints(constraints)
 	}
 }
 
 // JobStatus returns the eligibility status of the job.
-func (e *EvalEligibility) JobStatus(class uint64) ComputedClassEligibility {
+func (e *EvalEligibility) JobStatus(class uint64) ComputedClassFeasibility {
-	if len(e.JobEscapedConstraints) != 0 {
+	// COMPAT: Computed node class was introduced in 0.3. Clients running < 0.3
+	// will not have a computed class. The safest value to return is the escaped
+	// case, since it disables any optimization.
+	if len(e.jobEscapedConstraints) != 0 || class == 0 {
 		return EvalComputedClassEscaped
 	}
 
-	if status, ok := e.Job[class]; ok {
+	if status, ok := e.job[class]; ok {
 		return status
 	}
 	return EvalComputedClassUnknown
@@ -209,21 +222,28 @@ func (e *EvalEligibility) JobStatus(class uint64) ComputedClassEligibility {
 // node class.
 func (e *EvalEligibility) SetJobEligibility(eligible bool, class uint64) {
 	if eligible {
-		e.Job[class] = EvalComputedClassEligible
+		e.job[class] = EvalComputedClassEligible
 	} else {
-		e.Job[class] = EvalComputedClassIneligible
+		e.job[class] = EvalComputedClassIneligible
 	}
 }
 
 // TaskGroupStatus returns the eligibility status of the task group.
-func (e *EvalEligibility) TaskGroupStatus(tg string, class uint64) ComputedClassEligibility {
+func (e *EvalEligibility) TaskGroupStatus(tg string, class uint64) ComputedClassFeasibility {
-	if escaped, ok := e.TgEscapedConstraints[tg]; ok {
+	// COMPAT: Computed node class was introduced in 0.3. Clients running < 0.3
+	// will not have a computed class. The safest value to return is the escaped
+	// case, since it disables any optimization.
+	if class == 0 {
+		return EvalComputedClassEscaped
+	}
+
+	if escaped, ok := e.tgEscapedConstraints[tg]; ok {
 		if len(escaped) != 0 {
 			return EvalComputedClassEscaped
 		}
 	}
 
-	if classes, ok := e.TaskGroups[tg]; ok {
+	if classes, ok := e.taskGroups[tg]; ok {
 		if status, ok := classes[class]; ok {
 			return status
 		}
@@ -234,16 +254,16 @@ func (e *EvalEligibility) TaskGroupStatus(tg string, class uint64) ComputedClass
 // SetTaskGroupEligibility sets the eligibility status of the task group for the
 // computed node class.
 func (e *EvalEligibility) SetTaskGroupEligibility(eligible bool, tg string, class uint64) {
-	var eligibility ComputedClassEligibility
+	var eligibility ComputedClassFeasibility
 	if eligible {
 		eligibility = EvalComputedClassEligible
 	} else {
 		eligibility = EvalComputedClassIneligible
 	}
 
-	if classes, ok := e.TaskGroups[tg]; ok {
+	if classes, ok := e.taskGroups[tg]; ok {
 		classes[class] = eligibility
 	} else {
-		e.TaskGroups[tg] = map[uint64]ComputedClassEligibility{class: eligibility}
+		e.taskGroups[tg] = map[uint64]ComputedClassFeasibility{class: eligibility}
 	}
 }

scheduler/feasible.go

@@ -497,7 +497,7 @@ OUTER:
 		}
 
 		// Check if the job has been marked as eligible or ineligible.
-		jobEscaped := false
+		jobEscaped, jobUnknown := false, false
 		switch evalElig.JobStatus(option.ComputedClass) {
 		case EvalComputedClassIneligible:
 			// Fast path the ineligible case
@@ -505,6 +505,8 @@ OUTER:
 			continue
 		case EvalComputedClassEscaped:
 			jobEscaped = true
+		case EvalComputedClassUnknown:
+			jobUnknown = true
 		}
 
 		// Run the job feasibility checks.
@@ -520,13 +522,14 @@ OUTER:
 			}
 		}
 
-		// Set the job eligibility if the constraints weren't escaped.
+		// Set the job eligibility if the constraints weren't escaped and it
-		if !jobEscaped {
+		// hasn't been set before.
+		if !jobEscaped && jobUnknown {
 			evalElig.SetJobEligibility(true, option.ComputedClass)
 		}
 
 		// Check if the task group has been marked as eligible or ineligible.
-		tgEscaped := false
+		tgEscaped, tgUnknown := false, false
 		switch evalElig.TaskGroupStatus(w.tg, option.ComputedClass) {
 		case EvalComputedClassIneligible:
 			// Fast path the ineligible case
@@ -537,6 +540,8 @@ OUTER:
 			return option
 		case EvalComputedClassEscaped:
 			tgEscaped = true
+		case EvalComputedClassUnknown:
+			tgUnknown = true
 		}
 
 		// Run the task group feasibility checks.
@@ -552,8 +557,9 @@ OUTER:
 			}
 		}
 
-		// Set the task group eligibility if the constraints weren't escaped.
+		// Set the task group eligibility if the constraints weren't escaped and
-		if !tgEscaped {
+		// it hasn't been set before.
+		if !tgEscaped && tgUnknown {
 			evalElig.SetTaskGroupEligibility(true, w.tg, option.ComputedClass)
 		}
 

scheduler/feasible_test.go

@@ -661,7 +661,7 @@ func TestFeasibilityWrapper_JobEscapes(t *testing.T) {
 
 	// Set the job to escaped
 	cc := nodes[0].ComputedClass
-	ctx.Eligibility().Job[cc] = EvalComputedClassEscaped
+	ctx.Eligibility().job[cc] = EvalComputedClassEscaped
 
 	// Run the wrapper.
 	out := collectFeasible(wrapper)
@@ -687,7 +687,7 @@ func TestFeasibilityWrapper_JobAndTg_Eligible(t *testing.T) {
 
 	// Set the job to escaped
 	cc := nodes[0].ComputedClass
-	ctx.Eligibility().Job[cc] = EvalComputedClassEligible
+	ctx.Eligibility().job[cc] = EvalComputedClassEligible
 	ctx.Eligibility().SetTaskGroupEligibility(true, "foo", cc)
 	wrapper.SetTaskGroup("foo")
 
@@ -709,7 +709,7 @@ func TestFeasibilityWrapper_JobEligible_TgIneligible(t *testing.T) {
 
 	// Set the job to escaped
 	cc := nodes[0].ComputedClass
-	ctx.Eligibility().Job[cc] = EvalComputedClassEligible
+	ctx.Eligibility().job[cc] = EvalComputedClassEligible
 	ctx.Eligibility().SetTaskGroupEligibility(false, "foo", cc)
 	wrapper.SetTaskGroup("foo")
 
@@ -731,9 +731,9 @@ func TestFeasibilityWrapper_JobEligible_TgEscaped(t *testing.T) {
 
 	// Set the job to escaped
 	cc := nodes[0].ComputedClass
-	ctx.Eligibility().Job[cc] = EvalComputedClassEligible
+	ctx.Eligibility().job[cc] = EvalComputedClassEligible
-	ctx.Eligibility().TaskGroups["foo"] =
+	ctx.Eligibility().taskGroups["foo"] =
-		map[uint64]ComputedClassEligibility{cc: EvalComputedClassEscaped}
+		map[uint64]ComputedClassFeasibility{cc: EvalComputedClassEscaped}
 	wrapper.SetTaskGroup("foo")
 
 	// Run the wrapper.
@@ -743,7 +743,7 @@ func TestFeasibilityWrapper_JobEligible_TgEscaped(t *testing.T) {
 		t.Fatalf("bad: %#v %v", out, tgMock.calls())
 	}
 
-	if e, ok := ctx.Eligibility().TaskGroups["foo"][cc]; !ok || e != EvalComputedClassEscaped {
+	if e, ok := ctx.Eligibility().taskGroups["foo"][cc]; !ok || e != EvalComputedClassEscaped {
 		t.Fatalf("bad: %v %v", e, ok)
 	}
 }

scheduler/stack.go

@@ -73,7 +73,10 @@ func NewGenericStack(batch bool, ctx Context) *GenericStack {
 	// Filter on task group constraints second
 	s.taskGroupConstraint = NewConstraintChecker(ctx, nil)
 
-	// Create the feasibility wrapper.
+	// Create the feasibility wrapper which wraps all feasibility checks in
+	// which feasibility checking can be skipped if the computed node class has
+	// previously been marked as eligible or ineligible. Generally this will be
+	// checks that only needs to examine the single node to determine feasibility.
 	jobs := []FeasibilityChecker{s.jobConstraint}
 	tgs := []FeasibilityChecker{s.taskGroupDrivers, s.taskGroupConstraint}
 	s.wrappedChecks = NewFeasibilityWrapper(ctx, s.source, jobs, tgs)
@@ -197,7 +200,10 @@ func NewSystemStack(ctx Context) *SystemStack {
 	// Filter on task group constraints second
 	s.taskGroupConstraint = NewConstraintChecker(ctx, nil)
 
-	// Create the feasibility wrapper.
+	// Create the feasibility wrapper which wraps all feasibility checks in
+	// which feasibility checking can be skipped if the computed node class has
+	// previously been marked as eligible or ineligible. Generally this will be
+	// checks that only needs to examine the single node to determine feasibility.
 	jobs := []FeasibilityChecker{s.jobConstraint}
 	tgs := []FeasibilityChecker{s.taskGroupDrivers, s.taskGroupConstraint}
 	s.wrappedChecks = NewFeasibilityWrapper(ctx, s.source, jobs, tgs)