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

make task-name as an net-alias to the backing container #24973

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
@mavenugo
Contributor

mavenugo commented Jul 23, 2016

fixes #24466

This will help with resolving DNS names using task-name

Signed-off-by: Madhu Venugopal madhu@docker.com

@mavenugo

This comment has been minimized.

Show comment
Hide comment
@mavenugo

mavenugo Jul 23, 2016

Contributor

ping @tiborvass @thaJeztah . Do you think this is a useful thing to have ? I could not decide it myself if it is useful for 1.12 or nice to have....

Contributor

mavenugo commented Jul 23, 2016

ping @tiborvass @thaJeztah . Do you think this is a useful thing to have ? I could not decide it myself if it is useful for 1.12 or nice to have....

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jul 25, 2016

Member

So, for reference; given a service web;

docker service tasks web

ID                         NAME   SERVICE  IMAGE         LAST STATE                  DESIRED STATE  NODE
9tsk9hw2uac4tsharxkuzf981  web.1  web      nginx:alpine  Running about a minute ago  Running        moby
1opisbjf2ypok3cakoq2gmm4e  web.2  web      nginx:alpine  Running 6 seconds ago       Running        moby
3zqyv3sx6juzmlvxrcpl7adxo  web.3  web      nginx:alpine  Running 6 seconds ago       Running        moby
docker ps

CONTAINER ID        IMAGE               COMMAND                  CREATED              STATUS              PORTS               NAMES
561e2a934d6f        nginx:alpine        "nginx -g 'daemon off"   About a minute ago   Up 59 seconds       80/tcp, 443/tcp     web.2.1opisbjf2ypok3cakoq2gmm4e
fa73bbcbf766        nginx:alpine        "nginx -g 'daemon off"   About a minute ago   Up 59 seconds       80/tcp, 443/tcp     web.3.3zqyv3sx6juzmlvxrcpl7adxo
bec6bc5a579a        nginx:alpine        "nginx -g 'daemon off"   2 minutes ago        Up 2 minutes        80/tcp, 443/tcp     web.1.9tsk9hw2uac4tsharxkuzf981

Individual task-containers can currently be discovered by their full (container) name (e.g.web.1.9tsk9hw2uac4tsharxkuzf981), but not the task name that appears in docker service tasks (e.g. web.1); this change adds the task name web.1 as an alias.

Without this change, I think the only way to find the container that's related to a task is to docker inspect <task-id>, and get the container-id from that output, so I can see this being helpful.

It's related to #24241, however, so the same concern could be raised #24241 (comment), so:

ping @aluzzardi @dnephin @stevvooe PTAL

Member

thaJeztah commented Jul 25, 2016

So, for reference; given a service web;

docker service tasks web

ID                         NAME   SERVICE  IMAGE         LAST STATE                  DESIRED STATE  NODE
9tsk9hw2uac4tsharxkuzf981  web.1  web      nginx:alpine  Running about a minute ago  Running        moby
1opisbjf2ypok3cakoq2gmm4e  web.2  web      nginx:alpine  Running 6 seconds ago       Running        moby
3zqyv3sx6juzmlvxrcpl7adxo  web.3  web      nginx:alpine  Running 6 seconds ago       Running        moby
docker ps

CONTAINER ID        IMAGE               COMMAND                  CREATED              STATUS              PORTS               NAMES
561e2a934d6f        nginx:alpine        "nginx -g 'daemon off"   About a minute ago   Up 59 seconds       80/tcp, 443/tcp     web.2.1opisbjf2ypok3cakoq2gmm4e
fa73bbcbf766        nginx:alpine        "nginx -g 'daemon off"   About a minute ago   Up 59 seconds       80/tcp, 443/tcp     web.3.3zqyv3sx6juzmlvxrcpl7adxo
bec6bc5a579a        nginx:alpine        "nginx -g 'daemon off"   2 minutes ago        Up 2 minutes        80/tcp, 443/tcp     web.1.9tsk9hw2uac4tsharxkuzf981

