daemon: release sandbox even when NetworkDisabled#46651
daemon: release sandbox even when NetworkDisabled#46651thaJeztah merged 1 commit intomoby:masterfrom
Conversation
When the default bridge is disabled by setting dockerd's `--bridge=none` option, the daemon still creates a sandbox for containers with no network attachment specified. In that case `NetworkDisabled` will be set to true. However, currently the `releaseNetwork` call will early return if NetworkDisabled is true. Thus, these sandboxes won't be deleted until the daemon is restarted. If a high number of such containers are created, the daemon would then take few minutes to start. See moby#42461. Signed-off-by: payall4u <[email protected]> Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton
added
status/2-code-review
area/networking
Yeah, that's confusing. I also recall having discussions about docker network create foo
# this creates a container with a sandbox, but no networking connection
docker run -d --network=none --name hello nginx:alpine
# and connecting to a network produces an error
docker network connect foo hello
Error response from daemon: container cannot be connected to multiple networks with one of the networks in private (none) modeI could see use-cases where you'd want to create a container without networking (at first), and later connect it to a network, but not sure what'd be involved in doing so (if possible!) |
| // TODO(aker): If we hit this case, the endpoint state won't be cleaned up (ie. no call to cleanOperationalData). | ||
| if daemon.netController == nil { |
There was a problem hiding this comment.
Wondering if there's an actual case where daemon.netController CAN be nil 🤔
I'm considering that that should (probably?) never happen, and if it would we should probably make that a fatal error, and exit (before we even reach this code)
There was a problem hiding this comment.
Oh actually my comment isn't about an hypothetical situation:
There was a problem hiding this comment.
Ah, yeah, I recall the order of initialisation being... fun.. I once introduced a regression because of some of that; #43130 (comment)
|
should we cherry-pick this fix? |
|
@neersighted LGTY? |
|
Let me bring this one in; just wanted to have a second set of eyes before merging 👍 |
thaJeztah
added
process/cherry-picked
and removed
process/cherry-pick
process/cherry-pick/24.0
labels
4 participants
- What I did
When the default bridge is disabled by setting dockerd's
--bridge=noneoption, the daemon still creates a sandbox for containers with no network attachment specified. In that caseNetworkDisabledwill be set to true.However, currently the
releaseNetworkcall will early return if NetworkDisabled is true. Thus, these sandboxes won't be deleted until the daemon is restarted. If a high number of such containers are created, the daemon would then take few minutes to start.As a side note,
NetworkDisabledsemantics is weird/broken and should be revised:NetworkDisbledis set. Thus it allows these containers to be manually connected to other networks;- Description for the changelog
--bridge=noneoption is specified.- A picture of a cute animal (not mandatory but encouraged)