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

added --rm flag to docker run #1753

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mikemhenry
Copy link
Contributor

@mikemhenry mikemhenry commented Jul 28, 2023

Checklist

  • Added a news entry

Resolves #909

I spent an evening do a lot of python build-locally.py and blew through ~1.5TB of disk space. I am not sure if there are any downsides to passing in --rm by default. When making this PR I noticed that I could set CONDA_FORGE_DOCKER_RUN_ARGS https://github.com/conda-forge/conda-smithy/blob/main/conda_smithy/templates/run_docker_build.sh.tmpl#L67-L68 which is nice but I think that is something easy to forget.

@jakirkham
Copy link
Member

jakirkham commented Jul 28, 2023

Thanks Mike! 🙏

The reason we like having this be customizable is that it is useful in some cases to be able debug the container (if the build failed for example)

Though appreciate the issues around leftover containers eating up space (and forgetting to clean those). Struggle with the same issues 😅

Wonder if we can find a middle ground

For example, what if we add this to DOCKER_RUN_ARGS if CONDA_FORGE_DOCKER_RUN_ARGS is not specified? So changing this line...

DOCKER_RUN_ARGS="${CONDA_FORGE_DOCKER_RUN_ARGS}"

...to something like this...

DOCKER_RUN_ARGS="${CONDA_FORGE_DOCKER_RUN_ARGS:---rm}"

@mikemhenry
Copy link
Contributor Author

Perfect! I think that covers the (at least for me) common case of wanting --rm but letting someone override that choice.

Copy link
Member

@isuruf isuruf left a comment

Choose a reason for hiding this comment

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

-1 on this change. This requires me to set CONDA_FORGE_DOCKER_RUN_ARGS to something for me to debug which I'll probably forget and will have to waste time running again.

@jakirkham
Copy link
Member

What would be another approach that would be better?

@mikemhenry
Copy link
Contributor Author

I'm not even sure if setting CONDA_FORGE_DOCKER_RUN_ARGS=--rm even works, or perhaps something else is needed, since I find even setting that I still end up running out of disk space until I run docker system prune

For example, it looks like when I set CONDA_FORGE_DOCKER_RUN_ARGS=--rm the script does pull that in and do what I expect:

+ DOCKER_RUN_ARGS=--rm
+ '[' -z '' ']'
+ DOCKER_RUN_ARGS='-it --rm'

But post build, I almost free 1gb of space running docker system prune (I ran the command before I ran python build-locally.py so the images it came from this build)

$ docker system prune
WARNING! This will remove:
  - all stopped containers
  - all networks not used by at least one container
  - all dangling images
  - all dangling build cache

Are you sure you want to continue? [y/N] y
Deleted Images:
untagged: quay.io/condaforge/linux-anvil-cos7-x86_64@sha256:b2ee96143622c19449ef0de8bade84081dc3e3631ea5037ea217a258277e5d62
deleted: sha256:7b1cb965b45e86e0d2711843e332420206f2643feef164fdef55a9fe53cbe80e
deleted: sha256:4b06a18f310745e19ff44b567d1de6d0f2d4fa3bea86e39f2507e4c5cabe577d
deleted: sha256:9eb0d16efc207b5b63a22755964ff206cea275694611e6ec600e3f8507029407
deleted: sha256:e466f5e058278e5418b636baf1ed935a359524e866b28c4dbcc7c262258d204d
deleted: sha256:8d41a3418d63154f57d965441c32d61404cb616cc7e7ebb7e79c1a451a9775cd
deleted: sha256:6053c519ec0f516329fb726cb62e0c443a9d8e9980d5e01fc01155d3e3df2b75
deleted: sha256:74241802849ff8c15feecc3575845caa5b10cd1e379ed0148e5418067b94b497
deleted: sha256:abb92809548f6ad895ae8189869e5d1a0faaa61891b534669e2f83a72ee519cb
deleted: sha256:c8bb835780d7e72fec7de999be88990cd625d9844a9f259ac311129b44f0442d
deleted: sha256:15ec9339d84736535902287c71d227aea58d5005d237c0777761190f93dbaa4b
deleted: sha256:efb3c08aa8c7882ac0830bf4b684aa65ddb3e52c3d88da10fa80a499f07fa012
deleted: sha256:b6da3ef707d9ca6fbab4ad20210fa9789d53edf4289c0c5a26cc685246ac8ace
deleted: sha256:9094ad3e160f1e2535de42c604cb039215bc3a030cbe8c48df85ddcf36c55149

Total reclaimed space: 923.9MB

So I am fine with taking another approach (and it seems like a different approach is needed anyway?) to reduce the disk space usage from python build-locally.py if a user is trying to just catch errors before pushing changes to conda-forge CI, not inspect the images post build.

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.

Add --rm to docker run in run_docker_build.sh ?
4 participants