Individual task-containers can currently be discovered by their full (container) name (e.g.web.1.9tsk9hw2uac4tsharxkuzf981), but not the task name that appears in docker service tasks (e.g. web.1); this change adds the task name web.1 as an alias.

Without this change, I think the only way to find the container that's related to a task is to docker inspect <task-id>, and get the container-id from that output, so I can see this being helpful.

It's related to #24241, however, so the same concern could be raised #24241 (comment), so:

ping @aluzzardi @dnephin @stevvooe PTAL

@thaJeztah thaJeztah added this to the 1.12.0 milestone Jul 25, 2016

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jul 25, 2016

Member

Added to the milestone to make it easier to track, but we can move it or close it depending on what we think is best

Member

thaJeztah commented Jul 25, 2016

Added to the milestone to make it easier to track, but we can move it or close it depending on what we think is best

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jul 25, 2016

Member

Oh, this fixes #24466, so added that to the description; and contains a use-case / more background

Member

thaJeztah commented Jul 25, 2016

Oh, this fixes #24466, so added that to the description; and contains a use-case / more background

@dnephin

This comment has been minimized.

Show comment
Hide comment
@dnephin

dnephin Jul 25, 2016

Member

Providing the task short id as a discoverable alias sounds good to me. In the case where a name is set, could we still add the task.slot as an alias? Isn't the name already discoverable by default?

Member

dnephin commented Jul 25, 2016

Providing the task short id as a discoverable alias sounds good to me. In the case where a name is set, could we still add the task.slot as an alias? Isn't the name already discoverable by default?

make task-name as an net-alias to the backing container
This will help with resolving DNS names using task-name

Signed-off-by: Madhu Venugopal <madhu@docker.com>
@mavenugo

This comment has been minimized.

Show comment
Hide comment
@mavenugo

mavenugo Jul 25, 2016

Contributor

@dnephin @thaJeztah addressed your comments. PTAL.

Contributor

mavenugo commented Jul 25, 2016

@dnephin @thaJeztah addressed your comments. PTAL.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jul 25, 2016

Member

LGTM

