diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 4e368386a083..5829d886188e 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -58,8 +58,8 @@ func NewJobEndpoints(s *Server) *Job { srv: s, logger: s.logger.Named("job"), mutators: []jobMutator{ - jobConnectHook{}, jobCanonicalizer{}, + jobConnectHook{}, jobImpliedConstraints{}, }, validators: []jobValidator{ diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go index 9a670c5d8a84..b3392f415fa6 100644 --- a/nomad/job_endpoint_hook_connect.go +++ b/nomad/job_endpoint_hook_connect.go @@ -59,7 +59,7 @@ func (jobConnectHook) Mutate(job *structs.Job) (_ *structs.Job, warnings []error continue } - if err := groupConnectHook(g); err != nil { + if err := groupConnectHook(job, g); err != nil { return nil, nil, err } } @@ -96,7 +96,7 @@ func isSidecarForService(t *structs.Task, svc string) bool { return t.Kind == structs.TaskKind(fmt.Sprintf("%s:%s", structs.ConnectProxyPrefix, svc)) } -func groupConnectHook(g *structs.TaskGroup) error { +func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error { for _, service := range g.Services { if service.Connect.HasSidecar() { // Check to see if the sidecar task already exists @@ -104,7 +104,7 @@ func groupConnectHook(g *structs.TaskGroup) error { // If the task doesn't already exist, create a new one and add it to the job if task == nil { - task = newConnectTask(service) + task = newConnectTask(service.Name) // If there happens to be a task defined with the same name // append an UUID fragment to the task name @@ -121,6 +121,9 @@ func groupConnectHook(g *structs.TaskGroup) error { service.Connect.SidecarTask.MergeIntoTask(task) } + // Canonicalize task since this mutator runs after job canonicalization + task.Canonicalize(job, g) + // port to be added for the sidecar task's proxy port port := structs.Port{ Label: fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, service.Name), @@ -147,11 +150,11 @@ func groupConnectHook(g *structs.TaskGroup) error { return nil } -func newConnectTask(service *structs.Service) *structs.Task { +func newConnectTask(serviceName string) *structs.Task { task := &structs.Task{ // Name is used in container name so must start with '[A-Za-z0-9]' - Name: fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, service.Name), - Kind: structs.TaskKind(fmt.Sprintf("%s:%s", structs.ConnectProxyPrefix, service.Name)), + Name: fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, serviceName), + Kind: structs.TaskKind(fmt.Sprintf("%s:%s", structs.ConnectProxyPrefix, serviceName)), Driver: "docker", Config: connectDriverConfig, ShutdownDelay: 5 * time.Second, diff --git a/nomad/job_endpoint_hook_connect_test.go b/nomad/job_endpoint_hook_connect_test.go index c8a3cb2c1957..6f4a614c28b5 100644 --- a/nomad/job_endpoint_hook_connect_test.go +++ b/nomad/job_endpoint_hook_connect_test.go @@ -4,6 +4,8 @@ import ( "fmt" "testing" + "github.com/hashicorp/nomad/helper/testlog" + "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" "github.com/stretchr/testify/require" ) @@ -49,7 +51,8 @@ func Test_isSidecarForService(t *testing.T) { func Test_groupConnectHook(t *testing.T) { // Test that connect-proxy task is inserted for backend service - tgIn := &structs.TaskGroup{ + job := mock.Job() + job.TaskGroups[0] = &structs.TaskGroup{ Networks: structs.Networks{ { Mode: "bridge", @@ -73,10 +76,10 @@ func Test_groupConnectHook(t *testing.T) { }, } - tgOut := tgIn.Copy() + tgOut := job.TaskGroups[0].Copy() tgOut.Tasks = []*structs.Task{ - newConnectTask(tgOut.Services[0]), - newConnectTask(tgOut.Services[1]), + newConnectTask(tgOut.Services[0].Name), + newConnectTask(tgOut.Services[1].Name), } tgOut.Networks[0].DynamicPorts = []structs.Port{ { @@ -89,10 +92,31 @@ func Test_groupConnectHook(t *testing.T) { }, } - require.NoError(t, groupConnectHook(tgIn)) - require.Exactly(t, tgOut, tgIn) + require.NoError(t, groupConnectHook(job, job.TaskGroups[0])) + require.Exactly(t, tgOut, job.TaskGroups[0]) // Test that hook is idempotent - require.NoError(t, groupConnectHook(tgIn)) - require.Exactly(t, tgOut, tgIn) + require.NoError(t, groupConnectHook(job, job.TaskGroups[0])) + require.Exactly(t, tgOut, job.TaskGroups[0]) +} + +// TestJobEndpoint_ConnectInterpolation asserts that when a Connect sidecar +// proxy task is being created for a group service with an interpolated name, +// the service name is interpolated *before the task is created. +// +// See https://github.com/hashicorp/nomad/issues/6853 +func TestJobEndpoint_ConnectInterpolation(t *testing.T) { + t.Parallel() + + server := &Server{logger: testlog.HCLogger(t)} + jobEndpoint := NewJobEndpoints(server) + + j := mock.ConnectJob() + j.TaskGroups[0].Services[0].Name = "${JOB}-api" + j, warnings, err := jobEndpoint.admissionMutators(j) + require.NoError(t, err) + require.Nil(t, warnings) + + require.Len(t, j.TaskGroups[0].Tasks, 2) + require.Equal(t, "connect-proxy-my-job-api", j.TaskGroups[0].Tasks[1].Name) } diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index 63eeb5b6a36d..896b3f208830 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -385,6 +385,11 @@ func MaxParallelJob() *structs.Job { func ConnectJob() *structs.Job { job := Job() tg := job.TaskGroups[0] + tg.Networks = []*structs.NetworkResource{ + { + Mode: "bridge", + }, + } tg.Services = []*structs.Service{ { Name: "testconnect", diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index cf3bb5443830..268adea98629 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -5552,6 +5552,8 @@ func (t *Task) Validate(ephemeralDisk *EphemeralDisk, jobType string, tgServices if t.Leader { mErr.Errors = append(mErr.Errors, fmt.Errorf("Connect proxy task must not have leader set")) } + + // Ensure the proxy task has a corresponding service entry serviceErr := ValidateConnectProxyService(t.Kind.Value(), tgServices) if serviceErr != nil { mErr.Errors = append(mErr.Errors, serviceErr) @@ -5746,7 +5748,9 @@ const ConnectProxyPrefix = "connect-proxy" // valid Connect config. func ValidateConnectProxyService(serviceName string, tgServices []*Service) error { found := false - for _, svc := range tgServices { + names := make([]string, len(tgServices)) + for i, svc := range tgServices { + names[i] = svc.Name if svc.Name == serviceName && svc.Connect != nil && svc.Connect.SidecarService != nil { found = true break @@ -5754,7 +5758,7 @@ func ValidateConnectProxyService(serviceName string, tgServices []*Service) erro } if !found { - return fmt.Errorf("Connect proxy service name not found in services from task group") + return fmt.Errorf("Connect proxy service name (%q) not found in services from task group: %s", serviceName, strings.Join(names, ",")) } return nil