ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE

MarcoFalke commented at 6:54 am on May 30, 2023: member

This should prevent the persistent workers from running out of disk space. Containers are already removed, but not images. This is required since CI images are built and cached.

ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN fa9c65a74c

DrahtBot commented at 6:54 am on May 30, 2023: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto
Stale ACK	stickies-v

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label Tests on May 30, 2023

MarcoFalke commented at 6:54 am on May 30, 2023: member

This should fix the out-of-space error, see https://cirrus-ci.com/task/6118746146734080?logs=ci#L4903

MarcoFalke commented at 7:25 am on May 30, 2023: member

Looks like it is working, but not claiming a lot of space:

https://cirrus-ci.com/task/6656469207089152?logs=ci#L408

0Total reclaimed space: 949.4MB

DrahtBot added the label CI failed on May 30, 2023

DrahtBot removed the label CI failed on May 30, 2023

ci: Use podman for persistent workers fa123077bc

MarcoFalke commented at 8:28 am on May 30, 2023: member

Not sure what is going on. Seems there are unreachable zombie containers in the overlay2 directory. Let’s try podman instead.

hebasto commented at 8:35 am on May 30, 2023: member

Looks like it is working, but not claiming a lot of space:

cirrus-ci.com/task/6656469207089152?logs=ci#L408
0Total reclaimed space: 949.4MB

What amount do you expect?

MarcoFalke commented at 8:37 am on May 30, 2023: member

I already wiped all servers, but I was expecting that about 50GB of zombie stuff is cleared:

0# du -sh /var/lib/docker/overlay2/ 
153G	/var/lib/docker/overlay2/

hebasto commented at 8:45 am on May 30, 2023: member

Not sure what is going on. Seems there are unreachable zombie containers in the overlay2 directory. Let’s try podman instead.

Maybe add:

0docker volume prune --force

?

MarcoFalke commented at 8:48 am on May 30, 2023: member

I listed all images, volumes, containers, networks, and the log file(s) and the list was empty, apart from 3 small image layers.

MarcoFalke commented at 9:27 am on May 30, 2023: member

(CI is green, but it will probably take 6 months to find out if podman is running out of storage space as well)

hebasto commented at 9:58 am on May 30, 2023: member

(CI is green, but it will probably take 6 months to find out if podman is running out of storage space as well)

Could explicit logging of disk space usage help in the meanwhile?

MarcoFalke commented at 10:02 am on May 30, 2023: member

This is already done, see https://cirrus-ci.com/task/4965521926389760?logs=ci#L356

hebasto commented at 10:08 am on May 30, 2023: member

This is already done, see cirrus-ci.com/task/4965521926389760?logs=ci#L356

Right. I mean, no need to wait for 6 month. If numbers are the same after a week then this PR works as intended, no?

MarcoFalke commented at 10:09 am on May 30, 2023: member

Ok, I’ll check back a week after merge.

hebasto approved

hebasto commented at 10:10 am on May 30, 2023: member

ACK fa123077bc3f39aa0969d883e2d799a054cd4543

MarcoFalke assigned fanquake on May 30, 2023

in ci/test/04_install.sh:47 in fa123077bc

41@@ -42,7 +42,9 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then
42 
43   if [ -n "${RESTART_CI_DOCKER_BEFORE_RUN}" ] ; then
44     echo "Restart docker before run to stop and clear all containers started with --rm"
45-    systemctl restart docker
46+    podman container kill --all  # Similar to "systemctl restart docker"
47+    echo "Prune all dangling images"
48+    docker image prune --force

stickies-v commented at 12:37 pm on May 30, 2023:

Would it be prudent to use the --filter option here to only prune images created by the CI? This may be especially helpful to people running the CI locally/unsandboxed. We’d just need to add a --label "bitcoin-ci" argument to docker build.

0    docker image prune --force --filter label=bitcoin-ci

MarcoFalke commented at 2:01 pm on May 30, 2023:

Do dangling image (layer)s have a label? Anyway, this isn’t for local runs, only for the persistent runners, guarded via RESTART_CI_DOCKER_BEFORE_RUN. Happy to rename RESTART_CI_DOCKER_BEFORE_RUN to DANGER_RESTART_CI_DOCKER_BEFORE_RUN?

stickies-v commented at 2:42 pm on May 30, 2023:

Do dangling image (layer)s have a label?

Yeah they do, just tested it to confirm.

guarded via RESTART_CI_DOCKER_BEFORE_RUN

Fair enough, looks like this is unlikely to be hit locally. I don’t think renaming to DANGER_RESTART_CI_DOCKER_BEFORE_RUN is an improvement, so happy to keep as is. Just thought adding a label and only applying batch operations to our own images is a good practice, e.g. if in the future we remove the RESTART_CI_DOCKER_BEFORE_RUN condition, given that it’s almost no overhead. But up to you, it’s no big deal and I don’t have a strong opinion.

