Skip to content

Commit

Permalink
consul/connect: infer task name in service if possible
Browse files Browse the repository at this point in the history
Before, the service definition for a Connect Native service would always
require setting the `service.task` parameter. Now, that parameter is
automatically inferred when there is only one task in the task group.

Fixes #8274
  • Loading branch information
shoenig committed Jul 8, 2020
1 parent a7d7992 commit 44df264
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 43 deletions.
39 changes: 22 additions & 17 deletions nomad/job_endpoint_hook_connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/hashicorp/nomad/helper/uuid"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/pkg/errors"
)

var (
Expand Down Expand Up @@ -97,13 +98,23 @@ func isSidecarForService(t *structs.Task, svc string) bool {
return t.Kind == structs.TaskKind(fmt.Sprintf("%s:%s", structs.ConnectProxyPrefix, svc))
}

func getNamedTaskForNativeService(tg *structs.TaskGroup, taskName string) *structs.Task {
// getNamedTaskForNativeService retrieves the Task with the name specified in the
// group service definition. If the task name is empty and there is only one task
// in the group, infer the name from the only option.
func getNamedTaskForNativeService(tg *structs.TaskGroup, serviceName, taskName string) (*structs.Task, error) {
if taskName == "" {
if len(tg.Tasks) == 1 {
return tg.Tasks[0], nil
}
return nil, errors.Errorf("task for Consul Connect Native service %s->%s is ambiguous and must be set", tg.Name, serviceName)
}

for _, t := range tg.Tasks {
if t.Name == taskName {
return t
return t, nil
}
}
return nil
return nil, errors.Errorf("task %s named by Consul Connect Native service %s->%s does not exist", taskName, tg.Name, serviceName)
}

// probably need to hack this up to look for checks on the service, and if they
Expand Down Expand Up @@ -155,11 +166,13 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error {
// create a port for the sidecar task's proxy port
makePort(fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, service.Name))
} else if service.Connect.IsNative() {
// find the task backing this connect native service and set the kind
nativeTaskName := service.TaskName
if t := getNamedTaskForNativeService(g, nativeTaskName); t != nil {
t.Kind = structs.NewTaskKind(structs.ConnectNativePrefix, service.Name)
if t, err := getNamedTaskForNativeService(g, service.Name, nativeTaskName); err != nil {
return err
} else {
return fmt.Errorf("native task %s named by %s->%s does not exist", nativeTaskName, g.Name, service.Name)
t.Kind = structs.NewTaskKind(structs.ConnectNativePrefix, service.Name)
service.TaskName = t.Name // in case the task was inferred
}
}
}
Expand Down Expand Up @@ -220,16 +233,8 @@ func groupConnectSidecarValidate(g *structs.TaskGroup) error {
func groupConnectNativeValidate(g *structs.TaskGroup, s *structs.Service) error {
// note that network mode is not enforced for connect native services

// a native service must have the task identified in the service definition.
if len(s.TaskName) == 0 {
return fmt.Errorf("Consul Connect Native service %q requires task name", s.Name)
if _, err := getNamedTaskForNativeService(g, s.Name, s.TaskName); err != nil {
return err
}

// also make sure that task actually exists
for _, task := range g.Tasks {
if s.TaskName == task.Name {
return nil
}
}
return fmt.Errorf("Consul Connect Native service %q requires undefined task %q in group %q", s.Name, s.TaskName, g.Name)
return nil
}
54 changes: 31 additions & 23 deletions nomad/job_endpoint_hook_connect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,32 +157,40 @@ func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
})
}

func TestJobEndpointConnect_groupConnectNativeValidate(t *testing.T) {
t.Run("no task in service", func(t *testing.T) {
require.EqualError(t, groupConnectNativeValidate(&structs.TaskGroup{
Name: "g1",
}, &structs.Service{
Name: "s1",
TaskName: "",
}), `Consul Connect Native service "s1" requires task name`)
func TestJobEndpointConnect_getNamedTaskForNativeService(t *testing.T) {
t.Run("named exists", func(t *testing.T) {
task, err := getNamedTaskForNativeService(&structs.TaskGroup{
Name: "g1",
Tasks: []*structs.Task{{Name: "t1"}, {Name: "t2"}},
}, "s1", "t2")
require.NoError(t, err)
require.Equal(t, "t2", task.Name)
})

t.Run("no task for service", func(t *testing.T) {
require.EqualError(t, groupConnectNativeValidate(&structs.TaskGroup{
Name: "g2",
}, &structs.Service{
Name: "s2",
TaskName: "t1",
}), `Consul Connect Native service "s2" requires undefined task "t1" in group "g2"`)
t.Run("infer exists", func(t *testing.T) {
task, err := getNamedTaskForNativeService(&structs.TaskGroup{
Name: "g1",
Tasks: []*structs.Task{{Name: "t2"}},
}, "s1", "")
require.NoError(t, err)
require.Equal(t, "t2", task.Name)
})

t.Run("native okay", func(t *testing.T) {
require.NoError(t, groupConnectNativeValidate(&structs.TaskGroup{
Name: "g2",
Tasks: []*structs.Task{{Name: "t0"}, {Name: "t1"}, {Name: "t3"}},
}, &structs.Service{
Name: "s2",
TaskName: "t1",
}))
t.Run("infer ambiguous", func(t *testing.T) {
task, err := getNamedTaskForNativeService(&structs.TaskGroup{
Name: "g1",
Tasks: []*structs.Task{{Name: "t1"}, {Name: "t2"}},
}, "s1", "")
require.EqualError(t, err, "task for Consul Connect Native service g1->s1 is ambiguous and must be set")
require.Nil(t, task)
})

t.Run("named absent", func(t *testing.T) {
task, err := getNamedTaskForNativeService(&structs.TaskGroup{
Name: "g1",
Tasks: []*structs.Task{{Name: "t1"}, {Name: "t2"}},
}, "s1", "t3")
require.EqualError(t, err, "task t3 named by Consul Connect Native service g1->s1 does not exist")
require.Nil(t, task)
})
}
3 changes: 2 additions & 1 deletion nomad/structs/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,8 @@ func (s *Service) Validate() error {
mErr.Errors = append(mErr.Errors, err)
}

// if service is connect native, service task must be set
// if service is connect native, service task must be set (which may
// happen implicitly in a job mutation if there is only one task)
if s.Connect.IsNative() && len(s.TaskName) == 0 {
mErr.Errors = append(mErr.Errors, fmt.Errorf("Service %s is Connect Native and requires setting the task", s.Name))
}
Expand Down
4 changes: 2 additions & 2 deletions website/pages/docs/job-specification/service.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ Connect][connect] integration.

- `task` `(string: "")` - Specifies the name of the Nomad Task associated with
this service definition. Only available on group services. Must be set if this
service definition represents a Consul Connect Native service. The Nomad Task
must exist in the same Task Group.
service definition represents a Consul Connect Native service and there is more
than one task in the task group.

- `meta` <code>([Meta][]: nil)</code> - Specifies a key-value map that annotates
the Consul service with user-defined metadata.
Expand Down

0 comments on commit 44df264

Please sign in to comment.