caddy-docker-proxy icon indicating copy to clipboard operation
caddy-docker-proxy copied to clipboard

Caddyfile changes with every polling interval

Open magnetic-domelike-twisting opened this issue 10 months ago • 15 comments

For the past couple months that I've been using caddy-docker-proxy it's been fantastic, this is a great project. I'd notice certain things, particularly applications utilizing websockets had issues, seemingly random disconnects. I finally put some time into troubleshooting this and discovered that every 30-seconds caddy-docker-proxy was reloading the config with a new Caddyfile. This corresponded to the default polling interval for checks in changes of the Caddyfile. When checking the different Caddyfiles from one 30-second interval to the next I noticed this.

Caddyfile A

test.example.com {
	import common_config
	reverse_proxy 192.168.131.2:80 192.168.130.2:80
}

Caddyfile B (30-seconds later)

test.example.com {
	import common_config
	reverse_proxy 192.168.130.2:80 192.168.131.2:80
}

The reverse_proxy directive contains the same upstream targets, they're just sorted differently. Seems to only happen on my containers that are attached to more than one docker network. This looks like it's enough of a change that caddy-docker-file would consider it to be a new Caddyfile and reload the server, and I believe that's what was breaking my websocket applications. Anyway, Claude 3.7 helped me out and I believe this can be fixed by sorting the targets in go.cmd before they get put into the generated Caddyfile. I forked the repo, implemented a quick sort of the targets, rebuild caddy with my fork, and I haven't seen a single unexpected reload since.

Not sure if this is worth integrating, but I figured it'd be worth posting here. The fix is to just sort the targets in labels.go before they get transformed, not sure what else this might impact, but it appears to have solved this for me. I just imported sort and sorted the targets on line 18 before they get transformed.

labels.go

package generator

import (
        "strconv"
        "strings"
        "text/template"
        "sort"

        "github.com/lucaslorentz/caddy-docker-proxy/v2/caddyfile"
)

type targetsProvider func() ([]string, error)

func labelsToCaddyfile(labels map[string]string, templateData interface{}, getTargets targetsProvider) (*caddyfile.Container, error) {
        funcMap := template.FuncMap{
                "upstreams": func(options ...interface{}) (string, error) {
                        targets, err := getTargets()
                        sort.Strings(targets)
                        transformed := []string{}
                        for _, target := range targets {
                                for _, param := range options {
                                        if protocol, isProtocol := param.(string); isProtocol {
                                                target = protocol + "://" + target
                                        } else if port, isPort := param.(int); isPort {
                                                target = target + ":" + strconv.Itoa(port)
                                        }   
                                }   
                                transformed = append(transformed, target)
                        }   
                        sort.Strings(transformed)
                        return strings.Join(transformed, " "), err 
                },  
                "http": func() string {
                        return "http"
                },  
                "https": func() string {
                        return "https"
                },  
                "h2c": func() string {
                        return "h2c"
                },  
        }   

        return caddyfile.FromLabels(labels, templateData, funcMap)
}

I'm hitting this as well. Are there plans for an upstream fix for this? Would a PR that implements the fix from @magnetic-domelike-twisting be accepted @lucaslorentz ?

torarnv avatar Mar 16 '25 12:03 torarnv

The upstreams should probably not be sorted alphabetically though, but be based on the order of the ingress networks. So if you've set CADDY_INGRESS_NETWORKS explicitly to foo,bar that should be reflected in the upstreams list? And if not, the default (all networks) order is based on the eth0,eth1 etc network order (which is AFAIK based on internal/external + the alphabetical name of the docker network).

torarnv avatar Mar 16 '25 14:03 torarnv

I want to try out caddy docker proxy but this is definitely a blocker for me.

NeurekaSoftware avatar Apr 10 '25 16:04 NeurekaSoftware

@NeurekaSoftware it's probably only an issue if you rely on {{ upstreams }} to add the container IP. Instead you should be able to just rely on the container DNS names, and avoid this concern entirely.

services:
  hello-world:
    container_name: whoami
    hostname: whoami.example.com
    networks:
      default:
        aliases:
          - hello.example.net
      backend:
        aliases:
          - world

You should be able to specify hello-world, whoami, whoami.example.com as the DNS name with say curl from a container on either of the networks connected to the hello-world service and it should connect just fine. Whereas the aliases are only reachable via the specific networks shared between the two containers.

The networks basically become the TLD, so whoami would also be resolvable as whoami.backend (I think it'll actually be a generated network name based on the compose project, unless you specify an explicit network name when configuring the network top-level for backend).

These are all handled by Docker embedded DNS, the default dns per container if you haven't changed that setting. That runs at 127.0.0.11:53 for each container.

So try to use any of those instead of {{ upstreams }} in the CDP label, and it should get routed correctly. In the Caddyfile it should also just be that DNS name instead of the IP(s). Hence avoiding this issue 👍

polarathene avatar Apr 10 '25 23:04 polarathene

@polarathene Awesome, thank you! Looks like I get to dive in tonight. :)

NeurekaSoftware avatar Apr 11 '25 00:04 NeurekaSoftware

The workaround of manually specified upstreams works, but the convenience of {{ upstreams }} is very nice, so it would be great if it did the "right thing", by providing a stable ordering of upstreams that matches the order of the ingress networks that Docker uses natively.

torarnv avatar Apr 11 '25 10:04 torarnv

