RFC: pack Image Pull Policy

[jromero]

• Readable

[ekcasey]

I know this is still a draft, so maybe I should withhold my comments for now, but I like this idea a lot. Always checking for builder/run-image updates adds a surprising amount of time to each build.

[jromero]

This is now ready for review. 😄

[hone]

I'm 👍 on the flag, but I'm wary about the default of not pulling. I get people do it for performance, but the default was done for the reason of keeping people up to date so people don't have to think about it. It's super common when working with people that they forget (or really never) docker pull themselves and don't realize they're X versions behind latest.

[nebhale]

I specifically like bringing our default into alignment with the rest of the OCI-image based ecosystem. Regardless of the personal preferences around this, I think our inconsistency with all these other platforms is an additional cognitive burden for new users when on-boarding.

[sclevine]

I'm fine with the flag UX change, but share @hone and @jkutner's concerns about upgrades. Having a single command that produces a secure, up-to-date app image seems valuable.

Could we keep the default behavior, but allow you to set a pack-wide policy in config.toml?

[dfreilich]

I like this as well. Matching the docker/k8s pattern is great, as is the speed change - making initial builds (once images are pre-loaded) faster should definitely help users who want to optimize for speed.

[jabrown85]

How do we feel about buildpack pull policy compared to build and run image policy? Currently --no-pull is applied to buildpacks referenced by image but not by url. See buildpacks/pack#685.

I think we should clarify the stance on what type of actions this policy should gate. I'm of the mind that buildpacks should always be fetched unless we make a new policy specific to them to be consistent regardless of the way the buildpack is defined.

[jkutner]

I have concerns about the default being if-not-present even after our WG discussion today. Inevitably, people will not pull new images, and they may even end up having a bad experience with buildpacks because they are using outdated images. In the worst cases, they'll create out-of-date production images.

That said, I am concerned about performance of pack build, and with the Heroku stack being updated every 2-weeks the pulls will be common. I'd like us to consider some alternatives:

1. We "expire" images (i.e. if an image is old than 30 days we check for a new image and pull if there is one), or...
2. We add pull-policy key to .pack/config.toml so that you can switch your Pack CLI to favor not pulling. But it defaults to always.

I'm not ready to offer this as an alternative but I'm thinking about:

• Only pull builder image by default (but don't pull build and run images), then "rebase on push" or something.

[jabrown85]

I mentioned this to a couple of others, but we could perhaps adopt a special ❄️ case for :latest. k8s does this.

omit the imagePullPolicy and use :latest as the tag for the image to use.

We could do the same, so that you don't pay the cost for looking for anything other than :latest by default. Heroku uses :latest so the behavior wouldn't change for users of that builder image.

I would still propose we at least add a warning about how old your builder is compared to the remote, but do this check async while the build is happening and print the warning at the end. This way it is likely to be seen and does not incur a performance penalty.

[hone]

I also wonder if people would change their mind on this issue if pack develop was a thing. Regardless of what we keep saying about pack, it will be used in a production context. I would expect pack build to pull but pack develop to not since in the development context you're willing to make the tradeoffs for speed.

[drewpca]

If checking that we have the latest buildpack didn't take multiple seconds, we might not even be talking about adding pull policies nor about some users falling way behind. From the client side, it's just asking about one resource id and getting a HTTP 304 back most of the time, which should be quite acceptable. Have we ruled out fixing the slowness on the server side?

[jromero]

@drewpca to be honest, there hasn't been any real look into diagnosing the root cause for the performance hits. Although from observation, it seems to be a lot more apparent to gcr.io registries, whether that be the cred-helpers, docker client (as reported here) or the actual server, there are just too many variables dependent on the registry being used that I personally feel that from the client-side we need to find a viable solution to alleviate that pain point regardless of registry/server implementation.

At the end of the day, the perception and UX from the end-users' perspective is that pack (or buildpacks) are slow when in reality it could very well be related to external circumstances such as in this case.

With all that said, I would also like to add that I agree that we should look into the root cause and maybe provide feedback or fix to the upstream. The purpose of this proposal is to help aid with the majority concern of the performance hit regardless of registry but doesn't negate the need to improve the performance further.

[jromero]

Based on the last WG meeting conversations this RFC will be split into at least 2:

1. Focus on solely adding --pull-policy <arguments> along with a new policy if-not-present
2. Focus on relieving the performance hit of back-to-back build operations.
   Initial proposal:
   • New policy: --pull-policy periodic=1d
   • Setting new policy as the default
   • Making policy globally configurable via config.toml

[jromero]

@sclevine / @jkutner / @hone This PR/RFC has been update to only introduce the new flag and new option of if-not-present while leaving the default alone. Proposing the change of the default policy will have a follow-up RFC.

@jabrown85 as we've discussed, we can probably have a parallel RFC for updating the default and another for more granular control after this RFC is accepted.

[nebhale]

Final Comment Period with merge disposition, closing on 25 July, 2020.

[edmorley]

2\. New policy: `--pull-policy periodic=1d`

Hi! Has there been any further discussion of adding periodic option to --pull-policy?

For both local development and CI workflows, the default of always adds multiple seconds to the time to pack build.

In addition, now that Docker Hub has rate limiting, and with a no-op pull still counting against the rate limit (https://docs.docker.com/docker-hub/download-rate-limit/#definition-of-limits), preventing unnecessary pulls is even more important (think running 100 integration tests).

Whilst one can work around this using a manual pull combined with if-not-present in these scenarios, it would be great to just be able to use a periodic=1d policy everywhere and not have to (a) think about this, (b) have split CI/local workflows etc :-)

[buildpack-bot]

Maintainers,

As you review this RFC please queue up issues to be created using the following commands:

/queue-issue <repo> "<title>" [labels]...
/unqueue-issue <uid>

Issues

(none)

[edmorley]

Hi! Has there been any further discussion of adding periodic option to --pull-policy?

Filed:
buildpacks/pack#1368