MarcoFalke commented at 2:58 pm on May 30, 2023:

Ok, could make sense in a follow-up to apply the label to both places and then unguard the image prune from DANGER_RESTART_CI_DOCKER_BEFORE_RUN?

stickies-v commented at 10:46 am on May 31, 2023:

Done in https://github.com/bitcoin/bitcoin/pull/27793

in ci/test/04_install.sh:45 in fa123077bc

41@@ -42,7 +42,9 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then
42 
43   if [ -n "${RESTART_CI_DOCKER_BEFORE_RUN}" ] ; then
44     echo "Restart docker before run to stop and clear all containers started with --rm"
45-    systemctl restart docker
46+    podman container kill --all  # Similar to "systemctl restart docker"

stickies-v commented at 2:42 pm on May 30, 2023:

I’m not sure this works? Even though the CLIs have a compatible syntax, I think they operate on their own set of resources. I’m pretty sure podman ps right before this line won’t show anything, so I think no containers are killed after this line? At least, that’s what I get when I try locally spinning up a container with docker run and then try to show/kill it with podman ps or podman container kill --all.

MarcoFalke commented at 2:57 pm on May 30, 2023:

I already switched all machines to podman-docker, because it is not possible to install docker.io and podman-docker at the same time. So docker run is actually just an alias for podman run.

stickies-v commented at 5:32 pm on May 30, 2023:

Okay, I can’t comment on that. Since there is no mention of podman anywhere else in the codebase/documentation, I think this may make maintaining the CI more opaque, but perhaps that’s an okay tradeoff.

MarcoFalke commented at 5:38 pm on May 30, 2023:

Yeah, I don’t expect anyone to use RESTART_CI_DOCKER_BEFORE_RUN beside the persistent workers, so it is an undocumented setting. Anyone using it is expected to infer the documentation from the two echo calls and two lines of code.

MarcoFalke commented at 11:35 am on June 5, 2023:

Looks like this doesn’t work some of the time:

 0++ echo 'Restart docker before run to stop and clear all containers started with --rm'
 1Restart docker before run to stop and clear all containers started with --rm
 2++ podman container kill --all
 3++ echo 'Prune all dangling images'
 4Prune all dangling images
 5++ docker image prune --force
 6Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
 7+++ docker run --rm --interactive --detach --tty --mount type=bind,src=/tmp/cirrus-build,dst=/ro_base,readonly --mount type=volume,src=ci_native_qt5_ccache,dst=/tmp/ccache_dir --mount type=volume,src=ci_native_qt5_depends,dst=/tmp/cirrus-build/depends --mount type=volume,src=ci_native_qt5_previous_releases,dst=/tmp/cirrus-build/releases/x86_64-pc-linux-gnu -w /tmp/cirrus-build --env-file /tmp/env --name ci_native_qt5 ci_native_qt5
 8Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
 9time="2023-06-05T06:45:57+02:00" level=warning msg="The input device is not a TTY. The --tty and --interactive flags might not work properly"
10Error: error creating container storage: the container name "ci_native_qt5" is already in use by "1b176e38eff6bdc928f2e213260cb9c8abb9efaa5cd6b0cccd09033a1b707601". You have to remove that container to be able to reuse that name.: that name is already in use
11++ CI_CONTAINER_ID=
12Exit status: 125

stickies-v commented at 5:35 pm on May 30, 2023: contributor

ACK fa9c65a74cf18e9c75cd3472112d5197532ac2f2

I’ve not used podman and don’t fully understand the need for fa123077bc3f39aa0969d883e2d799a054cd4543, so I’m hoping that other people do and can comment on that so I don’t need to dive into it. Otherwise, happy to revisit this later.

DrahtBot requested review from stickies-v on May 30, 2023

fanquake merged this on May 31, 2023

fanquake closed this on May 31, 2023

sidhujag referenced this in commit 388f0173d5 on May 31, 2023

MarcoFalke deleted the branch on Jun 1, 2023

fanquake referenced this in commit 5111d8e02f on Jun 12, 2023

sidhujag referenced this in commit 0c2423005b on Jun 12, 2023

fanquake referenced this in commit 71f626ef2c on Jun 15, 2023

fanquake referenced this in commit de56daab41 on Jun 15, 2023

fanquake referenced this in commit 642b5dd1b4 on Jun 16, 2023

fanquake referenced this in commit 0db69a3d50 on Sep 26, 2023

fanquake referenced this in commit 7f1357de51 on Sep 26, 2023

fanquake commented at 3:29 pm on September 26, 2023: member

Backported for 24.x in #28535.

ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN #27777

Reviews