From 3fcac242c6a3a073d5c05e17a86d7c9106ba9cf8 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Wed, 20 Apr 2022 13:03:19 -0500 Subject: [PATCH] services: enable setting arbitrary address value in service registrations This PR introduces the `address` field in the `service` block so that Nomad or Consul services can be registered with a custom `.Address.` to advertise. The address can be an IP address or domain name. If the `address` field is set, the `service.address_mode` must be set in `auto` mode. --- .changelog/12720.txt | 3 + api/services.go | 16 +- client/serviceregistration/address.go | 65 ++++-- client/serviceregistration/address_test.go | 186 ++++++++++++------ client/serviceregistration/nsd/nsd.go | 2 +- client/taskenv/services.go | 1 + command/agent/consul/service_client.go | 14 +- command/agent/job_endpoint.go | 1 + command/agent/job_endpoint_test.go | 8 +- command/service_info.go | 9 +- command/service_info_test.go | 8 +- nomad/structs/diff_test.go | 51 +++++ nomad/structs/services.go | 24 ++- nomad/structs/services_test.go | 186 ++++++++++++++++++ nomad/structs/structs_test.go | 151 -------------- .../docs/job-specification/service.mdx | 30 +-- 16 files changed, 486 insertions(+), 269 deletions(-) create mode 100644 .changelog/12720.txt diff --git a/.changelog/12720.txt b/.changelog/12720.txt new file mode 100644 index 000000000..14d3edaab --- /dev/null +++ b/.changelog/12720.txt @@ -0,0 +1,3 @@ +```release-note:improvement +services: Enable setting arbitrary address on Nomad or Consul service registration +``` diff --git a/api/services.go b/api/services.go index f531c2964..0f4ea3d2e 100644 --- a/api/services.go +++ b/api/services.go @@ -86,7 +86,6 @@ type ServiceRegistrationStub struct { Tags []string } - // Services is used to query the service endpoints. type Services struct { client *Client @@ -195,10 +194,8 @@ func (c *CheckRestart) Merge(o *CheckRestart) *CheckRestart { return nc } -// ServiceCheck represents the consul health check that Nomad registers. +// ServiceCheck represents a Nomad job-submitters view of a Consul service health check. type ServiceCheck struct { - //FIXME Id is unused. Remove? - Id string `hcl:"id,optional"` Name string `hcl:"name,optional"` Type string `hcl:"type,optional"` Command string `hcl:"command,optional"` @@ -208,6 +205,7 @@ type ServiceCheck struct { PortLabel string `mapstructure:"port" hcl:"port,optional"` Expose bool `hcl:"expose,optional"` AddressMode string `mapstructure:"address_mode" hcl:"address_mode,optional"` + Advertise string `hcl:"advertise,optional"` Interval time.Duration `hcl:"interval,optional"` Timeout time.Duration `hcl:"timeout,optional"` InitialStatus string `mapstructure:"initial_status" hcl:"initial_status,optional"` @@ -224,16 +222,15 @@ type ServiceCheck struct { OnUpdate string `mapstructure:"on_update" hcl:"on_update,optional"` } -// Service represents a Consul service definition. +// Service represents a Nomad job-submitters view of a Consul or Nomad service. type Service struct { - //FIXME Id is unused. Remove? - Id string `hcl:"id,optional"` Name string `hcl:"name,optional"` Tags []string `hcl:"tags,optional"` CanaryTags []string `mapstructure:"canary_tags" hcl:"canary_tags,optional"` EnableTagOverride bool `mapstructure:"enable_tag_override" hcl:"enable_tag_override,optional"` PortLabel string `mapstructure:"port" hcl:"port,optional"` AddressMode string `mapstructure:"address_mode" hcl:"address_mode,optional"` + Address string `hcl:"address,optional"` Checks []ServiceCheck `hcl:"check,block"` CheckRestart *CheckRestart `mapstructure:"check_restart" hcl:"check_restart,block"` Connect *ConsulConnect `hcl:"connect,block"` @@ -242,9 +239,8 @@ type Service struct { TaskName string `mapstructure:"task" hcl:"task,optional"` OnUpdate string `mapstructure:"on_update" hcl:"on_update,optional"` - // Provider defines which backend system provides the service registration - // mechanism for this service. This supports either structs.ProviderConsul - // or structs.ProviderNomad and defaults for the former. + // Provider defines which backend system provides the service registration, + // either "consul" (default) or "nomad". Provider string `hcl:"provider,optional"` } diff --git a/client/serviceregistration/address.go b/client/serviceregistration/address.go index 4a67c94ab..af4974d0b 100644 --- a/client/serviceregistration/address.go +++ b/client/serviceregistration/address.go @@ -8,22 +8,49 @@ import ( "github.com/hashicorp/nomad/plugins/drivers" ) -// GetAddress returns the IP and port to use for a service or check. If no port -// label is specified (an empty value), zero values are returned because no -// address could be resolved. +// GetAddress returns the IP (or custom advertise address) and port to use for a +// service or check registration. If no port label is specified (an empty value), +// zero values are returned because no address could be resolved. func GetAddress( - addrMode, portLabel string, networks structs.Networks, driverNet *drivers.DriverNetwork, - ports structs.AllocatedPorts, netStatus *structs.AllocNetworkStatus) (string, int, error) { - - switch addrMode { + address, // custom address, if set + addressMode, + portLabel string, + networks structs.Networks, + driverNet *drivers.DriverNetwork, + ports structs.AllocatedPorts, + netStatus *structs.AllocNetworkStatus, +) (string, int, error) { + switch addressMode { case structs.AddressModeAuto: - if driverNet.Advertise() { - addrMode = structs.AddressModeDriver - } else { - addrMode = structs.AddressModeHost + switch { + case address != "": + // No port no problem, just return the advertise address. + if portLabel == "" { + return address, 0, nil + } + // A custom advertise address can be used with a port map; using the + // Value and ignoring the IP. The routing from your custom address to + // the group network address is DIY. + if mapping, exists := ports.Get(portLabel); exists { + return address, mapping.Value, nil + } + // If not a port map we can interpolate a numeric port for you. + port, err := strconv.Atoi(portLabel) + if err != nil { + return "", 0, fmt.Errorf("invalid port: %q: not a valid port mapping or numeric port", portLabel) + } + return address, port, nil + case driverNet.Advertise(): + return GetAddress("", structs.AddressModeDriver, portLabel, networks, driverNet, ports, netStatus) + default: + return GetAddress("", structs.AddressModeHost, portLabel, networks, driverNet, ports, netStatus) } - return GetAddress(addrMode, portLabel, networks, driverNet, ports, netStatus) case structs.AddressModeHost: + // Cannot use address mode host with custom advertise address. + if address != "" { + return "", 0, fmt.Errorf("cannot use custom advertise address with %q address mode", structs.AddressModeHost) + } + if portLabel == "" { if len(networks) != 1 { // If no networks are specified return zero @@ -32,7 +59,6 @@ func GetAddress( // some people rely on. return "", 0, nil } - return networks[0].IP, 0, nil } @@ -65,6 +91,11 @@ func GetAddress( return mapping.HostIP, mapping.Value, nil case structs.AddressModeDriver: + // Cannot use address mode driver with custom advertise address. + if address != "" { + return "", 0, fmt.Errorf("cannot use custom advertise address with %q address mode", structs.AddressModeDriver) + } + // Require a driver network if driver address mode is used if driverNet == nil { return "", 0, fmt.Errorf(`cannot use address_mode="driver": no driver network exists`) @@ -100,6 +131,12 @@ func GetAddress( return driverNet.IP, port, nil case structs.AddressModeAlloc: + // Cannot use address mode alloc with custom advertise address. + if address != "" { + return "", 0, fmt.Errorf("cannot use custom advertise address with %q address mode", structs.AddressModeAlloc) + } + + // Going to need a network for this. if netStatus == nil { return "", 0, fmt.Errorf(`cannot use address_mode="alloc": no allocation network status reported`) } @@ -131,6 +168,6 @@ func GetAddress( default: // Shouldn't happen due to validation, but enforce invariants - return "", 0, fmt.Errorf("invalid address mode %q", addrMode) + return "", 0, fmt.Errorf("invalid address mode %q", addressMode) } } diff --git a/client/serviceregistration/address_test.go b/client/serviceregistration/address_test.go index fbad6acd1..6e31c9176 100644 --- a/client/serviceregistration/address_test.go +++ b/client/serviceregistration/address_test.go @@ -14,7 +14,8 @@ func Test_GetAddress(t *testing.T) { testCases := []struct { name string - // Parameters + // Inputs + advertise string mode string portLabel string host map[string]int // will be converted to structs.Networks @@ -22,10 +23,10 @@ func Test_GetAddress(t *testing.T) { ports structs.AllocatedPorts status *structs.AllocNetworkStatus - // Results - expectedIP string - expectedPort int - expectedErr string + // Expectations + expIP string + expPort int + expErr string }{ // Valid Configurations { @@ -37,8 +38,8 @@ func Test_GetAddress(t *testing.T) { PortMap: map[string]int{"db": 6379}, IP: "10.1.2.3", }, - expectedIP: HostIP, - expectedPort: 12435, + expIP: HostIP, + expPort: 12435, }, { name: "host", @@ -49,8 +50,8 @@ func Test_GetAddress(t *testing.T) { PortMap: map[string]int{"db": 6379}, IP: "10.1.2.3", }, - expectedIP: HostIP, - expectedPort: 12345, + expIP: HostIP, + expPort: 12345, }, { name: "driver", @@ -61,8 +62,8 @@ func Test_GetAddress(t *testing.T) { PortMap: map[string]int{"db": 6379}, IP: "10.1.2.3", }, - expectedIP: "10.1.2.3", - expectedPort: 6379, + expIP: "10.1.2.3", + expPort: 6379, }, { name: "AutoDriver", @@ -74,8 +75,8 @@ func Test_GetAddress(t *testing.T) { IP: "10.1.2.3", AutoAdvertise: true, }, - expectedIP: "10.1.2.3", - expectedPort: 6379, + expIP: "10.1.2.3", + expPort: 6379, }, { name: "DriverCustomPort", @@ -86,18 +87,18 @@ func Test_GetAddress(t *testing.T) { PortMap: map[string]int{"db": 6379}, IP: "10.1.2.3", }, - expectedIP: "10.1.2.3", - expectedPort: 7890, + expIP: "10.1.2.3", + expPort: 7890, }, // Invalid Configurations { - name: "DriverWithoutNetwork", - mode: structs.AddressModeDriver, - portLabel: "db", - host: map[string]int{"db": 12345}, - driver: nil, - expectedErr: "no driver network exists", + name: "DriverWithoutNetwork", + mode: structs.AddressModeDriver, + portLabel: "db", + host: map[string]int{"db": 12345}, + driver: nil, + expErr: "no driver network exists", }, { name: "DriverBadPort", @@ -108,7 +109,7 @@ func Test_GetAddress(t *testing.T) { PortMap: map[string]int{"db": 6379}, IP: "10.1.2.3", }, - expectedErr: "invalid port", + expErr: "invalid port", }, { name: "DriverZeroPort", @@ -117,29 +118,29 @@ func Test_GetAddress(t *testing.T) { driver: &drivers.DriverNetwork{ IP: "10.1.2.3", }, - expectedErr: "invalid port", + expErr: "invalid port", }, { - name: "HostBadPort", - mode: structs.AddressModeHost, - portLabel: "bad-port-label", - expectedErr: "invalid port", + name: "HostBadPort", + mode: structs.AddressModeHost, + portLabel: "bad-port-label", + expErr: "invalid port", }, { - name: "InvalidMode", - mode: "invalid-mode", - portLabel: "80", - expectedErr: "invalid address mode", + name: "InvalidMode", + mode: "invalid-mode", + portLabel: "80", + expErr: "invalid address mode", }, { - name: "NoPort_AutoMode", - mode: structs.AddressModeAuto, - expectedIP: HostIP, + name: "NoPort_AutoMode", + mode: structs.AddressModeAuto, + expIP: HostIP, }, { - name: "NoPort_HostMode", - mode: structs.AddressModeHost, - expectedIP: HostIP, + name: "NoPort_HostMode", + mode: structs.AddressModeHost, + expIP: HostIP, }, { name: "NoPort_DriverMode", @@ -147,7 +148,7 @@ func Test_GetAddress(t *testing.T) { driver: &drivers.DriverNetwork{ IP: "10.1.2.3", }, - expectedIP: "10.1.2.3", + expIP: "10.1.2.3", }, // Scenarios using port 0.12 networking fields (NetworkStatus, AllocatedPortMapping) @@ -167,8 +168,8 @@ func Test_GetAddress(t *testing.T) { InterfaceName: "eth0", Address: "172.26.0.1", }, - expectedIP: HostIP, - expectedPort: 12435, + expIP: HostIP, + expPort: 12435, }, { name: "Host_withAllocatedPorts", @@ -186,8 +187,8 @@ func Test_GetAddress(t *testing.T) { InterfaceName: "eth0", Address: "172.26.0.1", }, - expectedIP: HostIP, - expectedPort: 12345, + expIP: HostIP, + expPort: 12345, }, { name: "Driver_withAllocatedPorts", @@ -208,8 +209,8 @@ func Test_GetAddress(t *testing.T) { InterfaceName: "eth0", Address: "172.26.0.1", }, - expectedIP: "10.1.2.3", - expectedPort: 6379, + expIP: "10.1.2.3", + expPort: 6379, }, { name: "AutoDriver_withAllocatedPorts", @@ -231,8 +232,8 @@ func Test_GetAddress(t *testing.T) { InterfaceName: "eth0", Address: "172.26.0.1", }, - expectedIP: "10.1.2.3", - expectedPort: 6379, + expIP: "10.1.2.3", + expPort: 6379, }, { name: "DriverCustomPort_withAllocatedPorts", @@ -253,8 +254,8 @@ func Test_GetAddress(t *testing.T) { InterfaceName: "eth0", Address: "172.26.0.1", }, - expectedIP: "10.1.2.3", - expectedPort: 7890, + expIP: "10.1.2.3", + expPort: 7890, }, { name: "Host_MultiHostInterface", @@ -272,8 +273,8 @@ func Test_GetAddress(t *testing.T) { InterfaceName: "eth0", Address: "172.26.0.1", }, - expectedIP: "127.0.0.100", - expectedPort: 12345, + expIP: "127.0.0.100", + expPort: 12345, }, { name: "Alloc", @@ -291,8 +292,8 @@ func Test_GetAddress(t *testing.T) { InterfaceName: "eth0", Address: "172.26.0.1", }, - expectedIP: "172.26.0.1", - expectedPort: 6379, + expIP: "172.26.0.1", + expPort: 6379, }, { name: "Alloc no to value", @@ -309,8 +310,8 @@ func Test_GetAddress(t *testing.T) { InterfaceName: "eth0", Address: "172.26.0.1", }, - expectedIP: "172.26.0.1", - expectedPort: 12345, + expIP: "172.26.0.1", + expPort: 12345, }, { name: "AllocCustomPort", @@ -320,8 +321,64 @@ func Test_GetAddress(t *testing.T) { InterfaceName: "eth0", Address: "172.26.0.1", }, - expectedIP: "172.26.0.1", - expectedPort: 6379, + expIP: "172.26.0.1", + expPort: 6379, + }, + // Cases for setting the address field + { + name: "Address", + mode: structs.AddressModeAuto, + advertise: "example.com", + expIP: "example.com", + expPort: 0, + }, + { + name: "Address with numeric port", + mode: structs.AddressModeAuto, + advertise: "example.com", + portLabel: "8080", + expIP: "example.com", + expPort: 8080, + }, + { + name: "Address with mapped port", + mode: structs.AddressModeAuto, + portLabel: "web", + advertise: "example.com", + ports: []structs.AllocatedPortMapping{ + { + Label: "web", + Value: 12345, + HostIP: HostIP, + }, + }, + expIP: "example.com", + expPort: 12345, + }, + { + name: "Address with invalid mapped port", + mode: structs.AddressModeAuto, + advertise: "example.com", + portLabel: "foobar", + expErr: `invalid port: "foobar": not a valid port mapping or numeric port`, + }, + { + name: "Address with host mode", + mode: structs.AddressModeHost, + advertise: "example.com", + expErr: `cannot use custom advertise address with "host" address mode`, + }, + { + name: "Address with driver mode", + mode: structs.AddressModeDriver, + advertise: "example.com", + expErr: `cannot use custom advertise address with "driver" address mode`, + }, + { + name: "Address with alloc mode", + mode: structs.AddressModeAlloc, + advertise: "example.com", + expErr: `cannot use custom advertise address with "alloc" address mode`, }, } @@ -345,16 +402,23 @@ func Test_GetAddress(t *testing.T) { // Run the GetAddress function. actualIP, actualPort, actualErr := GetAddress( - tc.mode, tc.portLabel, networks, tc.driver, tc.ports, tc.status) + tc.advertise, + tc.mode, + tc.portLabel, + networks, + tc.driver, + tc.ports, + tc.status, + ) // Assert the results - require.Equal(t, tc.expectedIP, actualIP, "IP mismatch") - require.Equal(t, tc.expectedPort, actualPort, "Port mismatch") - if tc.expectedErr == "" { + require.Equal(t, tc.expIP, actualIP, "IP mismatch") + require.Equal(t, tc.expPort, actualPort, "Port mismatch") + if tc.expErr == "" { require.Nil(t, actualErr) } else { require.Error(t, actualErr) - require.Contains(t, actualErr.Error(), tc.expectedErr) + require.Contains(t, actualErr.Error(), tc.expErr) } }) } diff --git a/client/serviceregistration/nsd/nsd.go b/client/serviceregistration/nsd/nsd.go index 312e1e4e2..32312d626 100644 --- a/client/serviceregistration/nsd/nsd.go +++ b/client/serviceregistration/nsd/nsd.go @@ -271,7 +271,7 @@ func (s *ServiceRegistrationHandler) generateNomadServiceRegistration( // Determine the address to advertise based on the mode. ip, port, err := serviceregistration.GetAddress( - addrMode, serviceSpec.PortLabel, workload.Networks, + serviceSpec.Address, addrMode, serviceSpec.PortLabel, workload.Networks, workload.DriverNetwork, workload.Ports, workload.NetworkStatus) if err != nil { return nil, fmt.Errorf("unable to get address for service %q: %v", serviceSpec.Name, err) diff --git a/client/taskenv/services.go b/client/taskenv/services.go index f809a8775..e2a251af4 100644 --- a/client/taskenv/services.go +++ b/client/taskenv/services.go @@ -36,6 +36,7 @@ func InterpolateServices(taskEnv *TaskEnv, services []*structs.Service) []*struc service.Name = taskEnv.ReplaceEnv(service.Name) service.PortLabel = taskEnv.ReplaceEnv(service.PortLabel) + service.Address = taskEnv.ReplaceEnv(service.Address) service.Tags = taskEnv.ParseAndReplace(service.Tags) service.CanaryTags = taskEnv.ParseAndReplace(service.CanaryTags) service.Meta = interpolateMapStringString(taskEnv, service.Meta) diff --git a/command/agent/consul/service_client.go b/command/agent/consul/service_client.go index 9c7f7cd4f..15ecdc6c8 100644 --- a/command/agent/consul/service_client.go +++ b/command/agent/consul/service_client.go @@ -948,7 +948,7 @@ func (c *ServiceClient) serviceRegs( // Determine the address to advertise based on the mode ip, port, err := serviceregistration.GetAddress( - addrMode, service.PortLabel, workload.Networks, workload.DriverNetwork, workload.Ports, workload.NetworkStatus) + service.Address, addrMode, service.PortLabel, workload.Networks, workload.DriverNetwork, workload.Ports, workload.NetworkStatus) if err != nil { return nil, fmt.Errorf("unable to get address for service %q: %v", service.Name, err) } @@ -1073,13 +1073,19 @@ func (c *ServiceClient) checkRegs(serviceID string, service *structs.Service, addrMode := check.AddressMode if addrMode == "" { - // pre-#3380 compat - addrMode = structs.AddressModeHost + if service.Address != "" { + // if the service is using a custom address, enable the check + // to use that address + addrMode = structs.AddressModeAuto + } else { + // otherwise default to the host address + addrMode = structs.AddressModeHost + } } var err error ip, port, err = serviceregistration.GetAddress( - addrMode, portLabel, workload.Networks, workload.DriverNetwork, workload.Ports, workload.NetworkStatus) + service.Address, addrMode, portLabel, workload.Networks, workload.DriverNetwork, workload.Ports, workload.NetworkStatus) if err != nil { return nil, fmt.Errorf("error getting address for check %q: %v", check.Name, err) } diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index fc2c6d5da..c25a527ad 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1374,6 +1374,7 @@ func ApiServicesToStructs(in []*api.Service, group bool) []*structs.Service { CanaryTags: s.CanaryTags, EnableTagOverride: s.EnableTagOverride, AddressMode: s.AddressMode, + Address: s.Address, Meta: helper.CopyMapStringString(s.Meta), CanaryMeta: helper.CopyMapStringString(s.CanaryMeta), OnUpdate: s.OnUpdate, diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 3c7cdb603..fc4310027 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2514,6 +2514,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { CanaryTags: []string{"d", "e"}, EnableTagOverride: true, PortLabel: "1234", + Address: "group.example.com", Meta: map[string]string{ "servicemeta": "foobar", }, @@ -2523,7 +2524,6 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { }, Checks: []api.ServiceCheck{ { - Id: "hello", Name: "bar", Type: "http", Command: "foo", @@ -2602,12 +2602,12 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { }, Services: []*api.Service{ { - Id: "id", Name: "serviceA", Tags: []string{"1", "2"}, CanaryTags: []string{"3", "4"}, EnableTagOverride: true, PortLabel: "foo", + Address: "task.example.com", Meta: map[string]string{ "servicemeta": "foobar", }, @@ -2617,7 +2617,6 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { }, Checks: []api.ServiceCheck{ { - Id: "hello", Name: "bar", Type: "http", Command: "foo", @@ -2639,7 +2638,6 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { }, }, { - Id: "check2id", Name: "check2", Type: "tcp", PortLabel: "foo", @@ -2914,6 +2912,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { EnableTagOverride: true, PortLabel: "1234", AddressMode: "auto", + Address: "group.example.com", Meta: map[string]string{ "servicemeta": "foobar", }, @@ -3007,6 +3006,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { EnableTagOverride: true, PortLabel: "foo", AddressMode: "auto", + Address: "task.example.com", Meta: map[string]string{ "servicemeta": "foobar", }, diff --git a/command/service_info.go b/command/service_info.go index f122c7fd3..91e47fd02 100644 --- a/command/service_info.go +++ b/command/service_info.go @@ -183,7 +183,7 @@ func (s *ServiceInfoCommand) formatOutput(jobIDs []string, jobServices map[strin outputTable = append(outputTable, fmt.Sprintf( "%s|%s|[%s]|%s|%s", service.JobID, - fmt.Sprintf("%s:%v", service.Address, service.Port), + formatAddress(service.Address, service.Port), strings.Join(service.Tags, ","), limit(service.NodeID, shortId), limit(service.AllocID, shortId), @@ -193,6 +193,13 @@ func (s *ServiceInfoCommand) formatOutput(jobIDs []string, jobServices map[strin s.Ui.Output(formatList(outputTable)) } +func formatAddress(address string, port int) string { + if port == 0 { + return address + } + return fmt.Sprintf("%s:%d", address, port) +} + // formatOutput produces the verbose output of service registration info for a // specific service by its name. func (s *ServiceInfoCommand) formatVerboseOutput(jobIDs []string, jobServices map[string][]*api.ServiceRegistration) { diff --git a/command/service_info_test.go b/command/service_info_test.go index c2c86bdb5..bb20b59c7 100644 --- a/command/service_info_test.go +++ b/command/service_info_test.go @@ -53,8 +53,8 @@ func TestServiceInfoCommand_Run(t *testing.T) { // Create a test job with a Nomad service. testJob := testJob("service-discovery-nomad-info") - testJob.TaskGroups[0].Tasks[0].Services = []*api.Service{ - {Name: "service-discovery-nomad-info", Provider: "nomad", Tags: []string{"foo", "bar"}}} + testJob.TaskGroups[0].Services = []*api.Service{ + {Name: "service-discovery-nomad-info", Provider: "nomad", PortLabel: "9999", Tags: []string{"foo", "bar"}}} // Register that job. regResp, _, err := client.Jobs().Register(testJob, nil) @@ -95,7 +95,7 @@ func TestServiceInfoCommand_Run(t *testing.T) { if !assert.Contains(t, s, "service-discovery-nomad-info") { return false } - if !assert.Contains(t, s, ":0") { + if !assert.Contains(t, s, ":9999") { return false } if !assert.Contains(t, s, "[foo,bar]") { @@ -114,7 +114,7 @@ func TestServiceInfoCommand_Run(t *testing.T) { require.Contains(t, s, "Namespace = default") require.Contains(t, s, "Job ID = service-discovery-nomad-info") require.Contains(t, s, "Datacenter = dc1") - require.Contains(t, s, "Address = :0") + require.Contains(t, s, "Address = :9999") require.Contains(t, s, "Tags = [foo,bar]") ui.OutputWriter.Reset() diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 946e350e3..07455c536 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -2841,6 +2841,12 @@ func TestTaskGroupDiff(t *testing.T) { Type: DiffTypeEdited, Name: "Service", Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Address", + Old: "", + New: "", + }, { Type: DiffTypeNone, Name: "AddressMode", @@ -5636,6 +5642,7 @@ func TestTaskDiff(t *testing.T) { Name: "foo", PortLabel: "bar", AddressMode: "driver", + Address: "a.example.com", TaskName: "task1", }, }, @@ -5647,6 +5654,12 @@ func TestTaskDiff(t *testing.T) { Type: DiffTypeEdited, Name: "Service", Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Address", + Old: "", + New: "a.example.com", + }, { Type: DiffTypeAdded, Name: "AddressMode", @@ -5805,6 +5818,10 @@ func TestTaskDiff(t *testing.T) { }, }, Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Address", + }, { Type: DiffTypeNone, Name: "AddressMode", @@ -6320,6 +6337,12 @@ func TestTaskDiff(t *testing.T) { Type: DiffTypeEdited, Name: "Service", Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Address", + Old: "", + New: "", + }, { Type: DiffTypeNone, Name: "AddressMode", @@ -7421,6 +7444,7 @@ func TestServicesDiff(t *testing.T) { Name: "webapp", PortLabel: "http", AddressMode: "host", + Address: "a.example.com", EnableTagOverride: true, Tags: []string{"prod"}, CanaryTags: []string{"canary"}, @@ -7431,6 +7455,7 @@ func TestServicesDiff(t *testing.T) { Name: "webapp-2", PortLabel: "https", AddressMode: "alloc", + Address: "b.example.com", EnableTagOverride: false, Tags: []string{"prod", "dev"}, CanaryTags: []string{"qa"}, @@ -7441,6 +7466,12 @@ func TestServicesDiff(t *testing.T) { Type: DiffTypeEdited, Name: "Service", Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "Address", + Old: "a.example.com", + New: "b.example.com", + }, { Type: DiffTypeEdited, Name: "AddressMode", @@ -7535,6 +7566,10 @@ func TestServicesDiff(t *testing.T) { Type: DiffTypeAdded, Name: "Service", Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Address", + }, { Type: DiffTypeNone, Name: "AddressMode", @@ -7598,6 +7633,10 @@ func TestServicesDiff(t *testing.T) { Type: DiffTypeAdded, Name: "Service", Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Address", + }, { Type: DiffTypeNone, Name: "AddressMode", @@ -7665,6 +7704,10 @@ func TestServicesDiff(t *testing.T) { Type: DiffTypeEdited, Name: "Service", Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Address", + }, { Type: DiffTypeNone, Name: "AddressMode", @@ -7738,6 +7781,10 @@ func TestServicesDiff(t *testing.T) { Type: DiffTypeEdited, Name: "Service", Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Address", + }, { Type: DiffTypeNone, Name: "AddressMode", @@ -7821,6 +7868,10 @@ func TestServicesDiff(t *testing.T) { Type: DiffTypeEdited, Name: "Service", Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Address", + }, { Type: DiffTypeNone, Name: "AddressMode", diff --git a/nomad/structs/services.go b/nomad/structs/services.go index dc9209220..8b6df3d9f 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -15,7 +15,7 @@ import ( "time" "github.com/hashicorp/consul/api" - multierror "github.com/hashicorp/go-multierror" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/args" "github.com/mitchellh/copystructure" @@ -49,7 +49,7 @@ type ServiceCheck struct { Protocol string // Protocol to use if check is http, defaults to http PortLabel string // The port to use for tcp/http checks Expose bool // Whether to have Envoy expose the check path (connect-enabled group-services only) - AddressMode string // 'host' to use host ip:port or 'driver' to use driver's + AddressMode string // Must be empty, "alloc", "host", or "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 @@ -449,10 +449,14 @@ type Service struct { // address, specify an empty host in the PortLabel (e.g. `:port`). PortLabel string - // AddressMode specifies whether or not to use the host ip:port for - // this service. + // AddressMode specifies how the Advertise address used in service registration + // is determined. Must be "auto" (default), "host", "driver", or "alloc". AddressMode string + // Advertise enables explicitly setting and advertise address used in service + // registration. AddressMode must be "auto" if Advertise is set. + Address string + // EnableTagOverride will disable Consul's anti-entropy mechanism for the // tags of this service. External updates to the service definition via // Consul will not be corrected to match the service definition set in the @@ -577,8 +581,11 @@ func (s *Service) Validate() error { } switch s.AddressMode { - case "", AddressModeAuto, AddressModeHost, AddressModeDriver, AddressModeAlloc: - // OK + case "", AddressModeAuto: + case AddressModeHost, AddressModeDriver, AddressModeAlloc: + if s.Address != "" { + mErr.Errors = append(mErr.Errors, fmt.Errorf("Service address_mode must be %q if address is set", AddressModeAuto)) + } default: mErr.Errors = append(mErr.Errors, fmt.Errorf("Service address_mode must be %q, %q, or %q; not %q", AddressModeAuto, AddressModeHost, AddressModeDriver, s.AddressMode)) } @@ -685,6 +692,7 @@ func (s *Service) Hash(allocID, taskName string, canary bool) string { hashString(h, s.Name) hashString(h, s.PortLabel) hashString(h, s.AddressMode) + hashString(h, s.Address) hashTags(h, s.Tags) hashTags(h, s.CanaryTags) hashBool(h, canary, "Canary") @@ -767,6 +775,10 @@ func (s *Service) Equals(o *Service) bool { return false } + if s.Address != o.Address { + return false + } + if s.OnUpdate != o.OnUpdate { return false } diff --git a/nomad/structs/services_test.go b/nomad/structs/services_test.go index 915a5a969..7917c3245 100644 --- a/nomad/structs/services_test.go +++ b/nomad/structs/services_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/helper" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -246,6 +247,10 @@ func TestService_Hash(t *testing.T) { // these tests use tweaker to modify 1 field and make the false assertion // on comparing the resulting hash output + t.Run("mod advertise", func(t *testing.T) { + try(t, func(s *svc) { s.Address = "example.com" }) + }) + t.Run("mod name", func(t *testing.T) { try(t, func(s *svc) { s.Name = "newName" }) }) @@ -1478,6 +1483,187 @@ func TestConsulMeshGateway_Validate(t *testing.T) { }) } +func TestService_Validate(t *testing.T) { + ci.Parallel(t) + + testCases := []struct { + input *Service + expErr bool + expErrStr string + name string + }{ + { + input: &Service{ + Name: "testservice", + }, + expErr: false, + name: "base service", + }, + { + input: &Service{ + Name: "testservice", + Connect: &ConsulConnect{ + Native: true, + }, + }, + expErr: true, + expErrStr: "Connect Native and requires setting the task", + name: "Native Connect without task name", + }, + { + input: &Service{ + Name: "testservice", + TaskName: "testtask", + Connect: &ConsulConnect{ + Native: true, + }, + }, + expErr: false, + name: "Native Connect with task name", + }, + { + input: &Service{ + Name: "testservice", + TaskName: "testtask", + Connect: &ConsulConnect{ + Native: true, + SidecarService: &ConsulSidecarService{}, + }, + }, + expErr: true, + expErrStr: "Consul Connect must be exclusively native", + name: "Native Connect with Sidecar", + }, + { + input: &Service{ + Name: "testservice", + Provider: "nomad", + Checks: []*ServiceCheck{ + { + Name: "servicecheck", + }, + }, + }, + expErr: true, + expErrStr: "Service with provider nomad cannot include Check blocks", + name: "provider nomad with checks", + }, + { + input: &Service{ + Name: "testservice", + Provider: "nomad", + Connect: &ConsulConnect{ + Native: true, + }, + }, + expErr: true, + expErrStr: "Service with provider nomad cannot include Connect blocks", + name: "provider nomad with connect", + }, + { + input: &Service{ + Name: "testservice", + Provider: "nomad", + }, + expErr: false, + name: "provider nomad valid", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.input.Canonicalize("testjob", "testgroup", "testtask", "testnamespace") + err := tc.input.Validate() + if tc.expErr { + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.expErrStr) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestService_Advertise(t *testing.T) { + try := func(mode, advertise string, exp error) { + s := &Service{Name: "s1", Provider: "consul", AddressMode: mode, Address: advertise} + result := s.Validate() + if exp == nil { + require.NoError(t, result) + } else { + // would be nice if multierror worked with errors.Is + require.Contains(t, result.Error(), exp.Error()) + } + } + + // advertise not set + try("", "", nil) + try("auto", "", nil) + try("host", "", nil) + try("alloc", "", nil) + try("driver", "", nil) + + // advertise is set + try("", "example.com", nil) + try("auto", "example.com", nil) + try("host", "example.com", errors.New(`Service address_mode must be "auto" if address is set`)) + try("alloc", "example.com", errors.New(`Service address_mode must be "auto" if address is set`)) + try("driver", "example.com", errors.New(`Service address_mode must be "auto" if address is set`)) +} + +func TestService_Equals(t *testing.T) { + ci.Parallel(t) + + s := Service{ + Name: "testservice", + } + + s.Canonicalize("testjob", "testgroup", "testtask", "default") + + o := s.Copy() + + // Base service should be equal to copy of itself + require.True(t, s.Equals(o)) + + // create a helper to assert a diff and reset the struct + assertDiff := func() { + require.False(t, s.Equals(o)) + o = s.Copy() + require.True(t, s.Equals(o), "bug in copy") + } + + // Changing any field should cause inequality + o.Name = "diff" + assertDiff() + + o.Address = "diff" + assertDiff() + + o.PortLabel = "diff" + assertDiff() + + o.AddressMode = AddressModeDriver + assertDiff() + + o.Tags = []string{"diff"} + assertDiff() + + o.CanaryTags = []string{"diff"} + assertDiff() + + o.Checks = []*ServiceCheck{{Name: "diff"}} + assertDiff() + + o.Connect = &ConsulConnect{Native: true} + assertDiff() + + o.EnableTagOverride = true + assertDiff() + + o.Provider = "nomad" + assertDiff() +} + func TestService_validateNomadService(t *testing.T) { ci.Parallel(t) diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index df387d8d8..004f82515 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -3506,157 +3506,6 @@ func TestService_Canonicalize(t *testing.T) { } } -func TestService_Validate(t *testing.T) { - ci.Parallel(t) - - testCases := []struct { - inputService *Service - expectedError bool - expectedErrorContains string - name string - }{ - { - inputService: &Service{ - Name: "testservice", - }, - expectedError: false, - name: "base service", - }, - { - inputService: &Service{ - Name: "testservice", - Connect: &ConsulConnect{ - Native: true, - }, - }, - expectedError: true, - expectedErrorContains: "Connect Native and requires setting the task", - name: "Native Connect without task name", - }, - { - inputService: &Service{ - Name: "testservice", - TaskName: "testtask", - Connect: &ConsulConnect{ - Native: true, - }, - }, - expectedError: false, - name: "Native Connect with task name", - }, - { - inputService: &Service{ - Name: "testservice", - TaskName: "testtask", - Connect: &ConsulConnect{ - Native: true, - SidecarService: &ConsulSidecarService{}, - }, - }, - expectedError: true, - expectedErrorContains: "Consul Connect must be exclusively native", - name: "Native Connect with Sidecar", - }, - { - inputService: &Service{ - Name: "testservice", - Provider: "nomad", - Checks: []*ServiceCheck{ - { - Name: "servicecheck", - }, - }, - }, - expectedError: true, - expectedErrorContains: "Service with provider nomad cannot include Check blocks", - name: "provider nomad with checks", - }, - { - inputService: &Service{ - Name: "testservice", - Provider: "nomad", - Connect: &ConsulConnect{ - Native: true, - }, - }, - expectedError: true, - expectedErrorContains: "Service with provider nomad cannot include Connect blocks", - name: "provider nomad with connect", - }, - { - inputService: &Service{ - Name: "testservice", - Provider: "nomad", - }, - expectedError: false, - name: "provider nomad valid", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - tc.inputService.Canonicalize("testjob", "testgroup", "testtask", "testnamespace") - err := tc.inputService.Validate() - if tc.expectedError { - assert.Error(t, err) - assert.Contains(t, err.Error(), tc.expectedErrorContains) - } else { - assert.NoError(t, err) - } - }) - } -} - -func TestService_Equals(t *testing.T) { - ci.Parallel(t) - - s := Service{ - Name: "testservice", - } - - s.Canonicalize("testjob", "testgroup", "testtask", "default") - - o := s.Copy() - - // Base service should be equal to copy of itself - require.True(t, s.Equals(o)) - - // create a helper to assert a diff and reset the struct - assertDiff := func() { - require.False(t, s.Equals(o)) - o = s.Copy() - require.True(t, s.Equals(o), "bug in copy") - } - - // Changing any field should cause inequality - o.Name = "diff" - assertDiff() - - o.PortLabel = "diff" - assertDiff() - - o.AddressMode = AddressModeDriver - assertDiff() - - o.Tags = []string{"diff"} - assertDiff() - - o.CanaryTags = []string{"diff"} - assertDiff() - - o.Checks = []*ServiceCheck{{Name: "diff"}} - assertDiff() - - o.Connect = &ConsulConnect{Native: true} - assertDiff() - - o.EnableTagOverride = true - assertDiff() - - o.Provider = "nomad" - assertDiff() -} - func TestJob_ExpandServiceNames(t *testing.T) { ci.Parallel(t) diff --git a/website/content/docs/job-specification/service.mdx b/website/content/docs/job-specification/service.mdx index e886e97cc..600856e3c 100644 --- a/website/content/docs/job-specification/service.mdx +++ b/website/content/docs/job-specification/service.mdx @@ -118,7 +118,7 @@ Connect][connect] integration. If a `to` value is not set, the port falls back to using the allocated host port. The `port` field may be a numeric port or a port label specified in the same group's network stanza. - - `driver` - Advertise the port determined by the driver (e.g. Docker or rkt). + - `driver` - Advertise the port determined by the driver (e.g. Docker). The `port` may be a numeric port or a port label specified in the driver's `ports` field. @@ -141,9 +141,12 @@ Connect][connect] integration. [documentation](https://www.consul.io/docs/internals/anti-entropy#enable-tag-override) for more information. Only available where `provider = "consul"`. +- `address` `(string: )` - Specifies a custom address to advertise in + Consul or Nomad service registration. Can be an IP address or domain name. If + set, `address_mode` must be in `auto` mode. + - `address_mode` `(string: "auto")` - Specifies which address (host, alloc or - driver-specific) this service should advertise. This setting is supported in - Docker since Nomad 0.6 and rkt since Nomad 0.7. See [below for + driver-specific) this service should advertise. See [below for examples.](#using-driver-address-mode) Valid options are: - `alloc` - For allocations which create a network namespace, this address mode @@ -161,7 +164,7 @@ Connect][connect] integration. port map. A numeric port may be specified since port maps aren't required by all network plugins. Useful for advertising SDN and overlay network addresses. Task will fail if driver network cannot be determined. Only - implemented for Docker and rkt. This mode can only be set for services + implemented for Docker. This mode can only be set for services which are defined in a "task" block. - `host` - Use the host IP and port. @@ -213,9 +216,11 @@ scripts. - `address_mode` `(string: "host")` - Same as `address_mode` on `service`. Unlike services, checks do not have an `auto` address mode as there's no way for Nomad to know which is the best address to use for checks. Consul needs - access to the address for any HTTP or TCP checks. Added in Nomad 0.7.1. See + access to the address for any HTTP or TCP checks. See [below for details.](#using-driver-address-mode) Unlike `port`, this setting is _not_ inherited from the `service`. + If the service `address` is set and the check `address_mode` is not set, the + service `address` value will be used for the check address. - `args` `(array: [])` - Specifies additional arguments to the `command`. This only applies to script-based health checks. @@ -588,13 +593,12 @@ selection. ### Using Driver Address Mode -The [Docker](/docs/drivers/docker#network_mode) and -[rkt](/docs/drivers/rkt#net) drivers support the `driver` setting for the -`address_mode` parameter in both `service` and `check` stanzas. The driver -address mode allows advertising and health checking the IP and port assigned to -a task by the driver. This way, if you're using a network plugin like Weave with -Docker, you can advertise the Weave address in Consul instead of the host's -address. +The [Docker](/docs/drivers/docker#network_mode) driver supports the `driver` +setting for the `address_mode` parameter in both `service` and `check` stanzas. +The driver address mode allows advertising and health checking the IP and port +assigned to a task by the driver. This way, if you're using a network plugin like +Weave with Docker, you can advertise the Weave address in Consul instead of the +host's address. For example if you were running the example Redis job in an environment with Weave but Consul was running on the host you could use the following @@ -643,7 +647,7 @@ job "example" { No explicit `address_mode` required. Services default to the `auto` address mode. When a Docker network mode other -than "host" or "bridge" is used, services will automatically advertise the +than `"host"` or `"bridge"` is used, services will automatically advertise the driver's address (in this case Weave's). The service will advertise the container's port: 6379.