diff --git a/client/allocrunner/taskrunner/envoy_bootstrap_hook.go b/client/allocrunner/taskrunner/envoy_bootstrap_hook.go index 0b286138dff8..eaa9f534c9e9 100644 --- a/client/allocrunner/taskrunner/envoy_bootstrap_hook.go +++ b/client/allocrunner/taskrunner/envoy_bootstrap_hook.go @@ -278,7 +278,8 @@ func buildEnvoyAdminBind(alloc *structs.Allocation, serviceName, taskName string case "host": for _, service := range tg.Services { if service.Name == serviceName { - _, port = tg.Networks.Port(service.PortLabel) + mapping := tg.Networks.Port(service.PortLabel) + port = mapping.Value break } } diff --git a/command/agent/consul/connect.go b/command/agent/consul/connect.go index 68904070f6db..3825c0da9320 100644 --- a/command/agent/consul/connect.go +++ b/command/agent/consul/connect.go @@ -2,6 +2,8 @@ package consul import ( "fmt" + "net" + "strconv" "github.com/hashicorp/consul/api" "github.com/hashicorp/nomad/helper" @@ -11,7 +13,7 @@ import ( // newConnect creates a new Consul AgentServiceConnect struct based on a Nomad // Connect struct. If the nomad Connect struct is nil, nil will be returned to // disable Connect for this service. -func newConnect(serviceName string, nc *structs.ConsulConnect, networks structs.Networks) (*api.AgentServiceConnect, error) { +func newConnect(serviceId string, serviceName string, nc *structs.ConsulConnect, networks structs.Networks, ports structs.AllocatedPorts) (*api.AgentServiceConnect, error) { switch { case nc == nil: // no connect stanza means there is no connect service to register @@ -27,7 +29,10 @@ func newConnect(serviceName string, nc *structs.ConsulConnect, networks structs. case nc.HasSidecar(): // must register the sidecar for this service - sidecarReg, err := connectSidecarRegistration(serviceName, nc.SidecarService, networks) + if nc.SidecarService.Port == "" { + nc.SidecarService.Port = fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, serviceName) + } + sidecarReg, err := connectSidecarRegistration(serviceId, nc.SidecarService, networks, ports) if err != nil { return nil, err } @@ -84,27 +89,38 @@ func newConnectGateway(serviceName string, connect *structs.ConsulConnect) *api. return &api.AgentServiceConnectProxyConfig{Config: envoyConfig} } -func connectSidecarRegistration(serviceName string, css *structs.ConsulSidecarService, networks structs.Networks) (*api.AgentServiceRegistration, error) { +func connectSidecarRegistration(serviceId string, css *structs.ConsulSidecarService, networks structs.Networks, ports structs.AllocatedPorts) (*api.AgentServiceRegistration, error) { if css == nil { // no sidecar stanza means there is no sidecar service to register return nil, nil } - cNet, cPort, err := connectPort(serviceName, networks) + cMapping, err := connectPort(css.Port, networks, ports) if err != nil { return nil, err } - proxy, err := connectSidecarProxy(css.Proxy, cPort.To, networks) + proxy, err := connectSidecarProxy(css.Proxy, cMapping.To, networks) if err != nil { return nil, err } return &api.AgentServiceRegistration{ Tags: helper.CopySliceString(css.Tags), - Port: cPort.Value, - Address: cNet.IP, + Port: cMapping.Value, + Address: cMapping.HostIP, Proxy: proxy, + Checks: api.AgentServiceChecks{ + { + Name: "Connect Sidecar Listening", + TCP: net.JoinHostPort(cMapping.HostIP, strconv.Itoa(cMapping.Value)), + Interval: "10s", + }, + { + Name: "Connect Sidecar Aliasing " + serviceId, + AliasService: serviceId, + }, + }, }, nil } @@ -200,17 +216,19 @@ func connectNetworkInvariants(networks structs.Networks) error { // connectPort returns the network and port for the Connect proxy sidecar // defined for this service. An error is returned if the network and port // cannot be determined. -func connectPort(serviceName string, networks structs.Networks) (*structs.NetworkResource, structs.Port, error) { +func connectPort(portLabel string, networks structs.Networks, ports structs.AllocatedPorts) (structs.AllocatedPortMapping, error) { if err := connectNetworkInvariants(networks); err != nil { - return nil, structs.Port{}, err + return structs.AllocatedPortMapping{}, err } - - port, ok := networks[0].PortForService(serviceName) + mapping, ok := ports.Get(portLabel) if !ok { - return nil, structs.Port{}, fmt.Errorf("No Connect port defined for service %q", serviceName) + mapping = networks.Port(portLabel) + if mapping.Value > 0 { + return mapping, nil + } + return structs.AllocatedPortMapping{}, fmt.Errorf("No port of label %q defined", portLabel) } - - return networks[0], port, nil + return mapping, nil } // connectExposePathPort returns the port for the exposed path for the exposed @@ -220,10 +238,10 @@ func connectExposePathPort(portLabel string, networks structs.Networks) (string, return "", 0, err } - ip, port := networks.Port(portLabel) - if port == 0 { + mapping := networks.Port(portLabel) + if mapping.Value == 0 { return "", 0, fmt.Errorf("No port of label %q defined", portLabel) } - return ip, port, nil + return mapping.HostIP, mapping.Value, nil } diff --git a/command/agent/consul/connect_test.go b/command/agent/consul/connect_test.go index d42f639957cb..7d89c55eca4a 100644 --- a/command/agent/consul/connect_test.go +++ b/command/agent/consul/connect_test.go @@ -21,34 +21,40 @@ var ( {Label: "connect-proxy-redis", Value: 3000, To: 3000}, }, }} + testConnectPorts = structs.AllocatedPorts{{ + Label: "connect-proxy-redis", + Value: 3000, + To: 3000, + HostIP: "192.168.30.1", + }} ) func TestConnect_newConnect(t *testing.T) { t.Parallel() t.Run("nil", func(t *testing.T) { - asr, err := newConnect("", nil, nil) + asr, err := newConnect("", "", nil, nil, nil) require.NoError(t, err) require.Nil(t, asr) }) t.Run("native", func(t *testing.T) { - asr, err := newConnect("", &structs.ConsulConnect{ + asr, err := newConnect("", "", &structs.ConsulConnect{ Native: true, - }, nil) + }, nil, nil) require.NoError(t, err) require.True(t, asr.Native) require.Nil(t, asr.SidecarService) }) t.Run("with sidecar", func(t *testing.T) { - asr, err := newConnect("redis", &structs.ConsulConnect{ + asr, err := newConnect("redis-service-id", "redis", &structs.ConsulConnect{ Native: false, SidecarService: &structs.ConsulSidecarService{ Tags: []string{"foo", "bar"}, - Port: "sidecarPort", + Port: "connect-proxy-redis", }, - }, testConnectNetwork) + }, testConnectNetwork, testConnectPorts) require.NoError(t, err) require.Equal(t, &api.AgentServiceRegistration{ Tags: []string{"foo", "bar"}, @@ -60,6 +66,17 @@ func TestConnect_newConnect(t *testing.T) { "bind_port": 3000, }, }, + Checks: api.AgentServiceChecks{ + { + Name: "Connect Sidecar Listening", + TCP: "192.168.30.1:3000", + Interval: "10s", + }, + { + Name: "Connect Sidecar Aliasing redis-service-id", + AliasService: "redis-service-id", + }, + }, }, asr.SidecarService) }) } @@ -68,20 +85,21 @@ func TestConnect_connectSidecarRegistration(t *testing.T) { t.Parallel() t.Run("nil", func(t *testing.T) { - sidecarReg, err := connectSidecarRegistration("", nil, testConnectNetwork) + sidecarReg, err := connectSidecarRegistration("", nil, testConnectNetwork, testConnectPorts) require.NoError(t, err) require.Nil(t, sidecarReg) }) t.Run("no service port", func(t *testing.T) { - _, err := connectSidecarRegistration("unknown", &structs.ConsulSidecarService{ - // irrelevant - }, testConnectNetwork) - require.EqualError(t, err, `No Connect port defined for service "unknown"`) + _, err := connectSidecarRegistration("unknown-id", &structs.ConsulSidecarService{ + Port: "unknown-label", + }, testConnectNetwork, testConnectPorts) + require.EqualError(t, err, `No port of label "unknown-label" defined`) }) t.Run("bad proxy", func(t *testing.T) { - _, err := connectSidecarRegistration("redis", &structs.ConsulSidecarService{ + _, err := connectSidecarRegistration("redis-service-id", &structs.ConsulSidecarService{ + Port: "connect-proxy-redis", Proxy: &structs.ConsulProxy{ Expose: &structs.ConsulExposeConfig{ Paths: []structs.ConsulExposePath{{ @@ -89,15 +107,15 @@ func TestConnect_connectSidecarRegistration(t *testing.T) { }}, }, }, - }, testConnectNetwork) + }, testConnectNetwork, testConnectPorts) require.EqualError(t, err, `No port of label "badPort" defined`) }) t.Run("normal", func(t *testing.T) { - proxy, err := connectSidecarRegistration("redis", &structs.ConsulSidecarService{ + proxy, err := connectSidecarRegistration("redis-service-id", &structs.ConsulSidecarService{ Tags: []string{"foo", "bar"}, - Port: "sidecarPort", - }, testConnectNetwork) + Port: "connect-proxy-redis", + }, testConnectNetwork, testConnectPorts) require.NoError(t, err) require.Equal(t, &api.AgentServiceRegistration{ Tags: []string{"foo", "bar"}, @@ -109,6 +127,17 @@ func TestConnect_connectSidecarRegistration(t *testing.T) { "bind_port": 3000, }, }, + Checks: api.AgentServiceChecks{ + { + Name: "Connect Sidecar Listening", + TCP: "192.168.30.1:3000", + Interval: "10s", + }, + { + Name: "Connect Sidecar Aliasing redis-service-id", + AliasService: "redis-service-id", + }, + }, }, proxy) }) } @@ -334,32 +363,39 @@ func TestConnect_getConnectPort(t *testing.T) { To: 23456, }}}} + ports := structs.AllocatedPorts{{ + Label: "foo", + Value: 23456, + To: 23456, + HostIP: "192.168.30.1", + }} + t.Run("normal", func(t *testing.T) { - nr, port, err := connectPort("foo", networks) + nr, err := connectPort("foo", networks, ports) require.NoError(t, err) - require.Equal(t, structs.Port{ - Label: "connect-proxy-foo", - Value: 23456, - To: 23456, - }, port) - require.Equal(t, "192.168.30.1", nr.IP) + require.Equal(t, structs.AllocatedPortMapping{ + Label: "foo", + Value: 23456, + To: 23456, + HostIP: "192.168.30.1", + }, nr) }) t.Run("no such service", func(t *testing.T) { - _, _, err := connectPort("other", networks) - require.EqualError(t, err, `No Connect port defined for service "other"`) + _, err := connectPort("other", networks, ports) + require.EqualError(t, err, `No port of label "other" defined`) }) t.Run("no network", func(t *testing.T) { - _, _, err := connectPort("foo", nil) + _, err := connectPort("foo", nil, nil) require.EqualError(t, err, "Connect only supported with exactly 1 network (found 0)") }) t.Run("multi network", func(t *testing.T) { - _, _, err := connectPort("foo", append(networks, &structs.NetworkResource{ + _, err := connectPort("foo", append(networks, &structs.NetworkResource{ Device: "eth1", IP: "10.0.10.0", - })) + }), nil) require.EqualError(t, err, "Connect only supported with exactly 1 network (found 2)") }) } diff --git a/command/agent/consul/service_client.go b/command/agent/consul/service_client.go index 762d3154c336..8af946c46f2d 100644 --- a/command/agent/consul/service_client.go +++ b/command/agent/consul/service_client.go @@ -866,7 +866,7 @@ func (c *ServiceClient) serviceRegs(ops *operations, service *structs.Service, w } // newConnect returns (nil, nil) if there's no Connect-enabled service. - connect, err := newConnect(service.Name, service.Connect, workload.Networks) + connect, err := newConnect(id, service.Name, service.Connect, workload.Networks, workload.Ports) if err != nil { return nil, fmt.Errorf("invalid Consul Connect configuration for service %q: %v", service.Name, err) } @@ -1500,9 +1500,9 @@ func getAddress(addrMode, portLabel string, networks structs.Networks, driverNet // Check in Networks struct for backwards compatibility if not found mapping, ok := ports.Get(portLabel) if !ok { - ip, port := networks.Port(portLabel) - if port > 0 { - return ip, port, nil + mapping = networks.Port(portLabel) + if mapping.Value > 0 { + return mapping.HostIP, mapping.Value, nil } // If port isn't a label, try to parse it as a literal port number diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go index fbd444fa2757..1e215f6196aa 100644 --- a/nomad/job_endpoint_hook_connect.go +++ b/nomad/job_endpoint_hook_connect.go @@ -248,7 +248,11 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error { task.Canonicalize(job, g) // create a port for the sidecar task's proxy port - injectPort(g, fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, service.Name)) + portLabel := service.Connect.SidecarService.Port + if portLabel == "" { + portLabel = fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, service.Name) + } + injectPort(g, portLabel) case service.Connect.IsNative(): // find the task backing this connect native service and set the kind diff --git a/nomad/job_endpoint_hook_expose_check.go b/nomad/job_endpoint_hook_expose_check.go index e1bc6d1ee2cb..ad60f706a466 100644 --- a/nomad/job_endpoint_hook_expose_check.go +++ b/nomad/job_endpoint_hook_expose_check.go @@ -206,13 +206,15 @@ func exposePathForCheck(tg *structs.TaskGroup, s *structs.Service, check *struct // The difference here is the address is predestined to be localhost since // it is binding inside the namespace. var port int - if _, port = tg.Networks.Port(s.PortLabel); port <= 0 { // try looking up by port label + if mapping := tg.Networks.Port(s.PortLabel); mapping.Value <= 0 { // try looking up by port label if port, _ = strconv.Atoi(s.PortLabel); port <= 0 { // then try direct port value return nil, errors.Errorf( "unable to determine local service port for service check %s->%s->%s", tg.Name, s.Name, check.Name, ) } + } else { + port = mapping.Value } // The Path, Protocol, and PortLabel are just copied over from the service diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 5b3b8d2f9d15..17b2c31083c2 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -2602,24 +2602,6 @@ func (n *NetworkResource) PortLabels() map[string]int { return labelValues } -// ConnectPort returns the Connect port for the given service. Returns false if -// no port was found for a service with that name. -func (n *NetworkResource) PortForService(serviceName string) (Port, bool) { - label := fmt.Sprintf("%s-%s", ConnectProxyPrefix, serviceName) - for _, port := range n.ReservedPorts { - if port.Label == label { - return port, true - } - } - for _, port := range n.DynamicPorts { - if port.Label == label { - return port, true - } - } - - return Port{}, false -} - // Networks defined for a task on the Resources struct. type Networks []*NetworkResource @@ -2636,20 +2618,30 @@ func (ns Networks) Copy() Networks { } // Port assignment and IP for the given label or empty values. -func (ns Networks) Port(label string) (string, int) { +func (ns Networks) Port(label string) AllocatedPortMapping { for _, n := range ns { for _, p := range n.ReservedPorts { if p.Label == label { - return n.IP, p.Value + return AllocatedPortMapping{ + Label: label, + Value: p.Value, + To: p.To, + HostIP: n.IP, + } } } for _, p := range n.DynamicPorts { if p.Label == label { - return n.IP, p.Value + return AllocatedPortMapping{ + Label: label, + Value: p.Value, + To: p.To, + HostIP: n.IP, + } } } } - return "", 0 + return AllocatedPortMapping{} } func (ns Networks) NetIndex(n *NetworkResource) int {