Support for random external port for containers (#103)

* fixes container port mapping by switching from set to list. Closes #102 
* adapts mapper and flattener
* updates CHANGELOG

CHANGELOG.md

@@ -1,4 +1,8 @@
 ## 1.0.4 (Unreleased)
+
+BUG FIXES
+* Support and fix for random external ports for containers [GH-102] and ([103](https://github.com/terraform-providers/terraform-provider-docker/pull/103))
+
 ## 1.0.3 (October 12, 2018)
 
 IMPROVEMENTS

docker/resource_docker_container.go

@@ -192,7 +192,7 @@ func resourceDockerContainer() *schema.Resource {
 			},
 
 			"ports": &schema.Schema{
-				Type:     schema.TypeSet,
+				Type:     schema.TypeList,
 				Optional: true,
 				ForceNew: true,
 				Elem: &schema.Resource{
@@ -205,8 +205,8 @@ func resourceDockerContainer() *schema.Resource {
 
 						"external": &schema.Schema{
 							Type:     schema.TypeInt,
-							Default:  "32678",
 							Optional: true,
+							Computed: true,
 							ForceNew: true,
 						},
 

docker/resource_docker_container_funcs.go

@@ -75,7 +75,7 @@ func resourceDockerContainerCreate(d *schema.ResourceData, meta interface{}) err
 	portBindings := map[nat.Port][]nat.PortBinding{}
 
 	if v, ok := d.GetOk("ports"); ok {
-		exposedPorts, portBindings = portSetToDockerPorts(v.(*schema.Set))
+		exposedPorts, portBindings = portSetToDockerPorts(v.([]interface{}))
 	}
 	if len(exposedPorts) != 0 {
 		config.ExposedPorts = exposedPorts
@@ -373,7 +373,7 @@ func resourceDockerContainerDelete(d *schema.ResourceData, meta interface{}) err
 }
 
 // TODO extract to structures_container.go
-func flattenContainerPorts(in nat.PortMap) *schema.Set {
+func flattenContainerPorts(in nat.PortMap) []interface{} {
 	var out = make([]interface{}, 0)
 	for port, portBindings := range in {
 		m := make(map[string]interface{})
@@ -388,9 +388,7 @@ func flattenContainerPorts(in nat.PortMap) *schema.Set {
 			out = append(out, m)
 		}
 	}
-	portsSpecResource := resourceDockerContainer().Schema["ports"].Elem.(*schema.Resource)
-	f := schema.HashResource(portsSpecResource)
-	return schema.NewSet(f, out)
+	return out
 }
 
 // TODO move to separate flattener file
@@ -452,11 +450,11 @@ func fetchDockerContainer(ID string, client *client.Client) (*types.Container, e
 	return nil, nil
 }
 
-func portSetToDockerPorts(ports *schema.Set) (map[nat.Port]struct{}, map[nat.Port][]nat.PortBinding) {
+func portSetToDockerPorts(ports []interface{}) (map[nat.Port]struct{}, map[nat.Port][]nat.PortBinding) {
 	retExposedPorts := map[nat.Port]struct{}{}
 	retPortBindings := map[nat.Port][]nat.PortBinding{}
 
-	for _, portInt := range ports.List() {
+	for _, portInt := range ports {
 		port := portInt.(map[string]interface{})
 		internal := port["internal"].(int)
 		protocol := port["protocol"].(string)

docker/resource_docker_container_test.go

@@ -420,15 +420,84 @@ func TestAccDockerContainer_port_internal(t *testing.T) {
 					testCheck,
 					resource.TestCheckResourceAttr("docker_container.foo", "name", "tf-test"),
 					resource.TestCheckResourceAttr("docker_container.foo", "ports.#", "1"),
-					resource.TestCheckResourceAttr("docker_container.foo", "ports.2978131916.internal", "80"),
-					resource.TestCheckResourceAttr("docker_container.foo", "ports.2978131916.ip", "0.0.0.0"),
-					resource.TestCheckResourceAttr("docker_container.foo", "ports.2978131916.protocol", "tcp"),
-					resource.TestCheckResourceAttr("docker_container.foo", "ports.2978131916.external", "32678"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.internal", "80"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.ip", "0.0.0.0"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.protocol", "tcp"),
+					testValueHigherEqualThan("docker_container.foo", "ports.0.external", 32768),
 				),
 			},
 		},
 	})
 }
+func TestAccDockerContainer_port_multiple_internal(t *testing.T) {
+	var c types.ContainerJSON
+
+	testCheck := func(*terraform.State) error {
+		portMap := c.NetworkSettings.NetworkSettingsBase.Ports
+		portBindings, ok := portMap["80/tcp"]
+		if !ok || len(portMap["80/tcp"]) == 0 {
+			return fmt.Errorf("Port 80 on tcp is not set")
+		}
+
+		portBindingsLength := len(portBindings)
+		if portBindingsLength != 1 {
+			return fmt.Errorf("Expected 1 binding on port 80, but was %d", portBindingsLength)
+		}
+
+		if len(portBindings[0].HostIP) == 0 {
+			return fmt.Errorf("Expected host IP to be set, but was empty")
+		}
+
+		if len(portBindings[0].HostPort) == 0 {
+			return fmt.Errorf("Expected host port to be set, but was empty")
+		}
+
+		portBindings, ok = portMap["81/tcp"]
+		if !ok || len(portMap["81/tcp"]) == 0 {
+			return fmt.Errorf("Port 81 on tcp is not set")
+		}
+
+		portBindingsLength = len(portBindings)
+		if portBindingsLength != 1 {
+			return fmt.Errorf("Expected 1 binding on port 81, but was %d", portBindingsLength)
+		}
+
+		if len(portBindings[0].HostIP) == 0 {
+			return fmt.Errorf("Expected host IP to be set, but was empty")
+		}
+
+		if len(portBindings[0].HostPort) == 0 {
+			return fmt.Errorf("Expected host port to be set, but was empty")
+		}
+
+		return nil
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccDockerContainerMultipleInternalPortConfig,
+				Check: resource.ComposeTestCheckFunc(
+					testAccContainerRunning("docker_container.foo", &c),
+					testCheck,
+					resource.TestCheckResourceAttr("docker_container.foo", "name", "tf-test"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.#", "2"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.internal", "80"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.ip", "0.0.0.0"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.protocol", "tcp"),
+					testValueHigherEqualThan("docker_container.foo", "ports.0.external", 32768),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.1.internal", "81"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.1.ip", "0.0.0.0"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.1.protocol", "tcp"),
+					testValueHigherEqualThan("docker_container.foo", "ports.1.external", 32768),
+				),
+			},
+		},
+	})
+}
+
 func TestAccDockerContainer_port(t *testing.T) {
 	var c types.ContainerJSON
 
@@ -466,10 +535,78 @@ func TestAccDockerContainer_port(t *testing.T) {
 					testCheck,
 					resource.TestCheckResourceAttr("docker_container.foo", "name", "tf-test"),
 					resource.TestCheckResourceAttr("docker_container.foo", "ports.#", "1"),
-					resource.TestCheckResourceAttr("docker_container.foo", "ports.2498386340.internal", "80"),
-					resource.TestCheckResourceAttr("docker_container.foo", "ports.2498386340.ip", "0.0.0.0"),
-					resource.TestCheckResourceAttr("docker_container.foo", "ports.2498386340.protocol", "tcp"),
-					resource.TestCheckResourceAttr("docker_container.foo", "ports.2498386340.external", "32787"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.internal", "80"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.ip", "0.0.0.0"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.protocol", "tcp"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.external", "32787"),
+				),
+			},
+		},
+	})
+}
+func TestAccDockerContainer_multiple_ports(t *testing.T) {
+	var c types.ContainerJSON
+
+	testCheck := func(*terraform.State) error {
+		portMap := c.NetworkSettings.NetworkSettingsBase.Ports
+		portBindings, ok := portMap["80/tcp"]
+		if !ok || len(portMap["80/tcp"]) == 0 {
+			return fmt.Errorf("Port 80 on tcp is not set")
+		}
+
+		portBindingsLength := len(portBindings)
+		if portBindingsLength != 1 {
+			return fmt.Errorf("Expected 1 binding on port 80, but was %d", portBindingsLength)
+		}
+
+		if len(portBindings[0].HostIP) == 0 {
+			return fmt.Errorf("Expected host IP to be set, but was empty")
+		}
+
+		if len(portBindings[0].HostPort) == 0 {
+			return fmt.Errorf("Expected host port to be set, but was empty")
+		}
+
+		portBindings, ok = portMap["81/tcp"]
+		if !ok || len(portMap["81/tcp"]) == 0 {
+			return fmt.Errorf("Port 81 on tcp is not set")
+		}
+
+		portBindingsLength = len(portBindings)
+		if portBindingsLength != 1 {
+			return fmt.Errorf("Expected 1 binding on port 81, but was %d", portBindingsLength)
+		}
+
+		if len(portBindings[0].HostIP) == 0 {
+			return fmt.Errorf("Expected host IP to be set, but was empty")
+		}
+
+		if len(portBindings[0].HostPort) == 0 {
+			return fmt.Errorf("Expected host port to be set, but was empty")
+		}
+
+		return nil
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccDockerContainerMultiplePortConfig,
+				Check: resource.ComposeTestCheckFunc(
+					testAccContainerRunning("docker_container.foo", &c),
+					testCheck,
+					resource.TestCheckResourceAttr("docker_container.foo", "name", "tf-test"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.#", "2"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.internal", "80"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.ip", "0.0.0.0"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.protocol", "tcp"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.0.external", "32787"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.1.internal", "81"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.1.ip", "0.0.0.0"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.1.protocol", "tcp"),
+					resource.TestCheckResourceAttr("docker_container.foo", "ports.1.external", "32788"),
 				),
 			},
 		},
@@ -508,6 +645,37 @@ func testAccContainerRunning(n string, container *types.ContainerJSON) resource.
 	}
 }
 
