Always advertise driver IP when in driver mode #3682
Conversation
client/driver/docker.go
Outdated
| @@ -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", | |||
There was a problem hiding this comment.
Would it be better to invert these logs and log on the task driver?
command/agent/consul/unit_test.go
Outdated
| Name: "EmptyIsOk", | ||
| Mode: structs.AddressModeHost, | ||
| Name: "NoPort_AutoMode", | ||
| Mode: structs.AddressModeHost, |
There was a problem hiding this comment.
This is host not auto. Also shouldn't the auto be with a driver network
schmichael
force-pushed
the
b-3681-always-set-driver-ip
branch
from
cd2e712
to
9a1cb46
Compare
schmichael
force-pushed
the
b-3681-always-set-driver-ip
branch
from
9a1cb46
to
c3f6dae
Compare
|
Pushed some more tests and an error message improvement to address #3681 (comment) Could use a re-review |
client/task_runner.go
Outdated
| } | ||
|
|
||
| if sresp.Network == nil || sresp.Network.IP == "" { | ||
| r.logger.Printf("[DEBUG] client: alloc %s task %s could not detect a driver IP", r.alloc.ID, r.task.Name) |
There was a problem hiding this comment.
These DEBUG logs feel like they may be better suited for TRACE? They are more or less expected on all task. What do you think?
There was a problem hiding this comment.
Glanced at the other DEBUG logs in TaskRunner and agree. These are pretty low level and could be needlessly confusing for people who never use this feature.
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.
Related to #3681 If a user specifies an invalid port *label* when using address_mode=driver they'll get an error message about the label being an invalid number which is very confusing. I also added a bunch of testing around Service.AddressMode validation since I was concerned by the linked issue that there were cases I was missing. Unfortunately when address_mode=driver is used there's only so much validation that can be done as structs/structs.go validation never peeks into the driver config which would be needed to verify the port labels/map.
For people not using driver networks these log lines would just be confusing.
schmichael
force-pushed
the
b-3681-always-set-driver-ip
branch
from
7ee3172
to
c624e5b
Compare
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #3681
When in driver 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.
Binaries from d1e2833
linux_amd64.zip
windows_amd64.zip
Old
Binaries from 9a1cb46 which include #3680:linux_amd64.zipwindows_amd64.zipBinaries from cd2e712:linux_amd64.zipwindows_amd64.zip