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.

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

Create docker temp files under packer.d when TMPDIR is not set #2846

Merged
merged 2 commits into from
Oct 27, 2015

Conversation

markpeek
Copy link
Collaborator

Packer uses a docker bind mount to upload files into the docker image via a temp directory. When run on a Mac with docker-machine (using vmware/virtualbox) the VM will only mount the /Users directory by default. This causes mounts using the temp directory to silently fail and cause packer to hang waiting for the Upload to finish which can't succeed.

A previous PR #2807 attempted to fix this by creating the temp dir in the CWD of the packer invocation. This caused an issue when a provisioner tried uploading "." and it recursively tried to copy up the intermediate upload directory.

This PR now creates the intermediate upload directory under the packer configuration directory (~/.packer.d/tmp). The ConfigDir()/ConfigFile() code has been moved from package main to package packer to allow the docker code to call it.

Note: the real fix for this issue is to switch to using the latest docker API directly suck as through go-dockerclient. However, this would require upgrading the minimum requirement for docker to version 1.8 which would need consensus before moving forward.

var err error
var td string
tempenv := "TMPDIR"
if runtime.GOOS == "windows" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to change the environment variable based on OS? Could we not use something like PACKER_TMP_DIR on all OSes?

@markpeek
Copy link
Collaborator Author

@cbednarski Thanks for the feedback. I pushed a new version that incorporates changes based on your review.

tempenv := "TMPDIR"
if runtime.GOOS == "windows" {
tempenv = "TMP"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this OS switch anymore

@cbednarski
Copy link
Contributor

Looks good! I think the implementation in ConfigTmpDir is clean and well-compartmentalized.

The temporary directories will be created under the packer config
directory. Setting PACKER_TMP_DIR will override this path.
markpeek added a commit that referenced this pull request Oct 27, 2015
Create docker temp files under packer.d when TMPDIR is not set
@markpeek markpeek merged commit ba7814b into hashicorp:master Oct 27, 2015
@ghost ghost locked and limited conversation to collaborators Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants