Merge pull request #18906 from coolljt0725/connect_to_created

Support network connect/disconnect to stopped container

daemon/container_operations_unix.go

@@ -696,13 +696,43 @@ func cleanOperationalData(es *networktypes.EndpointSettings) {
 	es.MacAddress = ""
 }
 
+func (daemon *Daemon) updateNetworkConfig(container *container.Container, idOrName string, updateSettings bool) (libnetwork.Network, error) {
+	if container.HostConfig.NetworkMode.IsContainer() {
+		return nil, runconfig.ErrConflictSharedNetwork
+	}
+
+	if containertypes.NetworkMode(idOrName).IsBridge() &&
+		daemon.configStore.DisableBridge {
+		container.Config.NetworkDisabled = true
+		return nil, nil
+	}
+
+	n, err := daemon.FindNetwork(idOrName)
+	if err != nil {
+		return nil, err
+	}
+
+	if updateSettings {
+		if err := daemon.updateNetworkSettings(container, n); err != nil {
+			return nil, err
+		}
+	}
+	return n, nil
+}
+
 // ConnectToNetwork connects a container to a network
 func (daemon *Daemon) ConnectToNetwork(container *container.Container, idOrName string, endpointConfig *networktypes.EndpointSettings) error {
 	if !container.Running {
-		return derr.ErrorCodeNotRunning.WithArgs(container.ID)
-	}
-	if err := daemon.connectToNetwork(container, idOrName, endpointConfig, true); err != nil {
-		return err
+		if container.RemovalInProgress || container.Dead {
+			return derr.ErrorCodeRemovalContainer.WithArgs(container.ID)
+		}
+		if _, err := daemon.updateNetworkConfig(container, idOrName, true); err != nil {
+			return err
+		}
+	} else {
+		if err := daemon.connectToNetwork(container, idOrName, endpointConfig, true); err != nil {
+			return err
+		}
 	}
 	if err := container.ToDiskLocking(); err != nil {
 		return fmt.Errorf("Error saving container to disk: %v", err)
@@ -711,37 +741,24 @@ func (daemon *Daemon) ConnectToNetwork(container *container.Container, idOrName
 }
 
 func (daemon *Daemon) connectToNetwork(container *container.Container, idOrName string, endpointConfig *networktypes.EndpointSettings, updateSettings bool) (err error) {
-	if container.HostConfig.NetworkMode.IsContainer() {
-		return runconfig.ErrConflictSharedNetwork
+	n, err := daemon.updateNetworkConfig(container, idOrName, updateSettings)
+	if err != nil {
+		return err
+	}
+	if n == nil {
+		return nil
 	}
 
 	if !containertypes.NetworkMode(idOrName).IsUserDefined() && hasUserDefinedIPAddress(endpointConfig) {
 		return runconfig.ErrUnsupportedNetworkAndIP
 	}
 
-	if containertypes.NetworkMode(idOrName).IsBridge() &&
-		daemon.configStore.DisableBridge {
-		container.Config.NetworkDisabled = true
-		return nil
-	}
-
 	controller := daemon.netController
 
-	n, err := daemon.FindNetwork(idOrName)
-	if err != nil {
-		return err
-	}
-
 	if err := validateNetworkingConfig(n, endpointConfig); err != nil {
 		return err
 	}
 
-	if updateSettings {
-		if err := daemon.updateNetworkSettings(container, n); err != nil {
-			return err
-		}
-	}
-
 	if endpointConfig != nil {
 		container.NetworkSettings.Networks[n.Name()] = endpointConfig
 	}
@@ -805,16 +822,22 @@ func (daemon *Daemon) connectToNetwork(container *container.Container, idOrName
 
 // DisconnectFromNetwork disconnects container from network n.
 func (daemon *Daemon) DisconnectFromNetwork(container *container.Container, n libnetwork.Network) error {
-	if !container.Running {
-		return derr.ErrorCodeNotRunning.WithArgs(container.ID)
-	}
-
 	if container.HostConfig.NetworkMode.IsHost() && containertypes.NetworkMode(n.Type()).IsHost() {
 		return runconfig.ErrConflictHostNetwork
 	}
-
-	if err := disconnectFromNetwork(container, n); err != nil {
-		return err
+	if !container.Running {
+		if container.RemovalInProgress || container.Dead {
+			return derr.ErrorCodeRemovalContainer.WithArgs(container.ID)
+		}
+		if _, ok := container.NetworkSettings.Networks[n.Name()]; ok {
+			delete(container.NetworkSettings.Networks, n.Name())
+		} else {
+			return fmt.Errorf("container %s is not connected to the network %s", container.ID, n.Name())
+		}
+	} else {
+		if err := disconnectFromNetwork(container, n); err != nil {
+			return err
+		}
 	}
 
 	if err := container.ToDiskLocking(); err != nil {

docs/reference/commandline/network_connect.md

@@ -16,7 +16,7 @@ parent = "smn_cli"
 
       --help             Print usage
 
-Connects a running container to a network. You can connect a container by name
+Connects a container to a network. You can connect a container by name
 or by ID. Once connected, the container can communicate with other containers in
 the same network.
 

docs/userguide/networking/work-with-networks.md

@@ -311,9 +311,8 @@ PING 172.17.0.2 (172.17.0.2): 56 data bytes
 
 ```
 
-To connect a container to a network, the container must be running. If you stop
-a container and inspect a network it belongs to, you won't see that container.
-The `docker network inspect` command only shows running containers.
+You can connect both running and non-running containers to a network. However,
+`docker network inspect` only displays information on running containers.
 
 ## Disconnecting containers
 

errors/daemon.go

@@ -46,6 +46,15 @@ var (
 		HTTPStatusCode: http.StatusInternalServerError,
 	})
 
+	// ErrorCodeRemovalContainer is generated when we attempt to connect or disconnect a
+	// container but it's marked for removal.
+	ErrorCodeRemovalContainer = errcode.Register(errGroup, errcode.ErrorDescriptor{
+		Value:          "REMOVALCONTAINER",
+		Message:        "Container %s is marked for removal and cannot be connected or disconnected to the network",
+		Description:    "The specified container is marked for removal and cannot be connected or disconnected to the network",
+		HTTPStatusCode: http.StatusInternalServerError,
+	})
+
 	// ErrorCodePausedContainer is generated when we attempt to attach a
 	// container but its paused.
 	ErrorCodePausedContainer = errcode.Register(errGroup, errcode.ErrorDescriptor{

integration-cli/docker_cli_network_unix_test.go

@@ -448,11 +448,6 @@ func (s *DockerNetworkSuite) TestDockerNetworkConnectDisconnect(c *check.C) {
 	c.Assert(nr.Name, checker.Equals, "test")
 	c.Assert(len(nr.Containers), checker.Equals, 0)
 
-	// check if network connect fails for inactive containers
-	dockerCmd(c, "stop", containerID)
-	_, _, err = dockerCmdWithError("network", "connect", "test", containerID)
-	c.Assert(err, check.NotNil)
-
 	dockerCmd(c, "network", "rm", "test")
 	assertNwNotAvailable(c, "test")
 }
@@ -938,7 +933,44 @@ func (s *DockerNetworkSuite) TestDockerNetworkRestartWithMulipleNetworks(c *chec
 	networks, err := inspectField("foo", "NetworkSettings.Networks")
 	c.Assert(err, checker.IsNil)
 	c.Assert(networks, checker.Contains, "bridge", check.Commentf("Should contain 'bridge' network"))
-	c.Assert(networks, checker.Contains, "test", check.Commentf("Should contain 'test' netwokr"))
+	c.Assert(networks, checker.Contains, "test", check.Commentf("Should contain 'test' network"))
+}
+
+func (s *DockerNetworkSuite) TestDockerNetworkConnectDisconnectToStoppedContainer(c *check.C) {
+	dockerCmd(c, "network", "create", "test")
+	dockerCmd(c, "create", "--name=foo", "busybox", "top")
+	dockerCmd(c, "network", "connect", "test", "foo")
+	networks, err := inspectField("foo", "NetworkSettings.Networks")
+	c.Assert(err, checker.IsNil)
+	c.Assert(networks, checker.Contains, "test", check.Commentf("Should contain 'test' network"))
+
+	// Restart docker daemon to test the config has persisted to disk
+	s.d.Restart()
+	networks, err = inspectField("foo", "NetworkSettings.Networks")
+	c.Assert(err, checker.IsNil)
+	c.Assert(networks, checker.Contains, "test", check.Commentf("Should contain 'test' network"))
+
+	// start the container and test if we can ping it from another container in the same network
+	dockerCmd(c, "start", "foo")
+	c.Assert(waitRun("foo"), checker.IsNil)
+	ip, err := inspectField("foo", "NetworkSettings.Networks.test.IPAddress")
+	ip = strings.TrimSpace(ip)
+	dockerCmd(c, "run", "--net=test", "busybox", "sh", "-c", fmt.Sprintf("ping -c 1 %s", ip))
+
+	dockerCmd(c, "stop", "foo")
+
+	// Test disconnect
+	dockerCmd(c, "network", "disconnect", "test", "foo")
+	networks, err = inspectField("foo", "NetworkSettings.Networks")
+	c.Assert(err, checker.IsNil)
+	c.Assert(networks, checker.Not(checker.Contains), "test", check.Commentf("Should not contain 'test' network"))
+
+	// Restart docker daemon to test the config has persisted to disk
+	s.d.Restart()
+	networks, err = inspectField("foo", "NetworkSettings.Networks")
+	c.Assert(err, checker.IsNil)
+	c.Assert(networks, checker.Not(checker.Contains), "test", check.Commentf("Should not contain 'test' network"))
+
 }
 
 func (s *DockerNetworkSuite) TestDockerNetworkConnectPreferredIP(c *check.C) {

man/docker-network-connect.1.md

@@ -11,7 +11,7 @@ NETWORK CONTAINER
 
 # DESCRIPTION
 
-Connects a running container to a network. You can connect a container by name
+Connects a container to a network. You can connect a container by name
 or by ID. Once connected, the container can communicate with other containers in
 the same network.
 

man/docker-network-disconnect.1.md

@@ -11,7 +11,7 @@ NETWORK CONTAINER
 
 # DESCRIPTION
 
-Disconnects a container from a network. The container must be running to disconnect it from the network.
+Disconnects a container from a network.
 
 ```bash
   $ docker network disconnect multi-host-network container1