lucaslorentz / caddy-docker-proxy

Caddy as a reverse proxy for Docker
MIT License
2.83k stars 168 forks source link

[Bug] New servers are sent to updateServer in loader.go when old server addresses should be #238

Closed regbo closed 9 months ago

regbo commented 3 years ago

For whatever reason, running this in docker swarm will occasionally result in an IP address being updated. If that happens after the admin address is assigned to localhost, the configs will never be loaded.

In loader.go, controlledServers are defined by the new caddyfile, which means that the file will be posted to new admin addresses that may not exist yet. I believe that's what's happening here:

https://github.com/lucaslorentz/caddy-docker-proxy/issues/205

To remedy this I created a PR that stores the previous controlled servers in the DockerLoader struct:

https://github.com/lucaslorentz/caddy-docker-proxy/pull/237

I'm not really a go programmer, so feel free to refactor if it doesn't work for you.

regbo commented 3 years ago

FWIW here is a Dockerfile that fixes the issue

ARG CADDY_VERSION=2.3.0
FROM caddy:builder AS builder

RUN wget https://github.com/lucaslorentz/caddy-docker-proxy/archive/refs/heads/master.zip -O caddy-docker-proxy.zip \
    && unzip caddy-docker-proxy.zip -d . \
    && rm caddy-docker-proxy.zip \
    && mv caddy-docker-proxy* caddy-docker-proxy

RUN wget https://raw.githubusercontent.com/regbo/caddy-docker-proxy/master/plugin/loader.go -O loader.go \
    && mv loader.go caddy-docker-proxy/plugin

RUN xcaddy build \
    --with github.com/lucaslorentz/caddy-docker-proxy/plugin/v2=./caddy-docker-proxy/plugin

FROM caddy:alpine

COPY --from=builder /usr/bin/caddy /usr/bin/caddy

CMD ["caddy", "docker-proxy"]
regbo commented 3 years ago

Turns out I know nothing about golang. I am not sure if my code was saving the previous servers correctly, so I ended up using the following logic:

1 attempt to load to all servers 2 record success/failure count 3 if failure > 0 try localhost

It's a hack, but it's the only solution I could come up with without understanding golang better.

    var errorCounter uint64
    var wg sync.WaitGroup
    for _, server := range controlledServers {
        wg.Add(1)
        go func() {
            if (!(dockerLoader.updateServer(server))) {
                atomic.AddUint64(&errorCounter, 1)
            }
            wg.Done()
        }()
    }
    wg.Wait()
    if errorCounter > 0 {
        server := "localhost"
        log.Printf("[INFO] Retrying after failure on %v", server)
        dockerLoader.updateServer(server)
    }
    return true
}

func (dockerLoader *DockerLoader) updateServer(server string) bool {

    // Skip servers that are being updated already
    if dockerLoader.serversUpdating.Get(server) {
        return true
    }

    // Flag and unflag updating
    dockerLoader.serversUpdating.Set(server, true)
    defer dockerLoader.serversUpdating.Delete(server)

    version := dockerLoader.lastVersion

    // Skip servers that already have this version
    if dockerLoader.serversVersions.Get(server) >= version {
        return true
    }

    log.Printf("[INFO] Sending configuration to %v", server)

    url := "http://" + server + ":2019/load"

    postBody, err := addAdminListen(dockerLoader.lastJSONConfig, "tcp/"+server+":2019")
    if err != nil {
        log.Printf("[ERROR] Failed to add admin listen to %v: %s", server, err)
        return false
    }

    req, err := http.NewRequest("POST", url, bytes.NewBuffer(postBody))
    if err != nil {
        log.Printf("[ERROR] Failed to create request to %v: %s", server, err)
        return false
    }
    req.Header.Set("Content-Type", "application/json")
    resp, err := http.DefaultClient.Do(req)

    if err != nil {
        log.Printf("[ERROR] Failed to send configuration to %v: %s", server, err)
        return false
    }

    bodyBytes, err := ioutil.ReadAll(resp.Body)
    if err != nil {
        log.Printf("[ERROR] Failed to read response from %v: %s", server, err)
        return false
    }

    if resp.StatusCode != 200 {
        log.Printf("[ERROR] Error response from %v: %v - %s", server, resp.StatusCode, bodyBytes)
        return false
    }

    dockerLoader.serversVersions.Set(server, version)

    log.Printf("[INFO] Successfully configured %v", server)
    return true
}
lucaslorentz commented 3 years ago

@regbo Can you help me with some steps to reproduce the issue? I would like to understand it better.

regbo commented 3 years ago

Can you help me with some steps to reproduce the issue? I would like to understand it better.

It is hard to reproduce because it happens sporadically in DockerSwarm. Basically when your code tries to send an update to caddy it will sometimes reference old admin urls, when they should be sent to new urls. Does that make sense?

lucaslorentz commented 9 months ago

I didn't get other reports about this issue. And I couldn't reproduce it as well. If this still happens, let's open a fresh issue.