Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Add anonymous container alias to service record on attachable network#1651

Merged
sanimej merged 1 commit intomoby:mastermoby/libnetwork:masterfrom
aboch:epaCopy head branch name to clipboard
Mar 2, 2017
Merged

Add anonymous container alias to service record on attachable network#1651
sanimej merged 1 commit intomoby:mastermoby/libnetwork:masterfrom
aboch:epaCopy head branch name to clipboard

Conversation

@aboch
Copy link
Copy Markdown
Contributor

@aboch aboch commented Feb 15, 2017 •
edited
Loading

  • Currently when a non-named container with network aliases
    is connected to a swarm attachable network, its aliases are
    not added to the service records.
    This is not in line with what we do when connecting to
    a local scope network, or to a kv-store based overlay network.

Related to moby/moby/issues/31015, moby/moby/issues/31238

Signed-off-by: Alessandro Boch aboch@docker.com

sanimej
sanimej reviewed Mar 2, 2017
View reviewed changes
Comment thread agent.go

c := n.getController()
agent := c.getAgent()
if !ep.isAnonymous() && ep.Iface().Address() != nil {
Copy link
Copy Markdown

@sanimej sanimej Mar 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ep.isAnonymous() check has been moved up, which is needed. But why is the nil check for ep.Iface().Address()` removed ?

Copy link
Copy Markdown
Contributor Author

@aboch aboch Mar 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this. I think it just got lost in the change.
I am not sure if the address can ever be nil, but better to restore the original check.

Copy link
Copy Markdown

@sanimej sanimej Mar 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. Since its an explicit check I assume there might be cases where its nil.

Copy link
Copy Markdown
Contributor Author

@aboch aboch Mar 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, PTAL

@aboch
Add anonymous container alias to service record on attachable network
f230726 
- Currently when a non-named container with network aliases
  is connected to a swarm attachable network, its aliases are
  not added to the service records.
  This is not in line with what we do when connecting to
  a local scope network, or to a kv-store based overlay network.

Signed-off-by: Alessandro Boch <aboch@docker.com>
@sanimej
Copy link
Copy Markdown

sanimej commented Mar 2, 2017

LGTM

@sanimej sanimej merged commit 1a01921 into moby:master Mar 2, 2017
@aboch aboch mentioned this pull request Mar 14, 2017
Merged
@XavM
Copy link
Copy Markdown

XavM commented Sep 16, 2017

I confirm it now works when using only one network, but not when using multiple nets and later connecting individual containers from the stack to some not previously connected networks (docker network connect --alias)

Tested with Docker version 17.06.2-ce, build cec0b72

To reproduce :

cat docker-compose.tnt.yml
version: '3.3'
services:
  svc1:
    image: alpine
    entrypoint: tail -f /dev/null
    networks:
      - net1
  svc2:
    image: alpine
    entrypoint: tail -f /dev/null
    networks:
      - net2
networks:
  net1:
    driver: overlay
    attachable: true
  net2:
    driver: overlay
    attachable: true

 

$>docker stack deploy -c docker-compose.tnt.yml --with-registry-auth tnt
Creating network tnt_net2
Creating network tnt_net1
Creating service tnt_svc1
Creating service tnt_svc2

 

$>docker ps -a 
CONTAINER ID        IMAGE               COMMAND               CREATED             STATUS              PORTS               NAMES
01f647e03085        alpine:latest       "tail -f /dev/null"   3 seconds ago       Up 2 seconds                            tnt_svc2.1.r4d3pmqo7iju7smrfs9vggyjz
e576057594c2        alpine:latest       "tail -f /dev/null"   6 seconds ago       Up 5 seconds                            tnt_svc1.1.sfe9jtslfc5hnpoy27xcouq29

 

$>docker network connect --alias svc2_ct1 tnt_net1 01f647e03085

Every thing looks fine so far :

$>docker inspect 01f647e03085 | jq  '[.[0].NetworkSettings.Networks["tnt_net1"]]|map({Aliases, IPAddress})[0]'
{
  "Aliases": [
    "svc2_ct1",
    "01f647e03085"
  ],
  "IPAddress": "10.0.0.2"
}

 

$>docker exec -it e576057594c2 ping -c 2 10.0.0.2
PING 10.0.0.2 (10.0.0.2): 56 data bytes
64 bytes from 10.0.0.2: seq=0 ttl=64 time=0.079 ms
64 bytes from 10.0.0.2: seq=1 ttl=64 time=0.117 ms

--- 10.0.0.2 ping statistics ---
2 packets transmitted, 2 packets received, 0% packet loss
round-trip min/avg/max = 0.079/0.098/0.117 ms

But both the manually added alias and the ct_id are not resolved by the embedded DNS

$>docker exec -it e576057594c2 ping -c 2 svc2_ct1
ping: bad address 'svc2_ct1'

 

$>docker exec -it e576057594c2 ping -c 2 01f647e03085
ping: bad address '01f647e03085'

@dolanor
Copy link
Copy Markdown

dolanor commented Sep 18, 2017 •
edited
Loading

I've been bitten by the same problem right this second.
Pretty problematic.
Should be create a fake TLD in the internal DNS so we can scope this way?

non_unique_container_name.unique_net_name
and
non_unique_container_name.unique_net_name2

My use case, is to have a service listening on 1 port (https server) to be able to redirect to 2 subnets.
grafana-prod.domain.com hitting my reverse proxy would go to the prod_dashboard_grafana.x.auienst container (aliased to prod_dashboard_grafana for example, I have 1 instance)
and
grafana-dev.domain.com hitting my reverse proxy would go to the dev_dashboard_grafana.y.nstrauie container (aliased to dev_dashboard_grafana)

so instead of using aliases (which don't work currently) maybe by having a TLD called dev_dashboard_default, we could redirect to grafana.dev_dashboard_default and the same for the prod for example.

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

Reviewers

1 more reviewer

@sanimej sanimej sanimej left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@aboch @sanimej @XavM @dolanor
Morty Proxy This is a proxified and sanitized view of the page, visit original site.