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

doc(ci): Fix incorrect docs about Docker build cache order #5190

Merged
merged 4 commits into from
Sep 27, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Sep 19, 2022

Motivation

We want to use the branch build cache first, because the changes to a PR are usually small.
This is a comment-only PR.

Closes #5186.

API documentation

The order and failure behaviour of caches is undocumented:
https://github.com/docker/build-push-action/blob/master/docs/advanced/cache.md#registry-cache

But it seems to be bottom to top, skip on failure.
(This is unusual, typically the order would be top to bottom.)

Review

This seems to be working in the most efficient way, but the docs are wrong.

Reviewer Checklist

  • Docs make sense

@teor2345 teor2345 added C-bug Category: This is a bug A-devops Area: Pipelines, CI/CD and Dockerfiles I-cost Zebra infrastructure costs labels Sep 19, 2022
@teor2345 teor2345 requested a review from a team as a code owner September 19, 2022 02:06
@teor2345 teor2345 removed the request for review from a team September 19, 2022 02:06
@teor2345 teor2345 self-assigned this Sep 19, 2022
@teor2345 teor2345 requested a review from dconnolly September 19, 2022 02:06
@teor2345 teor2345 changed the title Use branch then main build caches fix(ci): Use branch then main build caches for Docker builds Sep 19, 2022
@teor2345 teor2345 marked this pull request as draft September 19, 2022 03:49
@teor2345
Copy link
Contributor Author

Looking at the logs, it seems like this tries the main-cache first:
https://github.com/ZcashFoundation/zebra/actions/runs/3079487516/jobs/4975805832#step:9:218

So this might just need a doc update, I'll try reversing the order.

@teor2345
Copy link
Contributor Author

Looking at the logs, it seems like this tries the main-cache first: https://github.com/ZcashFoundation/zebra/actions/runs/3079487516/jobs/4975805832#step:9:218

So this might just need a doc update, I'll try reversing the order.

This order seems to work the way we want - checking the branch cache first. But annoyingly it doesn't say which cache it has used.

I'll wait for Gustavo to confirm before merging.

@teor2345 teor2345 added A-docs Area: Documentation P-Medium ⚡ and removed P-Medium ⚡ labels Sep 19, 2022
@teor2345 teor2345 changed the title fix(ci): Use branch then main build caches for Docker builds doc(ci): Fix incorrect docs about Docker build cache order Sep 19, 2022
@teor2345 teor2345 added C-cleanup Category: This is a cleanup S-incomplete and removed C-bug Category: This is a bug S-incomplete labels Sep 20, 2022
@teor2345
Copy link
Contributor Author

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Sep 20, 2022

update

✅ Branch has been successfully updated

@teor2345
Copy link
Contributor Author

sending_transactions_using_lightwalletd failed due to a timing issue with the "mempool transaction broadcast" logs.

This might be fixed by PR #5134 or #5164, if not we can fix it after that.

@teor2345
Copy link
Contributor Author

kex_exchange_identification: read: Connection reset by peer

https://github.com/ZcashFoundation/zebra/actions/runs/3087669400/jobs/4993361663#step:6:108

Failed due to #5069

@teor2345 teor2345 marked this pull request as ready for review September 27, 2022 01:14
@teor2345
Copy link
Contributor Author

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Sep 27, 2022

update

✅ Branch has been successfully updated

mergify bot added a commit that referenced this pull request Sep 27, 2022
mergify bot added a commit that referenced this pull request Sep 27, 2022
@mergify mergify bot merged commit 59a6ed2 into main Sep 27, 2022
@mergify mergify bot deleted the branch-cache-first branch September 27, 2022 06:54
@gustavovalverde
Copy link
Member

Yesterday I used the wrong button here.

I was trying to add this information with the previous action moby/moby#26839 (comment)

@teor2345 teor2345 mentioned this pull request Oct 11, 2022
32 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles A-docs Area: Documentation C-cleanup Category: This is a cleanup I-cost Zebra infrastructure costs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker build cache might be in the wrong order
3 participants