A service config with upstream limits can cause unnecessary catalog.register calls
- Dominant language
- Go
- Stars
- 30.1k
- Forks
- 4.6k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 43
Description
#### Overview of the Issue
The problem manifests as unexpected calls to the `catalog.register` RPC, and client agent logs that say "agent: Synced service".
When a service or sidecar is registered, and the registration contains values for `Proxy.Upstreams.Config` , those values may not be encoded and decoded properly by `go-msgpack`. `Proxy.Upstreams.Config` is "opaque configuration", Go sees it as a `map[string]interface{}`, so there's no type information available for any of the values under `Config`.
The client agent performs a full anti-entropy sync every few minutes (1-5 minutes is common). As part of this full sync it queries the server for all the catalog data. It then compares all the services in the catalog against the local registrations, and marks any service that is different as "needs sync". The second part of the full sync is to perform a `catalog.register` to sync the local registration to the catalog.
In this case, the local registration is semantically the same as the catalog service (same `ModifyIndex`), but `NodeService.IsSame` thinks they are different because of this line:
https://github.com/hashicorp/consul/blob/b2b84e7fc6b48365e11a54e39ed2d160e73acc05/agent/structs/structs.go#L1411
`reflect.DeepEqual` compare the two. Any difference in types is considered "not equal".
In this case, `msgpack` was converting the value of `Proxy.Upstreams.Config.Limits.Max*` to a `uint64`. The local service definition has type `int`. So comparing those values returned "false", and AE sync will attempt to sync this service to the catalog on every run.
#### Reproduction Steps
Steps to reproduce this issue, eg:
1. Start at least a single client and single server. The registration must be done to a client, the bug does not manifest when registrations are done to a server, because those never go through msgpack.
2. Register a service (to the client) with a `proxy.upstreams.config.limits.max_connections = 1000000000`
3. Watch the logs to see that the service is synced on every AE sync, even though it never changes.
### Consul info for both Client and Server
This bug was first reported in version 1.9.4, but I was able to reproduce with `main`, so I think it exists in all Consul releases since 1.9.x at least.
Contributor guide
Assessment
This issue has not been assessed yet.