+func testValueHigherEqualThan(name, key string, value int) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		ms := s.RootModule()
+		rs, ok := ms.Resources[name]
+		if !ok {
+			return fmt.Errorf("Not found: %s", name)
+		}
+
+		is := rs.Primary
+		if is == nil {
+			return fmt.Errorf("No primary instance: %s", name)
+		}
+
+		vRaw, ok := is.Attributes[key]
+		if !ok {
+			return fmt.Errorf("%s: Attribute '%s' not found", name, key)
+		}
+
+		v, err := strconv.Atoi(vRaw)
+		if err != nil {
+			return fmt.Errorf("'%s' is not a number", vRaw)
+		}
+
+		if v < value {
+			return fmt.Errorf("'%v' is smaller than '%v', but was expected to be equal or greater", v, value)
+		}
+
+		return nil
+	}
+}
+
 const testAccDockerContainerConfig = `
 resource "docker_image" "foo" {
 	name = "nginx:latest"
@@ -658,6 +826,27 @@ resource "docker_container" "foo" {
 	}
 }
 `
+
+const testAccDockerContainerMultipleInternalPortConfig = `
+resource "docker_image" "foo" {
+	name = "nginx:latest"
+	keep_locally = true
+}
+
+resource "docker_container" "foo" {
+	name = "tf-test"
+	image = "${docker_image.foo.latest}"
+	
+	ports = [
+		{
+			internal = "80"
+		},
+		{
+			internal = "81"
+		}
+	]
+}
+`
 const testAccDockerContainerPortConfig = `
 resource "docker_image" "foo" {
 	name = "nginx:latest"
@@ -674,3 +863,25 @@ resource "docker_container" "foo" {
 	}
 }
 `
