Build OCI-compatible image cache

[angonz]

Fixes #1118

Add image-manifest=true to build context to support OCI-compatible image cache.
This allows using the registry cache in other registries, like AWS ECR.

[regisb]

🤔 According to my tests this change should not break our CI. But I'm not sure what's the impact, to be honest. I fail to understand this part of the docs: https://docs.docker.com/build/cache/backends/

By default, the OCI media type generates an image index for the cache image. Some OCI registries, such as Amazon ECR, don't support the image index media type: application/vnd.oci.image.index.v1+json. If you export cache images to ECR, or any other registry that doesn't support image indices, set the image-manifest parameter to true to generate a single image manifest instead of an image index for the cache image.

I am tempted not to merge this change, for the following reasons:

1. I don't really have a clue what this does.
2. It makes the build command more complicated.
3. You can achieve the same result by implementing the DOCKER_BUILD_COMMAND filter https://docs.tutor.edly.io/reference/api/hooks/catalog.html#tutor.hooks.Filters.DOCKER_BUILD_COMMAND

[angonz]

Hi Régis!
To be honest, I stopped using the registry cache because it only slows the image building process. Registry cache seems to be tied to the image tag. As we create a new tag for every build, cache is always regenerated. Btw, I didn't know of the DOCKER_BUILD_COMMAND setting. That might be a better alternative.

[DawoudSheraz]

Given the conversations above, are we good to close this PR @angonz?

[angonz]

Yes.

[fghaas]

I opened PR #1161 not having checked for closed issues (sorry!), and @regisb asked me to look at this one and comment, so here goes:

2. It makes the build command more complicated.
3. You can achieve the same result by implementing the DOCKER_BUILD_COMMAND filter https://docs.tutor.edly.io/reference/api/hooks/catalog.html#tutor.hooks.Filters.DOCKER_BUILD_COMMAND

This is pushing complexity to the downstream user, which I'd argue is always a bad idea. "We won't merge a one-liner because you can just as well write a plugin" is not a compelling proposition in my book. :)

However, to illustrate why this should go in (not as a feature addition but rather a bug fix), let's address this question:

1. I don't really have a clue what this does.

Buckle up, because it's about to get ugly. :)

BuildKit had a long-standing issue (moby/buildkit#2251) which meant that the cache image index it was generating was essentially proprietary and not compliant with the relevant spec.

In the comments on that issue, it was proposed that OCI-compliant manifests could be enabled with oci-manifest=true. However, when the feature landed in moby/buildkit#3724, the option name ended up being image-manifest.

So, if (and only if) we specify image-manifest=true, BuildKit does the correct (OCI-compliant) thing and creates an OCI-compliant rather than a proprietary manifest format. And that's the proper way to support cached image builds across multiple registry providers.

Also, on this one:

To be honest, I stopped using the registry cache because it only slows the image building process. Registry cache seems to be tied to the image tag. As we create a new tag for every build, cache is always regenerated.

I cannot corroborate the "slows the image building process" part. And yes, we do have to build from scratch every time we create a new tag. However, it is very helpful to be able to use cached builds for multiple builds of the same image tag, which tends to happen during stages of code review.

[regisb]

Your explanations are very clear and helpful, thanks a lot. Now I understand why this proposed change should be a no-brainer -- on the condition that it doesn't break compatibility with docker.io. According to my superficial testing, image-manifest=true does work with docker.io. Can you confirm that?

EDIT: just realised that the answer to my question was here ("support across providers"). I assume that the reported issue for docker.io is now fixed. Not sure what's the status of Red Hat Quay..

[regisb]

Thanks to you both @angonz and @fghaas for bearing with me. I agree that we should merge this change.

@angonz your PR did not include a changelog entry; @fghaas your PR has a changelog entry, but no "by @fghaas" signature. It's by no means mandatory to include a signature, but if you do, then I think it would make sense that both your names should be mentioned.

[fghaas]

Your explanations are very clear and helpful, thanks a lot.

I hesitate to take credit for the explanation, as it was @kcollasarundell who very helpfully explained this to me, all credit should go to him, and any misinterpretations of his explanation are mine. :)

@angonz your PR did not include a changelog entry; @fghaas your PR has a changelog entry, but no "by @fghaas" signature. It's by no means mandatory to include a signature, but if you do, then I think it would make sense that both your names should be mentioned.

Sure. I have updated #1161 with an extended changelog entry, and have also added Co-authored-by and Fixes lines to the commit message for that patch.

[angonz]

Thank you all! I'm very happy to see that multiple visions on one problem end up in something much better.