-
Notifications
You must be signed in to change notification settings - Fork 37.8k
ci: Stop and remove CI container #26850
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
The head ref may contain hidden characters: "2301-ci-container-stop-\u{1F4BA}"
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
fa789b9
to
fa868d2
Compare
Concept ACK |
fa868d2
to
fad7ba8
Compare
Concept ACK. |
fad7ba8
to
fae3f90
Compare
DOCKER in names is confusingly used as synonym for "image", "container", and "ci". Fix the confusion by picking the term that fits the context. -BEGIN VERIFY SCRIPT- ren() { sed -i "s:$1:$2:g" $( git grep -l "$1" ) ; } ren DOCKER_PACKAGES CI_BASE_PACKAGES # This better reflects that they are the common base for all CI # containers. ren DOCKER_ID CI_CONTAINER_ID # This is according to the documentation of "--detach , -d: Run # container in background and print container ID". ren DOCKER_NAME_TAG CI_IMAGE_NAME_TAG # This avoids confusing with CONTAINER_NAME and clarifies that it is an # image. ren DOCKER_ADMIN CI_CONTAINER_CAP # This clarifies that it is a capability added to the container. ren DOCKER_CI_CMD_PREFIX CI_EXEC_CMD_PREFIX # This brings it in line with the CI_EXEC naming. -END VERIFY SCRIPT-
fae3f90
to
fa0584e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fa0584e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to pick another name, if there are suggestions. |
Maybe |
Will leave as-is for now. Can be changed in a follow-up, because merge conflicts in this area are rare. |
This should not affect CI runs that have
DANGER_RUN_CI_ON_HOST=1
set, for example.cirrus.yml
.However, when running CI locally with podman or docker, the container is stopped and thus deleted when all tests have passed. This feature was requested in #26843 (comment) and can help to reduce used disk space when running several CI tasks subsequently.