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

chore(ci) cache httpbin docker image #620

Closed
wants to merge 2 commits into from
Closed

Conversation

casimiro
Copy link
Contributor

@casimiro casimiro commented Oct 29, 2024

Also prevents dnsmasq from being started on installation.

@casimiro casimiro changed the title chore(ci) cache httpbin docker image and prevents dnsmasq from being started on installation chore(ci) cache httpbin docker image Oct 29, 2024
@casimiro casimiro force-pushed the chore/ci-improvements branch from 730db48 to 9b0a258 Compare October 29, 2024 12:42
Copy link

codecov bot commented Oct 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.84180%. Comparing base (741b22a) to head (ed025a5).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##                main        #620         +/-   ##
===================================================
- Coverage   90.85072%   90.84180%   -0.00892%     
===================================================
  Files             52          52                 
  Lines          11214       11214                 
===================================================
- Hits           10188       10187          -1     
- Misses          1026        1027          +1     

see 3 files with indirect coverage changes

Flag Coverage Δ
unit 90.58023% <ø> (-0.01016%) ⬇️
valgrind 82.48898% <ø> (-0.01960%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@casimiro casimiro force-pushed the chore/ci-improvements branch from 9b0a258 to e7244e7 Compare October 29, 2024 13:27
@thibaultcha
Copy link
Member

Fantastic! Should we also cache and load the nginx image before the "Build httpbin-proxy image" step? This way we also avoid the FROM nginx step to pull it from the hub as well.

@casimiro
Copy link
Contributor Author

Good idea! I'll include nginx image in the cache.

@casimiro
Copy link
Contributor Author

On a second thought, docker/build-push-action is configured to cache its build environment which makes me conclude that it's already preventing nginx image from being pulled whenever the action setup-httpbin-server is ran.

Do you think it's worth caching nginx image regardless of that?

@thibaultcha
Copy link
Member

Ah right! Is it possible to use the same docker/build-push-action to build the kennethreitz/httpbin and take care of the caching by itself then? It would simplify the caching process a lot.

@casimiro
Copy link
Contributor Author

casimiro commented Oct 30, 2024

Yes, we could have an additional docker/build-push-action step and Dockerfile to build an image that simply encapsulates httpbin's. Although less verbose in the setup-httpbin-server action, this seems a too implicit approach for caching an image that's simply pulled.

Maybe the ideal approach would be moving the new caching steps from setup-httpbin-server to a separate action, e.g. cache-docker-image, and have setup-httpbin-server invoking it in a single step. That way setup-httpbin-server doesn't get too cluttered and cache-docker-image represents an explicit intent that we can reuse in our workflows (although there's no demand for reusing it now).

What do you think?

@thibaultcha
Copy link
Member

Ok, let's do the separate action and encapsulate the httpbin image like we do the nginx one. Although let's name this separate action load-docker-image, because that seems to be the core usage of this new action: make an image available for use, and cache it as an implementation detail. Sounds good?

@casimiro
Copy link
Contributor Author

casimiro commented Nov 5, 2024

Yes it does.

@casimiro casimiro force-pushed the chore/ci-improvements branch from e7244e7 to c8ac677 Compare November 8, 2024 14:57
@casimiro casimiro force-pushed the chore/ci-improvements branch from c8ac677 to 30bde2b Compare November 8, 2024 15:27
cache_dir:
required: false
type: string
default: "~/.docker-images-cache"
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call!

CI has been failing rather frequently due to Docker registry returning
`502 Bad Gateway` when the `setup-httpbin-server` action attempts to
pull httpbin's docker image.

In fact, this image has been pulled by 16 jobs (Unit and Valgrind) for every GHA
workflow run. This might explain why we eventually fail to pull it.

Caching it should greatly reduce the number of times httpbin image is
actually pulled, which should prevent us from being rate limited.
@casimiro casimiro force-pushed the chore/ci-improvements branch from 30bde2b to ed025a5 Compare November 8, 2024 17:34
@thibaultcha thibaultcha closed this Nov 9, 2024
@thibaultcha thibaultcha deleted the chore/ci-improvements branch November 9, 2024 01:30
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.

2 participants