From 2360eb088f26236e8391c6e56d793c20e9c22078 Mon Sep 17 00:00:00 2001 From: Manuel Vogel Date: Thu, 25 Oct 2018 07:21:48 +0200 Subject: [PATCH] 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 | 7 +- docker/resource_docker_container.go | 47 +++++++-- docker/resource_docker_container_funcs.go | 47 ++++++++- docker/resource_docker_container_test.go | 120 ++++++++++++++++++++++ docker/resource_docker_network_funcs.go | 48 +++++++-- docker/resource_docker_service_funcs.go | 2 + website/docs/r/container.html.markdown | 15 +-- 7 files changed, 260 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a24deb53..02e049c0 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/docker/resource_docker_container.go b/docker/resource_docker_container.go index f853ba75..f0b997df 100644 --- a/docker/resource_docker_container.go +++ b/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{ diff --git a/docker/resource_docker_container_funcs.go b/docker/resource_docker_container_funcs.go index ca4ca9a4..1d28c010 100644 --- a/docker/resource_docker_container_funcs.go +++ b/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 { diff --git a/docker/resource_docker_container_test.go b/docker/resource_docker_container_test.go index 3d5a425c..43e33482 100644 --- a/docker/resource_docker_container_test.go +++ b/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"] +} +` diff --git a/docker/resource_docker_network_funcs.go b/docker/resource_docker_network_funcs.go index 908c856c..53b3810c 100644 --- a/docker/resource_docker_network_funcs.go +++ b/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 + } +} diff --git a/docker/resource_docker_service_funcs.go b/docker/resource_docker_service_funcs.go index 8c07a60a..806a4a16 100644 --- a/docker/resource_docker_service_funcs.go +++ b/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) diff --git a/website/docs/r/container.html.markdown b/website/docs/r/container.html.markdown index 72558fa6..6c1822c8 100644 --- a/website/docs/r/container.html.markdown +++ b/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.