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
  1. maflcko 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.
  2. ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN fa9c65a74c
  3. 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.

  4. DrahtBot added the label Tests on May 30, 2023
  5. maflcko 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
  6. 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
    
  7. DrahtBot added the label CI failed on May 30, 2023
  8. DrahtBot removed the label CI failed on May 30, 2023
  9. ci: Use podman for persistent workers fa123077bc
  10. maflcko 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.
  11. 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?

  12. 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/
    
  13. 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
    

    ?

  14. maflcko 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.
  15. 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)
  16. 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?

  17. maflcko commented at 10:02 am on May 30, 2023: member
  18. 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?

  19. maflcko commented at 10:09 am on May 30, 2023: member
    Ok, I’ll check back a week after merge.
  20. hebasto approved
  21. hebasto commented at 10:10 am on May 30, 2023: member
    ACK fa123077bc3f39aa0969d883e2d799a054cd4543
  22. maflcko assigned fanquake on May 30, 2023
  23. 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
    

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


    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 the image prune from DANGER_RESTART_CI_DOCKER_BEFORE_RUN?

    stickies-v commented at 10:46 am on May 31, 2023:
  24. 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.

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

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

    maflcko 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
    
  25. 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.

  26. DrahtBot requested review from stickies-v on May 30, 2023
  27. fanquake merged this on May 31, 2023
  28. fanquake closed this on May 31, 2023

  29. sidhujag referenced this in commit 388f0173d5 on May 31, 2023
  30. maflcko deleted the branch on Jun 1, 2023
  31. fanquake referenced this in commit 5111d8e02f on Jun 12, 2023
  32. sidhujag referenced this in commit 0c2423005b on Jun 12, 2023
  33. fanquake referenced this in commit 71f626ef2c on Jun 15, 2023
  34. fanquake referenced this in commit de56daab41 on Jun 15, 2023
  35. fanquake referenced this in commit 642b5dd1b4 on Jun 16, 2023
  36. fanquake referenced this in commit 0db69a3d50 on Sep 26, 2023
  37. fanquake referenced this in commit 7f1357de51 on Sep 26, 2023
  38. fanquake commented at 3:29 pm on September 26, 2023: member
    Backported for 24.x in #28535.
  39. fanquake referenced this in commit 1416d09cba on Oct 6, 2023
  40. fanquake referenced this in commit cee39d0628 on Oct 15, 2023
  41. Frank-GER referenced this in commit 991767f1c2 on Oct 21, 2023
  42. bitcoin locked this on Sep 25, 2024

github-metadata-mirror

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

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me