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

Wrong deduplication for service and connect dependencies #1458

Closed
danieleva opened this issue Mar 9, 2021 · 1 comment · Fixed by #1474
Closed

Wrong deduplication for service and connect dependencies #1458

danieleva opened this issue Mar 9, 2021 · 1 comment · Fixed by #1474
Labels
Milestone

Comments

@danieleva
Copy link
Contributor

dependency.HealthService.String() returns the same value for service and connect invocation, and that breaks templates when both functions are called with the same parameter.
In my case I have an external service registered in the catalog, and a terminating-gateway configured for it. I want to get IP:Port from connect, and some metadata from service, and that doesn't work

Consul Template version

consul-template v0.25.1

Configuration

consul-template -dry -once -template ./sample.tpl

This template is not very useful but it's perfect to pinpoint the bug

{{ range service "myservice" }} {{ .Name}} {{end}}
{{ range connect "myservice" }} {{ .Name}} {{end}}
{{ printf "%#v" (service "myservice") }}
{{ printf "%#v" (connect "myservice") }}

"myservice" is registered directly into consul catalog, and a terminating-gateway is assigned to it via config entry

Command

consul-template -dry -once -template ./sample.tpl

Expected behavior

I would expect the output to be:

[]*dependency.HealthService{(*dependency.HealthService)(0xc000001234)}
[]*dependency.HealthService{(*dependency.HealthService)(0xc000005678)}
 myservice
 terminating-gateway

Actual behavior

What actually happened?

[]*dependency.HealthService{(*dependency.HealthService)(0xc000001234)}
[]*dependency.HealthService{(*dependency.HealthService)(0xc000001234)}
 myservice
 myservice

Steps to reproduce

Any template that uses both functions will trigger it.

@eikenb eikenb added the bug label Mar 9, 2021
@eikenb
Copy link
Contributor

eikenb commented Mar 9, 2021

Thanks for the report @danieleva and much thanks for the simple case to reproduce, that always makes things way easier on my end which I appreciate.

Looks like like both connect and service look-ups are using the same keys for the cache/value-store. It should be a straightforward fix and will be in the next release, which should be soon as the latest release had an annoying regression in the shell handling.

@eikenb eikenb added this to the 0.25.3 milestone Mar 9, 2021
@eikenb eikenb assigned eikenb and unassigned eikenb Mar 9, 2021
@eikenb eikenb modified the milestones: 0.25.3, 0.26.0 Apr 27, 2021
eikenb added a commit that referenced this issue May 27, 2021
The IDs need to be different as they are used as the cache keys and
using the same ID means `connect` and `service` would overwrite each
other if both used. (this is bad)

The `connect` function re-uses the health `service` code as it is
identical except for an additional option. When adding connect the ID
was unintentially left the same, it should be different.

Fixes #1458
eikenb added a commit that referenced this issue May 27, 2021
The IDs need to be different as they are used as the cache keys and
using the same ID means `connect` and `service` would overwrite each
other if both used. (this is bad)

The `connect` function re-uses the health `service` code as it is
identical except for an additional option. When adding connect the ID
was unintentially left the same, it should be different.

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

Successfully merging a pull request may close this issue.

2 participants