Fix connect to host and prevent disconnect from host for host network#17476
Fix connect to host and prevent disconnect from host for host network#17476calavera merged 1 commit intomoby:masterfrom
Conversation
daemon/container_unix.go
Outdated
There was a problem hiding this comment.
We should not be needing this extra check. The check for IsPrivate() should disallow connecting to host. Are you able to connect to host after the container is started on a private network?
There was a problem hiding this comment.
@mrjana yes, I able to connect to host if I disconnect the container with private network from bridge
docker run -d --name c1 busybox top
docker network disconnect bridge c1
docker network connect host c1
docker inspect c1
....
"Networks": {
"host": {
"EndpointID": "6b09beacac0470bed008ee62c55f4161701c64d5a7ccaddd8a155eb0c779bf5f",
"Gateway": "",
"IPAddress": "",
"IPPrefixLen": 0,
"IPv6Gateway": "",
"GlobalIPv6Address": "",
"GlobalIPv6PrefixLen": 0,
"MacAddress": ""
}
}
because if we disconnect it from bridge, the "NetworkSettings" is nil and the check below for IsPrivate() has no chance to run
|
@coolljt0725 I am ok with the design. But had a comment on the code changes. Can you please clarify? When you try to connect a container to host, which is already connected to a private network, |
|
@coolljt0725 Also it needs a rebase |
f28e6d5 to
04baf0d
Compare
|
rebased |
|
@mrjana @tiborvass is this a 1.9.1 fix? |
|
@thaJeztah Yes |
|
LGTM |
|
Moving to code review since it's got @mrjana's blessing. |
daemon/container_unix.go
Outdated
There was a problem hiding this comment.
How come this is ErrConflictSharedNetwork and the other one is ErrConflictHostNetwork when they are both host?
There was a problem hiding this comment.
This is to prevent user from connecting to host and the other is prevent user from disconnecting from host
There was a problem hiding this comment.
But shouldn't these both be ErrConflictHostNetwork?
There was a problem hiding this comment.
Both be ErrConflictHostNetwork is also make sense, just need to modify the description of ErrConflictHostNetwork, I'll update
04baf0d to
75e5648
Compare
|
@cpuguy83 updated |
75e5648 to
8140ca3
Compare
|
@cpuguy83 updated |
|
LGTM |
8140ca3 to
e04435e
Compare
Container has private network namespace can not to connect to host and container with host network can not be disconnected from host. Signed-off-by: Lei Jitang <leijitang@huawei.com>
e04435e to
a2d8c93
Compare
|
LGTM. Unfortunately, this won't merge cleanly in the 1.9 release branch. @tiborvass should we open a new PR against that branch with a similar fix? |
|
It looks like this is okay for the release, no merged conflicts. Merging! |
Fix connect to host and prevent disconnect from host for host network
There are two commits in this PR.
The first one is to fix connect to host. If a container start with default network(
bridge) ornoneit can't be connect to
hostas desired. but if disconnect the container frombridgeornone, you canconnect to
host, but this will cause some problem.we able to connect to host after the container disconnect from
bridgebut actually, the container still use its own network namespace, docker inspect still show its own private sandbox the container use and
ifconfigin the container still the information of its own private network namespace.From what I understand the container has private network namespace cannot connect to other network namespace, https://github.com/docker/docker/blob/master/daemon/container_unix.go#L742 is supposed to do thar but may not go there because
container.NetworkSettings.Networkscould be nil ,correct me if I'm wrong :) @mavenugoThe second commit is to prevent from disconnecting from host for a container start with
--net=host. if a container with--net=hostdisconnect fromhost, it still in the host network namespace, but able to connect to other network, and cause some problem.though connect to
bridgefailed, but it has create aeth0network interface on my hostand docker inspect show the endpoint information
So I think we should prevent user from disconnect from host
ping @tiborvass