feat: Use container names for upstream names to avoid repeats

[rhansen]

Note: This depends on PR #2127, and will be marked as draft until that PR is merged.

[buchdag]

{{- /* TODO: Replace the next few lines with a call to join once nginx-proxy has a version of
docker-gen that includes nginx-proxy/docker-gen#418. */}}

I'm on it: #1940

[rhansen]

Thanks for updating docker-gen. I rebased and switched to the join function.

[buchdag]

Thanks for the PR, unfortunately we can't make this behaviour a default one because a lot of people have custom configurations that rely on upstream names being equal to the hostnames.

See #1725, #1693 (comment), #1736 plus the release notes of version 0.9.2 and 0.9.3.

[rhansen]

I updated the PR to keep the current behavior by default.

[rhansen]

Friendly ping @buchdag

[rhansen]

Marking as draft until #2127 is merged.

[buchdag]

@rhansen sorry for the incredibly long delay on this one. I've tested the PR, and I'm still a bit confused about it.

Does it provide any advantage / behaviour change when not using VIRTUAL_PATH ?

What kind of issue could the "repeat" of upstreams when using VIRTUAL_PATH cause ?

I feel like I'm completely missing the point but I seem unable to grasp what issue this PR is attempting to solve or its benefits against its non trivial added complexity. 🤔

I also don't think it could realistically be made default as it would made upstream name somewhere between very hard to predict and unpredictable on systems or setups where container names are dynamically generated or containers replica dynamically started / stopped.

[rhansen]

Does it provide any advantage / behaviour change when not using VIRTUAL_PATH ?

Yes. Currently, if a single container is used for multiple vhosts (e.g., VIRTUAL_HOST: foo.example,bar.example) then multiple identical upstream blocks are created, one per vhost. With this change only one upstream is generated and shared with both vhosts.

What kind of issue could the "repeat" of upstreams when using VIRTUAL_PATH cause ?

VIRTUAL_PATH is irrelevant to this change.

I think the repeated upstreams could theoretically cause minor load balancing issues.

I feel like I'm completely missing the point but I seem unable to grasp what issue this PR is attempting to solve or its benefits against its non trivial added complexity. thinking

It's been a year so a lot has slipped away from memory, but IIRC my main motivation was to facilitate a fix for #1504. Give me some time to recall what I was doing and I'll update the commit message with some rationale.

I also don't think it could realistically be made default as it would made upstream name somewhere between very hard to predict and unpredictable on systems or setups where container names are dynamically generated or containers replica dynamically started / stopped.

Hmm, that's a good point. Let me think about this some more to see if there's a good general solution that checks all of these boxes:

• avoids duplicates
• avoids the need to encode a path in the upstream name
• is stable
• is predictable
• is human-readable