bk: add post-checkout hook to cleanup leftover containers#621
Conversation
ivotron
requested review from
RafalKorepta,
andrewstucki,
chrisseto and
gene-redpanda
as code owners
ivotron
force-pushed
the
pre-exit-cleanup-hook
branch
from
ff8225e to
ccfdbf0
Compare
|
this can be seen in action on this cancelled step https://buildkite.com/redpanda/redpanda-operator/builds/5389#0195f7b7-dbcc-4d1e-9fcf-d0c9417adf50. or on any other cancelled step of that same job https://buildkite.com/redpanda/redpanda-operator/builds/5389 |
|
I see in CI job on your PR that kind cluster already exist. In other terms pre-start hook should prepare environment and delete kind and k3d clusters along with containers. Pre-exit hook could be good if the queue is shared among other project. |
ivotron
force-pushed
the
pre-exit-cleanup-hook
branch
from
ccfdbf0 to
8970872
Compare
ivotron
changed the title
post-checkout hook to cleanup leftover containers
that's because the previous job didn't run this |
.buildkite/hooks/post-checkout
Outdated
|
|
||
| echo "Cleaning up unused docker containers" | ||
| docker ps -a | ||
| for id in $(docker ps -qa); do |
There was a problem hiding this comment.
Why -a here? We're only interested in running containers, right?
-a, --all Show all containers (default shows just running)
FWIW, I find it useful to use the long version of arguments in scripts.
There was a problem hiding this comment.
the goal is to also freeup space. if not important then i can just remove it
.buildkite/hooks/post-checkout
Outdated
| docker ps -a | ||
| for id in $(docker ps -qa); do | ||
| echo "Killing $id" | ||
| docker stop -t 1 "$id" || true |
There was a problem hiding this comment.
Maybe docker kill instead of stop? No need to waste time waiting on a process to listen to SIGTERM in this context.
|| true isn't need because of set +e, right?
.buildkite/hooks/post-checkout
Outdated
| docker stop -t 1 "$id" || true | ||
| done | ||
|
|
||
| docker container prune --force |
There was a problem hiding this comment.
docker system prune --force --volumes --all will cover the first two lines here. I don't know if it'll clear out networks through 🤔
There was a problem hiding this comment.
based on docs, it seems like it does
ivotron
force-pushed
the
pre-exit-cleanup-hook
branch
2 times, most recently
from
b63d620 to
5edf8e8
Compare
Signed-off-by: Ivo Jimenez <ivo.jimenez@gmail.com>
ivotron
force-pushed
the
pre-exit-cleanup-hook
branch
from
5edf8e8 to
6b42095
Compare
Signed-off-by: Ivo Jimenez <ivo@redpanda.com> (cherry picked from commit a44a8f0)
Signed-off-by: Ivo Jimenez <ivo@redpanda.com> (cherry picked from commit a44a8f0)
Signed-off-by: Ivo Jimenez <ivo@redpanda.com> (cherry picked from commit a44a8f0)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
|
Thanks for the fix(es) @ivotron, I opened up the backports for you! |
|
ty! on a second thought, |
Signed-off-by: Ivo Jimenez <ivo@redpanda.com> (cherry picked from commit a44a8f0)
Signed-off-by: Ivo Jimenez <ivo@redpanda.com> (cherry picked from commit a44a8f0)
Signed-off-by: Ivo Jimenez <ivo@redpanda.com> (cherry picked from commit a44a8f0)
|
@ivotron I'm not sure if you are about to follow up with changing |
|
sorry, @chrisseto and i chatted and it seems like removing images is not harmful. are you seeing a negative effect? if yes then i can open a PR |
|
if it helps: #684 |
3 participants
Add a
post-checkouthook to properly cleanup leftover containers from previous jobs.fixes https://redpandadata.atlassian.net/browse/DEVPROD-2817