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

Allow users to set TMPDIR environment #5412

Merged
merged 1 commit into from
Mar 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmd/podman/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/containers/libpod/cmd/podman/cliconfig"
"github.com/containers/libpod/cmd/podman/shared/parse"
"github.com/containers/libpod/pkg/adapter"
"github.com/containers/libpod/pkg/util"
"github.com/pkg/errors"
"github.com/spf13/cobra"
"golang.org/x/crypto/ssh/terminal"
Expand Down Expand Up @@ -75,7 +76,7 @@ func loadCmd(c *cliconfig.LoadValues) error {
if terminal.IsTerminal(int(os.Stdin.Fd())) {
return errors.Errorf("cannot read from terminal. Use command-line redirection or the --input flag.")
}
outFile, err := ioutil.TempFile("/var/tmp", "podman")
outFile, err := ioutil.TempFile(util.Tmpdir(), "podman")
if err != nil {
return errors.Errorf("error creating file %v", err)
}
Expand Down
2 changes: 2 additions & 0 deletions docs/source/markdown/podman-create.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -1066,6 +1066,8 @@ b
**/etc/subuid**
**/etc/subgid**

NOTE: Use the environment variable `TMPDIR` to change the temporary storage location of downloaded container images. Podman defaults to use `/var/tmp`.

## SEE ALSO
subgid(5), subuid(5), libpod.conf(5), systemd.unit(5), setsebool(8), slirp4netns(1), fuse-overlayfs(1)

Expand Down
2 changes: 2 additions & 0 deletions docs/source/markdown/podman-load.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ Read from archive file, default is STDIN.

The remote client requires the use of this option.

NOTE: Use the environment variable `TMPDIR` to change the temporary storage location of container images. Podman defaults to use `/var/tmp`.

**--quiet**, **-q**

Suppress the progress output
Expand Down
2 changes: 2 additions & 0 deletions docs/source/markdown/podman-pull.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ Storing signatures

registries.conf is the configuration file which specifies which container registries should be consulted when completing image names which do not include a registry or domain portion.

NOTE: Use the environment variable `TMPDIR` to change the temporary storage location of downloaded container images. Podman defaults to use `/var/tmp`.

## SEE ALSO
podman(1), podman-push(1), podman-login(1), containers-registries.conf(5)

Expand Down
2 changes: 2 additions & 0 deletions docs/source/markdown/podman-run.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -1335,6 +1335,8 @@ b

**/etc/subgid**

NOTE: Use the environment variable `TMPDIR` to change the temporary storage location of downloaded container images. Podman defaults to use `/var/tmp`.

## SEE ALSO
**subgid**(5), **subuid**(5), **libpod.conf**(5), **systemd.unit**(5), **setsebool**(8), **slirp4netns**(1), **fuse-overlayfs**(1).

Expand Down
8 changes: 4 additions & 4 deletions libpod/runtime_img.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,11 @@ func (r *Runtime) Import(ctx context.Context, source string, reference string, c
}

// donwloadFromURL downloads an image in the format "https:/example.com/myimage.tar"
// and temporarily saves in it /var/tmp/importxyz, which is deleted after the image is imported
// and temporarily saves in it $TMPDIR/importxyz, which is deleted after the image is imported
func downloadFromURL(source string) (string, error) {
fmt.Printf("Downloading from %q\n", source)

outFile, err := ioutil.TempFile("/var/tmp", "import")
outFile, err := ioutil.TempFile(util.Tmpdir(), "import")
if err != nil {
return "", errors.Wrap(err, "error creating file")
}
Expand All @@ -234,9 +234,9 @@ func downloadFromURL(source string) (string, error) {
}

// DownloadFromFile reads all of the content from the reader and temporarily
// saves in it /var/tmp/importxyz, which is deleted after the image is imported
// saves in it $TMPDIR/importxyz, which is deleted after the image is imported
func DownloadFromFile(reader *os.File) (string, error) {
outFile, err := ioutil.TempFile("/var/tmp", "import")
outFile, err := ioutil.TempFile(util.Tmpdir(), "import")
if err != nil {
return "", errors.Wrap(err, "error creating file")
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/util/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,3 +600,12 @@ func HomeDir() (string, error) {
}
return home, nil
}

func Tmpdir() string {
tmpdir := os.Getenv("TMPDIR")
if tmpdir == "" {
tmpdir = "/var/tmp"
}

return tmpdir
}
Copy link
Member

Choose a reason for hiding this comment

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

Does this cause the --tmpdir global option to be ignored?

Copy link
Member Author

Choose a reason for hiding this comment

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

No.

Copy link
Member Author

Choose a reason for hiding this comment

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

There were already being ignored.

Man page for podman documents this.

--tmpdir

Path to the tmp directory, for libpod runtime content.

NOTE --tmpdir is not used for the temporary storage of downloaded images. Use the environment variable TMPDIR to change the temporary storage location of downloaded container images. Podman defaults to use /var/tmp.

Copy link
Member

Choose a reason for hiding this comment

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

My bad, I'd pulled up the man page and in my rush neglected to read the note. Ugh.
LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi! Thanks for making this change. I was surprised to see my /var filling up when I thought I had locked it down to only daemons.

It's unfortunate this is inconsistent with mktemp which defaults to /tmp when TMPDIR is unset. I don't suppose you would consider making /tmp the default?

Copy link
Member

Choose a reason for hiding this comment

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

@kouso unless my memory banks have fogged over, I believe we originally had /tmp as the default several versions ago, but ran into a lot of issues with container builds not working as there wasn't enough space.

Copy link
Contributor

@kousu kousu Jul 16, 2021

Choose a reason for hiding this comment

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

Interesting! Well, I suppose it depends on your setup. On my build system, /tmp is larger than /var.

I can just set up a /etc/profile.d/ file to get mktemp and podman to agree, so it's not a big deal. Thanks for taking the time to fill me in.