Allow users to set TMPDIR environment #5412

rhatdan · 2020-03-06T12:45:59Z

Some users have small /var/tmp directories and need to be able to specify a different location
for temporary files, which includes more space.

Fixes: #5411

Signed-off-by: Daniel J Walsh [email protected]

openshift-ci-robot · 2020-03-06T12:47:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

karlyeurl · 2020-03-06T13:34:29Z

Should cmd/podman/load.go line 78 be also adjusted?

Some users have small /var/tmp directories and need to be able to specify a different location for temporary files, which includes more space. Signed-off-by: Daniel J Walsh <[email protected]>

rhatdan · 2020-03-06T15:22:11Z

@karlyeurl Nice catch, I missed that one.

mheon · 2020-03-06T15:23:27Z

LGTM

TomSweeneyRedHat · 2020-03-06T21:48:00Z

pkg/util/utils.go

+	}
+
+	return tmpdir
+}


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

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.

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

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?

@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.

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.

TomSweeneyRedHat · 2020-03-07T16:19:19Z

/lgtm
/hold

rhatdan · 2020-03-08T16:01:42Z

/hold cancel

gcs278 · 2020-12-03T23:57:13Z

Hey guys,

Just curious - is there any way to set this via configuration files? Our /var/tmp is really small, and it seems a little sketchy adding TMPDIR to every scope (shell/scripts) that podman is running in. It appears the libpod.conf tmp_dir is not the same as TMPDIR which isn't the same as --tmpdir (--tmpdir != $TMPDIR != tmp_dir in libpod.conf).

Just seems like something you'd should be able to control via storage.conf or containers.conf. Our podman solution is looking a little messy with configuration files (storage.conf), environment variables (TMPDIR), some runtime arguments (--tmpdir) that we have to maintain.

Thoughts?

Thanks,

Grant

TomSweeneyRedHat · 2020-12-04T15:28:00Z

@rhatdan would this line in /etc/containers/containers.conf do the trick?

# Directory for temporary files. Must be tmpfs (wiped after reboot)
tmp_dir = "/var/run/libpod"

mheon · 2020-12-04T16:42:48Z

No, that's the libpod tmpdir, which is distinct.

mheon · 2020-12-04T16:44:25Z

However, there is a field for setting environment of the runtime in containers.conf.

# Environment variables to be used when running the container engine (e.g., Podman, Buildah).
# For example "http_proxy=internal.proxy.company.com".
# Note these environment variables will not be used within the container.
# Set the env section under [containers] table, if you want to set environment variables for the container.
# env = []

Setting that would let you set the TMPDIR environment variable for all calls to Podman/Buildah.

rhatdan · 2020-12-04T23:34:51Z

Correct
[engine]
env=["TMPDIR=/mybigdir"]

Should work.

saper · 2020-12-05T10:21:02Z

Ran into this running podman-1.9.3-2.module+el8.2.1+6867+366c07d6.x86_64 from fresh RHEL 8.2 - huge images did not fit into /var/tmp despite TMPDIR was set.

Copying blob 01baf108b52c done
  write /var/tmp/storage651390928/5: no space left on device
Error: error pulling image "docker-virtual.production.xxx/base:0.15": unable to pull docker-virtual.production.xxx/base:0.15: unable to pull image: Eror writing blob: error storing blob to file "/var/tmp/storage651390928/5": write /var/tmp/storage651390928/5: no space left on device

Should https://github.com/containers/storage also be changed?

rhatdan · 2020-12-05T13:52:31Z

This might be fixed in newer versions of Podman.

The containers.conf should not be an issue, if you can trigger this by doing

TMPDIR=/BIGDIR podman ...

That means that the code is not properly telling containers image where to store it's blobs.

saper · 2020-12-05T14:00:41Z

I had TMPDIR set to a large directory in the environment while invoking podman pull, that's the point. I have installed the update from Redhat on Dec 3rd, so it should be pretty up to date as far as 1.9.x is concerned.

rhatdan · 2020-12-07T21:27:01Z

Right, I am not sure if this was fixed in podman 1.9. Podman 2.* should have fixes for this though.

gcs278 · 2020-12-14T19:52:04Z

Correct
[engine]
env=["TMPDIR=/mybigdir"]

Should work.

Not sure if this is what @saper was saying - couldn't tell if @saper was were setting via env containers.conf, but the above doesn't work for me. Doing TMPDIR=<TMPDIR> podman images does indeed work for me. I think [engine] should be [containers], but I tried that and it didn't work either. AKA I can't get it to work via containers.conf.

Still got errors about /var/tmp.
podman version 2.0.5

rhatdan · 2020-12-15T11:23:21Z

It should definitely be [engine] not [containers]
[containers] will add the environment variable to the container, but not effect he engine launching the container.

rhatdan · 2020-12-15T11:27:12Z

This code is in the main function of podman.

	for _, env := range cfg.Engine.Env {
		splitEnv := strings.SplitN(env, "=", 2)
		if len(splitEnv) != 2 {
			return fmt.Errorf("invalid environment variable for engine %s, valid configuration is KEY=value pair", env)
		}
		// skip if the env is already defined
		if _, ok := os.LookupEnv(splitEnv[0]); ok {
			logrus.Debugf("environment variable %s is already defined, skip the settings from containers.conf", splitEnv[0])
			continue
		}
		if err := os.Setenv(splitEnv[0], splitEnv[1]); err != nil {
			return err
		}
	}

