json-proxy: Add GetEngineConfig to advertise retry policy

[cgwalters]

Basically podman as of recently retries by default, but we can't easily replicate that behavior in the proxy - the "client" needs to handle retries.

Expose the engine configuration (with retries) as an API so that projects like containers-image-proxy-rs can honor it.

Assisted-by: https://github.com/cgwalters/#llms

[mtrmac]

Thanks!

Expose the engine configuration (with retries) as an API

Hum, that doesn’t work exactly the same either, because c/common/libimage/copier has another level of defaults, setting a default for RetryDelay. I’m not sure how all these pieces are ideally expected to fit together in Podman / Buildah.

(@Luap99 PTAL at least at the config / defaults reading.)

[mtrmac]

• AFAICS the actual behavior of c/common/pkg/retry is to treat this as “no delay” (= wait for $delay immediately complete), not as exponential backoff
• I’m aesthetically worried about silently replacing invalid data with some other data. It will not be trivial to find out how the value is “lost” if anyone had to dig into it.

[Luap99]

I am missing the larger picture here, why does the proxy need to care about containers.conf at all? Just use whatever retries you want?

So far containers.conf is used by Podman and Buildah and nothing more.

[Luap99]

I know where little about that proxy but since it is used by skopeo never depended on containers.conf in any way.

This adds a large dependency chain thus to skopeo and makes it care about something it never did before, I do not maintain skopeo so I do not have a strong opinion but it is a design choice that likely should be looked at carefully.

[mtrmac]

Oops, yes. That’s pretty likely to be a big issue.

[cgwalters]

I am missing the larger picture here, why does the proxy need to care about containers.conf at all? Just use whatever retries you want?

The status quo is documented and I understand the rationale behind it, but from the user PoV at a high level it is kind of confusing (I hope you'd agree) for bootc to read policy.json and registries.conf but not containers.conf where the retries are.

Hmm....we could add retries into containers-registries.conf?

# New global values
retries = 10
retry-delay = "2s"

# Per-registry content
[[registry]]
prefix = "example.com/foo"
location = "internal-registry-for-example.com/bar"

[Luap99]

The status quo is documented and I understand the rationale behind it, but from the user PoV at a high level it is kind of confusing (I hope you'd agree) for bootc to read policy.json and registries.conf but not containers.conf where the retries are.

skopeo does not read this either but does expose retry cli options. containers.conf is documented only to work with podman and buildah so I am not sure why a user would think bootc needs to use it?
Why would they care that it uses the same default retry value as podman?

Practically sharing the config files has both benefits and downsides. It sounds good to have all tools use just one file but then it also means you cannot configure different defaults for different tools.

Hmm....we could add retries into containers-registries.conf?

I think the possible concern there would be that retries are driven by external callers outside of the image module so we would add an option that is not used there at all.

[mtrmac]

• Retries generally should exist only at “top levels” (unless there is a structural advantage to immediate retries, such as HTTP range requests allowing resuming without re-downloads); otherwise it’s pretty easy to end up with an exponential amount of retries over N layers each retrying.
• “How many times to retry” is not really a property of a registry but a cost/benefit balance to be decided by the calling application: Consider latency-sensitive accesses to check for presence of cache images vs. reliability-sensitive accesses to download a primary software package.

[cgwalters]

Why would they care that it uses the same default retry value as podman?

Because I want to inject a config into our CI that forces on more and longer retries for registry operations, and I want that to affect both podman and bootc.

but then it also means you cannot configure different defaults for different tools.

(One can do so by e.g. injecting BindReadOnlyPaths in a systemd unit, and ideally we honor env vars for all of them consistently)

For a situation in which I'd want them to be different, I think we could also offer CLI flags.

Retries generally should exist only at “top levels” (unless there is a structural advantage to immediate retries, such as HTTP range requests allowing resuming without re-downloads); otherwise it’s pretty easy to end up with an exponential amount of retries over N layers each retrying.

Agree.

“How many times to retry” is not really a property of a registry but a cost/benefit balance to be decided by the calling application: Consider latency-sensitive accesses to check for presence of cache images vs. reliability-sensitive accesses to download a primary software package.

At least in the use case in which I want to bump retries, I can't think of a case where I'd want "okay the registry for caching hasn't been responding for 30 seconds, I'm going to rebuild from source instead".

[mtrmac]

“How many times to retry” is not really a property of a registry but a cost/benefit balance to be decided by the calling application: Consider latency-sensitive accesses to check for presence of cache images vs. reliability-sensitive accesses to download a primary software package.

Or consider a CI job: When run nightly, users never want registry flakes, so giving it many retries 10 minutes apart is fine. When run against developers work (which they can’t test locally), it’s important for the developer to be told “the registry is unreliable now and you are not going to get this done”.

At least in the use case … I can't think of

My point really is about the proper location of the configuration — at the use case top level, not as a system-wide config file. I’m sure there are deployments where the two collapse into the same thing, but that does not erase the distinction nor the argument against putting the option in global config files. [Yes, we have it in containers.conf now.]