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

debian: use tmpfs on /tmp + bump /tmp size on fedora #351

Merged
merged 4 commits into from
May 13, 2024

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Apr 30, 2024

To make tests faster setup a tmpfs on /tmp like fedora does so that test
do not have to write everything onto persistent disk.

Fixes #350

Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

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

Suggestions inline, but not a real review because I'm not 100% sure how/if these will work (e.g., if debian already has a /tmp fstab entry, and other logistics).

Finally: the process for working in this repo is:

$ make IMG_SFX
$ git add IMG_SFX  <------- you have to commit it every single time, on every push

This is so incredibly annoying, that I encourage you to save this into .git/hooks/pre-push:

#!/bin/bash
#
# 2024-01-25 esm
#

# FIXME: figure out if this is a PR push or a tag push
#
imgsfx=$(<IMG_SFX)

imgsfx_history=.imgsfx.history
if [[ -e $imgsfx_history ]]; then
    if grep -q "$imgsfx" $imgsfx_history; then
        echo "FATAL: $imgsfx has already been used" >&2
        echo "Please rerun 'make IMG_SFX'" >&2
        exit 1
    fi
fi

echo $imgsfx >>$imgsfx_history

# they don't have to write to persistent disk.
# https://github.com/containers/podman/pull/22533
# We could create a systemd mount unit for it but appending fstab is easier.
$SUDO sh -c 'echo "tmpfs /tmp tmpfs size=75%,mode=1777 0 0" >> /etc/fstab'
Copy link
Member

Choose a reason for hiding this comment

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

I think the standard way to do this is via $SUDO tee -a:

echo "tmpfs ...." | $SUDO tee -a /etc/fstab

# Make /tmp tmpfs bigger, by default we only get 50%. Bump it to 75% so the tests have more storage.
# Do not use 100% so we do not run out of memory for the process itself if tests start leaking big
# files on /tmp.
$SUDO mkdir /etc/systemd/system/tmp.mount.d
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: mkdir -p, so command won't fail if (some day) that directory already exists

# Do not use 100% so we do not run out of memory for the process itself if tests start leaking big
# files on /tmp.
$SUDO mkdir /etc/systemd/system/tmp.mount.d
$SUDO sh -c 'echo -e "[Mount]\nOptions=size=75%%,mode=1777\n" > /etc/systemd/system/tmp.mount.d/override.conf'
Copy link
Member

Choose a reason for hiding this comment

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

Same tee suggestion as above.

@edsantiago
Copy link
Member

Also, did you look into bumping memory: "4Gb" on &standardvm in podman .cirrus.yml? Not instead of this, but possibly in addition to?

@Luap99
Copy link
Member Author

Luap99 commented Apr 30, 2024

make IMG_SFX

well yeah that is what I figured but it gave a bunch of timebomb warning that I should fix so I didn't bother and called it a day..., will continue on Thursday

@edsantiago
Copy link
Member

Oh yeah. Easiest solution is to just bump the timebombs up by a month.

@Luap99
Copy link
Member Author

Luap99 commented Apr 30, 2024

Also, did you look into bumping memory: "4Gb" on &standardvm in podman .cirrus.yml? Not instead of this, but possibly in addition to?

That is the next step, but I think 4 Gb RAM is enough (if we remove the gigantic toolbox image).
I just figured if I make changes anyway I give us some more head room.

@Luap99
Copy link
Member Author

Luap99 commented May 2, 2024

Suggestions inline, but not a real review because I'm not 100% sure how/if these will work (e.g., if debian already has a /tmp fstab entry, and other logistics).

I could make it a systemd unit like on fedora then we know it will always overwrite it (Note that on systemd systems it will parse the fstab file and convert to mounts) I am not sur ehow it interacts with the systemd parser but traditionally it would just mount line by line, so even if there would be a tmpfs mount specified before it would have just over mounted it again AFAIK.

Copy link

github-actions bot commented May 2, 2024

Cirrus CI build successful. Found built image names and IDs:

Stage Image Name IMAGE_SUFFIX
base debian do-not-use
base fedora do-not-use
base fedora-aws do-not-use
base fedora-aws-arm64 do-not-use
base image-builder do-not-use
base prior-fedora do-not-use
cache build-push c20240502t142051z-f39f38d13
cache debian c20240502t142051z-f39f38d13
cache fedora c20240502t142051z-f39f38d13
cache fedora-aws c20240502t142051z-f39f38d13
cache fedora-netavark c20240502t142051z-f39f38d13
cache fedora-netavark-aws-arm64 c20240502t142051z-f39f38d13
cache fedora-podman-aws-arm64 c20240502t142051z-f39f38d13
cache fedora-podman-py c20240502t142051z-f39f38d13
cache prior-fedora c20240502t142051z-f39f38d13
cache rawhide c20240502t142051z-f39f38d13
cache win-server-wsl c20240502t142051z-f39f38d13

@edsantiago
Copy link
Member

