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

Replace actions/cache with a more robust strategy #11

Merged
merged 20 commits into from
Jan 22, 2025
Merged

Conversation

gvegayon
Copy link
Member

@gvegayon gvegayon commented Jan 15, 2025

This pull request includes several changes to the twostep-container-build GitHub Action, focusing on improving caching mechanisms, updating documentation, and enhancing the build process. The most important changes include adding new jobs to test caching, updating the action version, and modifying the action's inputs and outputs. This version drops actions/cache to respond to a potential bug reported in #10.

Enhancements to caching and build process:

  • .github/workflows/test-twostep-container-build.yml: Added new jobs test-no-args-rerun and test-with-args to test caching and build actions. Updated cache keys to include prefixes (no-args- and with-args-). [1] [2] [3] [4]
  • twostep-container-build/action.yml: Introduced new inputs build-labels-1 and build-labels-2 for specifying labels during the build process. Added a new output summary to indicate the result of the build (built, rebuilt, or cached). Enhanced the caching mechanism by checking if the image exists and inspecting labels to determine cache validity. [1] [2] [3] [4] [5] [6]

Documentation updates:

Version updates:

Context information:

Copy link

github-actions bot commented Jan 15, 2025

Thank you for your contribution @gvegayon 🚀! Your readme-ubuntu-latest is ready for download 👉 here 👈!
(The artifact expires on 2025-04-17T19:37:32Z. You can re-generate it by re-running the workflow here.)

Copy link

github-actions bot commented Jan 15, 2025

Thank you for your contribution @gvegayon 🚀! Your readme-macos-latest is ready for download 👉 here 👈!
(The artifact expires on 2025-04-17T19:37:32Z. You can re-generate it by re-running the workflow here.)

Copy link

github-actions bot commented Jan 15, 2025

Thank you for your contribution @gvegayon 🚀! Your readme-windows-latest is ready for download 👉 here 👈!
(The artifact expires on 2025-04-17T19:37:32Z. You can re-generate it by re-running the workflow here.)

Copy link

github-actions bot commented Jan 15, 2025

Thank you for your contribution @gvegayon 🚀! Your readme-rocker is ready for download 👉 here 👈!
(The artifact expires on 2025-04-17T19:37:32Z. You can re-generate it by re-running the workflow here.)

@gvegayon gvegayon marked this pull request as ready for review January 15, 2025 23:31
@gvegayon gvegayon changed the title Adding new workflow Replace actions/cache with a more robust strategy Jan 15, 2025
Copy link
Collaborator

@zsusswein zsusswein left a comment

Choose a reason for hiding this comment

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

Hey George! Thanks for taking a crack at this. It's much appreciated.

I think this is generally what we want, but I have some specific questions about the implementation:

  • For the test-twostep-container-build.yml workflow, I'm having some trouble understanding the difference between the run with args- and with no-args- . Can you edit the names and add a comment to explain what the desired result is and how they're different? I get mechanically something is different in the argument specification, but it's not obvious how and why they're meaningfully different tests.
  • There seems to be some unrelated changes in here for the post-artifact action. Should they be in a separate PR?
  • It would be much faster to tag the dependencies image with the hash and, on re-runs, query the container registry for the tag. That would cut out the image pull and re-build.

@gvegayon
Copy link
Member Author

* For the `test-twostep-container-build.yml` workflow, I'm having some trouble understanding the difference between the run with `args-` and with `no-args-` . Can you edit the names and add a comment to explain what the desired result is and how they're different? I get mechanically something is different in the argument specification, but it's not obvious how and why they're meaningfully different tests.

The with-args run ueses the args parameter and checks it was passed properly, that's it!:

build-args-2: |
GH_SHA=${{ github.sha }}
. Now that I think, I should also check the labels args as well.

* There seems to be some unrelated changes in here for the `post-artifact` action. Should they be in a separate PR?

I'm adding a debugging step that posts the github context information, to both actions. Since I am bumping the version of the action, I figured it would be easier to do it here. I don't have a particular set of rules regarding dev, but that's something we could add after this is merged; what do you think?

* It would be much faster to tag the dependencies image with the hash and, on re-runs, query the container registry for the tag. That would cut out the image pull and re-build.

The problem with that is that it would make local builds harder. But I think time should be OK as the pull is done only once: You still need to pull the image in the second step, and I think docker/podman are smart enough to check the hash of the image itself. No?

@gvegayon gvegayon requested a review from zsusswein January 17, 2025 19:40
@gvegayon gvegayon merged commit 9390790 into main Jan 22, 2025
7 checks passed
@gvegayon gvegayon deleted the ggvy-passing-labels branch January 22, 2025 02:58
@gvegayon gvegayon linked an issue Jan 22, 2025 that may be closed by this pull request
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.

Two-step image build-and-push has erroneous cache hits
2 participants