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.

Sign up for GitHub

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

Jump to bottom

fix(build): avoid docker cache contamination and invalidation #4254

Merged
merged 4 commits into from
May 3, 2022
Merged

fix(build): avoid docker cache contamination and invalidation #4254

merged 4 commits into from
May 3, 2022

Conversation

gustavovalverde
Copy link
Member

@gustavovalverde gustavovalverde commented May 1, 2022 •
edited
Loading

Motivation

We've been having issues with image building, and cache is not being as effectively as when initially implemented. And jobs are failing mainly when an image takes more than 45 minutes to build. The specific root cause is unknown, but this is also a good workaround.

Fixes #4234

Solution

  • Fallback to use GHA cache
  • Do not invalidate the cache if not required, just add needed folders and files to .dockerignore

Review

@teor2345 and @dconnolly can review this

Reviewer Checklist

  • Re-running a job uses the cache

Follow Up Work

  • Confirm that cache does not expire as frequently as before, as we're using scopes to target specific images.

@gustavovalverde gustavovalverde added C-bug Category: This is a bug A-devops Area: Pipelines, CI/CD and Dockerfiles C-enhancement Category: This is an improvement P-Critical 🚑 I-integration-fail Continuous integration fails, including build and test failures lightwalletd any work associated with lightwalletd I-build-fail Zebra fails to build labels May 1, 2022
@gustavovalverde gustavovalverde requested a review from a team as a code owner May 1, 2022 14:02
@gustavovalverde gustavovalverde self-assigned this May 1, 2022
@gustavovalverde gustavovalverde requested a review from a team as a code owner May 1, 2022 14:02
@gustavovalverde gustavovalverde requested review from dconnolly and removed request for a team May 1, 2022 14:02
@gustavovalverde gustavovalverde changed the title Fix cache invalidation fix(build): use docker cache more frequently and avoid contamination May 1, 2022
@gustavovalverde gustavovalverde changed the title fix(build): use docker cache more frequently and avoid contamination fix(build): avoid docker cache contamination and invalidation May 1, 2022
@gustavovalverde gustavovalverde mentioned this pull request May 1, 2022
Merged
3 tasks
@gustavovalverde
Copy link
Member Author

Testing in another PR, there might be a missing fix on this one.

@gustavovalverde gustavovalverde added the do-not-merge Tells Mergify not to merge this PR label May 2, 2022
@gustavovalverde gustavovalverde marked this pull request as draft May 2, 2022 20:14
@gustavovalverde gustavovalverde marked this pull request as ready for review May 3, 2022 00:28
teor2345
teor2345 reviewed May 3, 2022
View reviewed changes
Copy link
Collaborator

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

I'm not sure if I understand how this cache is used, so it's hard for me to review this PR.

.github/workflows/docker-image-build.yml Show resolved Hide resolved
.dockerignore Show resolved Hide resolved
@gustavovalverde gustavovalverde merged commit 8b68651 into main May 3, 2022
@gustavovalverde gustavovalverde deleted the fix-cache-invalidation branch May 3, 2022 14:13
@gustavovalverde
Copy link
Member Author

gustavovalverde commented May 3, 2022 •
edited
Loading

It's fun how this change didn't fail in other PRs where I've been using it, but it failed when merged to main #4254

@gustavovalverde gustavovalverde mentioned this pull request May 3, 2022
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teor2345 teor2345 teor2345 left review comments

@dconnolly dconnolly dconnolly approved these changes

Assignees

@gustavovalverde gustavovalverde

Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles C-bug Category: This is a bug C-enhancement Category: This is an improvement do-not-merge Tells Mergify not to merge this PR I-build-fail Zebra fails to build I-integration-fail Continuous integration fails, including build and test failures lightwalletd any work associated with lightwalletd
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docker CI jobs fail with "failed to fetch oauth token: ... 401 Unauthorized"
3 participants
@gustavovalverde @dconnolly @teor2345