Fix Traefik reload if Consul Catalog tags change #2389
Conversation
mmatur
added
kind/bug/fix
mmatur
force-pushed
the
fix/consul-tag-change
branch
2 times, most recently
from
f04fea9
to
abe164d
Compare
provider/consul/consul_catalog.go
Outdated
| if len(removedServiceKeys) > 0 || len(addedServiceKeys) > 0 { | ||
| return true | ||
| } | ||
| return hasNodeOrTagsChanged(current, previous) |
There was a problem hiding this comment.
Can be merge in return len(removedServiceKeys) > 0 || len(addedServiceKeys) > 0 || hasNodeOrTagsChanged(current, previous)?
provider/consul/consul_catalog.go
Outdated
|
|
||
| if len(removedServiceKeys) > 0 || len(removedServiceNodeKeys) > 0 || len(addedServiceKeys) > 0 || len(addedServiceNodeKeys) > 0 { | ||
| log.WithField("MissingServices", removedServiceKeys).WithField("DiscoveredServices", addedServiceKeys).Debug("Catalog Services change detected.") | ||
| if changed { |
There was a problem hiding this comment.
Maybe use directly the method in the if statement?
if hasChanged(current, flashback) {
mmatur
force-pushed
the
fix/consul-tag-change
branch
from
abe164d
to
a851063
Compare
|
awesome @mmatur, thanks for looking at this so quickly! I'm afraid we run Traefik straight on metal and don't have a Consul infra in our containers environment. Any chance you have a built binary for me to test? (I guess I could just extract it from your image, that's probably easier) EDIT: Hm I guess its a minimal image without a shell in it. Not sure how I could extract the binary then.... |
|
@trondhindenes you can easily extract binary from the container docker run -tid --name=traefik-experimental mmatur/traefik:fix-consul-tag-change
docker cp traefik-experimental:/traefik . |
|
thanks! |
|
Hm, the web ui is giving me a 404 with this build. Is that intended? It redirects from root to |
|
Looking at live traefik logs while doing a deploy now I notice that Traefik is still not reloading its config when the service is flagged as "in maintenance using the Got me thinking, maybe this is the root cause of #2370? That Traefik doesn't watch for maintenance mode changes? |
|
For the webui it is because I have not build the webui when I have build my docker image. |
|
I'm also still seeing the behavior of Traefik not responding to changes in the service in general. I just tried rebooting a backend. Traefik updated its config when the node left, but not when it came back online (consul web ui shows that the consul db is up to date, and running |
|
"Traefik does not watch the maintenance mode" - okay that explains a good deal. That should probably go into documentation in a very clear way, as it's a core feature of Consul. |
|
Which version do you want the debug log from? The one from this custom build? Can do. But in general it doesn't say much, you can just see that it does not reload configuration so it just sits there. But I'm happy to send it over if you confirm the version. |
|
@trondhindenes Yes the log from the custom build. To have more verbose logs please add |
traefiker
added
status/4-merge-in-progress
bot/need-human-merge
and removed
status/4-merge-in-progress
labels
mmatur
force-pushed
the
fix/consul-tag-change
branch
from
4fe57d3
to
a8430e2
Compare
mmatur
force-pushed
the
fix/consul-tag-change
branch
from
a8430e2
to
4965903
Compare
|
Sure, will do - I'll send those over tomorrow morning. I saw this getting merge and the related issue was closed but just FYI: I'm still having issues with Consul not reloading config on changes. Should we track this in another issue? Tracking stuff in a merged PR seems a bit sub-optimal. |
|
@trondhindenes Could you please create a new issue with last configuration and traefik debug log |
|
Ah well I am still seeing this issue in 1.5.4 release as well. |
|
Hi @zaargy, Could you have a try with last Træfik version v1.6.3. |
What does this PR do?
This PR fix Træfik's reload if Consul Catalog tags change
More
Fixes #2370
@trondhindenes Could you please try with my experimental image
mmatur/traefik:fix-consul-tag-change