/cc-ing @sfsmithcha in case we want to document this somewhere (although I'm not sure where directly)

Member

thaJeztah commented Jul 25, 2016

LGTM

/cc-ing @sfsmithcha in case we want to document this somewhere (although I'm not sure where directly)

// fallback to service.slot.id.
return strings.Join([]string{c.task.ServiceAnnotations.Name, fmt.Sprint(c.task.Slot), c.task.ID}, ".")
// fallback to service.slot.id
return fmt.Sprintf("%s.%s", c.shortName(), c.task.ID)

This comment has been minimized.

@dnephin

dnephin Jul 25, 2016

Member

Sorry, I think my comment was misleading. Wont this use a name of service.slot.taskid ?

@dnephin

dnephin Jul 25, 2016

Member

Sorry, I think my comment was misleading. Wont this use a name of service.slot.taskid ?

This comment has been minimized.

@dnephin

dnephin Jul 25, 2016

Member

I was thinking that every task would get the following aliases:

  • container id
  • task name (service.slot)

(as well as the service name which is load balanced)

@dnephin

dnephin Jul 25, 2016

Member

I was thinking that every task would get the following aliases:

  • container id
  • task name (service.slot)

(as well as the service name which is load balanced)

This comment has been minimized.

@mavenugo

mavenugo Jul 25, 2016

Contributor

@dnephin you are looking at name() which is infact service.slot.taskid. whereas the shortName is service.slot which is the task-name and it is used as an net-alias.

In short, yes a container will have container-id and task-name as net-aliases. Ofcourse service-name is used for load-balancing.

@mavenugo

mavenugo Jul 25, 2016

Contributor

@dnephin you are looking at name() which is infact service.slot.taskid. whereas the shortName is service.slot which is the task-name and it is used as an net-alias.

In short, yes a container will have container-id and task-name as net-aliases. Ofcourse service-name is used for load-balancing.

This comment has been minimized.

@dnephin

dnephin Jul 25, 2016

Member

Thanks, makes sense

@dnephin

dnephin Jul 25, 2016

Member

Thanks, makes sense

}
// fallback to service.slot
return fmt.Sprintf("%s.%s", c.task.ServiceAnnotations.Name, fmt.Sprint(c.task.Slot))

This comment has been minimized.

@stevvooe

stevvooe Jul 25, 2016

Contributor

Make name() use this. Also, document the method.

@stevvooe

stevvooe Jul 25, 2016

Contributor

Make name() use this. Also, document the method.

This comment has been minimized.

@mavenugo

mavenugo Jul 25, 2016

Contributor

yes. name() is infact using this method above.

@mavenugo

mavenugo Jul 25, 2016

Contributor

yes. name() is infact using this method above.

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Jul 25, 2016

Contributor

@mavenugo Isn't this using a completely different method for generating the DNS name? Should we not have this all in the same place? What are the other projections of task into the dns space?

Contributor

stevvooe commented Jul 25, 2016

@mavenugo Isn't this using a completely different method for generating the DNS name? Should we not have this all in the same place? What are the other projections of task into the dns space?

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Jul 25, 2016

Contributor

Also, how are aliases on the network attachment propagated?

Contributor

stevvooe commented Jul 25, 2016

Also, how are aliases on the network attachment propagated?

@mavenugo

This comment has been minimized.

Show comment
Hide comment
@mavenugo

mavenugo Jul 25, 2016

Contributor

@stevvooe Sorry. I dont quite get your comment. The only addition that this change brings in is the additional dns entry for a container with the task-name. So with this change, a container can be reached by its

  • container-name (service.slot.taskid)
  • task-name (service.slot)
  • container-id
  • service-name (via LB)

container-name is the primary DNS entry for the container.
task-name & container-id are aliases.
service-name is the Virtual-name for the container.

Contributor

mavenugo commented Jul 25, 2016

@stevvooe Sorry. I dont quite get your comment. The only addition that this change brings in is the additional dns entry for a container with the task-name. So with this change, a container can be reached by its

  • container-name (service.slot.taskid)
  • task-name (service.slot)
  • container-id
  • service-name (via LB)

container-name is the primary DNS entry for the container.
task-name & container-id are aliases.
service-name is the Virtual-name for the container.

@mavenugo

This comment has been minimized.

Show comment
Hide comment
@mavenugo

mavenugo Jul 25, 2016

Contributor

@stevvooe the aliases are propagated the exact same way all other names are propagated (via Gossip SD mechanism).

Contributor

mavenugo commented Jul 25, 2016

@stevvooe the aliases are propagated the exact same way all other names are propagated (via Gossip SD mechanism).

@mavenugo

This comment has been minimized.

Show comment
Hide comment
@mavenugo

mavenugo Jul 26, 2016

Contributor

If this needs more discussions, we can consider it for later releases. I dont see this as mandatory for 1.12.0 release.

Contributor

mavenugo commented Jul 26, 2016

If this needs more discussions, we can consider it for later releases. I dont see this as mandatory for 1.12.0 release.

@cpuguy83 cpuguy83 removed this from the 1.12.0 milestone Jul 26, 2016

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Jul 26, 2016

Contributor

Removing from the milestone, setting to design review.

Contributor

cpuguy83 commented Jul 26, 2016

Removing from the milestone, setting to design review.

@toughIQ

This comment has been minimized.

Show comment
Hide comment
@toughIQ

toughIQ Jul 26, 2016

This feature is useful, if you have to create your own Loadbalancer service (with different balancing strategies than swarm provides) and need to know the backend tasks.

At first I thought, that service.slot-id would suffice, but this is only the case, if the number of backend tasks is fixed and gets not changed by scale command. Or your LB is set up with eg. service.1 to service.100 as backends by default and only uses the currently running serivce-ids.

Example: you start with 3 replicas. So you get service.1, service.2 and service.3. That's predictable. If you then scale up to 10, you have service IDs from 1-10. Also some kind of predictable. But if you scale down to 3 again, you are left with 3 tasks, but their slot IDs can be anything from 1-10 and you cannot say, what it will be.

In my current project https://github.com/toughIQ/docker-maxscale I used the dnsRR (by now undocumented feature) results of gentent hosts tasks.SERVICE to create a dynamic config at startup.

toughIQ commented Jul 26, 2016

This feature is useful, if you have to create your own Loadbalancer service (with different balancing strategies than swarm provides) and need to know the backend tasks.

At first I thought, that service.slot-id would suffice, but this is only the case, if the number of backend tasks is fixed and gets not changed by scale command. Or your LB is set up with eg. service.1 to service.100 as backends by default and only uses the currently running serivce-ids.

Example: you start with 3 replicas. So you get service.1, service.2 and service.3. That's predictable. If you then scale up to 10, you have service IDs from 1-10. Also some kind of predictable. But if you scale down to 3 again, you are left with 3 tasks, but their slot IDs can be anything from 1-10 and you cannot say, what it will be.

In my current project https://github.com/toughIQ/docker-maxscale I used the dnsRR (by now undocumented feature) results of gentent hosts tasks.SERVICE to create a dynamic config at startup.

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Jul 26, 2016

Contributor

@toughIQ For something like that, I'd really like to see something like DNAME.

In general, it is not critical that the load balancer is always synced with the current number of tasks if the load balancer is health checking backends. Even in the case where there replica holes, the load balancer will be able to work out which ones are up.

The other option here would be to have it consume scaling events.

Contributor

stevvooe commented Jul 26, 2016

@toughIQ For something like that, I'd really like to see something like DNAME.

In general, it is not critical that the load balancer is always synced with the current number of tasks if the load balancer is health checking backends. Even in the case where there replica holes, the load balancer will be able to work out which ones are up.

The other option here would be to have it consume scaling events.

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Jul 26, 2016

Contributor

@mavenugo To be compatible with the hierarchy we've been discussing, these really need to be the other way around:

container-name (service.slot.taskid)

taskid.slot.service

task-name (service.slot)

slot.service

container-id

container-id.service

service-name (via LB)

service.namespace.cluster

This will allow us to combine DNS spaces in the future in interesting ways.

Contributor

stevvooe commented Jul 26, 2016

@mavenugo To be compatible with the hierarchy we've been discussing, these really need to be the other way around:

container-name (service.slot.taskid)

taskid.slot.service

task-name (service.slot)

slot.service

container-id

container-id.service

service-name (via LB)

service.namespace.cluster

This will allow us to combine DNS spaces in the future in interesting ways.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jul 26, 2016

Member

@stevvooe can you open a separate issue for that for discussion? Also; can we make that change after 1.12 without breaking backward compatibility?

Member

thaJeztah commented Jul 26, 2016

@stevvooe can you open a separate issue for that for discussion? Also; can we make that change after 1.12 without breaking backward compatibility?

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe
Contributor

stevvooe commented Jul 26, 2016

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Aug 26, 2016

Contributor

Not sure how useful this would be without having some way to discover how many slots there are.

Contributor

cpuguy83 commented Aug 26, 2016

Not sure how useful this would be without having some way to discover how many slots there are.

@youngnicks

This comment has been minimized.

Show comment
Hide comment
@youngnicks

youngnicks Aug 28, 2016

This change would help dockerize apps that cluster themselves together, such as Zookeeper. With the current version of Zookeeper, you need to specify other servers in the cluster for it to connect to. With this change you could have the following in the zoo.cfg:

server.1=zoo.1:2888:3888
server.2=zoo.2:2888:3888
server.3=zoo.3:2888:3888

youngnicks commented Aug 28, 2016

This change would help dockerize apps that cluster themselves together, such as Zookeeper. With the current version of Zookeeper, you need to specify other servers in the cluster for it to connect to. With this change you could have the following in the zoo.cfg:

server.1=zoo.1:2888:3888
server.2=zoo.2:2888:3888
server.3=zoo.3:2888:3888
@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Aug 29, 2016

Contributor

@mavenugo @youngnicks We are looking at a comprehensive solution here. These use cases are a particular pain point and we need to address them.

Contributor

stevvooe commented Aug 29, 2016

@mavenugo @youngnicks We are looking at a comprehensive solution here. These use cases are a particular pain point and we need to address them.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Sep 22, 2016

Member

@mavenugo @stevvooe should we close this while you're working on an alternative?

Member

thaJeztah commented Sep 22, 2016

@mavenugo @stevvooe should we close this while you're working on an alternative?

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Sep 27, 2016

Contributor

@thaJeztah Until there is an actual plan here, we should not be changing the service mapping into DNS.

Contributor

stevvooe commented Sep 27, 2016

@thaJeztah Until there is an actual plan here, we should not be changing the service mapping into DNS.

@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure

mlaventure Oct 21, 2016

Contributor

ping @thaJeztah @mavenugo @stevvooe

Should this be closed and the discussion moved to #27080 until a decision is made?

Contributor

mlaventure commented Oct 21, 2016

ping @thaJeztah @mavenugo @stevvooe

Should this be closed and the discussion moved to #27080 until a decision is made?

@stevvooe

This comment has been minimized.

Show comment
Hide comment
@stevvooe

stevvooe Oct 21, 2016

Contributor

@mavenugo This needs a proposal around docker/swarmkit#1242. We can't keep adding bespoke DNS entries without documenting their guarantees. For example, I've see slots proposed as a method of identifying instances consistently, but we need to make sure that slots have the right guarantees for that to work and we comfortable supporting that.

I'm not wholly opposed to this addition, so I don't think we need to close this, but we do need to have a real plan for DNS-based service discovery.

Contributor

stevvooe commented Oct 21, 2016

@mavenugo This needs a proposal around docker/swarmkit#1242. We can't keep adding bespoke DNS entries without documenting their guarantees. For example, I've see slots proposed as a method of identifying instances consistently, but we need to make sure that slots have the right guarantees for that to work and we comfortable supporting that.

I'm not wholly opposed to this addition, so I don't think we need to close this, but we do need to have a real plan for DNS-based service discovery.

@mavenugo

This comment has been minimized.

Show comment
Hide comment
@mavenugo

mavenugo Oct 21, 2016

Contributor

apologies on the delay. Yes, I agree that we should have a complete proposal on this. lets continue the discussion in docker/swarmkit#1242.

Contributor

mavenugo commented Oct 21, 2016

apologies on the delay. Yes, I agree that we should have a complete proposal on this. lets continue the discussion in docker/swarmkit#1242.

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Nov 10, 2016

Contributor

@mavenugo can we close this? We have too many PRs now :)

Contributor

LK4D4 commented Nov 10, 2016

@mavenugo can we close this? We have too many PRs now :)

@mavenugo

This comment has been minimized.

Show comment
Hide comment
@mavenugo

mavenugo Nov 10, 2016

Contributor

@LK4D4 yes... closing this.

Contributor

mavenugo commented Nov 10, 2016

@LK4D4 yes... closing this.

@mavenugo mavenugo closed this Nov 10, 2016

@eyz

This comment has been minimized.

Show comment
Hide comment
@eyz

eyz May 26, 2017

This sounds very useful, as mentioned, for mapping in external service load balancers.

eyz commented May 26, 2017

This sounds very useful, as mentioned, for mapping in external service load balancers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment