Add advertise-address flag to handle the scheduler being behind a NAT #102
Conversation
lclarkmichalek
changed the title
| @@ -122,12 +124,16 @@ func main() { | |||
| } | |||
| } | |||
|
|
|||
| if *advertiseAddress == "" { | |||
| *advertiseAddress = *address | |||
There was a problem hiding this comment.
I pretty much copied the *address default assignment logic, which felt a little hacky also :/ This simply duplicates the current logic though
There was a problem hiding this comment.
ack. i should have scrolled up! the address logic could easily be rolled into a defaultAddress() func since it doesn't depend on any internal vars. this advertiseAddress value is basically defaulting to the value of another flag. it's not quite duplicating existing logic because PublishedAddress below will always have a value now. I'd rather see that param unset if the advertiseAddress flag is unspecified. does that make sense?
There was a problem hiding this comment.
Yeah, that's true. I don't believe the semantics will be different, but then I haven't tested it, so I'll happily update below to not set PublishedAddress if advertiseAddress is empty/nil :)
|
thanks for the PR, left some comments |
…on, only set PublishedAddress when advertiseAddress is explicitly set
|
Not 100% sure that's exactly what you wanted. Used the explicitAdvertise var to indicate if the advertiseAddress flag was ever set to anything different. |
In cases where the Docker daemon puts containers behind a NAT, the agent needs to use a different address to the address that is bound on to get the executor and related binaries. This change adds an 'advertise-address' flag that allow you to specify this.
I would have loved to call this flag address, and move the current address flag to bind-address or something, but I don't know how compatible that would be?