ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN #27777
pull maflcko wants to merge 2 commits into bitcoin:master from maflcko:2305-ci-prune-images-dangling- changing 1 files +3 −1-
maflcko commented at 6:54 am on May 30, 2023: memberThis 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
-
maflcko commented at 6:54 am on May 30, 2023: memberThis should fix the out-of-space error, see https://cirrus-ci.com/task/6118746146734080?logs=ci#L4903
-
maflcko 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
-
maflcko commented at 8:28 am on May 30, 2023: memberNot 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?
-
maflcko 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
?
-
maflcko commented at 8:48 am on May 30, 2023: memberI listed all images, volumes, containers, networks, and the log file(s) and the list was empty, apart from 3 small image layers.
-
maflcko 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?
-
maflcko commented at 10:02 am on May 30, 2023: memberThis 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?
-
maflcko commented at 10:09 am on May 30, 2023: memberOk, I’ll check back a week after merge.
-
hebasto approved
-
hebasto commented at 10:10 am on May 30, 2023: memberACK fa123077bc3f39aa0969d883e2d799a054cd4543
-
maflcko 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 todocker build
.0 docker image prune --force --filter label=bitcoin-ci
maflcko 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 viaRESTART_CI_DOCKER_BEFORE_RUN
. Happy to renameRESTART_CI_DOCKER_BEFORE_RUN
toDANGER_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 theRESTART_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.
maflcko 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 theimage prune
fromDANGER_RESTART_CI_DOCKER_BEFORE_RUN
?
stickies-v commented at 10:46 am on May 31, 2023: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 surepodman 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 withdocker run
and then try to show/kill it withpodman ps
orpodman container kill --all
.
maflcko commented at 2:57 pm on May 30, 2023:I already switched all machines topodman-docker
, because it is not possible to installdocker.io
andpodman-docker
at the same time. Sodocker run
is actually just an alias forpodman run
.
stickies-v commented at 5:32 pm on May 30, 2023:Okay, I can’t comment on that. Since there is no mention ofpodman
anywhere else in the codebase/documentation, I think this may make maintaining the CI more opaque, but perhaps that’s an okay tradeoff.
maflcko commented at 5:38 pm on May 30, 2023:Yeah, I don’t expect anyone to useRESTART_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.
maflcko commented at 11:35 am on June 5, 2023:Looks like this doesn’t work some of the time:
- https://cirrus-ci.com/task/5798762795237376?logs=ci#L267
- https://cirrus-ci.com/task/4640353173635072?logs=ci#L266
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: contributorACK 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, 2023fanquake merged this on May 31, 2023fanquake closed this on May 31, 2023
sidhujag referenced this in commit 388f0173d5 on May 31, 2023maflcko deleted the branch on Jun 1, 2023fanquake referenced this in commit 5111d8e02f on Jun 12, 2023sidhujag referenced this in commit 0c2423005b on Jun 12, 2023fanquake referenced this in commit 71f626ef2c on Jun 15, 2023fanquake referenced this in commit de56daab41 on Jun 15, 2023fanquake referenced this in commit 642b5dd1b4 on Jun 16, 2023fanquake referenced this in commit 0db69a3d50 on Sep 26, 2023fanquake referenced this in commit 7f1357de51 on Sep 26, 2023fanquake referenced this in commit 1416d09cba on Oct 6, 2023fanquake referenced this in commit cee39d0628 on Oct 15, 2023Frank-GER referenced this in commit 991767f1c2 on Oct 21, 2023bitcoin locked this on Sep 25, 2024
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-21 18:12 UTC
More mirrored repositories can be found on mirror.b10c.me