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

Use --pull-policy if-not-present with Pack CLI in libcnb-test #306

Closed
edmorley opened this issue Feb 4, 2022 · 1 comment · Fixed by #373
Closed

Use --pull-policy if-not-present with Pack CLI in libcnb-test #306

edmorley opened this issue Feb 4, 2022 · 1 comment · Fixed by #373
Assignees
@edmorley
Labels
faster tests Things that improve the runtime of tests libcnb-test

Comments

@edmorley
Copy link
Member

edmorley commented Feb 4, 2022 •
edited
Loading

By default (if the global config hasn't been updated), Pack CLI will attempt to pull the builder image every time a pack build is performed. Even if this image is up to date, it still:

  • adds ~2 seconds to the pack build time
  • counts against the container registry rate limits (ie Docker Hub or ECR)

This is not ideal when running pack build multiple times in a short period of time - as happens when running libcnb-test.

Currently to prevent around this in CI, we enable the if-not-present pull policy via Pack global config:
https://github.com/Malax/libcnb.rs/blob/552edb954cfc069814141baf4945921edb4e7a47/.github/workflows/ci.yml#L75-L80

However:

  1. This means every project using libcnb.rs (a) has to remember to set this in their CI (which is easy to forget, eg https://github.com/heroku/procfile-cnb-rust/pull/24), (b) now has extra boilerplate in their CI config.
  2. Solving for CI doesn't help usages locally, where users (a) might not know about the pull policy config, (b) might not want to set it globally since it affects all other pack usages outside of integration tests too.

One way to avoid both of these issues, would be if libcnb-test were to pass --pull-policy if-not-present to pack build as part of running the integration tests.

One downside to doing that, would be that if a user runs libcnb-test powered integration tests on a machine that has old builder images, then they would never get updated unless the user is also running pack build outside of integration tests too. However (a) users will presumably be running pack build reasonably often, (b) if there are any builder image changes that cause issues then CI would catch it anyway (since it will be pulling in new images each time, since none will be present).

Ideally Pack CLI would support an if-not-pulled-recently option, which would skip pulling if that builder image was pulled recently - and so give the best of both worlds. However in lieu of havingif-not-pulled-recently (we should file an issue), I think libcnb-test should pass --pull-policy if-not-present to Pack for now, as it's the least worst of the two approaches.
@edmorley edmorley mentioned this issue Feb 4, 2022
Open
@edmorley
Copy link
Member Author

edmorley commented Feb 4, 2022

Ideally Pack CLI would support an if-not-pulled-recently option, which would skip pulling if that builder image was pulled recently

Filed:
buildpacks/pack#1368

@edmorley edmorley mentioned this issue Feb 15, 2022
Merged
@edmorley edmorley added the faster tests Things that improve the runtime of tests label Mar 2, 2022
@edmorley edmorley self-assigned this Mar 6, 2022
@edmorley edmorley mentioned this issue Mar 6, 2022
Merged
edmorley added a commit that referenced this issue Mar 7, 2022
@edmorley
Use --pull-policy if-not-present when running pack build
d3ca146 
Since it:
1. Saves ~2 seconds per integration test re-pulling an image that's already up to date.
2. Helps prevent hitting Docker Hub or ECR rate limits from duplicate (and redundant)
   image pulling (that counts agains the rate limit even when it's a no-op).

Longer term, if Pack CLI supports a periodic pulling mode (buildpacks/pack#1368), we
can switch to that, however for now this is the lesser of two evils - and in most cases
Pack usage outside of `libcnb-test` will ensure that newer builder images are pulled
from time to time.

Fixes #306.
edmorley added a commit that referenced this issue Mar 7, 2022
@edmorley
Use --pull-policy if-not-present when running pack build (#373)
95feb4c 
Since it:
1. Saves ~2 seconds per integration test re-pulling an image that's already up to date.
2. Helps prevent hitting Docker Hub or ECR rate limits from duplicate (and redundant)
   image pulling (that counts agains the rate limit even when it's a no-op).

Longer term, if Pack CLI supports a periodic pulling mode (buildpacks/pack#1368), we
can switch to that, however for now this is the lesser of two evils - and in most cases
Pack usage outside of `libcnb-test` will ensure that newer builder images are pulled
from time to time.

See #306 for more details.

See also:
https://buildpacks.io/docs/tools/pack/cli/pack_config_pull-policy/

Fixes #306.
GUS-W-10799284.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@edmorley edmorley

Labels
faster tests Things that improve the runtime of tests libcnb-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use --pull-policy if-not-present when running pack build heroku/libcnb.rs
1 participant
@edmorley