Allow Windows Docker containers to map volumes

[derBroBro]

As windows is also support for Docker now we made our first testes and noticed that the notation "C:.." is not accepted yet.
The current code is requiring absolute paths for volume mounts in the Linux notation ("/etc...").
To correct this behavior I changed the regex to accept also the Windows pattern.

• Unit tests: Not affected
• Documentation updates: Not affected
• Well-formed Code: Not affected

[tombuildsstuff]

Hi @derBroBro

Thanks for submitting this PR - it looks good to me :)

Given we're expanding the scope of the ValidateFunc method - would it be possible to add some tests covering some sample values for this method? Here's an example of how we're doing that for the Azure Redis resource's validation method

Thanks!

[derBroBro]

Hi @tombuildsstuff

as always it depends ;-)
But if I am not able to hopefully someone else can support. What do you have in mind in detail?

[tombuildsstuff]

Hi @derBroBro

No worries, so that we can test this functionality is supported on both platforms in the future - it'd be great to add some test cases to the regular expression used in the Validation method :)

The simplest way to do that is to refactor the Validation method out into a separate function - as can be seen in this example - and then add some tests covering both the existing use-case (as can be seen in this example) (i.e. Linux Paths) for example:

• /var/log
• /tmp

and then some new use-cases (i.e. some Windows paths) - for example:

• C:\Windows\System32
• C:\Program Files\MSBuild

Presuming you've named the validation method validateDockerContainerPath - the associated test-case for that would look something like this:

func TestAccDockerContainerPath_validation(t *testing.T) {
	cases := []struct {
		Value    string
		ErrCount int
	}{
		{Value: "/var/log", ErrCount: 0},
		{Value: "/tmp", ErrCount: 0},
		{Value: "C:\Windows\System32", ErrCount: 0},
		{Value: "C:\Program Files\MSBuild", ErrCount: 0},
		{Value: "", ErrCount: 1},
	}

	for _, tc := range cases {
		_, errors := validateDockerContainerPath(tc.Value, "docker_container")

		if len(errors) != tc.ErrCount {
			t.Fatalf("Expected the Docker Container Path to trigger a validation error")
		}
	}
}

You should then be able to run this test via:

make testacc TEST=./builtin/providers/docker TESTARGS='-run= TestAccDockerContainerPath_validation'

Thanks! :)

[stack72]

ping @derBroBro ;)

[derBroBro]

I hope the changes are what you looked for. 😃

Output:

[root@ip-172-31-18-193 terraform]# make testacc TEST=./builtin/providers/docker TESTARGS='-run= TestAccDockerContainerPath_validation'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/13 21:19:00 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/docker -v -run= TestAccDockerContainerPath_validation -timeout 120m
=== RUN   TestAccDockerRegistryImage_basic
--- PASS: TestAccDockerRegistryImage_basic (4.05s)
=== RUN   TestAccDockerRegistryImage_private
--- PASS: TestAccDockerRegistryImage_private (2.96s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccDockerContainer_basic
--- PASS: TestAccDockerContainer_basic (7.01s)
=== RUN   TestAccDockerContainerPath_validation
--- PASS: TestAccDockerContainerPath_validation (0.00s)
=== RUN   TestAccDockerContainer_volume
--- PASS: TestAccDockerContainer_volume (6.92s)
=== RUN   TestAccDockerContainer_customized
--- PASS: TestAccDockerContainer_customized (17.10s)
=== RUN   TestAccDockerContainer_upload
--- PASS: TestAccDockerContainer_upload (6.73s)
=== RUN   TestAccDockerImage_basic
--- PASS: TestAccDockerImage_basic (1.97s)
=== RUN   TestAccDockerImage_private
--- PASS: TestAccDockerImage_private (1.75s)
=== RUN   TestAccDockerImage_destroy
--- PASS: TestAccDockerImage_destroy (1.04s)
=== RUN   TestAccDockerImage_data
--- PASS: TestAccDockerImage_data (4.28s)
=== RUN   TestAccDockerImage_data_pull_trigger
--- PASS: TestAccDockerImage_data_pull_trigger (4.29s)
=== RUN   TestAccDockerNetwork_basic
--- PASS: TestAccDockerNetwork_basic (15.13s)
=== RUN   TestAccDockerNetwork_internal
--- PASS: TestAccDockerNetwork_internal (15.08s)
=== RUN   TestAccDockerVolume_basic
--- PASS: TestAccDockerVolume_basic (0.05s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/docker 88.356s

[root@ip-172-31-18-193 terraform]# docker version
Client:
 Version:      17.05.0-ce
 API version:  1.29
 Go version:   go1.7.5
 Git commit:   89658be
 Built:        Thu May  4 22:06:25 2017
 OS/Arch:      linux/amd64

Server:
 Version:      17.05.0-ce
 API version:  1.29 (minimum version 1.12)
 Go version:   go1.7.5
 Git commit:   89658be
 Built:        Thu May  4 22:06:25 2017
 OS/Arch:      linux/amd64
 Experimental: false

[stack72]

LGTM! Thanks for adding the tests :)

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.