Add support for a pull policy that only pulls new images periodically

[edmorley]

Description

Pack's default pull policy is always, which means that on every pack build, it attempts to pull images even if they exist locally (eg the builder image, run image and any buildpacks specified via docker:// URNs).

This is great for ensuring users don't accidentally build against outdated images, however:

• It adds several seconds to every pack build (even for a no-op when images are up to date)
• It counts against the container registry's rate limits (even for a no-op), which is inconvenient given Docker Hub and ECR's fairly low rate limits for unauthenticated requests.

These issues are particularly problematic when running integration tests, where pack build will be run many times.

Using the if-not-present pull policy (either via Pack global config, or by passing --pull-policy if-not-present to pack build), prevents these issues, however:

• For CI: It's easy to forget to set the global pack pull policy to if-not-present, which can mean much slower integration tests (which can be hard to spot, given the Pack stdout mentioning pulling would typically be hidden in the test runner output unless the test fails).
• When using pack locally: It risks using out of date images.

Proposed solution

• Initially: Add support for a new Pack pull policy option (alongside the existing always and if-not-present options), that only pulls new images if the time since last pull of those images is greater than N hours/days.
• Later on: Make this new pull policy the default, instead of always. This would (a) mean users get the performance benefit locally without having to know that the pull policy option exists, (b) reduce the boilerplate needed (and chance of forgetting to set it) in CI configs.

Open questions

• Should the new pull policy allow the user to configure the time interval?
  • If yes, via what syntax? eg interval=1d
  • If no, what should the hardcoded interval be? 1 day?
• What should the new pull policy be called? Examples:
  • --pull-policy interval=1d
  • --pull-policy if-not-recent
  • --pull-policy if-not-pulled-recently
• How should the state of last pull time for each image be tracked?

Describe alternatives you've considered

• Modify integration test runners to pass --pull-policy if-not-present to pack build both locally and in CI:
  • Pros: This will speed up integration tests everywhere.
  • Cons: Users can potentially be integration testing against out of date images locally (if they don't also run pack build manually from time to time, which would pull). Doesn't help speed up manual repeat pack builds locally when developing.
• Modify integration test runners to pass --pull-policy if-not-present to pack build only in CI:
  • Pros: This will speed up integration tests in CI. No risk of running integration tests locally against stale images.
  • Cons: Doesn't help speed up integration tests locally, or running manual repeat pack builds locally when developing. Means having to add vendor-specific CI checks (eg known env vars) in integration test runners.
• In the CI config only, set the Pack global pull policy to if-not-present:
  • Pros: This will speed up integration tests in CI. No risk of running integration tests locally against stale images. No vendor-specific checks in integration test runners.
  • Cons: Doesn't help speed up integration tests locally, or running manual repeat pack builds locally when developing. Easy to forget to configure (particularly when the end user projects are owned by different users to the test runner, so less familiar with best practices).

Additional context

• Earlier discussion in the original pull-policy RFC: RFC: pack Image Pull Policy rfcs#80 (comment)
• https://github.com/Malax/libcnb.rs/issues/306

[jkutner]

I like this ideal of configurable intervals, but something as simple as the following might solve the 90% case:

• --pull-policy hourly
• --pull-policy daily
• --pull-policy weekly

[edmorley]

I like this ideal of configurable intervals, but something as simple as the following might solve the 90% case:

I like keeping this simpler - those names sound great to me.

How should the state of last pull time for each image be tracked?

I'm presuming there is no way to find out when a specific image was last pulled from Docker (even ignoring alternative implementations), since:

• Docker seems to only track the image created time (at least by looking at docker inspect), when we need the pull time instead
• Builder images have their timestamps normalised anyway (eg "Created": "1980-01-01T00:00:01Z",)

As such, Pack CLI will have to track the state somewhere itself, which leads to the following questions:

1. Where should that place be? I presume we wouldn't want to taint ~/.pack/config.toml with non-settings related state, so perhaps an additional file in that pack directory?
2. Do we also need to think about purging old entries in this file (eg if the max policy is weekly, then anything with a timestamp older than that is redundant, and could be dropped), or not worth it?

I'm presuming this would be implemented around here:

pack/pkg/image/fetcher.go

Lines 74 to 89 in 3e1f522

	switch options.PullPolicy {
	case PullNever:
	img, err := f.fetchDaemonImage(name)
	return img, err
	case PullIfNotPresent:
	img, err := f.fetchDaemonImage(name)
	if err == nil || !errors.Is(err, ErrNotFound) {
	return img, err
	}
	}
	f.logger.Debugf("Pulling image %s", style.Symbol(name))
	err = f.pullImage(ctx, name, options.Platform)
	if err != nil && !errors.Is(err, ErrNotFound) {
	return nil, err
	}

[jromero]

Personally, I'm all for adding new pull policies. Preferably those defined by Joe.

I'm a little more hesitant with "Make this new pull policy the default.". I think we should discuss the potential of unexpected behavior for end-users. If we limit this issue and its PR to simply adding new pull policies, that's a big 👍 from me.

Where should that place be? I presume we wouldn't want to taint ~/.pack/config.toml with non-settings related state, so perhaps an additional file in that pack directory?

Agree with some other file in ~/.pack.

I wonder if it's worth considering using a proper file-based db like sqlite or if that would be overkill. (Partly trying to look forward at other features that we've thought about that might also require persistence like storing OCI annotations associated to images on the daemon).

Do we also need to think about purging old entries in this file (eg if the max policy is weekly, then anything with a timestamp older than that is redundant, and could be dropped), or not worth it?

If we had sqlite, this purging would also be trivial. :)

[jromero]

@dfreilich I set this to status discussion-needed but if you have some thoughts that make this implementable please feel free to change it accordingly.

[jjbustamante]

@edmorley

I think this will be a great feature to be included into pack but because of the complexity and effort of what is asking for, I will put this issues as good for mentorship and we can propose it to GSoC or LFX program in the future or if someone else from the community want to work on this we will be more than happy to guide them.

[Parthiba-Hazra]

Hi everyone, @jjbustamante @edmorley @jkutner I want to work on this issue, will need some guidance from you guys.
I know it needs some moderate level of effort, I can do that, if I get some help from you here.

[jjbustamante]

Hi @Parthiba-Hazra, I was taking a look in your code but I am going to put my thoughts here first.

Note 🧵 with some discussion can be found here

From the slack thread

what if user delete some image from the local registry - If a user deletes an image from the local registry, we need to decide whether to retain or remove the log entry for that image from the JSON file.

I think that if we were not supposed to pull, because we haven't reached the amount of time after the last pull, and we expect to fetch the image from the daemon, but it is not there, because the end-user deleted.

• I will remove the log entry from the json file
• I will throw the not found error, that execution must fail, but next time the image will be pull

[Parthiba-Hazra]

I think that if we were not supposed to pull, because we haven't reached the amount of time after the last pull, and we expect to fetch the image from the daemon, but it is not there, because the end-user deleted.

• I will remove the log entry from the json file
• I will throw the not found error, that execution must fail, but next time the image will be pull

yeah got it.