consul/connect: infer task name in service if possible

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
This commit is contained in:
Seth Hoenig 2020-07-08 10:19:36 -05:00
parent ec96ddf648
commit 1a75da0ce0
4 changed files with 57 additions and 43 deletions

View file

@ -6,6 +6,7 @@ import (
"github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/helper/uuid"
"github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/nomad/structs"
"github.com/pkg/errors"
) )
var ( var (
@ -97,13 +98,23 @@ func isSidecarForService(t *structs.Task, svc string) bool {
return t.Kind == structs.TaskKind(fmt.Sprintf("%s:%s", structs.ConnectProxyPrefix, svc)) 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 { for _, t := range tg.Tasks {
if t.Name == taskName { 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 // probably need to hack this up to look for checks on the service, and if they
@ -155,11 +166,13 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error {
// create a port for the sidecar task's proxy port // create a port for the sidecar task's proxy port
makePort(fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, service.Name)) makePort(fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, service.Name))
} else if service.Connect.IsNative() { } else if service.Connect.IsNative() {
// find the task backing this connect native service and set the kind
nativeTaskName := service.TaskName nativeTaskName := service.TaskName
if t := getNamedTaskForNativeService(g, nativeTaskName); t != nil { if t, err := getNamedTaskForNativeService(g, service.Name, nativeTaskName); err != nil {
t.Kind = structs.NewTaskKind(structs.ConnectNativePrefix, service.Name) return err
} else { } 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
} }
} }
} }
@ -220,16 +233,8 @@ func groupConnectSidecarValidate(g *structs.TaskGroup) error {
func groupConnectNativeValidate(g *structs.TaskGroup, s *structs.Service) error { func groupConnectNativeValidate(g *structs.TaskGroup, s *structs.Service) error {
// note that network mode is not enforced for connect native services // note that network mode is not enforced for connect native services
// a native service must have the task identified in the service definition. if _, err := getNamedTaskForNativeService(g, s.Name, s.TaskName); err != nil {
if len(s.TaskName) == 0 { return err
return fmt.Errorf("Consul Connect Native service %q requires task name", s.Name)
} }
return nil
// 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)
} }

View file

@ -157,32 +157,40 @@ func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
}) })
} }
func TestJobEndpointConnect_groupConnectNativeValidate(t *testing.T) { func TestJobEndpointConnect_getNamedTaskForNativeService(t *testing.T) {
t.Run("no task in service", func(t *testing.T) { t.Run("named exists", func(t *testing.T) {
require.EqualError(t, groupConnectNativeValidate(&structs.TaskGroup{ task, err := getNamedTaskForNativeService(&structs.TaskGroup{
Name: "g1", Name: "g1",
}, &structs.Service{ Tasks: []*structs.Task{{Name: "t1"}, {Name: "t2"}},
Name: "s1", }, "s1", "t2")
TaskName: "", require.NoError(t, err)
}), `Consul Connect Native service "s1" requires task name`) require.Equal(t, "t2", task.Name)
}) })
t.Run("no task for service", func(t *testing.T) { t.Run("infer exists", func(t *testing.T) {
require.EqualError(t, groupConnectNativeValidate(&structs.TaskGroup{ task, err := getNamedTaskForNativeService(&structs.TaskGroup{
Name: "g2", Name: "g1",
}, &structs.Service{ Tasks: []*structs.Task{{Name: "t2"}},
Name: "s2", }, "s1", "")
TaskName: "t1", require.NoError(t, err)
}), `Consul Connect Native service "s2" requires undefined task "t1" in group "g2"`) require.Equal(t, "t2", task.Name)
}) })
t.Run("native okay", func(t *testing.T) { t.Run("infer ambiguous", func(t *testing.T) {
require.NoError(t, groupConnectNativeValidate(&structs.TaskGroup{ task, err := getNamedTaskForNativeService(&structs.TaskGroup{
Name: "g2", Name: "g1",
Tasks: []*structs.Task{{Name: "t0"}, {Name: "t1"}, {Name: "t3"}}, Tasks: []*structs.Task{{Name: "t1"}, {Name: "t2"}},
}, &structs.Service{ }, "s1", "")
Name: "s2", require.EqualError(t, err, "task for Consul Connect Native service g1->s1 is ambiguous and must be set")
TaskName: "t1", 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)
}) })
} }

View file

@ -481,7 +481,8 @@ func (s *Service) Validate() error {
mErr.Errors = append(mErr.Errors, err) 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 { 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)) mErr.Errors = append(mErr.Errors, fmt.Errorf("Service %s is Connect Native and requires setting the task", s.Name))
} }

View file

@ -149,8 +149,8 @@ Connect][connect] integration.
- `task` `(string: "")` - Specifies the name of the Nomad Task associated with - `task` `(string: "")` - Specifies the name of the Nomad Task associated with
this service definition. Only available on group services. Must be set if this this service definition. Only available on group services. Must be set if this
service definition represents a Consul Connect Native service. The Nomad Task service definition represents a Consul Connect Native service and there is more
must exist in the same Task Group. than one task in the task group.
- `meta` <code>([Meta][]: nil)</code> - Specifies a key-value map that annotates - `meta` <code>([Meta][]: nil)</code> - Specifies a key-value map that annotates
the Consul service with user-defined metadata. the Consul service with user-defined metadata.