Fix multiple ports named service

[aburnett]

If a container exposed multiple ports and a SERVICE_NAME was specified the backend would end up with both exposed ports under the same service.

docker run -p 443 -p 80 -e SERVICE_NAME=webserver nginx

would result in

/webserver/1233456:node01:80
/webserver/1233456:node01:443

With this patch we now get

/webserver-80/1233456:node01:80
/webserver-443/1233456:node01:443

[cgswong]

Hmmm. I actually prefer the current behavior since I can get to my service via the a single directory, then get the specific ports via keys and values.

[progrium]

@cgswong but they're different ports and therefore different services. esp when working with clients that use discovery to connect, you don't want to get a list of addresses/ports that speak different protocols.

@aburnett am i correct in the workaround to this being to specify specific service names for each port? SERVICE_80_NAME and SERVICE_443_NAME?

[cgswong]

By default in registrator:master for etcd each directory gets named as /services/[service]-[port] for each exposed port unless there's only one exposed port in which case it is named /services/[service]. For example, my Kibana service exposes port 5601 and is named /services/kibana whereas my Elasticsearch service exposes two ports so it gets two directories, i.e. /services/elasticsearch-9200 and /services/elasticsearch-9300.

If I want to call my cluster service on port 9300 something specific I can do -e SERVICE_9300_NAME=es-9300 which results in /services/es-9300. If I want to have a single directory for all my Elasticsearch services (which I prefer) I do -e SERVICE_NAME=elasticsearch which results in /services/elasticsearch. I can still see all my ports and IP under this directory as the keys are still in the form [hostname]:[container_name]:[port] with the values as [ip]:[port], it just centralizes things for me using the single directory entry.

[progrium]

You're saying that the default behavior using SERVICE_NAME is different than not specifying SERVICE_NAME, and I don't think that should be the case. Not only because of the reason I mentioned before, but just for predictability and consistency.

I think if you really want to centralize them, you should go out of your way to name them the same, eg SERVICE_9300_NAME=elasticsearch and SERVICE_9200_NAME=elasticsearch. The default behavior should be more "correct" and mixing protocols is not correct.

[aburnett]

Correct, the different behavior between the two scenarios was what I found confusing and prompted the patch.

[progrium]

So this is the right thing, but it will break expectations for some people. Can you add an entry to the CHANGELOG file? Also if you can update the relevant docs if necessary that would be super awesome.

[aburnett]

Updated.

[progrium]

Thanks!

[ptklechat]

The command below:

docker run -d --name nginx.0 -p 4443:443 -p 8000:80 -e "SERVICE_443_NAME=https" -e "SERVICE_80_NAME=http" progrium/nginx

should register the services "http" and "https" in Consul (according to the doc). Instead (since this commit in master I suppose), i see the services with names "http-80" and "https-443". Looks like -<PORT> is always appended in case of multiple services...

[JohnyDays]

I have experienced the same behaviour as @ptklechat

[aburnett]

Seems worth a revert for now perhaps?

[progrium]

I would much prefer to take a fix. Even if it is a PR to revert.

[aburnett]

@JohnyDays I don't think so, SERVICE_NAME and SERVICE_80_NAME will both cause the metadata to contain 'name' I believe.

[JohnyDays]

In this scope it is already only the port's metadata metadata := serviceMetaData(container.Config, port.ExposedPort) right?

[JohnyDays]

Tried it out in my fork, it did the job. The full change was if isgroup && metadata["name"] == ""

[progrium]

If you can submit a PR, please ask others to try it first to get some confirmations. Thanks for looking into it.

[aburnett]

@JohnyDays I tried that, but that gets us back to square one where only specifying a SERVICE_NAME creates a single service containing both ports.

[JohnyDays]

It didn't do that for me, but the fork i tried it on is a bit different (not much though)

[JohnyDays]

Can you checkout the changes I just made in #245?

[gtmtech]

This has disappeared into a hole.. What do we have to do to get it moving again? Frustrated that I need one of the features from master which isnt in v6, but I cant take it because of this which completely wrecks the ability for me to control my service names. There's been a few PRs about this, what need to happen for this to be either reverted, or sorted so that we still have the option to name services what we want to? Happy to help out, what do you need @progrium ?