-
Notifications
You must be signed in to change notification settings - Fork 18.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
integration-cli: Preserve DOCKER_TEST_HOST in env-clearing tests #10800
Conversation
@@ -863,6 +863,10 @@ func TestRunEnvironmentErase(t *testing.T) { | |||
// the container | |||
cmd := exec.Command(dockerBinary, "run", "-e", "FOO", "-e", "HOSTNAME", "busybox", "env") | |||
cmd.Env = []string{} | |||
if os.Getenv("DOCKER_HOST") != "" { // retain DOCKER_TEST_HOST | |||
cmd.Env = append(cmd.Env, fmt.Sprintf("DOCKER_HOST=%s", os.Getenv("DOCKER_HOST"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just do:
cmd.Env = []string{
fmt.Sprintf("DOCKER_HOST=%s", daemonHost()),
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will include an extra unix://var/run/docker.sock even though it's not needed by default. I'm not sure if that's the desired behavior.
For Windows, we run integration-cli with DOCKER_TEST_HOST env var b/c daemon is on some remote machine. This keeps the DOCKER_HOST set by bash scripts in the env. Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
+1 LGTM This approach will let us add TLS support to our test suite more easily later, too. |
LGTM! |
integration-cli: Preserve DOCKER_TEST_HOST in env-clearing tests
For Windows, we run integration-cli with DOCKER_TEST_HOST env var b/c
daemon is on some remote machine. This keeps the DOCKER_HOST set by
bash scripts in the env.
Signed-off-by: Ahmet Alp Balkan ahmetalpbalkan@gmail.com
Label:
#windows
Cc: @tianon @unclejack @duglin @jfrazelle @icecrime