but the convenience of {{ upstreams }} is very nice

I don't see much of a convenience beyond copy/paste vs typing the individual service name?

polarathene avatar Apr 11 '25 22:04 polarathene

The whole point of {{ upstreams }} is that it's dynamic based on the amount of replicas that are running in the Docker stack.

francislavoie avatar Apr 11 '25 22:04 francislavoie

Aren't the replicas all under the same DNS names too though, just load balanced? (assuming this is Docker Swarm specific)

Is there really a difference / benefit there with the IP addresses vs single DNS lookup (internal DNS via 127.0.0.11:53)?

polarathene avatar Apr 11 '25 23:04 polarathene

Lets you do active health checking with Caddy if they're separate upstreams instead of DNS round-robin.

francislavoie avatar Apr 11 '25 23:04 francislavoie

TL;DR:

If caddy.reverse_proxy: whoami is not convenient and you don't particularly care about the individual IP, just that upstreams is deterministic to avoid this issue, consider the alternative below (Docker Compose only) to infer the service name whoami:

services:
  whoami:
    image: traefik/whoami
    container_name: example
    labels:
      caddy: whoami.example.com
      caddy.reverse_proxy: '{{ index .Labels "com.docker.compose.service" }}'

NOTE: For an alternative that isn't specific to Docker Compose, you can instead use: {{ slice (index .Names 0) 1 }}. That will get the container name and remove the first character (assuming the equivalent API also returns the container name with / prepended). Thus in this case instead of whoami, you would get /example => example, and that should also resolve to the container IP address(es).

Understandably both look more awkward and verbose compared to the upstreams template function CDP provides. CDP could add something similar as an alias 🤷‍♂


Original response

Swarm is handled separately it seems via services.go, but for containers.go the upstreams template function loops over containers networks and gets the IP:

https://github.com/lucaslorentz/caddy-docker-proxy/blob/a955641355b4df8c5acb03b90e14704beb348072/generator/containers.go#L22

https://github.com/lucaslorentz/caddy-docker-proxy/blob/a955641355b4df8c5acb03b90e14704beb348072/generator/containers.go#L34

This is grabbing that from the standard container config you'd get via docker inspect container-name-here. The IP from NetworkSettings.Networks[network-name-here].IpAddress that network also has .Aliases and .DNSNames available for values I showed above.

Instead of NetworkSettings, if instead accessing the Config object it has Hostname and the more interesting com.docker.compose.service label under Labels (Docker Compose specific of course, so less flexible with other OCI containers that may lack this label annotation) which provides the service name.

Another top-level field in that container metadata is Name, the container name. That should be more standard outside of Docker Compose. I'm not sure how common it is for that to always have a DNS entry, but I assume Podman does the same at least. For some reason this prepends / to the value? 🤷‍♂


So instead of just upstreams you could have another template function to grab the container name, which should work if not using Docker Swarm (or I guess that could still work, you'd just get a list of container names that each resolve to one or more IP).

EDIT: Looks like that's already supported? (although the example documented there implies at some point there was not / prefix to this value)

  • Swarm: {{ .Spec.Name }}
  • Container: {{ index .Names 0 }} (appears to refer to this for .Names, from the containers entry in /containers/json/ API endpoint response)

I don't use Swarm, but assume the service name is roughly equivalent.

# The JSON returned to CDP appears to be the container specific entry from this endpoint:
curl --unix-socket /var/run/docker.sock localhost/containers/json | jq .[].Names
[ "/cdp" ]

# Alternatively specific container query, the field name is slightly different:
curl --unix-socket /var/run/docker.sock localhost/containers/${CONTAINER_NAME}/json | jq .Name
"/cdp"

A new template function could probably abstract those two and remove the / prefix? 🤔

In the meantime this works:

services:
  whoami:
    image: traefik/whoami
    labels:
      caddy: whoami.example.com
      caddy.reverse_proxy: '{{ index .Labels "com.docker.compose.service" }}'

That will get the service name when using Docker Compose, so in this case it's whoami. A little bit longer than {{ upstreams }} but it does provide a generic way to get a fixed DNS name that'll resolve to the container IP, without having to use caddy.reverse_proxy: whoami if that's somehow inconvenient 😅


Lets you do active health checking with Caddy if they're separate upstreams instead of DNS round-robin.

Uhh alright, but for those without multiple container instances, that don't use Swarm (which from what I've seen, typically reports aren't Docker Swarm oriented), that concern isn't really relevant?

This issue for example just cites a container with multiple networks, as opposed to multiple containers for a service on the same network.

polarathene avatar Apr 12 '25 00:04 polarathene

Would a PR that implements the fix from @magnetic-domelike-twisting be accepted @lucaslorentz ?

Sure, do a PR that sorts the upstreams IPs/names and I will review it.

lucaslorentz avatar Apr 15 '25 07:04 lucaslorentz

@magnetic-domelike-twisting did you get back to this?

Rumpick avatar Aug 03 '25 04:08 Rumpick

I ended up going to just a single network for ingress traffic and specifying CADDY_INGRESS_NETWORKS. So I solved sorting and having the order change by only having one IP address in there.

Not sure it'll fulfill the needs, but I put out https://github.com/lucaslorentz/caddy-docker-proxy/pull/737 for this. It's not elegant but it should at least keep the sorting order consistent. I'm not terribly invested in getting it merged, so if it's not worth it, not going to hurt me. But at least it's out there