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

ci: label docker images and prune dangling images selectively #27793

Merged
merged 2 commits into from Oct 15, 2023

Conversation

stickies-v
Copy link
Contributor

@stickies-v stickies-v commented May 31, 2023

Follow-up from #27777 (comment).

Labeling the docker images produced by the CI allows us/the user to apply batch operations to all images (including dangling ones) produced by the ci without affecting other, non-bitcoin-ci images. With labeling, we can safely always prune dangling bitcoin-ci-test images without checking for RESTART_CI_DOCKER_BEFORE_RUN, which we enable on our persistent runners.

@DrahtBot
Copy link
Contributor

DrahtBot commented May 31, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK fanquake
Concept ACK Sjors, hebasto, pablomartin4btc

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

@Sjors
Copy link
Member

Sjors commented Jun 6, 2023

Concept ACK. Haven't tested.

It would also be nice to have an incantation (e.g. in the help) to kill dangling images after CI failure / interruption (if you don't plan on running it again).

@stickies-v
Copy link
Contributor Author

Rebased to address merge conflict from #27844.

It would also be nice to have an incantation (e.g. in the help) to kill dangling images after CI failure / interruption (if you don't plan on running it again).

Thanks, added doc: ci: add instructions for pruning dangling images manually

@fanquake
Copy link
Member

Concept ACK

@stickies-v
Copy link
Contributor Author

Rebased to address merge conflict from #28138.

@stickies-v
Copy link
Contributor Author

Rebased to address merge conflict from #28161.

ci/README.md Outdated Show resolved Hide resolved
@hebasto
Copy link
Member

hebasto commented Aug 14, 2023

Concept ACK.

@stickies-v
Copy link
Contributor Author

Force pushed to update documentation addressing this review comment.

ci/README.md Outdated Show resolved Hide resolved
@stickies-v stickies-v force-pushed the ci-label-images branch 2 times, most recently from 234b61b to 8c0faec Compare August 16, 2023 12:03
@maflcko
Copy link
Member

maflcko commented Aug 17, 2023

Could rebase for green CI, if still relevant?

@stickies-v
Copy link
Contributor Author

Rebased to address merge conflict from #27976.

Copy link
Member

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

Concept ACK.

Perhaps description needs to be updated regarding the user instructions (ci/README.md).

@stickies-v
Copy link
Contributor Author

Perhaps description needs to be updated regarding the user instructions (ci/README.md).

Thanks, reverted that description.

@stickies-v
Copy link
Contributor Author

Closed, not an essential change and doesn't seem like there's sufficient interest.

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

lgtm, after the nit

ci/test/04_install.sh Outdated Show resolved Hide resolved
This allows us or the user to perform batch operations on all
images produced by the ci, e.g. to prune all dangling images,
without affecting non-ci images.
Since all bitcoin-ci-test images are now labeled, we can always
prune all dangling images, regardless of whether we are in
RESTART_CI_DOCKER_BEFORE_RUN.

To be safe, still prune all images if RESTART_CI_DOCKER_BEFORE_RUN
in case the filtering doesn't work, or if images were created on
an earlier version that did not assign labels.
@stickies-v
Copy link
Contributor Author

Force-pushed to address merge conflict from #28547 and incorporate suggestion from maflcko to keep the unfiltered pruning for now, as belt and suspenders approach.

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

utACK e44c574

@fanquake fanquake merged commit cee39d0 into bitcoin:master Oct 15, 2023
3 checks passed
@stickies-v stickies-v deleted the ci-label-images branch October 15, 2023 08:58
Frank-GER pushed a commit to syscoin/syscoin that referenced this pull request Oct 21, 2023
…s selectively

e44c574 ci: always prune all dangling bitcoin-ci-test images (stickies-v)
ce16997 ci: add label to docker images (stickies-v)

Pull request description:

  Follow-up from bitcoin#27777 (comment).

  Labeling the docker images produced by the CI allows us/the user to apply batch operations to all images (including dangling ones) produced by the ci without affecting other, non-bitcoin-ci images. With labeling, we can safely always prune dangling bitcoin-ci-test images without checking for `RESTART_CI_DOCKER_BEFORE_RUN`, which we enable on our persistent runners.

ACKs for top commit:
  fanquake:
    utACK e44c574

Tree-SHA512: 1009fb1be78fbc80b5341ba92eac2991e77d050e1ab6048d1d9a65af73413a6be7afc1e1c764eb3f347f363af31245b93fdb38f6ac016d775aad4a0f36e4c98f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants