Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

services: enable setting arbitrary address value in service registrations #12720

Merged
merged 4 commits into from
Apr 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/12720.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
services: Enable setting arbitrary address on Nomad or Consul service registration
```
16 changes: 6 additions & 10 deletions api/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ type ServiceRegistrationStub struct {
Tags []string
}


// Services is used to query the service endpoints.
type Services struct {
client *Client
Expand Down Expand Up @@ -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"`
Comment on lines -200 to -201
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You answered that question! Thanks for getting rid of deadcode 👍

Name string `hcl:"name,optional"`
Type string `hcl:"type,optional"`
Command string `hcl:"command,optional"`
Expand All @@ -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"`
Expand All @@ -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"`
Expand All @@ -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"`
}

Expand Down
66 changes: 52 additions & 14 deletions client/serviceregistration/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,50 @@ 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)
// and no custom address is specified, zero values are returned because no address
// could be resolved.
func GetAddress(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing for this PR, but noting for the future that I wonder if it's worth changing this function signature to take a struct rather than a growing list of types.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, and also I think the port mapping lookup and numeric interpolation fallback can be consolidated from now 4 places

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. (e.g. EC2 public address)
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
Expand All @@ -32,7 +60,6 @@ func GetAddress(
// some people rely on.
return "", 0, nil
}

return networks[0].IP, 0, nil
}

Expand Down Expand Up @@ -65,6 +92,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`)
Expand Down Expand Up @@ -100,6 +132,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`)
}
Expand Down Expand Up @@ -131,6 +169,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)
}
}
Loading