Container network fixes (#104)

* Feat/net-attr add IP address of each network to the computed attributes from #50.
* marks ip_address as deprecated and adds network data for a container. Closes #9
* adds wait for removal of a container. Closes #98 
* removes validator for container network_mode and checks error handling if container disconnect from default network fails. Closes #107

CHANGELOG.md

@@ -1,16 +1,19 @@
 ## 1.1.0 (Unreleased)
 
 IMPROVEMENTS
-* Adds labels for `network`, `volume` and `secret` to support docker stacks. [[92](https://github.com/terraform-providers/terraform-provider-docker/pull/92)] 
+* Adds labels for `network`, `volume` and `secret` to support docker stacks. [[#92](https://github.com/terraform-providers/terraform-provider-docker/pull/92)] 
 
 BUG FIXES
 * Fixes that new network were appended to the default bridge [GH-10]
 * Fixes that container resource returns a non-existent IP address [GH-36]
+* Fixes container's ip_address is empty when using custom network [GH-9] and [[#50](https://github.com/terraform-providers/terraform-provider-docker/pull/50)]
+* Fixes terraform destroy failing to remove a bridge network [GH-98] and [[#50](https://github.com/terraform-providers/terraform-provider-docker/pull/50)]
+
 
 ## 1.0.4 (October 17, 2018)
 
 BUG FIXES
-* Support and fix for random external ports for containers [[#102](https://github.com/terraform-providers/terraform-provider-docker/issues/102)] and ([103](https://github.com/terraform-providers/terraform-provider-docker/pull/103))
+* Support and fix for random external ports for containers [[#102](https://github.com/terraform-providers/terraform-provider-docker/issues/102)] and ([#103](https://github.com/terraform-providers/terraform-provider-docker/pull/103))
 
 ## 1.0.3 (October 12, 2018)
 

docker/resource_docker_container.go

@@ -291,18 +291,21 @@ func resourceDockerContainer() *schema.Resource {
 			},
 
 			"ip_address": &schema.Schema{
-				Type:     schema.TypeString,
-				Computed: true,
+				Type:       schema.TypeString,
+				Computed:   true,
+				Deprecated: "Use ip_adresses_data instead. This field exposes the data of the container's first network.",
 			},
 
 			"ip_prefix_length": &schema.Schema{
-				Type:     schema.TypeInt,
-				Computed: true,
+				Type:       schema.TypeInt,
+				Computed:   true,
+				Deprecated: "Use ip_prefix_length from ip_adresses_data instead. This field exposes the data of the container's first network.",
 			},
 
 			"gateway": &schema.Schema{
-				Type:     schema.TypeString,
-				Computed: true,
+				Type:       schema.TypeString,
+				Computed:   true,
+				Deprecated: "Use gateway from ip_adresses_data instead. This field exposes the data of the container's first network.",
 			},
 
 			"bridge": &schema.Schema{
@@ -310,6 +313,31 @@ func resourceDockerContainer() *schema.Resource {
 				Computed: true,
 			},
 
+			"network_data": &schema.Schema{
+				Type:     schema.TypeList,
+				Computed: true,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"network_name": &schema.Schema{
+							Type:     schema.TypeString,
+							Computed: true,
+						},
+						"ip_address": &schema.Schema{
+							Type:     schema.TypeString,
+							Computed: true,
+						},
+						"ip_prefix_length": &schema.Schema{
+							Type:     schema.TypeInt,
+							Computed: true,
+						},
+						"gateway": &schema.Schema{
+							Type:     schema.TypeString,
+							Computed: true,
+						},
+					},
+				},
+			},
+
 			"privileged": &schema.Schema{
 				Type:     schema.TypeBool,
 				Optional: true,
@@ -398,10 +426,9 @@ func resourceDockerContainer() *schema.Resource {
 			},
 
 			"network_mode": &schema.Schema{
-				Type:         schema.TypeString,
-				Optional:     true,
-				ForceNew:     true,
-				ValidateFunc: validateStringMatchesPattern(`^(bridge|host|none|container:.+|service:.+)$`),
+				Type:     schema.TypeString,
+				Optional: true,
+				ForceNew: true,
 			},
 
 			"networks": &schema.Schema{

docker/resource_docker_container_funcs.go

@@ -212,7 +212,9 @@ func resourceDockerContainerCreate(d *schema.ResourceData, meta interface{}) err
 		}
 
 		if err := client.NetworkDisconnect(context.Background(), "bridge", retContainer.ID, false); err != nil {
-			return fmt.Errorf("Unable to disconnect the default network: %s", err)
+			if !strings.Contains(err.Error(), "is not connected to the network bridge") {
+				return fmt.Errorf("Unable to disconnect the default network: %s", err)
+			}
 		}
 
 		for _, rawNetwork := range v.(*schema.Set).List() {
@@ -300,7 +302,7 @@ func resourceDockerContainerRead(d *schema.ResourceData, meta interface{}) error
 		}
 
 		jsonObj, _ := json.MarshalIndent(container, "", "\t")
-		log.Printf("[DEBUG] Docker container inspect: %s", jsonObj)
+		log.Printf("[INFO] Docker container inspect: %s", jsonObj)
 
 		if container.State.Running ||
 			!container.State.Running && !d.Get("must_run").(bool) {
@@ -333,13 +335,27 @@ func resourceDockerContainerRead(d *schema.ResourceData, meta interface{}) error
 
 	// Read Network Settings
 	if container.NetworkSettings != nil {
+		// TODO remove deprecated attributes in next major
 		d.Set("ip_address", container.NetworkSettings.IPAddress)
 		d.Set("ip_prefix_length", container.NetworkSettings.IPPrefixLen)
 		d.Set("gateway", container.NetworkSettings.Gateway)
+		if container.NetworkSettings != nil && len(container.NetworkSettings.Networks) > 0 {
+			// Still support deprecated outputs
+			for _, settings := range container.NetworkSettings.Networks {
+				d.Set("ip_address", settings.IPAddress)
+				d.Set("ip_prefix_length", settings.IPPrefixLen)
+				d.Set("gateway", settings.Gateway)
+				break
+			}
+		}
+
 		d.Set("bridge", container.NetworkSettings.Bridge)
 		if err := d.Set("ports", flattenContainerPorts(container.NetworkSettings.Ports)); err != nil {
 			log.Printf("[WARN] failed to set ports from API: %s", err)
 		}
+		if err := d.Set("network_data", flattenContainerNetworks(container.NetworkSettings)); err != nil {
+			log.Printf("[WARN] failed to set network settings from API: %s", err)
+		}
 	}
 
 	return nil
@@ -372,6 +388,16 @@ func resourceDockerContainerDelete(d *schema.ResourceData, meta interface{}) err
 		return fmt.Errorf("Error deleting container %s: %s", d.Id(), err)
 	}
 
+	waitOkC, errorC := client.ContainerWait(context.Background(), d.Id(), container.WaitConditionRemoved)
+	select {
+	case waitOk := <-waitOkC:
+		log.Printf("[INFO] Container exited with code [%v]: '%s'", waitOk.StatusCode, d.Id())
+	case err := <-errorC:
+		if !(strings.Contains(err.Error(), "No such container") || strings.Contains(err.Error(), "is already in progress")) {
+			return fmt.Errorf("Error waiting for container removal '%s': %s", d.Id(), err)
+		}
+	}
+
 	d.SetId("")
 	return nil
 }
@@ -394,6 +420,23 @@ func flattenContainerPorts(in nat.PortMap) []interface{} {
 	}
 	return out
 }
+func flattenContainerNetworks(in *types.NetworkSettings) []interface{} {
+	var out = make([]interface{}, 0)
+	if in == nil || in.Networks == nil || len(in.Networks) == 0 {
+		return out
+	}
+
+	networks := in.Networks
+	for networkName, networkData := range networks {
+		m := make(map[string]interface{})
+		m["network_name"] = networkName
+		m["ip_address"] = networkData.IPAddress
+		m["ip_prefix_length"] = networkData.IPPrefixLen
+		m["gateway"] = networkData.Gateway
+		out = append(out, m)
+	}
+	return out
+}
 
 // TODO move to separate flattener file
 func stringListToStringSlice(stringList []interface{}) []string {

docker/resource_docker_container_test.go

@@ -40,6 +40,73 @@ func TestAccDockerContainer_basic(t *testing.T) {
 		},
 	})
 }
+func TestAccDockerContainer_basic_network(t *testing.T) {
+	var c types.ContainerJSON
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccDockerContainerWith2BridgeNetworkConfig,
+				Check: resource.ComposeTestCheckFunc(
+					testAccContainerRunning("docker_container.foo", &c),
+					resource.TestCheckResourceAttr("docker_container.foo", "bridge", ""),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "ip_address"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "ip_prefix_length"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "gateway"),
+					resource.TestCheckResourceAttr("docker_container.foo", "network_data.#", "2"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.0.network_name"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.0.ip_address"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.0.ip_prefix_length"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.0.gateway"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.1.network_name"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.1.ip_address"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.1.ip_prefix_length"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.1.gateway"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccDockerContainer_2networks_withmode(t *testing.T) {
+	var c types.ContainerJSON
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccDockerContainer2NetworksConfig,
+				Check: resource.ComposeTestCheckFunc(
+					testAccContainerRunning("docker_container.foo", &c),
+					resource.TestCheckResourceAttr("docker_container.foo", "bridge", ""),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "ip_address"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "ip_prefix_length"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "gateway"),
+					resource.TestCheckResourceAttr("docker_container.foo", "network_data.#", "2"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.0.network_name"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.0.ip_address"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.0.ip_prefix_length"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.0.gateway"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.1.network_name"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.1.ip_address"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.1.ip_prefix_length"),
+					resource.TestCheckResourceAttrSet("docker_container.foo", "network_data.1.gateway"),
+					resource.TestCheckResourceAttr("docker_container.bar", "network_alias.#", "1"),
+					resource.TestCheckResourceAttr("docker_container.bar", "bridge", ""),
+					resource.TestCheckResourceAttrSet("docker_container.bar", "ip_address"),
+					resource.TestCheckResourceAttrSet("docker_container.bar", "ip_prefix_length"),
+					resource.TestCheckResourceAttrSet("docker_container.bar", "gateway"),
+					resource.TestCheckResourceAttr("docker_container.bar", "network_data.#", "1"),
+					resource.TestCheckResourceAttrSet("docker_container.bar", "network_data.0.network_name"),
+					resource.TestCheckResourceAttrSet("docker_container.bar", "network_data.0.ip_address"),
+					resource.TestCheckResourceAttrSet("docker_container.bar", "network_data.0.ip_prefix_length"),
+					resource.TestCheckResourceAttrSet("docker_container.bar", "network_data.0.gateway"),
+				),
+			},
+		},
+	})
+}
 
 func TestAccDockerContainerPath_validation(t *testing.T) {
 	cases := []struct {
@@ -687,6 +754,29 @@ resource "docker_container" "foo" {
 }
 `
 
+const testAccDockerContainerWith2BridgeNetworkConfig = `
+resource "docker_network" "tftest" {
+  name = "tftest-contnw"
+}
+
+resource "docker_network" "tftest_2" {
+  name = "tftest-contnw-2"
+}
+
+resource "docker_image" "foo" {
+	name = "nginx:latest"
+}
+
+resource "docker_container" "foo" {
+	name  	 = "tf-test"
+	image 	 = "${docker_image.foo.latest}"
+	networks = [
+		"${docker_network.tftest.name}",
+		"${docker_network.tftest_2.name}"
+	]
+}
+`
+
 const testAccDockerContainerVolumeConfig = `
 resource "docker_image" "foo" {
 	name = "nginx:latest"
@@ -885,3 +975,33 @@ resource "docker_container" "foo" {
 	] 
 }
 `
+const testAccDockerContainer2NetworksConfig = `
+resource "docker_image" "foo" {
+  name         = "nginx:latest"
+  keep_locally = true
+}
+
+resource "docker_network" "test_network_1" {
+  name = "tftest-1"
+}
+
+resource "docker_network" "test_network_2" {
+  name = "tftest-2"
+}
+
+resource "docker_container" "foo" {
+  name          = "tf-test"
+  image         = "${docker_image.foo.latest}"
+  network_mode  = "${docker_network.test_network_1.name}"
+  networks      = ["${docker_network.test_network_2.name}"]
+  network_alias = ["tftest-container"]
+}
+
+resource "docker_container" "bar" {
+  name          = "tf-test-bar"
+  image         = "${docker_image.foo.latest}"
+  network_mode  = "bridge"
+  networks      = ["${docker_network.test_network_2.name}"]
+  network_alias = ["tftest-container-foo"]
+}
+`

docker/resource_docker_network_funcs.go

@@ -2,15 +2,17 @@ package docker
 
 import (
 	"fmt"
+	"strings"
 
 	"context"
 	"encoding/json"
+	"log"
+	"time"
+
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/network"
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
-	"log"
-	"time"
 )
 
 func resourceDockerNetworkCreate(d *schema.ResourceData, meta interface{}) error {
@@ -90,10 +92,21 @@ func resourceDockerNetworkRead(d *schema.ResourceData, meta interface{}) error {
 }
 
 func resourceDockerNetworkDelete(d *schema.ResourceData, meta interface{}) error {
-	client := meta.(*ProviderConfig).DockerClient
+	log.Printf("[INFO] Waiting for network: '%s' to be removed: max '%v seconds'", d.Id(), 30)
 
-	if err := client.NetworkRemove(context.Background(), d.Id()); err != nil {
-		return fmt.Errorf("Error deleting network %s: %s", d.Id(), err)
+	stateConf := &resource.StateChangeConf{
+		Pending:    []string{"pending"},
+		Target:     []string{"removed"},
+		Refresh:    resourceDockerNetworkRemoveRefreshFunc(d, meta),
+		Timeout:    30 * time.Second,
+		MinTimeout: 5 * time.Second,
+		Delay:      2 * time.Second,
+	}
+
+	// Wait, catching any errors
+	_, err := stateConf.WaitForState()
+	if err != nil {
+		return err
 	}
 
 	d.SetId("")
@@ -133,7 +146,7 @@ func resourceDockerNetworkReadRefreshFunc(
 		if err != nil {
 			log.Printf("[WARN] Network (%s) not found, removing from state", networkID)
 			d.SetId("")
-			return networkID, "removed", err
+			return networkID, "removed", nil
 		}
 
 		jsonObj, _ := json.MarshalIndent(retNetwork, "", "\t")
@@ -160,3 +173,26 @@ func resourceDockerNetworkReadRefreshFunc(
 		return networkID, "all_fields", nil
 	}
 }
+
+func resourceDockerNetworkRemoveRefreshFunc(
+	d *schema.ResourceData, meta interface{}) resource.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		client := meta.(*ProviderConfig).DockerClient
+		networkID := d.Id()
+
+		_, _, err := client.NetworkInspectWithRaw(context.Background(), networkID, types.NetworkInspectOptions{})
+		if err != nil {
+			log.Printf("[INFO] Network (%s) not found. Already removed", networkID)
+			return networkID, "removed", nil
+		}
+
+		if err := client.NetworkRemove(context.Background(), networkID); err != nil {
+			if strings.Contains(err.Error(), "has active endpoints") {
+				return networkID, "pending", nil
+			}
+			return networkID, "other", err
+		}
+
+		return networkID, "removed", nil
+	}
+}

docker/resource_docker_service_funcs.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"encoding/base64"
+
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/filters"
@@ -275,6 +276,7 @@ func deleteService(serviceID string, d *schema.ResourceData, client *client.Clie
 			destroyGraceSeconds, _ := time.ParseDuration(v.(string))
 			log.Printf("[INFO] Waiting for container: '%s' to exit: max %v", containerID, destroyGraceSeconds)
 			ctx, cancel := context.WithTimeout(context.Background(), destroyGraceSeconds)
+			// TODO why defer? see container_resource with handling return channels! why not remove then wait?
 			defer cancel()
 			exitCode, _ := client.ContainerWait(ctx, containerID, container.WaitConditionRemoved)
 			log.Printf("[INFO] Container exited with code [%v]: '%s'", exitCode, containerID)

website/docs/r/container.html.markdown

@@ -205,13 +205,16 @@ the following:
 
 The following attributes are exported:
 
- * `ip_address` - The IP address of the container as read from its
+ * `network_data` - (Map of a block) The IP addresses of the container on each
+   network. Key are the network names, values are the IP addresses.
+  * `ip_address` - The IP address of the container.
+  * `ip_prefix_length` - The IP prefix length of the container.
+  * `gateway` - The network gateway of the container.
+ * `bridge` - The network bridge of the container as read from its NetworkSettings.
+ * `ip_address` - *Deprecated:* Use `network_data` instead. The IP address of the container's first network it.
+ * `ip_prefix_length` - *Deprecated:* Use `network_data` instead. The IP prefix length of the container as read from its
    NetworkSettings.
- * `ip_prefix_length` - The IP prefix length of the container as read from its
-   NetworkSettings.
- * `gateway` - The network gateway of the container as read from its
-   NetworkSettings.
- * `bridge` - The network bridge of the container as read from its
+ * `gateway` - *Deprecated:* Use `network_data` instead. The network gateway of the container as read from its
    NetworkSettings.