Which should be setting the environment variable.

rhatdan · 2020-12-15T11:38:16Z

Ok I found the bug.

gcs278 · 2021-01-12T02:54:20Z

Thanks Dan.

Another comment here is that - aren't we overloading the environment variable TMPDIR? I know once this fix goes through, I can set it selectively through containers.conf, but at the same time, I broke stuff by setting $TMPDIR in my bashrc. TMPDIR is used by the mktemp utility.

Why can't we have a new variable named like PODMAN_TMPDIR to avoid name conflicts since they do fundamentally different things? Or what this by design?

rhatdan · 2021-01-12T12:15:32Z

Well there are other places within GOLANG that will use TMPDIR, which makes having a different environment variable potentially confusing.

openshift-ci-robot added the size/S label Mar 6, 2020

openshift-ci-robot requested review from TomSweeneyRedHat and vrothberg March 6, 2020 12:47

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 6, 2020

Allow users to set TMPDIR environment

cb51707

Some users have small /var/tmp directories and need to be able to specify a different location for temporary files, which includes more space. Signed-off-by: Daniel J Walsh <[email protected]>

rhatdan force-pushed the tmpdir branch from d044511 to cb51707 Compare March 6, 2020 15:21

TomSweeneyRedHat reviewed Mar 6, 2020

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 7, 2020

openshift-ci-robot assigned TomSweeneyRedHat Mar 7, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 7, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 8, 2020

openshift-merge-robot merged commit f378e82 into containers:master Mar 8, 2020

rhatdan mentioned this pull request Dec 15, 2020

Allow users to specify TMPDIR in containers.conf #8725

Merged

mtrmac mentioned this pull request Sep 7, 2021

podman should use graphRoot/tmp for temporary image storage #11107

Closed

Bill0412 mentioned this pull request Aug 18, 2022

set podman image storage location #1916

Closed

msbrod mentioned this pull request Jan 11, 2023

[Bug]: Podman loads but does not remove temporary docker-tar#### files from under tmpdir (e.g. /var/tmp/) #17078

Closed

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow users to set TMPDIR environment #5412

Allow users to set TMPDIR environment #5412

rhatdan commented Mar 6, 2020

openshift-ci-robot commented Mar 6, 2020

karlyeurl commented Mar 6, 2020

rhatdan commented Mar 6, 2020

mheon commented Mar 6, 2020

TomSweeneyRedHat Mar 6, 2020

rhatdan Mar 7, 2020

rhatdan Mar 7, 2020

TomSweeneyRedHat Mar 7, 2020

kousu Jul 16, 2021

TomSweeneyRedHat Jul 16, 2021

kousu Jul 16, 2021 •

edited

Loading

TomSweeneyRedHat commented Mar 7, 2020

rhatdan commented Mar 8, 2020

gcs278 commented Dec 3, 2020 •

edited

Loading

TomSweeneyRedHat commented Dec 4, 2020

mheon commented Dec 4, 2020

mheon commented Dec 4, 2020

rhatdan commented Dec 4, 2020

saper commented Dec 5, 2020

rhatdan commented Dec 5, 2020

saper commented Dec 5, 2020

rhatdan commented Dec 7, 2020

gcs278 commented Dec 14, 2020 •

edited

Loading

rhatdan commented Dec 15, 2020

rhatdan commented Dec 15, 2020

rhatdan commented Dec 15, 2020

gcs278 commented Jan 12, 2021

rhatdan commented Jan 12, 2021

Allow users to set TMPDIR environment #5412

Allow users to set TMPDIR environment #5412

Conversation

rhatdan commented Mar 6, 2020

openshift-ci-robot commented Mar 6, 2020

karlyeurl commented Mar 6, 2020

rhatdan commented Mar 6, 2020

mheon commented Mar 6, 2020

TomSweeneyRedHat Mar 6, 2020

Choose a reason for hiding this comment

rhatdan Mar 7, 2020

Choose a reason for hiding this comment

rhatdan Mar 7, 2020

Choose a reason for hiding this comment

TomSweeneyRedHat Mar 7, 2020

Choose a reason for hiding this comment

kousu Jul 16, 2021

Choose a reason for hiding this comment

TomSweeneyRedHat Jul 16, 2021

Choose a reason for hiding this comment

kousu Jul 16, 2021 • edited Loading

Choose a reason for hiding this comment

TomSweeneyRedHat commented Mar 7, 2020

rhatdan commented Mar 8, 2020

gcs278 commented Dec 3, 2020 • edited Loading

TomSweeneyRedHat commented Dec 4, 2020

mheon commented Dec 4, 2020

mheon commented Dec 4, 2020

rhatdan commented Dec 4, 2020

saper commented Dec 5, 2020

rhatdan commented Dec 5, 2020

saper commented Dec 5, 2020

rhatdan commented Dec 7, 2020

gcs278 commented Dec 14, 2020 • edited Loading

rhatdan commented Dec 15, 2020

rhatdan commented Dec 15, 2020

rhatdan commented Dec 15, 2020

gcs278 commented Jan 12, 2021

rhatdan commented Jan 12, 2021

kousu Jul 16, 2021 •

edited

Loading

gcs278 commented Dec 3, 2020 •

edited

Loading

gcs278 commented Dec 14, 2020 •

edited

Loading