Skip to content

Commit

Permalink
Always advertise driver IP when in driver mode
Browse files Browse the repository at this point in the history 
Fixes #3681

When in drive address mode Nomad should always advertise the driver's IP
in Consul even when no network exists. This matches the 0.6 behavior.

When in host address mode Nomad advertises the alloc's network's IP if
one exists. Otherwise it lets Consul determine the IP.

I also added some much needed logging around Docker's network discovery.
  • Loading branch information
@schmichael
schmichael committed Jan 18, 2018
1 parent 3b95662 commit cde7961
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 31 deletions.
10 changes: 9 additions & 1 deletion client/driver/docker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,9 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (*StartRespon

// Detect container address
ip, autoUse := d.detectIP(container)
if ip == "" {
d.logger.Printf("[DEBUG] driver.docker: task %s could not detect a container IP", d.taskName)
}

// Create a response with the driver handle and container network metadata
resp := &StartResponse{
Expand Down Expand Up @@ -860,13 +863,18 @@ func (d *DockerDriver) detectIP(c *docker.Container) (string, bool) {
// Linux, nat on Windows)
if name != "bridge" && name != "nat" {
auto = true
d.logger.Printf("[INFO] driver.docker: task %s auto-advertising detected IP %s on network %q",
d.taskName, ip, name)
} else {
d.logger.Printf("[DEBUG] driver.docker task %s detect IP %s on network %q but not auto-advertising",
d.taskName, ip, name)
}

break
}

if n := len(c.NetworkSettings.Networks); n > 1 {
d.logger.Printf("[WARN] driver.docker: multiple (%d) Docker networks for container %q but Nomad only supports 1: choosing %q", n, c.ID, ipName)
d.logger.Printf("[WARN] driver.docker: task %s multiple (%d) Docker networks for container %q but Nomad only supports 1: choosing %q", d.taskName, n, c.ID, ipName)
}

return ip, auto
Expand Down
22 changes: 17 additions & 5 deletions command/agent/consul/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1098,11 +1098,6 @@ func isOldNomadService(id string) bool {
// 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 *cstructs.DriverNetwork) (string, int, error) {
// No port label specified, no address can be assembled
if portLabel == "" {
return "", 0, nil
}

switch addrMode {
case structs.AddressModeAuto:
if driverNet.Advertise() {
Expand All @@ -1112,6 +1107,18 @@ func getAddress(addrMode, portLabel string, networks structs.Networks, driverNet
}
return getAddress(addrMode, portLabel, networks, driverNet)
case structs.AddressModeHost:
if portLabel == "" {
if len(networks) != 1 {
// If no networks are specified return zero
// values. Consul will advertise the host IP
// with no port. This is the pre-0.7.1 behavior
// some people rely on.
return "", 0, nil
}

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

// Default path: use host ip:port
ip, port := networks.Port(portLabel)
if ip == "" && port <= 0 {
Expand All @@ -1125,6 +1132,11 @@ func getAddress(addrMode, portLabel string, networks structs.Networks, driverNet
return "", 0, fmt.Errorf(`cannot use address_mode="driver": no driver network exists`)
}

// If no port label is specified just return the IP
if portLabel == "" {
return driverNet.IP, 0, nil
}

// If the port is a label, use the driver's port (not the host's)
if port, ok := driverNet.PortMap[portLabel]; ok {
return driverNet.IP, port, nil
Expand Down
64 changes: 39 additions & 25 deletions command/agent/consul/unit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1464,9 +1464,9 @@ func TestGetAddress(t *testing.T) {
Driver *cstructs.DriverNetwork

// Results
IP string
Port int
ErrContains string
ExpectedIP string
ExpectedPort int
ExpectedErr string
}{
{
Name: "ExampleService",
Expand All @@ -1477,8 +1477,8 @@ func TestGetAddress(t *testing.T) {
PortMap: map[string]int{"db": 6379},
IP: "10.1.2.3",
},
IP: HostIP,
Port: 12435,
ExpectedIP: HostIP,
ExpectedPort: 12435,
},
{
Name: "Host",
Expand All @@ -1489,8 +1489,8 @@ func TestGetAddress(t *testing.T) {
PortMap: map[string]int{"db": 6379},
IP: "10.1.2.3",
},
IP: HostIP,
Port: 12345,
ExpectedIP: HostIP,
ExpectedPort: 12345,
},
{
Name: "Driver",
Expand All @@ -1501,8 +1501,8 @@ func TestGetAddress(t *testing.T) {
PortMap: map[string]int{"db": 6379},
IP: "10.1.2.3",
},
IP: "10.1.2.3",
Port: 6379,
ExpectedIP: "10.1.2.3",
ExpectedPort: 6379,
},
{
Name: "AutoDriver",
Expand All @@ -1514,8 +1514,8 @@ func TestGetAddress(t *testing.T) {
IP: "10.1.2.3",
AutoAdvertise: true,
},
IP: "10.1.2.3",
Port: 6379,
ExpectedIP: "10.1.2.3",
ExpectedPort: 6379,
},
{
Name: "DriverCustomPort",
Expand All @@ -1526,16 +1526,16 @@ func TestGetAddress(t *testing.T) {
PortMap: map[string]int{"db": 6379},
IP: "10.1.2.3",
},
IP: "10.1.2.3",
Port: 7890,
ExpectedIP: "10.1.2.3",
ExpectedPort: 7890,
},
{
Name: "DriverWithoutNetwork",
Mode: structs.AddressModeDriver,
PortLabel: "db",
Host: map[string]int{"db": 12345},
Driver: nil,
ErrContains: "no driver network exists",
ExpectedErr: "no driver network exists",
},
{
Name: "DriverBadPort",
Expand All @@ -1546,7 +1546,7 @@ func TestGetAddress(t *testing.T) {
PortMap: map[string]int{"db": 6379},
IP: "10.1.2.3",
},
ErrContains: "invalid port",
ExpectedErr: "invalid port",
},
{
Name: "DriverZeroPort",
Expand All @@ -1555,23 +1555,37 @@ func TestGetAddress(t *testing.T) {
Driver: &cstructs.DriverNetwork{
IP: "10.1.2.3",
},
ErrContains: "invalid port",
ExpectedErr: "invalid port",
},
{
Name: "HostBadPort",
Mode: structs.AddressModeHost,
PortLabel: "bad-port-label",
ErrContains: "invalid port",
ExpectedErr: "invalid port",
},
{
Name: "InvalidMode",
Mode: "invalid-mode",
PortLabel: "80",
ErrContains: "invalid address mode",
ExpectedErr: "invalid address mode",
},
{
Name: "EmptyIsOk",
Mode: structs.AddressModeHost,
Name: "NoPort_AutoMode",
Mode: structs.AddressModeHost,
ExpectedIP: HostIP,
},
{
Name: "NoPort_HostMode",
Mode: structs.AddressModeHost,
ExpectedIP: HostIP,
},
{
Name: "NoPort_DriverMode",
Mode: structs.AddressModeDriver,
Driver: &cstructs.DriverNetwork{
IP: "10.1.2.3",
},
ExpectedIP: "10.1.2.3",
},
}

Expand All @@ -1596,15 +1610,15 @@ func TestGetAddress(t *testing.T) {
ip, port, err := getAddress(tc.Mode, tc.PortLabel, networks, tc.Driver)

// Assert the results
assert.Equal(t, tc.IP, ip, "IP mismatch")
assert.Equal(t, tc.Port, port, "Port mismatch")
if tc.ErrContains == "" {
assert.Equal(t, tc.ExpectedIP, ip, "IP mismatch")
assert.Equal(t, tc.ExpectedPort, port, "Port mismatch")
if tc.ExpectedErr == "" {
assert.Nil(t, err)
} else {
if err == nil {
t.Fatalf("expected error containing %q but err=nil", tc.ErrContains)
t.Fatalf("expected error containing %q but err=nil", tc.ExpectedErr)
} else {
assert.Contains(t, err.Error(), tc.ErrContains)
assert.Contains(t, err.Error(), tc.ExpectedErr)
}
}
})
Expand Down

0 comments on commit cde7961

Please sign in to comment.