consul connect proxy -register-id is not used during registration

kneufeld commented 5 years ago

I've starting testing consul connect and noticed that when using -register the catalog did not match what I expected. After reading the docs and realizing that -proxy is appended to the -service name one mystery was solved. But then I added -register-id to the command line and the catalog was very wrong and didn't match consul output.

Side note, IMHO appending -proxy to service name is non-intuitive and makes the service name wrong in a everything-is-consul-connected world. Also, the outbound proxy connects to the service name (which may not exist) and instead connects to service-proxy which is not what you said on the command line. Way too much magic there.

Reproduction Steps

consul connect proxy  -service redis --service-addr 127.0.0.1:6379 -listen :6377 -register -register-id inbound
...
2018/12/26 12:16:15 [INFO] proxy: registered Consul service: redis-proxy-inbound
...

consul catalog services | grep redis
redis-proxy
# should be redis-proxy-inbound according to previous output

Consul info for both Client and Server

Consul v1.4.0

agent:
    check_monitors = 0
    check_ttls = 1
    checks = 1
    services = 1
build:
    prerelease =
    revision = 0bddfa2+
    version = 1.4.0
consul:
    acl = disabled
    known_servers = 1
    server = false
runtime:
    arch = amd64
    cpu_count = 4
    goroutines = 78
    max_procs = 4
    os = darwin
    version = go1.11.2
serf_lan:
    coordinate_resets = 0
    encrypted = false
    event_queue = 0
    event_time = 16
    failed = 0
    health_score = 0
    intent_queue = 0
    left = 0
    member_time = 58
    members = 5
    query_queue = 0
    query_time = 1

Operating system and Environment details

Running on OSX

pearkes commented 5 years ago

Good feedback here. I think the -register-id type parameters are not favored vs. the consul service register command as noted in the documentation. We could potentially make that clearer.

Part of the confusion here is I think it is often easy to mix up ID and name for a service. In the case of the catalog command, it returns names, but the flag there allows adding a suffix only to the ID. The code makes the confusing part pretty clear.

https://github.com/hashicorp/consul/blob/249babc2447c4b5851cfb1fbe716b397139806b1/command/connect/proxy/register.go#L283-L296

If you query the Consul API directly with your example you can also see the issue.

$ curl -s localhost:8500/v1/catalog/service/redis-proxy?pretty | grep proxy
        "ServiceKind": "connect-proxy",
        "ServiceID": "redis-proxy-inbound",
        "ServiceName": "redis-proxy",

Going to leave this open as something we could fix in docs I think by stating this will not effect the suffix used for the name registered.

kneufeld commented 5 years ago

Ah, I see what you mean name vs id. Yeah, id is used when there are multiple proxy right?

I made a blog post to help future me with all this, maybe it'll be inspirational to whomever updates the docs?

https://www.burgundywall.com/post/consul-connect

For the record I'm okay with you closing this bug.

hashicorp / consul