debian prior-fedora fedora fedora-aws rawhide
base 13.1 38-1.6 39-1.5 ? 41-0
13 ⇑
kernel 6.7.12-1 6.8.7-100 6.8.8-200 6.8.7-200 6.9.0-0.rc6.52
6.7.9-2 ⇑ 6.8.4-100 ⇑ 6.8.5-200 ⇑ 6.8.4-200 ⇑ 6.9.0-0.rc3.30 ⇑
grub2-common 2.12-2 2.06-116 2.06-120 2.06-120 2.06-121
2.06-118 ⇑ 2.06-118 ⇑ 2.06-119 ⇑
aardvark-dns 1.4.0-5 1.10.0-1 1.10.0-1 1.10.0-1 1.10.0-1
netavark 1.4.0-4 1.10.3-1 1.10.3-1 1.10.3-1 1.10.3-3
buildah 1.33.7+ds1-1 1.35.3-1 1.35.3-1 1.35.3-1 1.35.3-1
conmon 2.1.10+ds1-1+b1 2.1.10-1 2.1.10-1 2.1.10-1 2.1.10-1
container-selinux ? 2.230.0-1 2.231.0-1 2.230.0-1 2.231.0-1
2.230.0-1 ⇑ 2.230.0-1 ⇑
containers-common ? 1-89 1-99 1-99 0.58.0-18
0.58.0-8 ⇑
criu 3.17.1-3 3.18-1 3.19-2 3.19-2 3.19-4
crun 1.14.4-1 1.14.4-1 1.14.4-1 1.14.4-1 1.14.4-5
golang 2:1.22~3 1.21.9-1 1.21.9-1 1.21.9-1 1.22.2-3
1.21.8-1 ⇑
gvisor-tap-vsock ? 0.7.3-1 0.7.3-1 0.7.3-1 0.7.3-2
nmap-ncat 7.94+git20230807.3be01efb1+dfsg-3+b1 7.93-2 7.95-1 7.95-1 7.95-1
7.94-1 ⇑ 7.94-1 ⇑ 7.94-1 ⇑
passt 2024-04-26 2024-03-26 2024-04-26 2024-04-26 2024-04-26
2024-03-26 ⇑ 2024-04-05 ⇑ 2024-04-05 ⇑ 2024-04-05 ⇑ 2024-04-05 ⇑
podman 4.9.4+ds1-1 4.9.4-1 4.9.4-1 4.9.4-1 5.0.2-1
5.0.1-1 ⇑
runc 1.1.12+ds1-2 1.1.12-1 1.1.12-1 1.1.12-1 1.1.12-3
skopeo 1.13.3+ds1-2+b1 1.15.0-1 1.15.0-1 1.15.0-1 1.15.0-1
slirp4netns 1.2.1-1+b1 1.2.2-1 1.2.2-1 1.2.2-1 1.2.2-2
systemd 255.5-1 253.17-1 254.10-1 254.10-1 255.5-1
255.4-1+b1 ⇑ 255.4-1 ⇑
tar 1.35+dfsg-3 1.34-8 1.35-2 1.35-2 1.35-3
1.34+dfsg-1.2+deb12u1 ⇑

Luap99 added 2 commits May 8, 2024 11:41
To make tests faster setup a tmpfs on /tmp like fedora does so that test
do not have to write everything onto persistent disk.

Fixes containers#350

Signed-off-by: Paul Holzinger <[email protected]>
By default we only get 50% of all memory, given our programs don't take
this much we should instead use more /tmp space in case we have to store
more images.

Signed-off-by: Paul Holzinger <[email protected]>
Copy link

github-actions bot commented May 8, 2024

Cirrus CI build successful. Found built image names and IDs:

Stage Image Name IMAGE_SUFFIX
base debian do-not-use
base fedora do-not-use
base fedora-aws do-not-use
base fedora-aws-arm64 do-not-use
base image-builder do-not-use
base prior-fedora do-not-use
cache build-push c20240508t094144z-f40f39d13
cache debian c20240508t094144z-f40f39d13
cache fedora c20240508t094144z-f40f39d13
cache fedora-aws c20240508t094144z-f40f39d13
cache fedora-netavark c20240508t094144z-f40f39d13
cache fedora-netavark-aws-arm64 c20240508t094144z-f40f39d13
cache fedora-podman-aws-arm64 c20240508t094144z-f40f39d13
cache fedora-podman-py c20240508t094144z-f40f39d13
cache prior-fedora c20240508t094144z-f40f39d13
cache rawhide c20240508t094144z-f40f39d13
cache win-server-wsl c20240508t094144z-f40f39d13

@edsantiago
Copy link
Member

