diff --git a/nomad/job_endpoint_hooks.go b/nomad/job_endpoint_hooks.go index c1df901af..61bb42b20 100644 --- a/nomad/job_endpoint_hooks.go +++ b/nomad/job_endpoint_hooks.go @@ -23,6 +23,17 @@ var ( RTarget: ">= 0.6.1", Operand: structs.ConstraintSemver, } + + // nativeServiceDiscoveryConstraint is the constraint injected into task + // groups that utilise Nomad's native service discovery feature. This is + // needed, as operators can disable the client functionality, and therefore + // we need to ensure task groups are placed where they can run + // successfully. + nativeServiceDiscoveryConstraint = &structs.Constraint{ + LTarget: "${attr.nomad.service_discovery}", + RTarget: "true", + Operand: "=", + } ) type admissionController interface { @@ -120,8 +131,11 @@ func (jobImpliedConstraints) Mutate(j *structs.Job) (*structs.Job, []error, erro // Get the required signals signals := j.RequiredSignals() + // Identify which task groups are utilising Nomad native service discovery. + nativeServiceDisco := j.RequiredNativeServiceDiscovery() + // Hot path - if len(signals) == 0 && len(policies) == 0 { + if len(signals) == 0 && len(policies) == 0 && len(nativeServiceDisco) == 0 { return j, nil, nil } @@ -171,6 +185,25 @@ func (jobImpliedConstraints) Mutate(j *structs.Job) (*structs.Job, []error, erro } } + // Add the Nomad service discovery constraints. + for _, tg := range j.TaskGroups { + if ok := nativeServiceDisco[tg.Name]; !ok { + continue + } + + found := false + for _, c := range tg.Constraints { + if c.Equals(nativeServiceDiscoveryConstraint) { + found = true + break + } + } + + if !found { + tg.Constraints = append(tg.Constraints, nativeServiceDiscoveryConstraint) + } + } + return j, nil, nil } diff --git a/nomad/job_endpoint_hooks_test.go b/nomad/job_endpoint_hooks_test.go new file mode 100644 index 000000000..8e146972a --- /dev/null +++ b/nomad/job_endpoint_hooks_test.go @@ -0,0 +1,169 @@ +package nomad + +import ( + "testing" + + "github.com/hashicorp/nomad/nomad/structs" + "github.com/stretchr/testify/require" +) + +func Test_jobImpliedConstraints_Mutate(t *testing.T) { + t.Parallel() + + testCases := []struct { + inputJob *structs.Job + expectedOutputJob *structs.Job + expectedOutputWarnings []error + expectedOutputError error + name string + }{ + { + inputJob: &structs.Job{ + Name: "example", + TaskGroups: []*structs.TaskGroup{ + { + Name: "example-group-1", + }, + }, + }, + expectedOutputJob: &structs.Job{ + Name: "example", + TaskGroups: []*structs.TaskGroup{ + { + Name: "example-group-1", + }, + }, + }, + expectedOutputWarnings: nil, + expectedOutputError: nil, + name: "no needed constraints", + }, + { + inputJob: &structs.Job{ + Name: "example", + TaskGroups: []*structs.TaskGroup{ + { + Name: "example-group-1", + Services: []*structs.Service{ + { + Name: "example-group-service-1", + Provider: structs.ServiceProviderNomad, + }, + }, + }, + }, + }, + expectedOutputJob: &structs.Job{ + Name: "example", + TaskGroups: []*structs.TaskGroup{ + { + Name: "example-group-1", + Services: []*structs.Service{ + { + Name: "example-group-service-1", + Provider: structs.ServiceProviderNomad, + }, + }, + Constraints: []*structs.Constraint{nativeServiceDiscoveryConstraint}, + }, + }, + }, + expectedOutputWarnings: nil, + expectedOutputError: nil, + name: "task group nomad discovery", + }, + { + inputJob: &structs.Job{ + Name: "example", + TaskGroups: []*structs.TaskGroup{ + { + Name: "example-group-1", + Services: []*structs.Service{ + { + Name: "example-group-service-1", + Provider: structs.ServiceProviderNomad, + }, + }, + Constraints: []*structs.Constraint{nativeServiceDiscoveryConstraint}, + }, + }, + }, + expectedOutputJob: &structs.Job{ + Name: "example", + TaskGroups: []*structs.TaskGroup{ + { + Name: "example-group-1", + Services: []*structs.Service{ + { + Name: "example-group-service-1", + Provider: structs.ServiceProviderNomad, + }, + }, + Constraints: []*structs.Constraint{nativeServiceDiscoveryConstraint}, + }, + }, + }, + expectedOutputWarnings: nil, + expectedOutputError: nil, + name: "task group nomad discovery constraint found", + }, + { + inputJob: &structs.Job{ + Name: "example", + TaskGroups: []*structs.TaskGroup{ + { + Name: "example-group-1", + Services: []*structs.Service{ + { + Name: "example-group-service-1", + Provider: structs.ServiceProviderNomad, + }, + }, + Constraints: []*structs.Constraint{ + { + LTarget: "${node.class}", + RTarget: "high-memory", + Operand: "=", + }, + }, + }, + }, + }, + expectedOutputJob: &structs.Job{ + Name: "example", + TaskGroups: []*structs.TaskGroup{ + { + Name: "example-group-1", + Services: []*structs.Service{ + { + Name: "example-group-service-1", + Provider: structs.ServiceProviderNomad, + }, + }, + Constraints: []*structs.Constraint{ + { + LTarget: "${node.class}", + RTarget: "high-memory", + Operand: "=", + }, + nativeServiceDiscoveryConstraint, + }, + }, + }, + }, + expectedOutputWarnings: nil, + expectedOutputError: nil, + name: "task group nomad discovery other constraints", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + impl := jobImpliedConstraints{} + actualJob, actualWarnings, actualError := impl.Mutate(tc.inputJob) + require.Equal(t, tc.expectedOutputJob, actualJob) + require.ElementsMatch(t, tc.expectedOutputWarnings, actualWarnings) + require.Equal(t, tc.expectedOutputError, actualError) + }) + } +} diff --git a/nomad/structs/job.go b/nomad/structs/job.go index 07c79023a..7d638f51d 100644 --- a/nomad/structs/job.go +++ b/nomad/structs/job.go @@ -22,3 +22,41 @@ type JobServiceRegistrationsResponse struct { Services []*ServiceRegistration QueryMeta } + +// RequiredNativeServiceDiscovery identifies which task groups, if any, within +// the job are utilising Nomad native service discovery. +func (j *Job) RequiredNativeServiceDiscovery() map[string]bool { + groups := make(map[string]bool) + + for _, tg := range j.TaskGroups { + + // It is possible for services using the Nomad provider to be + // configured at the task group level, so check here first. + if requiresNativeServiceDiscovery(tg.Services) { + groups[tg.Name] = true + continue + } + + // Iterate the tasks within the task group to check the services + // configured at this more traditional level. + for _, task := range tg.Tasks { + if requiresNativeServiceDiscovery(task.Services) { + groups[tg.Name] = true + continue + } + } + } + + return groups +} + +// requiresNativeServiceDiscovery identifies whether any of the services passed +// to the function are utilising Nomad native service discovery. +func requiresNativeServiceDiscovery(services []*Service) bool { + for _, tgService := range services { + if tgService.Provider == ServiceProviderNomad { + return true + } + } + return false +} diff --git a/nomad/structs/job_test.go b/nomad/structs/job_test.go index e64339510..254c315b1 100644 --- a/nomad/structs/job_test.go +++ b/nomad/structs/job_test.go @@ -10,3 +10,147 @@ func TestServiceRegistrationsRequest_StaleReadSupport(t *testing.T) { req := &AllocServiceRegistrationsRequest{} require.True(t, req.IsRead()) } + +func TestJob_RequiresNativeServiceDiscovery(t *testing.T) { + testCases := []struct { + inputJob *Job + expectedOutput map[string]bool + name string + }{ + { + inputJob: &Job{ + TaskGroups: []*TaskGroup{ + { + Name: "group1", + Services: []*Service{ + {Provider: "nomad"}, + {Provider: "nomad"}, + }, + }, + { + Name: "group2", + Services: []*Service{ + {Provider: "nomad"}, + {Provider: "nomad"}, + }, + }, + }, + }, + expectedOutput: map[string]bool{"group1": true, "group2": true}, + name: "multiple group services with Nomad provider", + }, + { + inputJob: &Job{ + TaskGroups: []*TaskGroup{ + { + Name: "group1", + Tasks: []*Task{ + { + Services: []*Service{ + {Provider: "nomad"}, + {Provider: "nomad"}, + }, + }, + { + Services: []*Service{ + {Provider: "nomad"}, + {Provider: "nomad"}, + }, + }, + }, + }, + { + Name: "group2", + Tasks: []*Task{ + { + Services: []*Service{ + {Provider: "nomad"}, + {Provider: "nomad"}, + }, + }, + { + Services: []*Service{ + {Provider: "nomad"}, + {Provider: "nomad"}, + }, + }, + }, + }, + }, + }, + expectedOutput: map[string]bool{"group1": true, "group2": true}, + name: "multiple task services with Nomad provider", + }, + { + inputJob: &Job{ + TaskGroups: []*TaskGroup{ + { + Name: "group1", + Services: []*Service{ + {Provider: "consul"}, + {Provider: "consul"}, + }, + }, + { + Name: "group2", + Services: []*Service{ + {Provider: "consul"}, + {Provider: "consul"}, + }, + }, + }, + }, + expectedOutput: map[string]bool{}, + name: "multiple group services with Consul provider", + }, + { + inputJob: &Job{ + TaskGroups: []*TaskGroup{ + { + Name: "group1", + Tasks: []*Task{ + { + Services: []*Service{ + {Provider: "consul"}, + {Provider: "consul"}, + }, + }, + { + Services: []*Service{ + {Provider: "consul"}, + {Provider: "consul"}, + }, + }, + }, + }, + { + Name: "group2", + Tasks: []*Task{ + { + Services: []*Service{ + {Provider: "consul"}, + {Provider: "consul"}, + }, + }, + { + Services: []*Service{ + {Provider: "consul"}, + {Provider: "consul"}, + }, + }, + }, + }, + }, + }, + expectedOutput: map[string]bool{}, + name: "multiple task services with Consul provider", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actualOutput := tc.inputJob.RequiredNativeServiceDiscovery() + require.Equal(t, tc.expectedOutput, actualOutput) + }) + } +}