Store entire bundle, generate relocation map #65
Conversation
| if len(b.InvocationImages) == 0 { | ||
| return nil, fmt.Errorf("unknown invocation image: %q", d.Digest) | ||
| } | ||
| relocationMap[b.InvocationImages[0].Image] = refFamiliar |
There was a problem hiding this comment.
Here we still only handle the first invocation image from a bundle - at some future point we should handle multiple invocation images.
There was a problem hiding this comment.
Agreed. Is the current restriction captured in an issue?
silvin-lubecki
force-pushed
the
relocation-map
branch
2 times, most recently
from
7b5c27b
to
45fdf5a
Compare
There was a problem hiding this comment.
Manually tested the latest rebase, LGTM.
Thanks a lot, @silvin-lubecki!
|
PTAL @glyn 🙏 |
silvin-lubecki
force-pushed
the
relocation-map
branch
from
51fc7a3
to
541c483
Compare
| } | ||
| // Fixup images | ||
| for name, original := range b.Images { | ||
| if original.BaseImage, err = fixupImage(ctx, original.BaseImage, cfg, events, cfg.componentImagePlatformFilter); err != nil { | ||
| return err | ||
| if err := fixupImage(ctx, &original.BaseImage, relocationMap, cfg, events, cfg.componentImagePlatformFilter); err != nil { |
There was a problem hiding this comment.
I don't understand why the output from fixupImage is sent to a separate goroutine in order to be printed to stderr. Since the calls to fixupImage are executed serially, why can't it just print to stderr directly?
There was a problem hiding this comment.
As the fixup will push or mount several images, we thought that having a more sophisticated event base mechanism will help the consumer of this library to create a good UX on this. For example with docker/app we use https://github.com/morikuni/aec to have a nice UI in the term. Does that make sense?
There was a problem hiding this comment.
I suppose so. The only issue I can think of immediately is if one of these event listeners blocks, it will hold up the termination of the whole process, but I guess that's ok. The user can always Ctrl-C.
- Fixup does not mutate the bundle anymore, unless we want it too using auto-update-bundle, then it will update the digest, size and media type sections - Fixup now generates a relocation map, with original service/invocation images as keys and the digested references of the pushed images as values - Pull now generates the relocation map too. The result of pulling a bundle is a bundle.json and a relocation map. - Bundle is now stored as is in the registry, it is no more re-generated from the OCI index. Bundle is stored using the canonical form. - Split --output flag for fixup and pull commands into 2 flags: --bundle and --relocation-map - Harden e2e test, checking fixed bundle and generated relocation map Signed-off-by: Radu M <root@radu.sh> Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
silvin-lubecki
force-pushed
the
relocation-map
branch
from
541c483
to
ea06786
Compare
|
I addressed your comments @glyn, thank you 👍 |
closes #58
Changes in this PR:
push, it can be safely dropped - not entirely sure if it's better to just not return it at all - WDYT?)bundle.jsonfile in canonical formbundle.jsonin canonical formfixupflag for automatically updating the bundle when media type, content digest, and size are missing from an image.Thanks to @silvin-lubecki for pairing on this!