From 4e326d455d47ebfe92b00e06394206d848b8b7d9 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Wed, 20 Dec 2017 15:02:34 -0800 Subject: [PATCH] Always advertise driver IP when in driver mode 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. --- client/driver/docker.go | 10 ++++- command/agent/consul/client.go | 22 ++++++++--- command/agent/consul/unit_test.go | 64 +++++++++++++++++++------------ 3 files changed, 65 insertions(+), 31 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index d2b3827c064..81cffb44891 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -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{ @@ -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 diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go index b8db963aeb7..9955d7e99e1 100644 --- a/command/agent/consul/client.go +++ b/command/agent/consul/client.go @@ -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() { @@ -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 { @@ -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 diff --git a/command/agent/consul/unit_test.go b/command/agent/consul/unit_test.go index 4d8123b881e..fa8b52ab4cd 100644 --- a/command/agent/consul/unit_test.go +++ b/command/agent/consul/unit_test.go @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -1526,8 +1526,8 @@ 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", @@ -1535,7 +1535,7 @@ func TestGetAddress(t *testing.T) { PortLabel: "db", Host: map[string]int{"db": 12345}, Driver: nil, - ErrContains: "no driver network exists", + ExpectedErr: "no driver network exists", }, { Name: "DriverBadPort", @@ -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", @@ -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", }, } @@ -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) } } })