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

consul: all services being re-registered all the time #14914

Closed
shoenig opened this issue Oct 17, 2022 · 1 comment · Fixed by #14917
Closed

consul: all services being re-registered all the time #14914

shoenig opened this issue Oct 17, 2022 · 1 comment · Fixed by #14917

Comments

@shoenig
Copy link
Member

shoenig commented Oct 17, 2022

While looking into #3935, it looks like we are improperly re-registering every service on every iteration of the reconciliation loop. This is quite inefficient, we should only re-register a service if the service definition has changed on the Consul side from what the Nomad job definition declares.

E.g.

GET_SERVICES namespace=default why=sync
got services keys=[_nomad-server-o5s5vxlab3cgpyi3x2i5cm24xa2r2hbz _nomad-server-tl5dgauqxnxsq7pgcvloyq7d6gf75mj7 _nomad-client-rxajoy6bie7cqxttksivmlv2ucz7qxfl _nomad-server-icsfum2knxzafc7jtsv5hmelx35hkufm]
different H
SERVICE_REGISTER id=_nomad-server-o5s5vxlab3cgpyi3x2i5cm24xa2r2hbz name=nomad why=missing exists=true
different H
SERVICE_REGISTER id=_nomad-server-icsfum2knxzafc7jtsv5hmelx35hkufm name=nomad why=missing exists=true
different H
SERVICE_REGISTER id=_nomad-client-rxajoy6bie7cqxttksivmlv2ucz7qxfl name=nomad-client why=missing exists=true
different H
SERVICE_REGISTER id=_nomad-server-tl5dgauqxnxsq7pgcvloyq7d6gf75mj7 name=nomad why=missing exists=true
GET_CHECKS namespace=default
GET_SERVICES namespace=default why=sync
got services keys=[_nomad-server-tl5dgauqxnxsq7pgcvloyq7d6gf75mj7 _nomad-client-rxajoy6bie7cqxttksivmlv2ucz7qxfl _nomad-server-icsfum2knxzafc7jtsv5hmelx35hkufm _nomad-server-o5s5vxlab3cgpyi3x2i5cm24xa2r2hbz]
different H
SERVICE_REGISTER id=_nomad-server-tl5dgauqxnxsq7pgcvloyq7d6gf75mj7 name=nomad why=missing exists=true
different H
SERVICE_REGISTER id=_nomad-server-o5s5vxlab3cgpyi3x2i5cm24xa2r2hbz name=nomad why=missing exists=true
different H
SERVICE_REGISTER id=_nomad-server-icsfum2knxzafc7jtsv5hmelx35hkufm name=nomad why=missing exists=true
different H
SERVICE_REGISTER id=_nomad-client-rxajoy6bie7cqxttksivmlv2ucz7qxfl name=nomad-client why=missing exists=true

The cause of course being the mis-use of reflect.DeepEqual to compare slices of strings.

case !reflect.DeepEqual(wanted.TaggedAddresses, existing.TaggedAddresses):
    netlog.Cyan("different H")
    return true

edit: on closer look, it's actually also because of the default tagged addresses being set by Consul on our agent registrations

LOOP id=_nomad-server-tl5dgauqxnxsq7pgcvloyq7d6gf75mj7 service=nomad
DIFFERENT
H wanted=map[] existing=map[lan_ipv4:{127.0.0.1 4647} wan_ipv4:{127.0.0.1 4647}]
LOOP id=_nomad-server-o5s5vxlab3cgpyi3x2i5cm24xa2r2hbz service=nomad
DIFFERENT
H wanted=map[] existing=map[lan_ipv4:{127.0.0.1 4648} wan_ipv4:{127.0.0.1 4648}]
LOOP id=_nomad-server-icsfum2knxzafc7jtsv5hmelx35hkufm service=nomad
DIFFERENT
H wanted=map[] existing=map[lan_ipv4:{127.0.0.1 4646} wan_ipv4:{127.0.0.1 4646}]
LOOP id=_nomad-client-rxajoy6bie7cqxttksivmlv2ucz7qxfl service=nomad-client
DIFFERENT
H wanted=map[] existing=map[lan_ipv4:{127.0.0.1 4646} wan_ipv4:{127.0.0.1 4646}]
LOOP id=_nomad-task-0a45376d-cfd3-54b8-d801-99d5f93e4bd7-group-group-s1-8080 service=s1
DIFFERENT
SAME
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant