Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Change Docker ENV behavior #407

Merged
merged 4 commits into from
Nov 11, 2015
Merged

Change Docker ENV behavior #407

merged 4 commits into from
Nov 11, 2015

Conversation

cbednarski
Copy link
Contributor

  • Docker ENV variables now work the same way in production, dev, and test
  • Docker ENV variables are ignored if docker.endpoint is present in the Nomad config file
  • Remote tests now work correctly
  • Docker fingerprints correctly on Windows

Fixes #295

@cbednarski
Changed behavior for Docker ENV
acb8f1d 
- Docker ENV variables now work the same way in production, dev, and test
- Docker ENV variables are *ignored* if docker.endpoint is present in the Nomad config file
- Remote tests now work correctly
@cbednarski
Change Docker fingerprinter to INFO and not error when the connection…
4b071f6 
… to the daemon fails; we simply assume docker isn't there.
dadgar
dadgar reviewed Nov 11, 2015
View reviewed changes
client/driver/docker_test.go
@@ -147,7 +172,7 @@ func TestDockerDriver_Start_Wait(t *testing.T) {
}

func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) {
if !dockerLocated() {
if !dockerIsConnected() || dockerIsRemote() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need the || dockerIsRemote()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails with a remote docker host because the alloc dir cannot be mounted into the remote daemon.

Since remote docker is only supported for dev/test I think the skip is OK here.

@dadgar
Copy link
Contributor

dadgar commented Nov 11, 2015

LGTM

@dadgar
Copy link
Contributor

dadgar commented Nov 11, 2015

Ah I see now why you can't just skip in dockerIs methods. Looks good

@diptanu
Copy link
Contributor

diptanu commented Nov 11, 2015

LGTM

cbednarski added a commit that referenced this pull request Nov 11, 2015
@cbednarski
Merge pull request #407 from hashicorp/f-docker-host-env
33d281f 
Change Docker ENV behavior
@cbednarski cbednarski merged commit 33d281f into master Nov 11, 2015
@cbednarski cbednarski deleted the f-docker-host-env branch November 11, 2015 03:30
benbuzbee pushed a commit to benbuzbee/nomad that referenced this pull request Jul 21, 2022
@alwindoss
Removed the travis-ci build status label (hashicorp#407)
5927dcd 
Signed-off-by: Alwin Doss <[email protected]>
@github-actions
Copy link

github-actions bot commented May 3, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cbednarski @dadgar @diptanu