luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Merge pull request #3682 from hashicorp/b-3681-always-set-driver-ip 

Always advertise driver IP when in driver mode
This commit is contained in:
Michael Schurter 2018-01-22 16:41:34 -08:00 committed by GitHub
parent e5cddc80e4 1b5bb6c24c
commit 694b547a6b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 197 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -17,6 +17,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)]

2
client/driver/docker.go
View File

@ -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

15
client/task_runner.go
View File

@ -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)

29
command/agent/consul/client.go
View File

@ -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

67
command/agent/consul/unit_test.go
View File

@ -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: "EmptyIsOk",
Mode: structs.AddressModeHost,
Name: "NoPort_AutoMode",
Mode: structs.AddressModeAuto,
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 +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)
}
}
})

116
nomad/structs/structs_test.go
View File

@ -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{