Not sure what to use as baseline, so I choose the last result of #349. Only one diff:

  • 20240508t094144z-f40f39d13
  • 20240506t132946z-f40f39d13 ⇑ (built in Fedora40 #349)
debian prior-fedora fedora fedora-aws rawhide
kernel 6.7.12-1 6.8.8-200 6.8.9-300 6.8.8-300 6.9.0-0.rc7.58
6.9.0-0.rc6.20240503gitf03359bca01b.56 ⇑

@Luap99 Luap99 force-pushed the debian-tmpfs branch 2 times, most recently from 264ba1d to a531bef Compare May 8, 2024 16:44
Copy link

github-actions bot commented May 8, 2024

Cirrus CI build successful. Found built image names and IDs:

Stage Image Name IMAGE_SUFFIX
base debian do-not-use
base fedora do-not-use
base fedora-aws do-not-use
base fedora-aws-arm64 do-not-use
base image-builder do-not-use
base prior-fedora do-not-use
cache build-push c20240508t164413z-f40f39d13
cache debian c20240508t164413z-f40f39d13
cache fedora c20240508t164413z-f40f39d13
cache fedora-aws c20240508t164413z-f40f39d13
cache fedora-netavark c20240508t164413z-f40f39d13
cache fedora-netavark-aws-arm64 c20240508t164413z-f40f39d13
cache fedora-podman-aws-arm64 c20240508t164413z-f40f39d13
cache fedora-podman-py c20240508t164413z-f40f39d13
cache prior-fedora c20240508t164413z-f40f39d13
cache rawhide c20240508t164413z-f40f39d13
cache win-server-wsl c20240508t164413z-f40f39d13

@edsantiago
Copy link
Member

Comparing against the previous build in this PR:

debian prior-fedora fedora fedora-aws rawhide
kernel 6.7.12-1 6.8.8-200 6.8.9-300 6.8.8-300 6.9.0-0.rc7.20240507gitdccb07f2914c.59
6.9.0-0.rc7.58 ⇑

Now that we use /tmp we do not have to include the changes for /var/tmp.
However we need r (read) access to /tmp as pasta opens the path with
read access.

Signed-off-by: Paul Holzinger <[email protected]>
Signed-off-by: Paul Holzinger <[email protected]>
Copy link

Cirrus CI build successful. Found built image names and IDs:

Stage Image Name IMAGE_SUFFIX
base debian do-not-use
base fedora do-not-use
base fedora-aws do-not-use
base fedora-aws-arm64 do-not-use
base image-builder do-not-use
base prior-fedora do-not-use
cache build-push c20240513t140131z-f40f39d13
cache debian c20240513t140131z-f40f39d13
cache fedora c20240513t140131z-f40f39d13
cache fedora-aws c20240513t140131z-f40f39d13
cache fedora-netavark c20240513t140131z-f40f39d13
cache fedora-netavark-aws-arm64 c20240513t140131z-f40f39d13
cache fedora-podman-aws-arm64 c20240513t140131z-f40f39d13
cache fedora-podman-py c20240513t140131z-f40f39d13
cache prior-fedora c20240513t140131z-f40f39d13
cache rawhide c20240513t140131z-f40f39d13
cache win-server-wsl c20240513t140131z-f40f39d13

@edsantiago
Copy link
Member

Again, comparing against the previous build in this same PR. New pasta in f40 and rawhide, be careful

debian prior-fedora fedora fedora-aws rawhide
kernel 6.7.12-1 6.8.9-200 6.8.9-300 6.8.9-300 6.9.0-0.rc7.20240510git448b3fe5a0ea.62
6.8.8-200 ⇑ 6.8.8-300 ⇑ 6.9.0-0.rc7.20240507gitdccb07f2914c.59 ⇑
buildah 1.33.7+ds1-1 1.35.3-1 1.35.4-1 1.35.3-1 1.35.4-1
1.35.3-1 ⇑ 1.35.3-1 ⇑
golang 2:1.22~3 1.21.9-1 1.22.3-1 1.22.2-1 1.22.3-1
1.22.2-1 ⇑ 1.22.2-3 ⇑
passt 2024-04-26 2024-04-26 2024-05-10 2024-04-26 2024-05-10
2024-04-26 ⇑ 2024-04-26 ⇑
podman 4.9.4+ds1-1 4.9.4-1 5.0.3-1 5.0.2-1 5.0.2-1
5.0.2-1 ⇑
systemd 255.5-1 254.10-1 255.6-1 255.6-1 255.5-1
255.4-1 ⇑ 255.4-1 ⇑

Luap99 added a commit to Luap99/libpod that referenced this pull request May 13, 2024
@Luap99
Copy link
Member Author

Luap99 commented May 13, 2024

@edsantiago PTAL and merge, images were merged in podman containers/podman#22533

@edsantiago
Copy link
Member

/lgtm

@Luap99 do you know the debian pasta status? Should I tag this (so renovatebot pulls it into every repo), or should we wait?

@Luap99
Copy link
Member Author

Luap99 commented May 13, 2024

@Luap99 do you know the debian pasta status? Should I tag this (so renovatebot pulls it into every repo), or should we wait?

If it passes podman I see no reason why not. AFAIK pasta is only used in buildah and podman so I don't see any particular risk with pasta on debian.

@edsantiago edsantiago merged commit b7395d1 into containers:main May 13, 2024
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

debian doesn't use tmpfs /tmp
2 participants