diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f9f7f562fe2..054c977336bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ BUG FIXES: * core: Fix an issue in which batch jobs with queued placements and lost allocations could result in improper placement counts [[GH-3717](https://github.com/hashicorp/nomad/issues/3717)] * client: Migrated ephemeral_disk's maintain directory permissions [[GH-3723](https://github.com/hashicorp/nomad/issues/3723)] + * client: Always advertise driver IP when in driver address mode [[GH-3682](https://github.com/hashicorp/nomad/issues/3682)] * client/vault: Recognize renewing non-renewable Vault lease as fatal [[GH-3727](https://github.com/hashicorp/nomad/issues/3727)] * config: Revert minimum CPU limit back to 20 from 100. * ui: Fix ui on non-leaders when ACLs are enabled [[GH-3722](https://github.com/hashicorp/nomad/issues/3722)] diff --git a/client/driver/docker.go b/client/driver/docker.go index d2b3827c0647..df6ee0fcacff 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -866,7 +866,7 @@ func (d *DockerDriver) detectIP(c *docker.Container) (string, bool) { } 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/client/task_runner.go b/client/task_runner.go index e01cc34cdd62..9d45d91f7f6a 100644 --- a/client/task_runner.go +++ b/client/task_runner.go @@ -1440,6 +1440,21 @@ func (r *TaskRunner) startTask() error { } + // Log driver network information + if sresp.Network != nil && sresp.Network.IP != "" { + if sresp.Network.AutoAdvertise { + r.logger.Printf("[INFO] client: alloc %s task %s auto-advertising detected IP %s", + r.alloc.ID, r.task.Name, sresp.Network.IP) + } else { + r.logger.Printf("[TRACE] client: alloc %s task %s detected IP %s but not auto-advertising", + r.alloc.ID, r.task.Name, sresp.Network.IP) + } + } + + if sresp.Network == nil || sresp.Network.IP == "" { + r.logger.Printf("[TRACE] client: alloc %s task %s could not detect a driver IP", r.alloc.ID, r.task.Name) + } + // Update environment with the network defined by the driver's Start method. r.envBuilder.SetDriverNetwork(sresp.Network) diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go index b8db963aeb70..93547c26eea9 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 @@ -1133,10 +1145,13 @@ func getAddress(addrMode, portLabel string, networks structs.Networks, driverNet // If port isn't a label, try to parse it as a literal port number port, err := strconv.Atoi(portLabel) if err != nil { - return "", 0, fmt.Errorf("invalid port %q: %v", portLabel, err) + // Don't include Atoi error message as user likely + // never intended it to be a numeric and it creates a + // confusing error message + return "", 0, fmt.Errorf("invalid port label %q: port labels in driver address_mode must be numeric or in the driver's port map", portLabel) } if port <= 0 { - return "", 0, fmt.Errorf("invalid port: %q: port 0 is invalid", portLabel) + return "", 0, fmt.Errorf("invalid port: %q: port must be >0", portLabel) } return driverNet.IP, port, nil diff --git a/command/agent/consul/unit_test.go b/command/agent/consul/unit_test.go index 4d8123b881ed..dd6b3477b853 100644 --- a/command/agent/consul/unit_test.go +++ b/command/agent/consul/unit_test.go @@ -1464,10 +1464,11 @@ func TestGetAddress(t *testing.T) { Driver *cstructs.DriverNetwork // Results - IP string - Port int - ErrContains string + ExpectedIP string + ExpectedPort int + ExpectedErr string }{ + // Valid Configurations { Name: "ExampleService", Mode: structs.AddressModeAuto, @@ -1477,8 +1478,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 +1490,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 +1502,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 +1515,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,16 +1527,18 @@ 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, }, + + // Invalid Configurations { 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", @@ -1546,7 +1549,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 +1558,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: "NoPort_AutoMode", + Mode: structs.AddressModeAuto, + ExpectedIP: HostIP, + }, + { + Name: "NoPort_HostMode", + Mode: structs.AddressModeHost, + ExpectedIP: HostIP, }, { - Name: "EmptyIsOk", - Mode: structs.AddressModeHost, + Name: "NoPort_DriverMode", + Mode: structs.AddressModeDriver, + Driver: &cstructs.DriverNetwork{ + IP: "10.1.2.3", + }, + ExpectedIP: "10.1.2.3", }, } @@ -1596,15 +1613,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) } } }) diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index c4e52038fe99..87d820ca332d 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -1178,6 +1178,122 @@ func TestTask_Validate_Services(t *testing.T) { } } +func TestTask_Validate_Service_AddressMode_Ok(t *testing.T) { + ephemeralDisk := DefaultEphemeralDisk() + getTask := func(s *Service) *Task { + task := &Task{ + Name: "web", + Driver: "docker", + Resources: DefaultResources(), + Services: []*Service{s}, + LogConfig: DefaultLogConfig(), + } + task.Resources.Networks = []*NetworkResource{ + { + MBits: 10, + DynamicPorts: []Port{ + { + Label: "http", + Value: 80, + }, + }, + }, + } + return task + } + + cases := []*Service{ + { + // https://github.com/hashicorp/nomad/issues/3681#issuecomment-357274177 + Name: "DriverModeWithLabel", + PortLabel: "http", + AddressMode: AddressModeDriver, + }, + { + Name: "DriverModeWithPort", + PortLabel: "80", + AddressMode: AddressModeDriver, + }, + { + Name: "HostModeWithLabel", + PortLabel: "http", + AddressMode: AddressModeHost, + }, + { + Name: "HostModeWithoutLabel", + AddressMode: AddressModeHost, + }, + { + Name: "DriverModeWithoutLabel", + AddressMode: AddressModeDriver, + }, + } + + for _, service := range cases { + task := getTask(service) + t.Run(service.Name, func(t *testing.T) { + if err := task.Validate(ephemeralDisk); err != nil { + t.Fatalf("unexpected err: %v", err) + } + }) + } +} + +func TestTask_Validate_Service_AddressMode_Bad(t *testing.T) { + ephemeralDisk := DefaultEphemeralDisk() + getTask := func(s *Service) *Task { + task := &Task{ + Name: "web", + Driver: "docker", + Resources: DefaultResources(), + Services: []*Service{s}, + LogConfig: DefaultLogConfig(), + } + task.Resources.Networks = []*NetworkResource{ + { + MBits: 10, + DynamicPorts: []Port{ + { + Label: "http", + Value: 80, + }, + }, + }, + } + return task + } + + cases := []*Service{ + { + // https://github.com/hashicorp/nomad/issues/3681#issuecomment-357274177 + Name: "DriverModeWithLabel", + PortLabel: "asdf", + AddressMode: AddressModeDriver, + }, + { + Name: "HostModeWithLabel", + PortLabel: "asdf", + AddressMode: AddressModeHost, + }, + { + Name: "HostModeWithPort", + PortLabel: "80", + AddressMode: AddressModeHost, + }, + } + + for _, service := range cases { + task := getTask(service) + t.Run(service.Name, func(t *testing.T) { + err := task.Validate(ephemeralDisk) + if err == nil { + t.Fatalf("expected an error") + } + //t.Logf("err: %v", err) + }) + } +} + func TestTask_Validate_Service_Check(t *testing.T) { invalidCheck := ServiceCheck{