+const testAccDockerContainerMultiplePortConfig = `
+resource "docker_image" "foo" {
+	name = "nginx:latest"
+	keep_locally = true
+}
+
+resource "docker_container" "foo" {
+	name = "tf-test"
+	image = "${docker_image.foo.latest}"
+
+	ports = [
+		{
+			internal = "80"
+			external = "32787"
+		},
+		{
+			internal = "81"
+			external = "32788"
+		}
+	] 
+}
+`

scripts/runAccTests.sh

@@ -20,7 +20,7 @@ run() {
   TF_ACC=1 go test ./docker -v -timeout 120m
   
   # for a single test comment the previous line and uncomment the next line
-  #TF_LOG=INFO TF_ACC=1 go test -v github.com/terraform-providers/terraform-provider-docker/docker -run ^TestAccDockerService_full$ -timeout 360s
+  #TF_LOG=INFO TF_ACC=1 go test -v github.com/terraform-providers/terraform-provider-docker/docker -run ^TestAccDockerContainer_port$ -timeout 360s
   
   # keep the return value for the scripts to fail and clean properly
   return $?

website/docs/r/container.html.markdown

@@ -124,10 +124,10 @@ the port mappings of the container. Each `ports` block supports
 the following:
 
 * `internal` - (Required, int) Port within the container.
-* `external` - (Optional, int) Port exposed out of the container, defaults to `32768`.
+* `external` - (Optional, int) Port exposed out of the container. If not given a free random port `>= 32768` will be used.
 * `ip` - (Optional, string) IP address/mask that can access this port, default to `0.0.0.0`
 * `protocol` - (Optional, string) Protocol that can be used over this port,
-  defaults to TCP.
+  defaults to `tcp`.
 
 <a id="extra_hosts"></a>
 ### Extra Hosts