Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort nodes before creating consul catalog config #478

Merged
merged 1 commit into from
Jun 23, 2016

Conversation

keis
Copy link

@keis keis commented Jun 22, 2016

I noticed traefik was reload constantly from config changes when nothing had actually changed and while I got to the point where the config produced is the same it is still reloading. Any ideas?

The watch of consul can return for various reasons and not of all of
them require a reload of the config. The order of nodes provided by
consul is not stable so to ensure a identical config is generated for an
identical server set the nodes needs to be sorted before creating the
config.

@emilevauge
Copy link
Member

emilevauge commented Jun 22, 2016

@keis: great 👍
Tests are failing ;)
I'm going to publish the rc3 tomorrow, that would be interesting to also get this one.

@keis keis force-pushed the consul-stable-ordering-of-nodes branch 2 times, most recently from e3cf89d to 8e0f255 Compare June 23, 2016 08:59
@keis
Copy link
Author

keis commented Jun 23, 2016

oops 😓 tests should work now

I'm still trying figure out why the deep equal check is not working, I tried writing the json blobs to disk and they are the exact same to the checksum. I saw you where changing some other stuff in the area so maybe I'll wait for that and see if that changes anything.

@vdemeester
Copy link
Contributor

hum this is weird, it's only failing on docker 1.10 😓
@keis you need to rebase too 👼 😅

The watch of consul can return for various reasons and not of all of
them require a reload of the config. The order of nodes provided by
consul is not stable so to ensure a identical config is generated for an
identical server set the nodes needs to be sorted before creating the
config.
@keis keis force-pushed the consul-stable-ordering-of-nodes branch from 8e0f255 to 7030526 Compare June 23, 2016 11:08
@keis
Copy link
Author

keis commented Jun 23, 2016

Some random timing issue with the integration tests I think.

@emilevauge
Copy link
Member

emilevauge commented Jun 23, 2016

@keis :

I'm still trying figure out why the deep equal check is not working, I tried writing the json blobs to disk and they are the exact same to the checksum. I saw you where changing some other stuff in the area so maybe I'll wait for that and see if that changes anything.

I made a fix on that. The deepEquals is made earlier, and the entrypoints from defaultEntryPoints are added to the configuration BEFORE the deepEquals now.

You can test the PR using image containous/traefik:pr-477 if you want.

@emilevauge
Copy link
Member

LGTM

@vdemeester
Copy link
Contributor

LGTM 🐯

@vdemeester vdemeester merged commit 2a209c2 into traefik:master Jun 23, 2016
@keis
Copy link
Author

keis commented Jun 23, 2016

Hmm. that did still not fix it :'(

but writing even more json to the disk I found that the "old" config also have loadBalancer blocks in the backend config where the new from the provider does not

        "backend-eremetic": {
            "servers": {
                "eremetic--10-4-72-161--8182--monitor--api--monitor--6": {
                    "url": "http://10.4.72.161:8182",
                    "weight": 0
                }
            },
            "loadBalancer": {
                "method": "wrr"
            }
        },
        "backend-eremetic": {
            "servers": {
                "eremetic--10-4-72-161--8182--monitor--api--monitor--6": {
                    "url": "http://10.4.72.161:8182",
                    "weight": 0
                }
            }
        },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants