From cde796162ceda93e270852328fd211c5c97de16a 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 d2b3827c0..81cffb448 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 b8db963ae..9955d7e99 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 4d8123b88..fa8b52ab4 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) } } })