Merge f0851bc into backport/b-consul-task-service-constraint-missing/…

…seriously-supreme-alpaca

.changelog/22229.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+job endpoint: fix implicit constraint mutation for task-level services
+```

nomad/job_endpoint_hooks.go

@@ -333,12 +333,12 @@ func (jobImpliedConstraints) Mutate(j *structs.Job) (*structs.Job, []error, erro
 				if service.IsConsul() {
 					mutateConstraint(constraintMatcherLeft, tg, consulConstraintFn(service))
 				}
+			}
 
-				for _, task := range tg.Tasks {
-					for _, service := range task.Services {
-						if service.IsConsul() {
-							mutateConstraint(constraintMatcherLeft, tg, consulConstraintFn(service))
-						}
+			for _, task := range tg.Tasks {
+				for _, service := range task.Services {
+					if service.IsConsul() {
+						mutateConstraint(constraintMatcherLeft, task, consulConstraintFn(service))
 					}
 				}
 			}
@@ -414,9 +414,17 @@ const (
 	constraintMatcherLeft
 )
 
+// both Tasks and TaskGroups can have constraints, and since current (1.22) Go
+// still doesn't allow us accessing fields of generic type structs, we have to
+// resort to an interface
+type hasConstraints interface {
+	GetConstraints() []*structs.Constraint
+	SetConstraints([]*structs.Constraint)
+}
+
 // mutateConstraint is a generic mutator used to set implicit constraints
 // within the task group if they are needed.
-func mutateConstraint(matcher constraintMatcher, taskGroup *structs.TaskGroup, constraint *structs.Constraint) {
+func mutateConstraint[T hasConstraints](matcher constraintMatcher, taskOrTG T, constraint *structs.Constraint) {
 
 	var found bool
 
@@ -425,14 +433,14 @@ func mutateConstraint(matcher constraintMatcher, taskGroup *structs.TaskGroup, c
 	// therefore we do it here.
 	switch matcher {
 	case constraintMatcherFull:
-		for _, c := range taskGroup.Constraints {
+		for _, c := range taskOrTG.GetConstraints() {
 			if c.Equal(constraint) {
 				found = true
 				break
 			}
 		}
 	case constraintMatcherLeft:
-		for _, c := range taskGroup.Constraints {
+		for _, c := range taskOrTG.GetConstraints() {
 			if c.LTarget == constraint.LTarget {
 				found = true
 				break
@@ -442,7 +450,9 @@ func mutateConstraint(matcher constraintMatcher, taskGroup *structs.TaskGroup, c
 
 	// If we didn't find a suitable constraint match, add one.
 	if !found {
-		taskGroup.Constraints = append(taskGroup.Constraints, constraint)
+		constraints := taskOrTG.GetConstraints()
+		constraints = append(constraints, constraint)
+		taskOrTG.SetConstraints(constraints)
 	}
 }
 

nomad/job_endpoint_hooks_test.go

@@ -1117,6 +1117,48 @@ func Test_jobImpliedConstraints_Mutate(t *testing.T) {
 			expectedOutputWarnings: nil,
 			expectedOutputError:    nil,
 		},
+		{
+			name: "task-level service",
+			inputJob: &structs.Job{
+				Name: "example",
+				TaskGroups: []*structs.TaskGroup{
+					{
+						Name: "example-group-1",
+						Tasks: []*structs.Task{
+							{
+								Name: "example-task-1",
+								Services: []*structs.Service{
+									{
+										Name: "example-task-service-1",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedOutputJob: &structs.Job{
+				Name: "example",
+				TaskGroups: []*structs.TaskGroup{
+					{
+						Name: "example-group-1",
+						Tasks: []*structs.Task{
+							{
+								Name: "example-task-1",
+								Services: []*structs.Service{
+									{
+										Name: "example-task-service-1",
+									},
+								},
+								Constraints: []*structs.Constraint{consulServiceDiscoveryConstraint},
+							},
+						},
+					},
+				},
+			},
+			expectedOutputWarnings: nil,
+			expectedOutputError:    nil,
+		},
 		{
 			name: "task group with numa block",
 			inputJob: &structs.Job{

nomad/job_endpoint_test.go

@@ -1662,12 +1662,13 @@ func TestJobEndpoint_Register_Vault_OverrideConstraint(t *testing.T) {
 		Policies:   []string{"foo"},
 		ChangeMode: structs.VaultChangeModeRestart,
 	}
-	job.TaskGroups[0].Tasks[0].Constraints = []*structs.Constraint{
-		{
-			LTarget: "${attr.vault.version}",
-			Operand: "is_set",
-		},
+
+	vaultConstraint := &structs.Constraint{
+		LTarget: "${attr.vault.version}",
+		Operand: "is_set",
 	}
+	job.TaskGroups[0].Tasks[0].Constraints = []*structs.Constraint{vaultConstraint}
+
 	req := &structs.JobRegisterRequest{
 		Job: job,
 		WriteRequest: structs.WriteRequest{
@@ -1679,20 +1680,24 @@ func TestJobEndpoint_Register_Vault_OverrideConstraint(t *testing.T) {
 	// Fetch the response
 	var resp structs.JobRegisterResponse
 	err := msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp)
-	require.NoError(t, err)
+	must.NoError(t, err)
 
 	// Check for the job in the FSM
 	state := s1.fsm.State()
 	ws := memdb.NewWatchSet()
 	out, err := state.JobByID(ws, job.Namespace, job.ID)
-	require.NoError(t, err)
-	require.NotNil(t, out)
-	require.Equal(t, resp.JobModifyIndex, out.CreateIndex)
+	must.NoError(t, err)
+	must.NotNil(t, out)
+	must.Eq(t, resp.JobModifyIndex, out.CreateIndex)
 
 	// Assert constraint was not overridden by the server
 	outConstraints := out.TaskGroups[0].Tasks[0].Constraints
-	require.Len(t, outConstraints, 1)
-	require.True(t, job.TaskGroups[0].Tasks[0].Constraints[0].Equal(outConstraints[0]))
+	for _, constraint := range outConstraints {
+		if constraint.LTarget == "${attr.vault.version}" {
+			must.Eq(t, constraint, vaultConstraint)
+		}
+	}
+	must.True(t, job.TaskGroups[0].Tasks[0].Constraints[0].Equal(outConstraints[0]))
 }
 
 func TestJobEndpoint_Register_Vault_NoToken(t *testing.T) {

nomad/mock/job.go

@@ -89,6 +89,13 @@ func Job() *structs.Job {
 								Args:    []string{"hello world"},
 							},
 						},
+						Constraints: []*structs.Constraint{
+							{
+								LTarget: "${attr.consul.version}",
+								RTarget: ">= 1.8.0",
+								Operand: structs.ConstraintSemver,
+							},
+						},
 						Services: []*structs.Service{
 							{
 								Name:      "${TASK}-frontend",

nomad/structs/structs.go

@@ -7543,6 +7543,14 @@ func (tg *TaskGroup) GetDisconnectStopTimeout() *time.Duration {
 	return nil
 }
 
+func (tg *TaskGroup) GetConstraints() []*Constraint {
+	return tg.Constraints
+}
+
+func (tg *TaskGroup) SetConstraints(newConstraints []*Constraint) {
+	tg.Constraints = newConstraints
+}
+
 // CheckRestart describes if and when a task should be restarted based on
 // failing health checks.
 type CheckRestart struct {
@@ -8377,6 +8385,14 @@ func (t *Task) Warnings() error {
 	return mErr.ErrorOrNil()
 }
 
+func (t *Task) GetConstraints() []*Constraint {
+	return t.Constraints
+}
+
+func (t *Task) SetConstraints(newConstraints []*Constraint) {
+	t.Constraints = newConstraints
+}
+
 // TaskKind identifies the special kinds of tasks using the following format:
 // '<kind_name>(:<identifier>)`. The TaskKind can optionally include an identifier that
 // is opaque to the Task. This identifier can be used to relate the task to some