Repository: kreuzwerker/terraform-provider-docker

Issue:
default value of `network_mode` changed when deploy from linux to a windows runner
<!---
Please note the following potential times when an issue might be in Terraform core:

* [Configuration Language](https://www.terraform.io/docs/configuration/index.html) or resource ordering issues
* [State](https://www.terraform.io/docs/state/index.html) and [State Backend](https://www.terraform.io/docs/backends/index.html) issues
* [Provisioner](https://www.terraform.io/docs/provisioners/index.html) issues
* [Registry](https://registry.terraform.io/) issues
* Spans resources across multiple providers

If you are running into one of these scenarios, we recommend opening an issue in the [Terraform core repository](https://github.com/hashicorp/terraform/) instead.
--->

<!--- Please keep this note for the community --->

### Community Note

* Please vote on this issue by adding a 👍 [reaction](https://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/) to the original issue to help the community and maintainers prioritize this request
* Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
* If you are interested in working on this issue or have submitted a pull request, please leave a comment

<!--- Thank you for keeping this note for the community --->

### Terraform (and docker Provider) Version

<!--- Please run `terraform -v` to show the Terraform core version and provider version(s). If you are not running the latest version of Terraform or the provider, please upgrade because your issue may have already been fixed. [Terraform documentation on provider versioning](https://www.terraform.io/docs/configuration/providers.html#provider-versions). --->

- Terraform: v1.11.4
- Provider: 3.4.0

### Affected Resource(s)

<!--- Please list the affected resources and data sources. --->

* `docker_container`

### Terraform Configuration Files

<!--- Information about code formatting: https://help.github.com/articles/basic-writing-and-formatting-syntax/#quoting-code --->

```hcl
terraform {
  required_providers {
    docker = {
      source = "kreuzwerker/docker"
      version = "3.4.0"
    }
  }
}

variable "username" {
  type = string
}

variable "hostname" {
  type = string
}

variable "docuum_threshold" {
  type    = string
  default = "30 GB"
}

provider "docker" {
  host = "ssh://${var.username}@${var.hostname}"
}

locals {
  image_docuum = "ghcr.io/visualon/docuum:0.25.0@sha256:0ffa6174581b49d6886a807cac5f7477c70813d17696ef116b1357885b0c0f31" # renovate: depName=docuum
}

resource "docker_image" "docuum" {
  name = replace(local.image_docuum, "ghcr.io", "ghcr.visualon.de")
}

resource "docker_container" "docuum" {
  name    = "docuum"
  image   = docker_image.docuum.image_id
  restart = "unless-stopped"

  command = ["--threshold", var.docuum_threshold]

  env = [
    "TZ=Europe/Berlin"
  ]

  volumes {
    container_path = "//./pipe/docker_engine/"
    host_path      = "//./pipe/docker_engine/"
  }

  volumes {
    container_path = "C:\\Users\\ContainerAdministrator\\AppData\\Local\\docuum"
    volume_name    = docker_volume.docuum_data.name
  }

  // network_mode = "nat" // defaults to bridge on windows since provider v3.1 or v3.2
}

resource "docker_volume" "docuum_data" {
  name = "docuum-data"
}
```

### Debug Output

<!---
Please provide a link to a GitHub Gist containing the complete debug output. Please do NOT paste the debug output in the issue; just paste a link to the Gist.

To obtain the debug output, see the [Terraform documentation on debugging](https://www.terraform.io/docs/internals/debugging.html).
--->

no debug logs

### Panic Output

<!--- If Terraform produced a panic, please provide a link to a GitHub Gist containing the output of the `crash.log`. --->

### Expected Behaviour

<!--- What should have happened? --->
It should deploy the container to a windows docker instance.

### Actual Behaviour

<!--- What actually happened? --->
It fails to start the container because it's trying to use bridge network mode, which isn't available on windows.

### Steps to Reproduce

<!--- Please list the steps required to reproduce the issue. --->

1. `terraform apply`

### Important Factoids

<!--- Are there anything atypical about your setup that we should know? For example: Running the docker daemon with TLS? --->

Terraform is running inside a linux docker container and connecting to the windows node via ssh.
The ssh key is preloaded to ssh agent.

My workaround is to set `network_mode = "nat"` explicit.

### References

<!---
Information about referencing Github Issues: https://help.github.com/articles/basic-writing-and-formatting-syntax/#referencing-issues-and-pull-requests

Are there any other GitHub issues (open or closed) or pull requests that should be linked here? Such as vendor documentation?
--->

* #603


Discussion:
@Junkern: I have made a PR which adds a warning to your terraform output when you are using a non-linux host and the value is set to `bridge`.
A proper fix would need a major release, which I would like to avoid for the next couple of releases. But it is on my list :)@Junkern: Closed with #773 

Solution patch:
diff --git a/docs/resources/container.md b/docs/resources/container.md
index d9750f2..96ee4f2 100644
--- a/docs/resources/container.md
+++ b/docs/resources/container.md
@@ -67,10 +67,11 @@ resource "docker_image" "ubuntu" {
 - `logs` (Boolean) Save the container logs (`attach` must be enabled). Defaults to `false`.
 - `max_retry_count` (Number) The maximum amount of times to an attempt a restart when `restart` is set to 'on-failure'.
 - `memory` (Number) The memory limit for the container in MBs.
+- `memory_reservation` (Number) The memory-resveration for the container in MBs. Defaults to 0. Allows you to specify a soft limit smaller than `memory` which is activated when Docker detects contention or low memory on the host machine. If you use `memory-reservation`, it must be set lower than `memory` for it to take precedence. Because it is a soft limit, it doesn't guarantee that the container doesn't exceed the limit.
 - `memory_swap` (Number) The total memory limit (memory + swap) for the container in MBs. This setting may compute to `-1` after `terraform apply` if the target host doesn't support memory swap, when that is the case docker will use a soft limitation.
 - `mounts` (Block Set) Specification for mounts to be added to containers created as part of the service. (see [below for nested schema](#nestedblock--mounts))
 - `must_run` (Boolean) If `true`, then the Docker container will be kept running. If `false`, then as long as the container exists, Terraform assumes it is successful. Defaults to `true`.
-- `network_mode` (String) Network mode of the container. See https://docs.docker.com/engine/network/ for more information.
+- `network_mode` (String) Network mode of the container. Defaults to `bridge`. If your host OS is any other OS, you need to set this value explicitly, e.g. `nat` when your container will be running on an Windows host. See https://docs.docker.com/engine/network/ for more information.
 - `networks_advanced` (Block Set) The networks the container is attached to (see [below for nested schema](#nestedblock--networks_advanced))
 - `pid_mode` (String) he PID (Process) Namespace mode for the container. Either `container:<name|id>` or `host`.
 - `ports` (Block List) Publish a container's port(s) to the host. (see [below for nested schema](#nestedblock--ports))
diff --git a/internal/provider/resource_docker_container.go b/internal/provider/resource_docker_container.go
index 5f3d7d4..6b1d5bd 100644
--- a/internal/provider/resource_docker_container.go
+++ b/internal/provider/resource_docker_container.go
@@ -684,6 +684,13 @@ func resourceDockerContainer() *schema.Resource {
 				Optional:         true,
 				ValidateDiagFunc: validateIntegerGeqThan(0),
 			},
+			"memory_reservation": {
+				Type:             schema.TypeInt,
+				Description:      "The memory-resveration for the container in MBs. Defaults to 0. Allows you to specify a soft limit smaller than `memory` which is activated when Docker detects contention or low memory on the host machine. If you use `memory-reservation`, it must be set lower than `memory` for it to take precedence. Because it is a soft limit, it doesn't guarantee that the container doesn't exceed the limit.",
+				Optional:         true,
+				ValidateDiagFunc: validateIntegerGeqThan(0),
+				Default:          0,
+			},
 
 			"memory_swap": {
 				Type:             schema.TypeInt,
@@ -732,7 +739,7 @@ func resourceDockerContainer() *schema.Resource {
 
 			"network_mode": {
 				Type:        schema.TypeString,
-				Description: "Network mode of the container. See https://docs.docker.com/engine/network/ for more information.",
+				Description: "Network mode of the container. Defaults to `bridge`. If your host OS is any other OS, you need to set this value explicitly, e.g. `nat` when your container will be running on an Windows host. See https://docs.docker.com/engine/network/ for more information.",
 				Optional:    true,
 				ForceNew:    true,
 				Default:     "bridge",
diff --git a/internal/provider/resource_docker_container_funcs.go b/internal/provider/resource_docker_container_funcs.go
index 809ac21..8860f0e 100644
--- a/internal/provider/resource_docker_container_funcs.go
+++ b/internal/provider/resource_docker_container_funcs.go
@@ -311,6 +311,10 @@ func resourceDockerContainerCreate(ctx context.Context, d *schema.ResourceData,
 		hostConfig.Memory = int64(v.(int)) * 1024 * 1024
 	}
 
+	if v, ok := d.GetOk("memory_reservation"); ok {
+		hostConfig.MemoryReservation = int64(v.(int)) * 1024 * 1024
+	}
+
 	if v, ok := d.GetOk("memory_swap"); ok {
 		swap := int64(v.(int))
 		if swap > 0 {
@@ -362,6 +366,11 @@ func resourceDockerContainerCreate(ctx context.Context, d *schema.ResourceData,
 
 	networkingConfig := &network.NetworkingConfig{}
 	if v, ok := d.GetOk("network_mode"); ok {
+		// if the client OS is any other than Linux and the value is "bridge", print a warning
+		info, _ := client.Info(ctx)
+		if v.(string) == "bridge" && !strings.Contains(info.OSType, "linux") {
+			log.Printf("[WARN] You are using a non-Linux host OS, but the network_mode is set to 'bridge'. This may not work as expected. Please set the network_mode explicitly to 'nat' or another appropriate value for your host OS.")
+		}
 		hostConfig.NetworkMode = container.NetworkMode(v.(string))
 	}
 
@@ -783,6 +792,13 @@ func resourceDockerContainerRead(ctx context.Context, d *schema.ResourceData, me
 	}
 	// "destroy_grace_seconds" can't be imported
 	d.Set("memory", container.HostConfig.Memory/1024/1024)
+
+	if container.HostConfig.MemoryReservation > 0 {
+		d.Set("memory_reservation", container.HostConfig.MemoryReservation/1024/1024)
+	} else {
+		d.Set("memory_reservation", container.HostConfig.MemoryReservation)
+	}
+
 	if container.HostConfig.MemorySwap > 0 {
 		d.Set("memory_swap", container.HostConfig.MemorySwap/1024/1024)
 	} else {
@@ -884,7 +900,7 @@ func resourceDockerContainerReadRefreshFunc(ctx context.Context,
 
 func resourceDockerContainerUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	attrs := []string{
-		"restart", "max_retry_count", "cpu_shares", "memory", "cpu_set", "memory_swap",
+		"restart", "max_retry_count", "cpu_shares", "memory", "memory_reservation", "cpu_set", "memory_swap",
 	}
 	for _, attr := range attrs {
 		if d.HasChange(attr) {
@@ -904,9 +920,10 @@ func resourceDockerContainerUpdate(ctx context.Context, d *schema.ResourceData,
 					MaximumRetryCount: d.Get("max_retry_count").(int),
 				},
 				Resources: container.Resources{
-					CPUShares:  int64(d.Get("cpu_shares").(int)),
-					Memory:     int64(d.Get("memory").(int)) * 1024 * 1024,
-					CpusetCpus: d.Get("cpu_set").(string),
+					CPUShares:         int64(d.Get("cpu_shares").(int)),
+					Memory:            int64(d.Get("memory").(int)) * 1024 * 1024,
+					MemoryReservation: int64(d.Get("memory_reservation").(int)) * 1024 * 1024,
+					CpusetCpus:        d.Get("cpu_set").(string),
 					// Ulimits:    ulimits,
 				},
 			}



Test patch:
diff --git a/internal/provider/resource_docker_container_test.go b/internal/provider/resource_docker_container_test.go
index c32e62d..807f326 100644
--- a/internal/provider/resource_docker_container_test.go
+++ b/internal/provider/resource_docker_container_test.go
@@ -496,6 +496,9 @@ func TestAccDockerContainer_customized(t *testing.T) {
 		if c.HostConfig.Memory != (512 * 1024 * 1024) {
 			return fmt.Errorf("Container has wrong memory setting: %d", c.HostConfig.Memory)
 		}
+		if c.HostConfig.MemoryReservation != (256 * 1024 * 1024) {
+			return fmt.Errorf("Container has wrong memory-reservation setting: %d", c.HostConfig.MemoryReservation)
+		}
 
 		if c.HostConfig.MemorySwap != (2048 * 1024 * 1024) {
 			return fmt.Errorf("Container has wrong memory swap setting: %d\n\r\tPlease check that you machine supports memory swap (you can do that by running 'docker info' command).", c.HostConfig.MemorySwap)
diff --git a/testdata/resources/docker_container/testAccDockerContainerCustomizedConfig.tf b/testdata/resources/docker_container/testAccDockerContainerCustomizedConfig.tf
index 1e7532d..91bb3c4 100644
--- a/testdata/resources/docker_container/testAccDockerContainerCustomizedConfig.tf
+++ b/testdata/resources/docker_container/testAccDockerContainerCustomizedConfig.tf
@@ -11,6 +11,7 @@ resource "docker_container" "foo" {
   destroy_grace_seconds = 10
   max_retry_count       = 5
   memory                = 512
+  memory_reservation    = 256
   shm_size              = 128
   memory_swap           = 2048
   cpu_shares            = 32