From a5a3dce8639d806d2ef660d59022be5e3e04fab3 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 3 Mar 2020 17:30:22 -0600 Subject: [PATCH 1/5] client: use consistent name for struct receiver parameter This helps reduce the number of squiggly lines in Goland. --- command/agent/consul/structs.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/command/agent/consul/structs.go b/command/agent/consul/structs.go index 0aa21f3cc03..b41387a77df 100644 --- a/command/agent/consul/structs.go +++ b/command/agent/consul/structs.go @@ -73,22 +73,22 @@ func BuildAllocServices(node *structs.Node, alloc *structs.Allocation, restarter } // Copy method for easing tests -func (t *WorkloadServices) Copy() *WorkloadServices { +func (ws *WorkloadServices) Copy() *WorkloadServices { newTS := new(WorkloadServices) - *newTS = *t + *newTS = *ws // Deep copy Services - newTS.Services = make([]*structs.Service, len(t.Services)) - for i := range t.Services { - newTS.Services[i] = t.Services[i].Copy() + newTS.Services = make([]*structs.Service, len(ws.Services)) + for i := range ws.Services { + newTS.Services[i] = ws.Services[i].Copy() } return newTS } -func (w *WorkloadServices) Name() string { - if w.Task != "" { - return w.Task +func (ws *WorkloadServices) Name() string { + if ws.Task != "" { + return ws.Task } - return "group-" + w.Group + return "group-" + ws.Group } From 08e3eee9612c890de9236a588dcfab65097d97b3 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Fri, 6 Mar 2020 21:15:22 -0600 Subject: [PATCH 2/5] connect: enable proxy.passthrough configuration Enable configuration of HTTP and gRPC endpoints which should be exposed by the Connect sidecar proxy. This changeset is the first "non-magical" pass that lays the groundwork for enabling Consul service checks for tasks running in a network namespace because they are Connect-enabled. The changes here provide for full configuration of the connect { sidecar_service { proxy { expose { paths = [{ path = protocol = local_path_port = listener_port = }, ... ] } } } stanza. Everything from `expose` and below is new, and partially implements the precedent set by Consul: https://www.consul.io/docs/connect/registration/service-registration.html#expose-paths-configuration-reference Combined with a task-group level network port-mapping in the form: port "exposeExample" { to = -1 } it is now possible to "punch a hole" through the network namespace to a specific HTTP or gRPC path, with the anticipated use case of creating Consul checks on Connect enabled services. A future PR may introduce more automagic behavior, where we can do things like 1) auto-fill the 'expose.path.local_path_port' with the default value of the 'service.port' value for task-group level connect-enabled services. 2) automatically generate a port-mapping 3) enable an 'expose.checks' flag which automatically creates exposed endpoints for every compatible consul service check (http/grpc checks on connect enabled services). --- api/services.go | 17 +- client/allocrunner/networking_bridge_linux.go | 6 +- command/agent/agent.go | 2 +- command/agent/consul/client.go | 93 +---- command/agent/consul/connect.go | 176 ++++++++ command/agent/consul/connect_test.go | 376 ++++++++++++++++++ command/agent/job_endpoint.go | 139 ++++--- command/agent/job_endpoint_test.go | 166 ++++++++ jobspec/parse_service.go | 64 ++- jobspec/parse_test.go | 28 ++ .../test-fixtures/tg-service-proxy-expose.hcl | 21 + nomad/job_endpoint_hook_connect.go | 45 ++- nomad/structs/services.go | 100 ++++- nomad/structs/services_test.go | 98 ++++- nomad/structs/structs.go | 12 +- 15 files changed, 1145 insertions(+), 198 deletions(-) create mode 100644 command/agent/consul/connect.go create mode 100644 command/agent/consul/connect_test.go create mode 100644 jobspec/test-fixtures/tg-service-proxy-expose.hcl diff --git a/api/services.go b/api/services.go index d371148a926..9a48ea06c47 100644 --- a/api/services.go +++ b/api/services.go @@ -168,8 +168,9 @@ type SidecarTask struct { // ConsulProxy represents a Consul Connect sidecar proxy jobspec stanza. type ConsulProxy struct { - LocalServiceAddress string `mapstructure:"local_service_address"` - LocalServicePort int `mapstructure:"local_service_port"` + LocalServiceAddress string `mapstructure:"local_service_address"` + LocalServicePort int `mapstructure:"local_service_port"` + ExposeConfig *ConsulExposeConfig `mapstructure:"expose"` Upstreams []*ConsulUpstream Config map[string]interface{} } @@ -179,3 +180,15 @@ type ConsulUpstream struct { DestinationName string `mapstructure:"destination_name"` LocalBindPort int `mapstructure:"local_bind_port"` } + +type ConsulExposeConfig struct { + Paths []*ConsulExposePath `mapstructure:"paths"` + // todo(shoenig): add magic for 'checks' option +} + +type ConsulExposePath struct { + Path string + Protocol string + LocalPathPort int `mapstructure:"local_path_port"` + ListenerPort string `mapstructure:"listener_port"` +} diff --git a/client/allocrunner/networking_bridge_linux.go b/client/allocrunner/networking_bridge_linux.go index 4bb17cb63f5..75e9612d1fe 100644 --- a/client/allocrunner/networking_bridge_linux.go +++ b/client/allocrunner/networking_bridge_linux.go @@ -86,8 +86,6 @@ func newBridgeNetworkConfigurator(log hclog.Logger, bridgeName, ipRange, cniPath // ensureForwardingRules ensures that a forwarding rule is added to iptables // to allow traffic inbound to the bridge network -// // ensureForwardingRules ensures that a forwarding rule is added to iptables -// to allow traffic inbound to the bridge network func (b *bridgeNetworkConfigurator) ensureForwardingRules() error { ipt, err := iptables.New() if err != nil { @@ -154,9 +152,9 @@ func (b *bridgeNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Al return err } - // Depending on the version of bridge cni plugin used, a known race could occure + // Depending on the version of bridge cni plugin (< 0.8.4) a known race could occur // where two alloc attempt to create the nomad bridge at the same time, resulting - // in one of them to fail. This rety attempts to overcome any + // in one of them to fail. This retry attempts to overcome those erroneous failures. const retry = 3 for attempt := 1; ; attempt++ { //TODO eventually returning the IP from the result would be nice to have in the alloc diff --git a/command/agent/agent.go b/command/agent/agent.go index db67ce710fe..85ca02ad8ca 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -586,7 +586,7 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) { conf.ACLTokenTTL = agentConfig.ACL.TokenTTL conf.ACLPolicyTTL = agentConfig.ACL.PolicyTTL - // Setup networking configration + // Setup networking configuration conf.CNIPath = agentConfig.Client.CNIPath conf.BridgeNetworkName = agentConfig.Client.BridgeNetworkName conf.BridgeNetworkAllocSubnet = agentConfig.Client.BridgeNetworkSubnet diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go index 578c1bfc9d5..e86d1f2a9d1 100644 --- a/command/agent/consul/client.go +++ b/command/agent/consul/client.go @@ -1265,7 +1265,7 @@ func MakeCheckID(serviceID string, check *structs.ServiceCheck) string { // createCheckReg creates a Check that can be registered with Consul. // // Script checks simply have a TTL set and the caller is responsible for -// running the script and heartbeating. +// running the script and heart-beating. func createCheckReg(serviceID, checkID string, check *structs.ServiceCheck, host string, port int) (*api.AgentCheckRegistration, error) { chkReg := api.AgentCheckRegistration{ ID: checkID, @@ -1298,8 +1298,8 @@ func createCheckReg(serviceID, checkID string, check *structs.ServiceCheck, host if err != nil { return nil, err } - url := base.ResolveReference(relative) - chkReg.HTTP = url.String() + checkURL := base.ResolveReference(relative) + chkReg.HTTP = checkURL.String() chkReg.Method = check.Method chkReg.Header = check.Header @@ -1456,90 +1456,3 @@ func getAddress(addrMode, portLabel string, networks structs.Networks, driverNet return "", 0, fmt.Errorf("invalid address mode %q", addrMode) } } - -// 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) { - if nc == nil { - // No Connect stanza, returning nil is fine - return nil, nil - } - - cc := &api.AgentServiceConnect{ - Native: nc.Native, - } - - if nc.SidecarService == nil { - return cc, nil - } - - net, port, err := getConnectPort(serviceName, networks) - if err != nil { - return nil, err - } - - // Bind to netns IP(s):port - proxyConfig := map[string]interface{}{} - localServiceAddress := "" - localServicePort := 0 - if nc.SidecarService.Proxy != nil { - localServiceAddress = nc.SidecarService.Proxy.LocalServiceAddress - localServicePort = nc.SidecarService.Proxy.LocalServicePort - if nc.SidecarService.Proxy.Config != nil { - proxyConfig = nc.SidecarService.Proxy.Config - } - } - proxyConfig["bind_address"] = "0.0.0.0" - proxyConfig["bind_port"] = port.To - - // Advertise host IP:port - cc.SidecarService = &api.AgentServiceRegistration{ - Tags: helper.CopySliceString(nc.SidecarService.Tags), - Address: net.IP, - Port: port.Value, - - // Automatically configure the proxy to bind to all addresses - // within the netns. - Proxy: &api.AgentServiceConnectProxyConfig{ - LocalServiceAddress: localServiceAddress, - LocalServicePort: localServicePort, - Config: proxyConfig, - }, - } - - // If no further proxy settings were explicitly configured, exit early - if nc.SidecarService.Proxy == nil { - return cc, nil - } - - numUpstreams := len(nc.SidecarService.Proxy.Upstreams) - if numUpstreams == 0 { - return cc, nil - } - - upstreams := make([]api.Upstream, numUpstreams) - for i, nu := range nc.SidecarService.Proxy.Upstreams { - upstreams[i].DestinationName = nu.DestinationName - upstreams[i].LocalBindPort = nu.LocalBindPort - } - cc.SidecarService.Proxy.Upstreams = upstreams - - return cc, nil -} - -// getConnectPort 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 getConnectPort(serviceName string, networks structs.Networks) (*structs.NetworkResource, structs.Port, error) { - if n := len(networks); n != 1 { - return nil, structs.Port{}, fmt.Errorf("Connect only supported with exactly 1 network (found %d)", n) - } - - port, ok := networks[0].PortForService(serviceName) - if !ok { - return nil, structs.Port{}, fmt.Errorf("No Connect port defined for service %q", serviceName) - } - - return networks[0], port, nil -} diff --git a/command/agent/consul/connect.go b/command/agent/consul/connect.go new file mode 100644 index 00000000000..5adba80fa49 --- /dev/null +++ b/command/agent/consul/connect.go @@ -0,0 +1,176 @@ +package consul + +import ( + "fmt" + + "github.com/hashicorp/consul/api" + "github.com/hashicorp/nomad/helper" + "github.com/hashicorp/nomad/nomad/structs" +) + +// 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) { + if nc == nil { + // no connect stanza means there is no connect service to register + return nil, nil + } + + if nc.Native { + return &api.AgentServiceConnect{Native: true}, nil + } + + sidecarReg, err := connectSidecarRegistration(serviceName, nc.SidecarService, networks) + if err != nil { + return nil, err + } + + return &api.AgentServiceConnect{ + Native: false, + SidecarService: sidecarReg, + }, nil +} + +func connectSidecarRegistration(serviceName string, css *structs.ConsulSidecarService, networks structs.Networks) (*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) + if err != nil { + return nil, err + } + + proxy, err := connectProxy(css.Proxy, cPort.To, networks) + if err != nil { + return nil, err + } + + return &api.AgentServiceRegistration{ + Tags: helper.CopySliceString(css.Tags), + Port: cPort.Value, + Address: cNet.IP, + Proxy: proxy, + }, nil +} + +func connectProxy(proxy *structs.ConsulProxy, cPort int, networks structs.Networks) (*api.AgentServiceConnectProxyConfig, error) { + if proxy == nil { + return nil, nil + } + + expose, err := connectProxyExpose(proxy.Expose, networks) + if err != nil { + return nil, err + } + + return &api.AgentServiceConnectProxyConfig{ + LocalServiceAddress: proxy.LocalServiceAddress, + LocalServicePort: proxy.LocalServicePort, + Config: connectProxyConfig(proxy.Config, cPort), + Upstreams: connectUpstreams(proxy.Upstreams), + Expose: expose, + }, nil +} + +func connectProxyExpose(expose *structs.ConsulExposeConfig, networks structs.Networks) (api.ExposeConfig, error) { + if expose == nil { + return api.ExposeConfig{}, nil + } + + paths, err := connectProxyExposePaths(expose.Paths, networks) + if err != nil { + return api.ExposeConfig{}, err + } + + return api.ExposeConfig{ + Checks: false, + Paths: paths, + }, nil +} + +func connectProxyExposePaths(in []structs.ConsulExposePath, networks structs.Networks) ([]api.ExposePath, error) { + if len(in) == 0 { + return nil, nil + } + + paths := make([]api.ExposePath, len(in)) + for i, path := range in { + if _, exposedPort, err := connectExposePathPort(path.ListenerPort, networks); err != nil { + return nil, err + } else { + paths[i] = api.ExposePath{ + ListenerPort: exposedPort, + Path: path.Path, + LocalPathPort: path.LocalPathPort, + Protocol: path.Protocol, + ParsedFromCheck: false, + } + } + } + return paths, nil +} + +func connectUpstreams(in []structs.ConsulUpstream) []api.Upstream { + if len(in) == 0 { + return nil + } + + upstreams := make([]api.Upstream, len(in)) + for i, upstream := range in { + upstreams[i] = api.Upstream{ + DestinationName: upstream.DestinationName, + LocalBindPort: upstream.LocalBindPort, + } + } + return upstreams +} + +func connectProxyConfig(cfg map[string]interface{}, port int) map[string]interface{} { + if cfg == nil { + cfg = make(map[string]interface{}) + } + cfg["bind_address"] = "0.0.0.0" + cfg["bind_port"] = port + return cfg +} + +func connectNetworkInvariants(networks structs.Networks) error { + if n := len(networks); n != 1 { + return fmt.Errorf("Connect only supported with exactly 1 network (found %d)", n) + } + return nil +} + +// 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) { + if err := connectNetworkInvariants(networks); err != nil { + return nil, structs.Port{}, err + } + + port, ok := networks[0].PortForService(serviceName) + if !ok { + return nil, structs.Port{}, fmt.Errorf("No Connect port defined for service %q", serviceName) + } + + return networks[0], port, nil +} + +// connectExposePathPort returns the port for the exposed path for the exposed +// proxy path. +func connectExposePathPort(portLabel string, networks structs.Networks) (string, int, error) { + if err := connectNetworkInvariants(networks); err != nil { + return "", 0, err + } + + ip, port := networks.Port(portLabel) + if port == 0 { + return "", 0, fmt.Errorf("No port of label %q defined", portLabel) + } + + return ip, port, nil +} diff --git a/command/agent/consul/connect_test.go b/command/agent/consul/connect_test.go new file mode 100644 index 00000000000..5f14988c63e --- /dev/null +++ b/command/agent/consul/connect_test.go @@ -0,0 +1,376 @@ +package consul + +import ( + "testing" + + "github.com/hashicorp/consul/api" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/stretchr/testify/require" +) + +var ( + testConnectNetwork = structs.Networks{{ + Mode: "bridge", + Device: "eth0", + IP: "192.168.30.1", + DynamicPorts: []structs.Port{ + {Label: "healthPort", Value: 23100, To: 23100}, + {Label: "metricsPort", Value: 23200, To: 23200}, + {Label: "connect-proxy-redis", Value: 3000, To: 3000}, + }, + }} +) + +func TestConnect_newConnect(t *testing.T) { + t.Parallel() + + t.Run("nil", func(t *testing.T) { + asr, err := newConnect("", nil, nil) + require.NoError(t, err) + require.Nil(t, asr) + }) + + t.Run("native", func(t *testing.T) { + asr, err := newConnect("", &structs.ConsulConnect{ + Native: true, + }, 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{ + Native: false, + SidecarService: &structs.ConsulSidecarService{ + Tags: []string{"foo", "bar"}, + Port: "sidecarPort", + }, + }, testConnectNetwork) + require.NoError(t, err) + require.Equal(t, &api.AgentServiceRegistration{ + Tags: []string{"foo", "bar"}, + Port: 3000, + Address: "192.168.30.1", + }, asr.SidecarService) + }) +} + +func TestConnect_connectSidecarRegistration(t *testing.T) { + t.Parallel() + + t.Run("nil", func(t *testing.T) { + sidecarReg, err := connectSidecarRegistration("", nil, testConnectNetwork) + 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"`) + }) + + t.Run("bad proxy", func(t *testing.T) { + _, err := connectSidecarRegistration("redis", &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Paths: []structs.ConsulExposePath{{ + ListenerPort: "badPort", + }}, + }, + }, + }, testConnectNetwork) + require.EqualError(t, err, `No port of label "badPort" defined`) + }) + + t.Run("normal", func(t *testing.T) { + proxy, err := connectSidecarRegistration("redis", &structs.ConsulSidecarService{ + Tags: []string{"foo", "bar"}, + Port: "sidecarPort", + }, testConnectNetwork) + require.NoError(t, err) + require.Equal(t, &api.AgentServiceRegistration{ + Tags: []string{"foo", "bar"}, + Port: 3000, + Address: "192.168.30.1", + }, proxy) + }) +} + +func TestConnect_connectProxy(t *testing.T) { + t.Parallel() + + t.Run("nil", func(t *testing.T) { + proxy, err := connectProxy(nil, 2000, nil) + require.NoError(t, err) + require.Nil(t, proxy) + }) + + t.Run("bad proxy", func(t *testing.T) { + _, err := connectProxy(&structs.ConsulProxy{ + LocalServiceAddress: "0.0.0.0", + LocalServicePort: 2000, + Upstreams: nil, + Expose: &structs.ConsulExposeConfig{ + Paths: []structs.ConsulExposePath{{ + ListenerPort: "badPort", + }}, + }, + Config: nil, + }, 2000, testConnectNetwork) + require.EqualError(t, err, `No port of label "badPort" defined`) + }) + + t.Run("normal", func(t *testing.T) { + proxy, err := connectProxy(&structs.ConsulProxy{ + LocalServiceAddress: "0.0.0.0", + LocalServicePort: 2000, + Upstreams: nil, + Expose: &structs.ConsulExposeConfig{ + Paths: []structs.ConsulExposePath{{ + Path: "/health", + Protocol: "http", + LocalPathPort: 8000, + ListenerPort: "healthPort", + }}, + }, + Config: nil, + }, 2000, testConnectNetwork) + require.NoError(t, err) + require.Equal(t, &api.AgentServiceConnectProxyConfig{ + LocalServiceAddress: "0.0.0.0", + LocalServicePort: 2000, + Upstreams: nil, + Expose: api.ExposeConfig{ + Paths: []api.ExposePath{{ + Path: "/health", + Protocol: "http", + LocalPathPort: 8000, + ListenerPort: 23100, + }}, + }, + Config: map[string]interface{}{ + "bind_address": "0.0.0.0", + "bind_port": 2000, + }, + }, proxy) + }) +} + +func TestConnect_connectProxyExpose(t *testing.T) { + t.Parallel() + + t.Run("nil", func(t *testing.T) { + exposeConfig, err := connectProxyExpose(nil, nil) + require.NoError(t, err) + require.Equal(t, api.ExposeConfig{}, exposeConfig) + }) + + t.Run("bad port", func(t *testing.T) { + _, err := connectProxyExpose(&structs.ConsulExposeConfig{ + Paths: []structs.ConsulExposePath{{ + ListenerPort: "badPort", + }}, + }, testConnectNetwork) + require.EqualError(t, err, `No port of label "badPort" defined`) + }) + + t.Run("normal", func(t *testing.T) { + expose, err := connectProxyExpose(&structs.ConsulExposeConfig{ + Paths: []structs.ConsulExposePath{{ + Path: "/health", + Protocol: "http", + LocalPathPort: 8000, + ListenerPort: "healthPort", + }}, + }, testConnectNetwork) + require.NoError(t, err) + require.Equal(t, api.ExposeConfig{ + Checks: false, + Paths: []api.ExposePath{{ + Path: "/health", + ListenerPort: 23100, + LocalPathPort: 8000, + Protocol: "http", + ParsedFromCheck: false, + }}, + }, expose) + }) +} + +func TestConnect_connectProxyExposePaths(t *testing.T) { + t.Parallel() + + t.Run("nil", func(t *testing.T) { + upstreams, err := connectProxyExposePaths(nil, nil) + require.NoError(t, err) + require.Empty(t, upstreams) + }) + + t.Run("no network", func(t *testing.T) { + original := []structs.ConsulExposePath{{Path: "/path"}} + _, err := connectProxyExposePaths(original, nil) + require.EqualError(t, err, `Connect only supported with exactly 1 network (found 0)`) + }) + + t.Run("normal", func(t *testing.T) { + original := []structs.ConsulExposePath{{ + Path: "/health", + Protocol: "http", + LocalPathPort: 8000, + ListenerPort: "healthPort", + }, { + Path: "/metrics", + Protocol: "grpc", + LocalPathPort: 9500, + ListenerPort: "metricsPort", + }} + exposePaths, err := connectProxyExposePaths(original, testConnectNetwork) + require.NoError(t, err) + require.Equal(t, []api.ExposePath{ + { + Path: "/health", + Protocol: "http", + LocalPathPort: 8000, + ListenerPort: 23100, + ParsedFromCheck: false, + }, + { + Path: "/metrics", + Protocol: "grpc", + LocalPathPort: 9500, + ListenerPort: 23200, + ParsedFromCheck: false, + }, + }, exposePaths) + }) +} + +func TestConnect_connectUpstreams(t *testing.T) { + t.Parallel() + + t.Run("nil", func(t *testing.T) { + require.Nil(t, connectUpstreams(nil)) + }) + + t.Run("not empty", func(t *testing.T) { + require.Equal(t, + []api.Upstream{{ + DestinationName: "foo", + LocalBindPort: 8000, + }, { + DestinationName: "bar", + LocalBindPort: 9000, + }}, + connectUpstreams([]structs.ConsulUpstream{{ + DestinationName: "foo", + LocalBindPort: 8000, + }, { + DestinationName: "bar", + LocalBindPort: 9000, + }}), + ) + }) +} + +func TestConnect_connectProxyConfig(t *testing.T) { + t.Parallel() + + t.Run("nil map", func(t *testing.T) { + require.Equal(t, map[string]interface{}{ + "bind_address": "0.0.0.0", + "bind_port": 42, + }, connectProxyConfig(nil, 42)) + }) + + t.Run("pre-existing map", func(t *testing.T) { + require.Equal(t, map[string]interface{}{ + "bind_address": "0.0.0.0", + "bind_port": 42, + "foo": "bar", + }, connectProxyConfig(map[string]interface{}{ + "foo": "bar", + }, 42)) + }) +} + +func TestConnect_getConnectPort(t *testing.T) { + t.Parallel() + + networks := structs.Networks{{ + IP: "192.168.30.1", + DynamicPorts: []structs.Port{{ + Label: "connect-proxy-foo", + Value: 23456, + To: 23456, + }}}} + + t.Run("normal", func(t *testing.T) { + nr, port, err := connectPort("foo", networks) + 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) + }) + + t.Run("no such service", func(t *testing.T) { + _, _, err := connectPort("other", networks) + require.EqualError(t, err, `No Connect port defined for service "other"`) + }) + + t.Run("no network", func(t *testing.T) { + _, _, err := connectPort("foo", 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{ + Device: "eth1", + IP: "10.0.10.0", + })) + require.EqualError(t, err, "Connect only supported with exactly 1 network (found 2)") + }) +} + +func TestConnect_getExposePathPort(t *testing.T) { + t.Parallel() + + networks := structs.Networks{{ + Device: "eth0", + IP: "192.168.30.1", + DynamicPorts: []structs.Port{{ + Label: "myPort", + Value: 23456, + To: 23456, + }}}} + + t.Run("normal", func(t *testing.T) { + ip, port, err := connectExposePathPort("myPort", networks) + require.NoError(t, err) + require.Equal(t, ip, "192.168.30.1") + require.Equal(t, 23456, port) + }) + + t.Run("no such port label", func(t *testing.T) { + _, _, err := connectExposePathPort("otherPort", networks) + require.EqualError(t, err, `No port of label "otherPort" defined`) + }) + + t.Run("no network", func(t *testing.T) { + _, _, err := connectExposePathPort("myPort", nil) + require.EqualError(t, err, "Connect only supported with exactly 1 network (found 0)") + }) + + t.Run("multi network", func(t *testing.T) { + _, _, err := connectExposePathPort("myPort", append(networks, &structs.NetworkResource{ + Device: "eth1", + IP: "10.0.10.0", + })) + require.EqualError(t, err, "Connect only supported with exactly 1 network (found 2)") + }) +} diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index b76329c5691..4a11ccc6d99 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1092,67 +1092,110 @@ func ApiConsulConnectToStructs(in *api.ConsulConnect) *structs.ConsulConnect { if in == nil { return nil } + return &structs.ConsulConnect{ + Native: in.Native, + SidecarService: apiConnectSidecarServiceToStructs(in.SidecarService), + SidecarTask: apiConnectSidecarTaskToStructs(in.SidecarTask), + } +} - out := &structs.ConsulConnect{ - Native: in.Native, +func apiConnectSidecarServiceToStructs(in *api.ConsulSidecarService) *structs.ConsulSidecarService { + if in == nil { + return nil + } + return &structs.ConsulSidecarService{ + Port: in.Port, + Tags: helper.CopySliceString(in.Tags), + Proxy: apiConnectSidecarServiceProxyToStructs(in.Proxy), } +} - if in.SidecarService != nil { +func apiConnectSidecarServiceProxyToStructs(in *api.ConsulProxy) *structs.ConsulProxy { + if in == nil { + return nil + } + return &structs.ConsulProxy{ + LocalServiceAddress: in.LocalServiceAddress, + LocalServicePort: in.LocalServicePort, + Upstreams: apiUpstreamsToStructs(in.Upstreams), + Expose: apiConsulExposeConfigToStructs(in.ExposeConfig), + Config: in.Config, + } +} - out.SidecarService = &structs.ConsulSidecarService{ - Tags: helper.CopySliceString(in.SidecarService.Tags), - Port: in.SidecarService.Port, +func apiUpstreamsToStructs(in []*api.ConsulUpstream) []structs.ConsulUpstream { + if len(in) == 0 { + return nil + } + upstreams := make([]structs.ConsulUpstream, len(in)) + for i, upstream := range in { + upstreams[i] = structs.ConsulUpstream{ + DestinationName: upstream.DestinationName, + LocalBindPort: upstream.LocalBindPort, } + } + return upstreams +} - if in.SidecarService.Proxy != nil { - - out.SidecarService.Proxy = &structs.ConsulProxy{ - LocalServiceAddress: in.SidecarService.Proxy.LocalServiceAddress, - LocalServicePort: in.SidecarService.Proxy.LocalServicePort, - Config: in.SidecarService.Proxy.Config, - } - - upstreams := make([]structs.ConsulUpstream, len(in.SidecarService.Proxy.Upstreams)) - for i, p := range in.SidecarService.Proxy.Upstreams { - upstreams[i] = structs.ConsulUpstream{ - DestinationName: p.DestinationName, - LocalBindPort: p.LocalBindPort, - } - } +func apiConsulExposeConfigToStructs(in *api.ConsulExposeConfig) *structs.ConsulExposeConfig { + if in == nil { + return nil + } + return &structs.ConsulExposeConfig{ + Paths: apiConsulExposePathsToStructs(in.Paths), + } +} - out.SidecarService.Proxy.Upstreams = upstreams +func apiConsulExposePathsToStructs(in []*api.ConsulExposePath) []structs.ConsulExposePath { + if len(in) == 0 { + return nil + } + paths := make([]structs.ConsulExposePath, len(in)) + for i, path := range in { + paths[i] = structs.ConsulExposePath{ + Path: path.Path, + Protocol: path.Protocol, + LocalPathPort: path.LocalPathPort, + ListenerPort: path.ListenerPort, } } + return paths +} - if in.SidecarTask != nil { - out.SidecarTask = &structs.SidecarTask{ - Name: in.SidecarTask.Name, - Driver: in.SidecarTask.Driver, - Config: in.SidecarTask.Config, - User: in.SidecarTask.User, - Env: in.SidecarTask.Env, - Resources: ApiResourcesToStructs(in.SidecarTask.Resources), - Meta: in.SidecarTask.Meta, - LogConfig: &structs.LogConfig{}, - ShutdownDelay: in.SidecarTask.ShutdownDelay, - KillSignal: in.SidecarTask.KillSignal, - } +func apiConnectSidecarTaskToStructs(in *api.SidecarTask) *structs.SidecarTask { + if in == nil { + return nil + } + return &structs.SidecarTask{ + Name: in.Name, + Driver: in.Driver, + User: in.User, + Config: in.Config, + Env: in.Env, + Resources: ApiResourcesToStructs(in.Resources), + Meta: in.Meta, + ShutdownDelay: in.ShutdownDelay, + KillSignal: in.KillSignal, + KillTimeout: in.KillTimeout, + LogConfig: apiLogConfigToStructs(in.LogConfig), + } +} - if in.SidecarTask.KillTimeout != nil { - out.SidecarTask.KillTimeout = in.SidecarTask.KillTimeout - } - if in.SidecarTask.LogConfig != nil { - out.SidecarTask.LogConfig = &structs.LogConfig{} - if in.SidecarTask.LogConfig.MaxFiles != nil { - out.SidecarTask.LogConfig.MaxFiles = *in.SidecarTask.LogConfig.MaxFiles - } - if in.SidecarTask.LogConfig.MaxFileSizeMB != nil { - out.SidecarTask.LogConfig.MaxFileSizeMB = *in.SidecarTask.LogConfig.MaxFileSizeMB - } - } +func apiLogConfigToStructs(in *api.LogConfig) *structs.LogConfig { + if in == nil { + return nil } + return &structs.LogConfig{ + MaxFiles: dereferenceInt(in.MaxFiles), + MaxFileSizeMB: dereferenceInt(in.MaxFileSizeMB), + } +} - return out +func dereferenceInt(in *int) int { + if in == nil { + return 0 + } + return *in } func ApiConstraintsToStructs(in []*api.Constraint) []*structs.Constraint { diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 807a5837517..73e191538ea 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2425,3 +2425,169 @@ func TestHTTP_JobValidate_SystemMigrate(t *testing.T) { require.Contains(t, resp.Error, `Job type "system" does not allow migrate block`) }) } + +func TestConversion_dereferenceInt(t *testing.T) { + t.Parallel() + require.Equal(t, 0, dereferenceInt(nil)) + require.Equal(t, 42, dereferenceInt(helper.IntToPtr(42))) +} + +func TestConversion_apiLogConfigToStructs(t *testing.T) { + t.Parallel() + require.Nil(t, apiLogConfigToStructs(nil)) + require.Equal(t, &structs.LogConfig{ + MaxFiles: 2, + MaxFileSizeMB: 8, + }, apiLogConfigToStructs(&api.LogConfig{ + MaxFiles: helper.IntToPtr(2), + MaxFileSizeMB: helper.IntToPtr(8), + })) +} + +func TestConversion_apiConnectSidecarTaskToStructs(t *testing.T) { + t.Parallel() + require.Nil(t, apiConnectSidecarTaskToStructs(nil)) + delay := time.Duration(200) + timeout := time.Duration(1000) + config := make(map[string]interface{}) + env := make(map[string]string) + meta := make(map[string]string) + require.Equal(t, &structs.SidecarTask{ + Name: "name", + Driver: "driver", + User: "user", + Config: config, + Env: env, + Resources: &structs.Resources{ + CPU: 1, + MemoryMB: 128, + }, + Meta: meta, + KillTimeout: &timeout, + LogConfig: &structs.LogConfig{ + MaxFiles: 2, + MaxFileSizeMB: 8, + }, + ShutdownDelay: &delay, + KillSignal: "SIGTERM", + }, apiConnectSidecarTaskToStructs(&api.SidecarTask{ + Name: "name", + Driver: "driver", + User: "user", + Config: config, + Env: env, + Resources: &api.Resources{ + CPU: helper.IntToPtr(1), + MemoryMB: helper.IntToPtr(128), + }, + Meta: meta, + KillTimeout: &timeout, + LogConfig: &api.LogConfig{ + MaxFiles: helper.IntToPtr(2), + MaxFileSizeMB: helper.IntToPtr(8), + }, + ShutdownDelay: &delay, + KillSignal: "SIGTERM", + })) +} + +func TestConversion_apiConsulExposePathsToStructs(t *testing.T) { + t.Parallel() + require.Nil(t, apiConsulExposePathsToStructs(nil)) + require.Nil(t, apiConsulExposePathsToStructs(make([]*api.ConsulExposePath, 0))) + require.Equal(t, []structs.ConsulExposePath{{ + Path: "/health", + Protocol: "http", + LocalPathPort: 8080, + ListenerPort: "hcPort", + }}, apiConsulExposePathsToStructs([]*api.ConsulExposePath{{ + Path: "/health", + Protocol: "http", + LocalPathPort: 8080, + ListenerPort: "hcPort", + }})) +} + +func TestConversion_apiConsulExposeConfigToStructs(t *testing.T) { + t.Parallel() + require.Nil(t, apiConsulExposeConfigToStructs(nil)) + require.Equal(t, &structs.ConsulExposeConfig{ + Paths: []structs.ConsulExposePath{{Path: "/health"}}, + }, apiConsulExposeConfigToStructs(&api.ConsulExposeConfig{ + Paths: []*api.ConsulExposePath{{Path: "/health"}}, + })) +} + +func TestConversion_apiUpstreamsToStructs(t *testing.T) { + t.Parallel() + require.Nil(t, apiUpstreamsToStructs(nil)) + require.Nil(t, apiUpstreamsToStructs(make([]*api.ConsulUpstream, 0))) + require.Equal(t, []structs.ConsulUpstream{{ + DestinationName: "upstream", + LocalBindPort: 8000, + }}, apiUpstreamsToStructs([]*api.ConsulUpstream{{ + DestinationName: "upstream", + LocalBindPort: 8000, + }})) +} + +func TestConversion_apiConnectSidecarServiceProxyToStructs(t *testing.T) { + t.Parallel() + require.Nil(t, apiConnectSidecarServiceProxyToStructs(nil)) + config := make(map[string]interface{}) + require.Equal(t, &structs.ConsulProxy{ + LocalServiceAddress: "192.168.30.1", + LocalServicePort: 9000, + Config: config, + Upstreams: []structs.ConsulUpstream{{ + DestinationName: "upstream", + }}, + Expose: &structs.ConsulExposeConfig{ + Paths: []structs.ConsulExposePath{{Path: "/health"}}, + }, + }, apiConnectSidecarServiceProxyToStructs(&api.ConsulProxy{ + LocalServiceAddress: "192.168.30.1", + LocalServicePort: 9000, + Config: config, + Upstreams: []*api.ConsulUpstream{{ + DestinationName: "upstream", + }}, + ExposeConfig: &api.ConsulExposeConfig{ + Paths: []*api.ConsulExposePath{{ + Path: "/health", + }}, + }, + })) +} + +func TestConversion_apiConnectSidecarServiceToStructs(t *testing.T) { + t.Parallel() + require.Nil(t, apiConnectSidecarTaskToStructs(nil)) + require.Equal(t, &structs.ConsulSidecarService{ + Tags: []string{"foo"}, + Port: "myPort", + Proxy: &structs.ConsulProxy{ + LocalServiceAddress: "192.168.30.1", + }, + }, apiConnectSidecarServiceToStructs(&api.ConsulSidecarService{ + Tags: []string{"foo"}, + Port: "myPort", + Proxy: &api.ConsulProxy{ + LocalServiceAddress: "192.168.30.1", + }, + })) +} + +func TestConversion_ApiConsulConnectToStructs(t *testing.T) { + t.Parallel() + require.Nil(t, ApiConsulConnectToStructs(nil)) + require.Equal(t, &structs.ConsulConnect{ + Native: false, + SidecarService: &structs.ConsulSidecarService{Port: "myPort"}, + SidecarTask: &structs.SidecarTask{Name: "task"}, + }, ApiConsulConnectToStructs(&api.ConsulConnect{ + Native: false, + SidecarService: &api.ConsulSidecarService{Port: "myPort"}, + SidecarTask: &api.SidecarTask{Name: "task"}, + })) +} diff --git a/jobspec/parse_service.go b/jobspec/parse_service.go index e44754a301a..ff591160b18 100644 --- a/jobspec/parse_service.go +++ b/jobspec/parse_service.go @@ -306,7 +306,7 @@ func parseSidecarTask(item *ast.ObjectItem) (*api.SidecarTask, error) { KillSignal: task.KillSignal, } - // Parse ShutdownDelay seperatly to get pointer + // Parse ShutdownDelay separately to get pointer var m map[string]interface{} if err := hcl.DecodeObject(&m, item.Val); err != nil { return nil, err @@ -336,6 +336,7 @@ func parseProxy(o *ast.ObjectItem) (*api.ConsulProxy, error) { "local_service_address", "local_service_port", "upstreams", + "expose", "config", } @@ -353,15 +354,27 @@ func parseProxy(o *ast.ObjectItem) (*api.ConsulProxy, error) { } // Parse the proxy + uo := listVal.Filter("upstreams") - proxy.Upstreams = make([]*api.ConsulUpstream, len(uo.Items)) - for i := range uo.Items { - u, err := parseUpstream(uo.Items[i]) - if err != nil { - return nil, err + if len(uo.Items) > 0 { + proxy.Upstreams = make([]*api.ConsulUpstream, len(uo.Items)) + for i := range uo.Items { + u, err := parseUpstream(uo.Items[i]) + if err != nil { + return nil, err + } + proxy.Upstreams[i] = u } + } - proxy.Upstreams[i] = u + if eo := listVal.Filter("expose"); len(eo.Items) > 1 { + return nil, fmt.Errorf("only 1 expose object supported") + } else if len(eo.Items) == 1 { + if e, err := parseExpose(eo.Items[0]); err != nil { + return nil, err + } else { + proxy.ExposeConfig = e + } } // If we have config, then parse that @@ -389,6 +402,42 @@ func parseProxy(o *ast.ObjectItem) (*api.ConsulProxy, error) { return &proxy, nil } +func parseExpose(eo *ast.ObjectItem) (*api.ConsulExposeConfig, error) { + var listVal *ast.ObjectList + if eoType, ok := eo.Val.(*ast.ObjectType); ok { + listVal = eoType.List + } else { + return nil, fmt.Errorf("expose: should be an object") + } + + valid := []string{ + "paths", + } + + if err := helper.CheckHCLKeys(listVal, valid); err != nil { + return nil, multierror.Prefix(err, "expose ->") + } + + var m map[string]interface{} + if err := hcl.DecodeObject(&m, eo.Val); err != nil { + return nil, err + } + + // Build the expose block + var expose api.ConsulExposeConfig + dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + Result: &expose, + }) + if err != nil { + return nil, err + } + if err := dec.Decode(m); err != nil { + return nil, err + } + + return &expose, nil +} + func parseUpstream(uo *ast.ObjectItem) (*api.ConsulUpstream, error) { valid := []string{ "destination_name", @@ -420,6 +469,7 @@ func parseUpstream(uo *ast.ObjectItem) (*api.ConsulUpstream, error) { return &upstream, nil } + func parseChecks(service *api.Service, checkObjs *ast.ObjectList) error { service.Checks = make([]api.ServiceCheck, len(checkObjs.Items)) for idx, co := range checkObjs.Items { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 924d3ab719e..9a73878e2d2 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -1114,6 +1114,34 @@ func TestParse(t *testing.T) { }, false, }, + { + "tg-service-proxy-expose.hcl", + &api.Job{ + ID: helper.StringToPtr("group_service_proxy_expose"), + Name: helper.StringToPtr("group_service_proxy_expose"), + TaskGroups: []*api.TaskGroup{{ + Name: helper.StringToPtr("group"), + Services: []*api.Service{{ + Name: "example", + Connect: &api.ConsulConnect{ + SidecarService: &api.ConsulSidecarService{ + Proxy: &api.ConsulProxy{ + ExposeConfig: &api.ConsulExposeConfig{ + Paths: []*api.ConsulExposePath{{ + Path: "/health", + Protocol: "http", + LocalPathPort: 2222, + ListenerPort: "healthcheck", + }}, + }, + }, + }, + }, + }}, + }}, + }, + false, + }, { "tg-service-enable-tag-override.hcl", &api.Job{ diff --git a/jobspec/test-fixtures/tg-service-proxy-expose.hcl b/jobspec/test-fixtures/tg-service-proxy-expose.hcl new file mode 100644 index 00000000000..2195a716f7c --- /dev/null +++ b/jobspec/test-fixtures/tg-service-proxy-expose.hcl @@ -0,0 +1,21 @@ +job "group_service_proxy_expose" { + group "group" { + service { + name = "example" + connect { + sidecar_service { + proxy { + expose { + paths = [{ + path = "/health" + protocol = "http" + local_path_port = 2222 + listener_port = "healthcheck" + }] + } + } + } + } + } + } +} diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go index b18b4c1ec0f..fca81ef1d5e 100644 --- a/nomad/job_endpoint_hook_connect.go +++ b/nomad/job_endpoint_hook_connect.go @@ -30,9 +30,9 @@ var ( } // connectVersionConstraint is used when building the sidecar task to ensure - // the proper Consul version is used that supports the nessicary Connect - // features. This includes bootstraping envoy with a unix socket for Consul's - // grpc xDS api. + // the proper Consul version is used that supports the necessary Connect + // features. This includes bootstrapping envoy with a unix socket for Consul's + // gRPC xDS API. connectVersionConstraint = func() *structs.Constraint { return &structs.Constraint{ LTarget: "${attr.consul.version}", @@ -97,6 +97,8 @@ func isSidecarForService(t *structs.Task, svc string) bool { return t.Kind == structs.TaskKind(fmt.Sprintf("%s:%s", structs.ConnectProxyPrefix, svc)) } +// probably need to hack this up to look for checks on the service, and if they +// qualify, configure a port for envoy to use to expose their paths. func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error { for _, service := range g.Services { if service.Connect.HasSidecar() { @@ -125,29 +127,28 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error { // 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), - - // -1 is a sentinel value to instruct the - // scheduler to map the host's dynamic port to - // the same port in the netns. - To: -1, - } - - // check that port hasn't already been defined before adding it to tg - var found bool - for _, p := range g.Networks[0].DynamicPorts { - if p.Label == port.Label { - found = true - break + makePort := func(label string) { + // check that port hasn't already been defined before adding it to tg + for _, p := range g.Networks[0].DynamicPorts { + if p.Label == label { + return + } } + g.Networks[0].DynamicPorts = append(g.Networks[0].DynamicPorts, structs.Port{ + Label: label, + // -1 is a sentinel value to instruct the + // scheduler to map the host's dynamic port to + // the same port in the netns. + To: -1, + }) } - if !found { - g.Networks[0].DynamicPorts = append(g.Networks[0].DynamicPorts, port) - } + + // create a port for the sidecar task's proxy port + makePort(fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, service.Name)) + // todo(shoenig) magic port for 'expose.checks' } } + return nil } diff --git a/nomad/structs/services.go b/nomad/structs/services.go index dbafdfd1164..5b9f38f2fa6 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -852,6 +852,10 @@ type ConsulProxy struct { // connect to. Upstreams []ConsulUpstream + // Expose configures the consul proxy.expose stanza to "open up" endpoints + // used by task-group level service checks using HTTP or gRPC protocols. + Expose *ConsulExposeConfig + // Config is a proxy configuration. It is opaque to Nomad and passed // directly to Consul. Config map[string]interface{} @@ -863,9 +867,11 @@ func (p *ConsulProxy) Copy() *ConsulProxy { return nil } - newP := ConsulProxy{} - newP.LocalServiceAddress = p.LocalServiceAddress - newP.LocalServicePort = p.LocalServicePort + newP := &ConsulProxy{ + LocalServiceAddress: p.LocalServiceAddress, + LocalServicePort: p.LocalServicePort, + Expose: p.Expose, + } if n := len(p.Upstreams); n > 0 { newP.Upstreams = make([]ConsulUpstream, n) @@ -883,7 +889,7 @@ func (p *ConsulProxy) Copy() *ConsulProxy { } } - return &newP + return newP } // Equals returns true if the structs are recursively equal. @@ -895,24 +901,16 @@ func (p *ConsulProxy) Equals(o *ConsulProxy) bool { if p.LocalServiceAddress != o.LocalServiceAddress { return false } + if p.LocalServicePort != o.LocalServicePort { return false } - if len(p.Upstreams) != len(o.Upstreams) { + + if !p.Expose.Equals(o.Expose) { return false } - // Order doesn't matter -OUTER: - for _, up := range p.Upstreams { - for _, innerUp := range o.Upstreams { - if up.Equals(&innerUp) { - // Match; find next upstream - continue OUTER - } - } - - // No match + if !upstreamsEquals(p.Upstreams, o.Upstreams) { return false } @@ -936,7 +934,24 @@ type ConsulUpstream struct { LocalBindPort int } -// Copy the stanza recursively. Returns nil if nil. +func upstreamsEquals(a, b []ConsulUpstream) bool { + if len(a) != len(b) { + return false + } + +LOOP: // order does not matter + for _, upA := range a { + for _, upB := range b { + if upA.Equals(&upB) { + continue LOOP + } + } + return false + } + return true +} + +// Copy the stanza recursively. Returns nil if u is nil. func (u *ConsulUpstream) Copy() *ConsulUpstream { if u == nil { return nil @@ -956,3 +971,54 @@ func (u *ConsulUpstream) Equals(o *ConsulUpstream) bool { return (*u) == (*o) } + +// ExposeConfig represents a Consul Connect expose jobspec stanza. +type ConsulExposeConfig struct { + Paths []ConsulExposePath +} + +type ConsulExposePath struct { + Path string + Protocol string + LocalPathPort int + ListenerPort string +} + +func exposePathsEqual(pathsA, pathsB []ConsulExposePath) bool { + if len(pathsA) != len(pathsB) { + return false + } + +LOOP: // order does not matter + for _, pathA := range pathsA { + for _, pathB := range pathsB { + if pathA == pathB { + continue LOOP + } + } + return false + } + return true +} + +// Copy the stanza. Returns nil if e is nil. +func (e *ConsulExposeConfig) Copy() *ConsulExposeConfig { + if e == nil { + return nil + } + paths := make([]ConsulExposePath, len(e.Paths)) + for i := 0; i < len(e.Paths); i++ { + paths[i] = e.Paths[i] + } + return &ConsulExposeConfig{ + Paths: paths, + } +} + +// Equals returns true if the structs are recursively equal. +func (e *ConsulExposeConfig) Equals(o *ConsulExposeConfig) bool { + if e == nil || o == nil { + return e == o + } + return exposePathsEqual(e.Paths, o.Paths) +} diff --git a/nomad/structs/services_test.go b/nomad/structs/services_test.go index ac5c73ee551..b889a9ec7c3 100644 --- a/nomad/structs/services_test.go +++ b/nomad/structs/services_test.go @@ -174,7 +174,6 @@ func TestConsulConnect_CopyEquals(t *testing.T) { } func TestSidecarTask_MergeIntoTask(t *testing.T) { - task := MockJob().TaskGroups[0].Tasks[0] sTask := &SidecarTask{ Name: "sidecar", @@ -226,5 +225,102 @@ func TestSidecarTask_MergeIntoTask(t *testing.T) { sTask.MergeIntoTask(task) require.Exactly(t, expected, task) +} + +func TestConsulUpstream_upstreamEquals(t *testing.T) { + t.Parallel() + + up := func(name string, port int) ConsulUpstream { + return ConsulUpstream{ + DestinationName: name, + LocalBindPort: port, + } + } + + t.Run("size mismatch", func(t *testing.T) { + a := []ConsulUpstream{up("foo", 8000)} + b := []ConsulUpstream{up("foo", 8000), up("bar", 9000)} + require.False(t, upstreamsEquals(a, b)) + }) + + t.Run("different", func(t *testing.T) { + a := []ConsulUpstream{up("bar", 9000)} + b := []ConsulUpstream{up("foo", 8000)} + require.False(t, upstreamsEquals(a, b)) + }) + + t.Run("identical", func(t *testing.T) { + a := []ConsulUpstream{up("foo", 8000), up("bar", 9000)} + b := []ConsulUpstream{up("foo", 8000), up("bar", 9000)} + require.True(t, upstreamsEquals(a, b)) + }) + + t.Run("unsorted", func(t *testing.T) { + a := []ConsulUpstream{up("foo", 8000), up("bar", 9000)} + b := []ConsulUpstream{up("bar", 9000), up("foo", 8000)} + require.True(t, upstreamsEquals(a, b)) + }) +} + +func TestConsulExposePath_exposePathsEqual(t *testing.T) { + t.Parallel() + + expose := func(path, protocol, listen string, local int) ConsulExposePath { + return ConsulExposePath{ + Path: path, + Protocol: protocol, + LocalPathPort: local, + ListenerPort: listen, + } + } + + t.Run("size mismatch", func(t *testing.T) { + a := []ConsulExposePath{expose("/1", "http", "myPort", 8000)} + b := []ConsulExposePath{expose("/1", "http", "myPort", 8000), expose("/2", "http", "myPort", 8000)} + require.False(t, exposePathsEqual(a, b)) + }) + + t.Run("different", func(t *testing.T) { + a := []ConsulExposePath{expose("/1", "http", "myPort", 8000)} + b := []ConsulExposePath{expose("/2", "http", "myPort", 8000)} + require.False(t, exposePathsEqual(a, b)) + }) + + t.Run("identical", func(t *testing.T) { + a := []ConsulExposePath{expose("/1", "http", "myPort", 8000)} + b := []ConsulExposePath{expose("/1", "http", "myPort", 8000)} + require.True(t, exposePathsEqual(a, b)) + }) + + t.Run("unsorted", func(t *testing.T) { + a := []ConsulExposePath{expose("/2", "http", "myPort", 8000), expose("/1", "http", "myPort", 8000)} + b := []ConsulExposePath{expose("/1", "http", "myPort", 8000), expose("/2", "http", "myPort", 8000)} + require.True(t, exposePathsEqual(a, b)) + }) +} + +func TestConsulExposeConfig_Copy(t *testing.T) { + require.Nil(t, (*ConsulExposeConfig)(nil).Copy()) + require.Equal(t, &ConsulExposeConfig{ + Paths: []ConsulExposePath{{ + Path: "/health", + }}, + }, (&ConsulExposeConfig{ + Paths: []ConsulExposePath{{ + Path: "/health", + }}, + }).Copy()) +} +func TestConsulExposeConfig_Equals(t *testing.T) { + require.True(t, (*ConsulExposeConfig)(nil).Equals(nil)) + require.True(t, (&ConsulExposeConfig{ + Paths: []ConsulExposePath{{ + Path: "/health", + }}, + }).Equals(&ConsulExposeConfig{ + Paths: []ConsulExposePath{{ + Path: "/health", + }}, + })) } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 3c1fa2b6ca0..d540512d381 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -2341,7 +2341,7 @@ type RequestedDevice struct { // to use. Constraints Constraints - // Affinities are a set of affinites to apply when selecting the device + // Affinities are a set of affinities to apply when selecting the device // to use. Affinities Affinities } @@ -2534,18 +2534,18 @@ func (n *NodeResources) Equals(o *NodeResources) bool { } // Equals equates Networks as a set -func (n *Networks) Equals(o *Networks) bool { - if n == o { +func (ns *Networks) Equals(o *Networks) bool { + if ns == o { return true } - if n == nil || o == nil { + if ns == nil || o == nil { return false } - if len(*n) != len(*o) { + if len(*ns) != len(*o) { return false } SETEQUALS: - for _, ne := range *n { + for _, ne := range *ns { for _, oe := range *o { if ne.Equals(oe) { continue SETEQUALS From 5c5a3ab38b459d4d9b5ac39c9822d8d01ef9c293 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Thu, 12 Mar 2020 09:37:31 -0500 Subject: [PATCH 3/5] jobspec: parse multi expose.path instead of explicit slice --- api/services.go | 2 +- command/agent/job_endpoint.go | 2 +- command/agent/job_endpoint_test.go | 4 +- jobspec/parse_service.go | 48 +++++++++++++++---- jobspec/parse_test.go | 7 ++- .../test-fixtures/tg-service-proxy-expose.hcl | 11 ++++- 6 files changed, 59 insertions(+), 15 deletions(-) diff --git a/api/services.go b/api/services.go index 9a48ea06c47..823634ac09a 100644 --- a/api/services.go +++ b/api/services.go @@ -182,7 +182,7 @@ type ConsulUpstream struct { } type ConsulExposeConfig struct { - Paths []*ConsulExposePath `mapstructure:"paths"` + Path []*ConsulExposePath `mapstructure:"path"` // todo(shoenig): add magic for 'checks' option } diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 4a11ccc6d99..c5135bb545c 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1142,7 +1142,7 @@ func apiConsulExposeConfigToStructs(in *api.ConsulExposeConfig) *structs.ConsulE return nil } return &structs.ConsulExposeConfig{ - Paths: apiConsulExposePathsToStructs(in.Paths), + Paths: apiConsulExposePathsToStructs(in.Path), } } diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 73e191538ea..443e26f8400 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2514,7 +2514,7 @@ func TestConversion_apiConsulExposeConfigToStructs(t *testing.T) { require.Equal(t, &structs.ConsulExposeConfig{ Paths: []structs.ConsulExposePath{{Path: "/health"}}, }, apiConsulExposeConfigToStructs(&api.ConsulExposeConfig{ - Paths: []*api.ConsulExposePath{{Path: "/health"}}, + Path: []*api.ConsulExposePath{{Path: "/health"}}, })) } @@ -2553,7 +2553,7 @@ func TestConversion_apiConnectSidecarServiceProxyToStructs(t *testing.T) { DestinationName: "upstream", }}, ExposeConfig: &api.ConsulExposeConfig{ - Paths: []*api.ConsulExposePath{{ + Path: []*api.ConsulExposePath{{ Path: "/health", }}, }, diff --git a/jobspec/parse_service.go b/jobspec/parse_service.go index ff591160b18..f89b7d0ab5e 100644 --- a/jobspec/parse_service.go +++ b/jobspec/parse_service.go @@ -403,6 +403,17 @@ func parseProxy(o *ast.ObjectItem) (*api.ConsulProxy, error) { } func parseExpose(eo *ast.ObjectItem) (*api.ConsulExposeConfig, error) { + valid := []string{ + "path", // an array of path blocks + // todo(shoenig) checks boolean + } + + if err := helper.CheckHCLKeys(eo.Val, valid); err != nil { + return nil, multierror.Prefix(err, "expose ->") + } + + var expose api.ConsulExposeConfig + var listVal *ast.ObjectList if eoType, ok := eo.Val.(*ast.ObjectType); ok { listVal = eoType.List @@ -410,32 +421,53 @@ func parseExpose(eo *ast.ObjectItem) (*api.ConsulExposeConfig, error) { return nil, fmt.Errorf("expose: should be an object") } + // Parse the expose block + + po := listVal.Filter("path") // array + if len(po.Items) > 0 { + expose.Path = make([]*api.ConsulExposePath, len(po.Items)) + for i := range po.Items { + p, err := parseExposePath(po.Items[i]) + if err != nil { + return nil, err + } + expose.Path[i] = p + } + } + + return &expose, nil +} + +func parseExposePath(epo *ast.ObjectItem) (*api.ConsulExposePath, error) { valid := []string{ - "paths", + "path", + "protocol", + "local_path_port", + "listener_port", } - if err := helper.CheckHCLKeys(listVal, valid); err != nil { - return nil, multierror.Prefix(err, "expose ->") + if err := helper.CheckHCLKeys(epo.Val, valid); err != nil { + return nil, multierror.Prefix(err, "path ->") } + var path api.ConsulExposePath var m map[string]interface{} - if err := hcl.DecodeObject(&m, eo.Val); err != nil { + if err := hcl.DecodeObject(&m, epo.Val); err != nil { return nil, err } - // Build the expose block - var expose api.ConsulExposeConfig dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ - Result: &expose, + Result: &path, }) if err != nil { return nil, err } + if err := dec.Decode(m); err != nil { return nil, err } - return &expose, nil + return &path, nil } func parseUpstream(uo *ast.ObjectItem) (*api.ConsulUpstream, error) { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 9a73878e2d2..4f3c5699df1 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -1127,11 +1127,16 @@ func TestParse(t *testing.T) { SidecarService: &api.ConsulSidecarService{ Proxy: &api.ConsulProxy{ ExposeConfig: &api.ConsulExposeConfig{ - Paths: []*api.ConsulExposePath{{ + Path: []*api.ConsulExposePath{{ Path: "/health", Protocol: "http", LocalPathPort: 2222, ListenerPort: "healthcheck", + }, { + Path: "/metrics", + Protocol: "grpc", + LocalPathPort: 3000, + ListenerPort: "metrics", }}, }, }, diff --git a/jobspec/test-fixtures/tg-service-proxy-expose.hcl b/jobspec/test-fixtures/tg-service-proxy-expose.hcl index 2195a716f7c..b5032f9d46a 100644 --- a/jobspec/test-fixtures/tg-service-proxy-expose.hcl +++ b/jobspec/test-fixtures/tg-service-proxy-expose.hcl @@ -6,12 +6,19 @@ job "group_service_proxy_expose" { sidecar_service { proxy { expose { - paths = [{ + path = { path = "/health" protocol = "http" local_path_port = 2222 listener_port = "healthcheck" - }] + } + + path = { + path = "/metrics" + protocol = "grpc" + local_path_port = 3000 + listener_port = "metrics" + } } } } From dcac09cc92568a37ab14e8abf697990851768bbd Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 17 Mar 2020 08:24:17 -0600 Subject: [PATCH 4/5] connect: enable expose.checks configuration Similar to a Consul Service Definition, enable users to configure a single boolean which extrapolates proxy expose rules from existing service checks. By setting 'expose.checks', Nomad will automatically generate minimal 'proxy.expose.path' values for compatible service checks for connect-enabled services. In doing so, Consul checks should then Just Work if they are of type 'http' or 'grpc'. --- api/services.go | 4 +- .../taskrunner/envoybootstrap_hook.go | 3 + command/agent/job_endpoint.go | 3 +- command/agent/job_endpoint_test.go | 14 +- jobspec/parse_service.go | 25 +- jobspec/parse_test.go | 1 + .../test-fixtures/tg-service-proxy-expose.hcl | 3 +- nomad/job_endpoint.go | 2 + nomad/job_endpoint_hook_connect.go | 2 - nomad/job_endpoint_hook_expose.go | 185 ++++++++ nomad/job_endpoint_hook_expose_test.go | 439 ++++++++++++++++++ nomad/job_endpoint_test.go | 127 ++++- nomad/structs/services.go | 15 +- 13 files changed, 791 insertions(+), 32 deletions(-) create mode 100644 nomad/job_endpoint_hook_expose.go create mode 100644 nomad/job_endpoint_hook_expose_test.go diff --git a/api/services.go b/api/services.go index 823634ac09a..62c70a3d4f1 100644 --- a/api/services.go +++ b/api/services.go @@ -182,8 +182,8 @@ type ConsulUpstream struct { } type ConsulExposeConfig struct { - Path []*ConsulExposePath `mapstructure:"path"` - // todo(shoenig): add magic for 'checks' option + Checks bool `mapstructure:"checks"` + Path []*ConsulExposePath `mapstructure:"path"` } type ConsulExposePath struct { diff --git a/client/allocrunner/taskrunner/envoybootstrap_hook.go b/client/allocrunner/taskrunner/envoybootstrap_hook.go index 09efccb798c..87d7d7d2837 100644 --- a/client/allocrunner/taskrunner/envoybootstrap_hook.go +++ b/client/allocrunner/taskrunner/envoybootstrap_hook.go @@ -191,6 +191,9 @@ func buildEnvoyAdminBind(alloc *structs.Allocation, taskName string) string { break } } + + // note: bind to 0.0.0.0 and create a port-map to 19001 for envoy debugging + return fmt.Sprintf("localhost:%d", port) } diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index c5135bb545c..28fbe993fe2 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1142,7 +1142,8 @@ func apiConsulExposeConfigToStructs(in *api.ConsulExposeConfig) *structs.ConsulE return nil } return &structs.ConsulExposeConfig{ - Paths: apiConsulExposePathsToStructs(in.Path), + Checks: in.Checks, + Paths: apiConsulExposePathsToStructs(in.Path), } } diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 443e26f8400..120ac0da6b4 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2512,9 +2512,11 @@ func TestConversion_apiConsulExposeConfigToStructs(t *testing.T) { t.Parallel() require.Nil(t, apiConsulExposeConfigToStructs(nil)) require.Equal(t, &structs.ConsulExposeConfig{ - Paths: []structs.ConsulExposePath{{Path: "/health"}}, + Checks: true, + Paths: []structs.ConsulExposePath{{Path: "/health"}}, }, apiConsulExposeConfigToStructs(&api.ConsulExposeConfig{ - Path: []*api.ConsulExposePath{{Path: "/health"}}, + Checks: true, + Path: []*api.ConsulExposePath{{Path: "/health"}}, })) } @@ -2543,7 +2545,8 @@ func TestConversion_apiConnectSidecarServiceProxyToStructs(t *testing.T) { DestinationName: "upstream", }}, Expose: &structs.ConsulExposeConfig{ - Paths: []structs.ConsulExposePath{{Path: "/health"}}, + Checks: true, + Paths: []structs.ConsulExposePath{{Path: "/health"}}, }, }, apiConnectSidecarServiceProxyToStructs(&api.ConsulProxy{ LocalServiceAddress: "192.168.30.1", @@ -2553,9 +2556,8 @@ func TestConversion_apiConnectSidecarServiceProxyToStructs(t *testing.T) { DestinationName: "upstream", }}, ExposeConfig: &api.ConsulExposeConfig{ - Path: []*api.ConsulExposePath{{ - Path: "/health", - }}, + Checks: true, + Path: []*api.ConsulExposePath{{Path: "/health"}}, }, })) } diff --git a/jobspec/parse_service.go b/jobspec/parse_service.go index f89b7d0ab5e..8b7b44e7e12 100644 --- a/jobspec/parse_service.go +++ b/jobspec/parse_service.go @@ -404,8 +404,8 @@ func parseProxy(o *ast.ObjectItem) (*api.ConsulProxy, error) { func parseExpose(eo *ast.ObjectItem) (*api.ConsulExposeConfig, error) { valid := []string{ - "path", // an array of path blocks - // todo(shoenig) checks boolean + "path", // an array of path blocks + "checks", // single boolean } if err := helper.CheckHCLKeys(eo.Val, valid); err != nil { @@ -414,6 +414,25 @@ func parseExpose(eo *ast.ObjectItem) (*api.ConsulExposeConfig, error) { var expose api.ConsulExposeConfig + // Parse the checks boolean + + var m map[string]interface{} + if err := hcl.DecodeObject(&m, eo.Val); err != nil { + return nil, err + } + dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + Result: &expose, + }) + if err != nil { + return nil, err + } + delete(m, "path") // skip for now + if err := dec.Decode(m); err != nil { + return nil, err + } + + // Parse the expose block + var listVal *ast.ObjectList if eoType, ok := eo.Val.(*ast.ObjectType); ok { listVal = eoType.List @@ -421,8 +440,6 @@ func parseExpose(eo *ast.ObjectItem) (*api.ConsulExposeConfig, error) { return nil, fmt.Errorf("expose: should be an object") } - // Parse the expose block - po := listVal.Filter("path") // array if len(po.Items) > 0 { expose.Path = make([]*api.ConsulExposePath, len(po.Items)) diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 4f3c5699df1..46b70ec2258 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -1127,6 +1127,7 @@ func TestParse(t *testing.T) { SidecarService: &api.ConsulSidecarService{ Proxy: &api.ConsulProxy{ ExposeConfig: &api.ConsulExposeConfig{ + Checks: true, Path: []*api.ConsulExposePath{{ Path: "/health", Protocol: "http", diff --git a/jobspec/test-fixtures/tg-service-proxy-expose.hcl b/jobspec/test-fixtures/tg-service-proxy-expose.hcl index b5032f9d46a..970aeb62f25 100644 --- a/jobspec/test-fixtures/tg-service-proxy-expose.hcl +++ b/jobspec/test-fixtures/tg-service-proxy-expose.hcl @@ -1,11 +1,12 @@ job "group_service_proxy_expose" { group "group" { service { - name = "example" connect { sidecar_service { proxy { expose { + checks = true + path = { path = "/health" protocol = "http" diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index a0b17323add..b46abe748ea 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -60,10 +60,12 @@ func NewJobEndpoints(s *Server) *Job { mutators: []jobMutator{ jobCanonicalizer{}, jobConnectHook{}, + jobExposeHook{}, jobImpliedConstraints{}, }, validators: []jobValidator{ jobConnectHook{}, + jobExposeHook{}, jobValidate{}, }, } diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go index fca81ef1d5e..019417529b6 100644 --- a/nomad/job_endpoint_hook_connect.go +++ b/nomad/job_endpoint_hook_connect.go @@ -128,7 +128,6 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error { task.Canonicalize(job, g) makePort := func(label string) { - // check that port hasn't already been defined before adding it to tg for _, p := range g.Networks[0].DynamicPorts { if p.Label == label { return @@ -145,7 +144,6 @@ 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)) - // todo(shoenig) magic port for 'expose.checks' } } diff --git a/nomad/job_endpoint_hook_expose.go b/nomad/job_endpoint_hook_expose.go new file mode 100644 index 00000000000..551d125f0e0 --- /dev/null +++ b/nomad/job_endpoint_hook_expose.go @@ -0,0 +1,185 @@ +package nomad + +import ( + "strconv" + + "github.com/pkg/errors" + + "github.com/hashicorp/nomad/nomad/structs" +) + +// jobExposeHook implements a job Mutating and Validating admission controller. +type jobExposeHook struct{} + +func (jobExposeHook) Name() string { + return "expose" +} + +// Mutate will extrapolate and append each of the the necessary expose.path +// configurations for each compatible service check in each service in each +// task group. +func (jobExposeHook) Mutate(job *structs.Job) (*structs.Job, []error, error) { + for _, tg := range job.TaskGroups { + for _, s := range tg.Services { + if serviceEnablesExposeChecks(s) { + for _, check := range s.Checks { + // create an expose path for each check that is compatible + if ePath, err := exposePathForCheck(tg, s, check); err != nil { + return nil, nil, err + } else if ePath != nil { + // insert only if compatible & not already present + if !containsExposePath(s.Connect.SidecarService.Proxy.Expose.Paths, *ePath) { + s.Connect.SidecarService.Proxy.Expose.Paths = append( + s.Connect.SidecarService.Proxy.Expose.Paths, + *ePath, + ) + } + } + } + } + } + } + return job, nil, nil +} + +// Validate will ensure the job contains valid network configuration for each +// task group in which an expose path is configured. The network must be of type +// bridge mode. +func (jobExposeHook) Validate(job *structs.Job) ([]error, error) { + // Make sure expose config exists only along with a bridge network. We could + // also validate existence of named port mappings, but Mutate will also + // necessarily make that check when it looks up the service port. + for _, tg := range job.TaskGroups { + if tgEnablesExpose(tg) { + if mode, group, ok := tgUsesBridgeNetwork(tg); !ok { + return nil, errors.Errorf( + "expose configuration requires bridge network, found %q network in task group %q", + mode, group, + ) + } + } + } + return nil, nil +} + +func containsExposePath(paths []structs.ConsulExposePath, path structs.ConsulExposePath) bool { + for _, p := range paths { + if p == path { + return true + } + } + return false +} + +// exposePathForCheck extrapolates the necessary expose path configuration for +// the given consul service check. If the check is not compatible, nil is +// returned. +func exposePathForCheck(tg *structs.TaskGroup, s *structs.Service, check *structs.ServiceCheck) (*structs.ConsulExposePath, error) { + if !checkIsExposable(check) { + return nil, nil + } + + // Determine the local service port (i.e. what port the service is actually + // listening to inside the network namespace). + // + // Similar logic exists in getAddress of client.go which is used for + // creating check & service registration objects. + // + // 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 port, _ = strconv.Atoi(s.PortLabel); port <= 0 { // then try direct port value + return nil, errors.Errorf( + "unable to determine local service port for check %q of service %q in group %q", + check.Name, s.Name, tg.Name, + ) + } + } + + // The Path, Protocol, and PortLabel are just copied over from the service + // check definition. It is required that the user configure their own port + // mapping for each check, including setting the 'to = -1' sentinel value + // enabling the network namespace pass-through. + return &structs.ConsulExposePath{ + Path: check.Path, + Protocol: check.Protocol, + LocalPathPort: port, + ListenerPort: check.PortLabel, + }, nil +} + +// checkIsExposable returns true if check is qualified for automatic generation +// of connect proxy expose path configuration based on configured consul checks. +// To qualify, the check must be of type "http" or "grpc", and must have a Path +// configured. +func checkIsExposable(check *structs.ServiceCheck) bool { + switch check.Type { + case "grpc", "http": + return check.Path != "" + default: + return false + } +} + +// tgEnablesExpose returns true if any group-level service in tg makes use of +// the connect proxy expose configuration. +func tgEnablesExpose(tg *structs.TaskGroup) bool { + for _, s := range tg.Services { + if serviceEnablesExpose(s) { + return true + } + } + return false +} + +// serviceEnablesExpose returns true if s is configured to expose endpoints +// through the connect proxy, whether with explicit path configurations or using +// automatic configuration based on consul checks. +func serviceEnablesExpose(s *structs.Service) bool { + exposeConfig := serviceExposeConfig(s) + if exposeConfig == nil { + return false + } + return exposeConfig.Checks || len(exposeConfig.Paths) > 0 +} + +// serviceEnablesExposeChecks returns true if s is configured to automatically +// expose consul service check endpoints through the connect proxy. +func serviceEnablesExposeChecks(s *structs.Service) bool { + exposeConfig := serviceExposeConfig(s) + if exposeConfig == nil { + return false + } + return exposeConfig.Checks +} + +// serviceExposeConfig digs through s to extract the connect sidecar service +// proxy expose configuration. If any layer is nil, nil is returned. +func serviceExposeConfig(s *structs.Service) *structs.ConsulExposeConfig { + if s == nil { + return nil + } + + if s.Connect == nil { + return nil + } + + if s.Connect.SidecarService == nil { + return nil + } + + if s.Connect.SidecarService.Proxy == nil { + return nil + } + + return s.Connect.SidecarService.Proxy.Expose +} + +// tgUsesBridgeNetwork detects whether the 0th network in tg uses the bridge +// networking mode, and returns the mode and name as well to the caller for +// convenient error reporting. +func tgUsesBridgeNetwork(tg *structs.TaskGroup) (string, string, bool) { + mode := tg.Networks[0].Mode + return mode, tg.Name, mode == "bridge" +} diff --git a/nomad/job_endpoint_hook_expose_test.go b/nomad/job_endpoint_hook_expose_test.go new file mode 100644 index 00000000000..66e641c930d --- /dev/null +++ b/nomad/job_endpoint_hook_expose_test.go @@ -0,0 +1,439 @@ +package nomad + +import ( + "testing" + + "github.com/hashicorp/nomad/nomad/structs" + "github.com/stretchr/testify/require" +) + +func TestJobExposeHook_Name(t *testing.T) { + t.Parallel() + + require.Equal(t, "expose", new(jobExposeHook).Name()) +} + +func TestJobExposeHook_tgUsesBridgeNetwork(t *testing.T) { + t.Parallel() + + t.Run("uses bridge", func(t *testing.T) { + mode, name, result := tgUsesBridgeNetwork(&structs.TaskGroup{ + Name: "group", + Networks: structs.Networks{{ + Mode: "bridge", + }}, + }) + require.True(t, result) + require.Equal(t, "bridge", mode) + require.Equal(t, "group", name) + }) + t.Run("uses host", func(t *testing.T) { + mode, name, result := tgUsesBridgeNetwork(&structs.TaskGroup{ + Name: "group", + Networks: structs.Networks{{ + Mode: "host", + }}, + }) + require.False(t, result) + require.Equal(t, "host", mode) + require.Equal(t, "group", name) + }) +} + +func TestJobExposeHook_serviceExposeConfig(t *testing.T) { + t.Parallel() + + t.Run("nil service", func(t *testing.T) { + require.Nil(t, serviceExposeConfig(nil)) + }) + t.Run("nil connect", func(t *testing.T) { + require.Nil(t, serviceExposeConfig(&structs.Service{ + Connect: nil, + })) + }) + t.Run("nil sidecar", func(t *testing.T) { + require.Nil(t, serviceExposeConfig(&structs.Service{ + Connect: &structs.ConsulConnect{ + SidecarService: nil, + }})) + }) + t.Run("nil proxy", func(t *testing.T) { + require.Nil(t, serviceExposeConfig(&structs.Service{ + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: nil, + }}})) + }) + t.Run("expose", func(t *testing.T) { + require.True(t, serviceExposeConfig(&structs.Service{ + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Checks: true, + }}}}}).Checks) + }) +} + +func TestJobExposeHook_serviceEnablesExposeChecks(t *testing.T) { + t.Parallel() + + t.Run("expose checks true", func(t *testing.T) { + require.True(t, serviceEnablesExposeChecks(&structs.Service{ + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Checks: true, + }}}}})) + }) + t.Run("expose checks false", func(t *testing.T) { + require.False(t, serviceEnablesExposeChecks(&structs.Service{ + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Checks: false, + }}}}})) + }) +} + +func TestJobExposeHook_serviceEnablesExpose(t *testing.T) { + t.Parallel() + + t.Run("no expose", func(t *testing.T) { + require.False(t, serviceEnablesExpose(&structs.Service{ + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Checks: false, + Paths: nil, + }}}}})) + }) + + t.Run("expose checks", func(t *testing.T) { + require.True(t, serviceEnablesExpose(&structs.Service{ + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Checks: true, + Paths: nil, + }}}}})) + }) + + t.Run("expose paths", func(t *testing.T) { + require.True(t, serviceEnablesExpose(&structs.Service{ + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Checks: false, + Paths: []structs.ConsulExposePath{{ + Path: "/example", + }}, + }}}}})) + }) +} + +func TestJobExposeHook_checkIsExposable(t *testing.T) { + t.Parallel() + + t.Run("type http", func(t *testing.T) { + require.True(t, checkIsExposable(&structs.ServiceCheck{ + Type: "http", + Path: "/path", + })) + }) + + t.Run("type grpc", func(t *testing.T) { + require.True(t, checkIsExposable(&structs.ServiceCheck{ + Type: "grpc", + Path: "/path", + })) + }) + + t.Run("type http no path", func(t *testing.T) { + require.False(t, checkIsExposable(&structs.ServiceCheck{ + Type: "http", + })) + }) + + t.Run("type tcp", func(t *testing.T) { + require.False(t, checkIsExposable(&structs.ServiceCheck{ + Type: "tcp", + })) + }) +} + +func TestJobExposeHook_tgEnablesExpose(t *testing.T) { + t.Parallel() + + tg := &structs.TaskGroup{ + Services: []*structs.Service{ + {Name: "no_expose"}, + { + Name: "with_expose", Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Checks: true, + }}}}, + }, + }, + } + + t.Run("tg enables expose", func(t *testing.T) { + require.True(t, tgEnablesExpose(tg)) + }) + + t.Run("tg does not enable expose", func(t *testing.T) { + tg.Services[1].Connect.SidecarService.Proxy.Expose.Checks = false + require.False(t, tgEnablesExpose(tg)) + }) +} + +func TestJobExposeHook_containsExposePath(t *testing.T) { + t.Parallel() + + t.Run("contains path", func(t *testing.T) { + require.True(t, containsExposePath([]structs.ConsulExposePath{{ + Path: "/v2/health", + Protocol: "grpc", + LocalPathPort: 8080, + ListenerPort: "v2Port", + }, { + Path: "/health", + Protocol: "http", + LocalPathPort: 8080, + ListenerPort: "hcPort", + }}, structs.ConsulExposePath{ + Path: "/health", + Protocol: "http", + LocalPathPort: 8080, + ListenerPort: "hcPort", + })) + }) + + t.Run("no such path", func(t *testing.T) { + require.False(t, containsExposePath([]structs.ConsulExposePath{{ + Path: "/v2/health", + Protocol: "grpc", + LocalPathPort: 8080, + ListenerPort: "v2Port", + }, { + Path: "/health", + Protocol: "http", + LocalPathPort: 8080, + ListenerPort: "hcPort", + }}, structs.ConsulExposePath{ + Path: "/v3/health", + Protocol: "http", + LocalPathPort: 8080, + ListenerPort: "hcPort", + })) + }) +} + +func TestJobExposeHook_exposePathForCheck(t *testing.T) { + t.Parallel() + + t.Run("not expose compatible", func(t *testing.T) { + c := &structs.ServiceCheck{ + Type: "tcp", // not expose compatible + } + s := &structs.Service{ + Checks: []*structs.ServiceCheck{c}, + } + ePath, err := exposePathForCheck(&structs.TaskGroup{ + Services: []*structs.Service{s}, + }, s, c) + require.NoError(t, err) + require.Nil(t, ePath) + }) + + t.Run("direct port", func(t *testing.T) { + c := &structs.ServiceCheck{ + Name: "check1", + Type: "http", + Path: "/health", + PortLabel: "hcPort", + } + s := &structs.Service{ + Name: "service1", + PortLabel: "4000", + Checks: []*structs.ServiceCheck{c}, + } + ePath, err := exposePathForCheck(&structs.TaskGroup{ + Name: "group1", + Services: []*structs.Service{s}, + }, s, c) + require.NoError(t, err) + require.Equal(t, &structs.ConsulExposePath{ + Path: "/health", + Protocol: "", // often blank, consul does the Right Thing + LocalPathPort: 4000, + ListenerPort: "hcPort", + }, ePath) + }) + + t.Run("labeled port", func(t *testing.T) { + c := &structs.ServiceCheck{ + Name: "check1", + Type: "http", + Path: "/health", + PortLabel: "hcPort", + } + s := &structs.Service{ + Name: "service1", + PortLabel: "sPort", // port label indirection + Checks: []*structs.ServiceCheck{c}, + } + ePath, err := exposePathForCheck(&structs.TaskGroup{ + Name: "group1", + Services: []*structs.Service{s}, + Networks: structs.Networks{{ + Mode: "bridge", + DynamicPorts: []structs.Port{ + {Label: "sPort", Value: 4000}, + }, + }}, + }, s, c) + require.NoError(t, err) + require.Equal(t, &structs.ConsulExposePath{ + Path: "/health", + Protocol: "", + LocalPathPort: 4000, + ListenerPort: "hcPort", + }, ePath) + }) + + t.Run("missing port", func(t *testing.T) { + c := &structs.ServiceCheck{ + Name: "check1", + Type: "http", + Path: "/health", + PortLabel: "hcPort", + } + s := &structs.Service{ + Name: "service1", + PortLabel: "sPort", // port label indirection + Checks: []*structs.ServiceCheck{c}, + } + _, err := exposePathForCheck(&structs.TaskGroup{ + Name: "group1", + Services: []*structs.Service{s}, + Networks: structs.Networks{{ + Mode: "bridge", + DynamicPorts: []structs.Port{ + // service declares "sPort", but does not exist + }, + }}, + }, s, c) + require.EqualError(t, err, `unable to determine local service port for check "check1" of service "service1" in group "group1"`) + }) +} + +func TestJobExposeHook_Validate(t *testing.T) { + t.Parallel() + + t.Run("expose without bridge", func(t *testing.T) { + warnings, err := new(jobExposeHook).Validate(&structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Name: "group", + Networks: structs.Networks{{ + Mode: "host", + }}, + Services: []*structs.Service{{ + Name: "service", + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Checks: true, + }}}}}}}}, + }) + require.Empty(t, warnings) + require.EqualError(t, err, `expose configuration requires bridge network, found "host" network in task group "group"`) + }) + + t.Run("valid", func(t *testing.T) { + warnings, err := new(jobExposeHook).Validate(&structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Name: "group", + Networks: structs.Networks{{ + Mode: "bridge", + }}, + Services: []*structs.Service{{ + Name: "service", + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Checks: true, + }}}}}}}}, + }) + require.Empty(t, warnings) + require.NoError(t, err) + }) +} + +func TestJobExposeHook_Mutate(t *testing.T) { + t.Parallel() + + result, warnings, err := new(jobExposeHook).Mutate(&structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Name: "group1", + Services: []*structs.Service{{ + Name: "service1", + PortLabel: "8080", + Checks: []*structs.ServiceCheck{{ + Name: "check1", + Type: "tcp", + PortLabel: "5000", + }, { + Name: "check2", + Type: "http", + Protocol: "http", + PortLabel: "forChecks", + Path: "/health", + }, { + Name: "check3", + Type: "grpc", + Protocol: "grpc", + PortLabel: "forChecks", + Path: "/v2/health", + }}, + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Checks: true, + Paths: []structs.ConsulExposePath{{ + Path: "/pre-existing", + Protocol: "http", + LocalPathPort: 9000, + ListenerPort: "otherPort", + }}}}}}}}}}, + }) + + require.NoError(t, err) + require.Empty(t, warnings) + require.Equal(t, []structs.ConsulExposePath{{ + Path: "/pre-existing", + Protocol: "http", + LocalPathPort: 9000, + ListenerPort: "otherPort", + }, { + Path: "/health", + Protocol: "http", + LocalPathPort: 8080, + ListenerPort: "forChecks", + }, { + Path: "/v2/health", + Protocol: "grpc", + LocalPathPort: 8080, + ListenerPort: "forChecks", + }}, result.TaskGroups[0].Services[0].Connect.SidecarService.Proxy.Expose.Paths) +} diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 5954a2bc414..eb7fc355b05 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -120,19 +120,15 @@ func TestJobEndpoint_Register_Connect(t *testing.T) { // Create the register request job := mock.Job() - job.TaskGroups[0].Networks = structs.Networks{ - { - Mode: "bridge", - }, - } - job.TaskGroups[0].Services = []*structs.Service{ - { - Name: "backend", - PortLabel: "8080", - Connect: &structs.ConsulConnect{ - SidecarService: &structs.ConsulSidecarService{}, - }, - }, + job.TaskGroups[0].Networks = structs.Networks{{ + Mode: "bridge", + }} + job.TaskGroups[0].Services = []*structs.Service{{ + Name: "backend", + PortLabel: "8080", + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{}, + }}, } req := &structs.JobRegisterRequest{ Job: job, @@ -177,7 +173,112 @@ func TestJobEndpoint_Register_Connect(t *testing.T) { require.Len(out.TaskGroups[0].Tasks, 2) require.Exactly(sidecarTask, out.TaskGroups[0].Tasks[1]) +} + +func TestJobEndpoint_Register_ConnectProxyExposeChecks(t *testing.T) { + t.Parallel() + r := require.New(t) + + s1, cleanupS1 := TestServer(t, func(c *Config) { + c.NumSchedulers = 0 // Prevent automatic dequeue + }) + defer cleanupS1() + codec := rpcClient(t, s1) + testutil.WaitForLeader(t, s1.RPC) + + // Create the register request + job := mock.Job() + job.TaskGroups[0].Networks = structs.Networks{{ + Mode: "bridge", + DynamicPorts: []structs.Port{{ + Label: "hcPort", + To: -1, + }, { + Label: "v2Port", + To: -1, + }}, + }} + job.TaskGroups[0].Services = []*structs.Service{{ + Name: "backend", + PortLabel: "8080", + Checks: []*structs.ServiceCheck{{ + Name: "check1", + Type: "http", + Protocol: "http", + Path: "/health", + PortLabel: "hcPort", + Interval: 1 * time.Second, + Timeout: 1 * time.Second, + }, { + Name: "check2", + Type: "grpc", + Protocol: "grpc", + Path: "/v2/health", + PortLabel: "v2Port", + Interval: 1 * time.Second, + Timeout: 1 * time.Second, + }}, + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + Expose: &structs.ConsulExposeConfig{ + Checks: true, + }}}}, + }} + req := &structs.JobRegisterRequest{ + Job: job, + WriteRequest: structs.WriteRequest{ + Region: "global", + Namespace: job.Namespace, + }, + } + + // Fetch the response + var resp structs.JobRegisterResponse + r.NoError(msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp)) + r.NotZero(resp.Index) + + // Check for the node in the FSM + state := s1.fsm.State() + ws := memdb.NewWatchSet() + out, err := state.JobByID(ws, job.Namespace, job.ID) + r.NoError(err) + r.NotNil(out) + r.Equal(resp.JobModifyIndex, out.CreateIndex) + + // Check that the new expose paths got created + r.Len(out.TaskGroups[0].Services[0].Connect.SidecarService.Proxy.Expose.Paths, 2) + httpPath := out.TaskGroups[0].Services[0].Connect.SidecarService.Proxy.Expose.Paths[0] + r.Equal(structs.ConsulExposePath{ + Path: "/health", + Protocol: "http", + LocalPathPort: 8080, + ListenerPort: "hcPort", + }, httpPath) + grpcPath := out.TaskGroups[0].Services[0].Connect.SidecarService.Proxy.Expose.Paths[1] + r.Equal(structs.ConsulExposePath{ + Path: "/v2/health", + Protocol: "grpc", + LocalPathPort: 8080, + ListenerPort: "v2Port", + }, grpcPath) + + // todo: round tripping is probably gonna duplicate since we only append + out.Meta["test"] = "abc" + req.Job = out + r.NoError(msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp)) + r.NotZero(resp.Index) + + // Check for the new node in the FSM + state = s1.fsm.State() + ws = memdb.NewWatchSet() + out, err = state.JobByID(ws, job.Namespace, job.ID) + r.NoError(err) + r.NotNil(out) + r.Equal(resp.JobModifyIndex, out.CreateIndex) // todo uhh + // make sure we are not re-adding what has already been added + r.Len(out.TaskGroups[0].Services[0].Connect.SidecarService.Proxy.Expose.Paths, 2) } func TestJobEndpoint_Register_ConnectWithSidecarTask(t *testing.T) { diff --git a/nomad/structs/services.go b/nomad/structs/services.go index 5b9f38f2fa6..8a1b21a8ad0 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -47,7 +47,7 @@ type ServiceCheck struct { Path string // path of the health check url for http type check Protocol string // Protocol to use if check is http, defaults to http PortLabel string // The port to use for tcp/http checks - AddressMode string // 'host' to use host ip:port or 'driver' to use driver's + AddressMode string // 'host' to use host ip:port or 'driver' to use driver' Interval time.Duration // Interval of the check Timeout time.Duration // Timeout of the response from the check before consul fails the check InitialStatus string // Initial status of the check @@ -974,7 +974,8 @@ func (u *ConsulUpstream) Equals(o *ConsulUpstream) bool { // ExposeConfig represents a Consul Connect expose jobspec stanza. type ConsulExposeConfig struct { - Paths []ConsulExposePath + Checks bool + Paths []ConsulExposePath } type ConsulExposePath struct { @@ -984,6 +985,10 @@ type ConsulExposePath struct { ListenerPort string } +func (p *ConsulExposePath) String() string { + return fmt.Sprintf("", p.Protocol, p.Path, p.LocalPathPort, p.ListenerPort) +} + func exposePathsEqual(pathsA, pathsB []ConsulExposePath) bool { if len(pathsA) != len(pathsB) { return false @@ -1011,7 +1016,8 @@ func (e *ConsulExposeConfig) Copy() *ConsulExposeConfig { paths[i] = e.Paths[i] } return &ConsulExposeConfig{ - Paths: paths, + Checks: e.Checks, + Paths: paths, } } @@ -1020,5 +1026,8 @@ func (e *ConsulExposeConfig) Equals(o *ConsulExposeConfig) bool { if e == nil || o == nil { return e == o } + if e.Checks != o.Checks { + return false + } return exposePathsEqual(e.Paths, o.Paths) } From 9da0de2d7f67b53e73134de5ecd10c4e7799fc69 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Fri, 20 Mar 2020 09:24:25 -0600 Subject: [PATCH 5/5] connect: fixup some leftover debugging comments --- nomad/job_endpoint_hook_connect.go | 1 + nomad/job_endpoint_test.go | 4 ++-- nomad/structs/services.go | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go index 019417529b6..4270796e524 100644 --- a/nomad/job_endpoint_hook_connect.go +++ b/nomad/job_endpoint_hook_connect.go @@ -128,6 +128,7 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error { task.Canonicalize(job, g) makePort := func(label string) { + // check that port hasn't already been defined before adding it to tg for _, p := range g.Networks[0].DynamicPorts { if p.Label == label { return diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index eb7fc355b05..2c53ff14247 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -263,7 +263,7 @@ func TestJobEndpoint_Register_ConnectProxyExposeChecks(t *testing.T) { ListenerPort: "v2Port", }, grpcPath) - // todo: round tripping is probably gonna duplicate since we only append + // make sure round tripping does not create duplicate expose paths out.Meta["test"] = "abc" req.Job = out r.NoError(msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp)) @@ -275,7 +275,7 @@ func TestJobEndpoint_Register_ConnectProxyExposeChecks(t *testing.T) { out, err = state.JobByID(ws, job.Namespace, job.ID) r.NoError(err) r.NotNil(out) - r.Equal(resp.JobModifyIndex, out.CreateIndex) // todo uhh + r.Equal(resp.JobModifyIndex, out.CreateIndex) // make sure we are not re-adding what has already been added r.Len(out.TaskGroups[0].Services[0].Connect.SidecarService.Proxy.Expose.Paths, 2) diff --git a/nomad/structs/services.go b/nomad/structs/services.go index 8a1b21a8ad0..603344ff1a5 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -47,7 +47,7 @@ type ServiceCheck struct { Path string // path of the health check url for http type check Protocol string // Protocol to use if check is http, defaults to http PortLabel string // The port to use for tcp/http checks - AddressMode string // 'host' to use host ip:port or 'driver' to use driver' + AddressMode string // 'host' to use host ip:port or 'driver' to use driver's Interval time.Duration // Interval of the check Timeout time.Duration // Timeout of the response from the check before consul fails the check InitialStatus string // Initial status of the check