Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consul/connect: infer task name for native service if possible #8392

Merged
merged 1 commit into from
Jul 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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