On Tue, Oct 24, 2023 at 12:58 PM Bruce Ashfield
<[email protected]> wrote:
>
> On Mon, Oct 16, 2023 at 3:34 PM Joshua Watt <[email protected]> wrote:
> >
> > On Mon, Oct 16, 2023 at 12:04 PM Bruce Ashfield
> > <[email protected]> wrote:
> > >
> > > On Wed, Oct 4, 2023 at 10:54 AM Joshua Watt <[email protected]> wrote:
> > > >
> > > > umoci treats the --image parameter as <DIRECTORY>:<NAME>, where the OCI
> > > > spec ref.name for the image it set to <NAME>. Therefore, if the
> > > > intention is for the ref.name to be in the format <IMAGE_NAME>:<TAG> (as
> > > > is expected by e.g. `podman load`, then the complete argument that is
> > > > passed to umoci must be `--image <DIRECTORY>:<IMAGE_NAME>:<TAG>`
> > >
> > > This is going to take a few iterations as well, the changing spec
> > > and umoci uses have made this a non-trivial area to modify.
> > >
> > > I assume you are talking about the oci ref.name annotation ?
> >
> > Correct. If you pass <DIRECTORY>:<IMAGE_NAME>:<TAG> to umoci, ref.name
> > will be <IMAGE_NAME>:<TAG>
> >
> > >
> > > >
> > > > Signed-off-by: Joshua Watt <[email protected]>
> > > > ---
> > > > classes/image-oci-umoci.inc | 52 +++++++++++++++++++------------------
> > > > 1 file changed, 27 insertions(+), 25 deletions(-)
> > > >
> > > > diff --git a/classes/image-oci-umoci.inc b/classes/image-oci-umoci.inc
> > > > index 58e4668..a58cb4c 100644
> > > > --- a/classes/image-oci-umoci.inc
> > > > +++ b/classes/image-oci-umoci.inc
> > > > @@ -35,14 +35,16 @@ IMAGE_CMD:oci() {
> > > > fi
> > > >
> > > > if [ -z "${OCI_IMAGE_TAG}" ]; then
> > > > - OCI_IMAGE_TAG="initial-tag"
> > > > - fi
> > > > + image_spec="$image_name:${IMAGE_BASENAME}:initial-tag"
> > > > + else
> > > > + image_spec="$image_name:${IMAGE_BASENAME}:${OCI_IMAGE_TAG}"
> > >
> > > I'm not seeing any examples in the umoci documentation that shows
> > > a "triplet" of values for the image.
> > >
> > > It is always image-path[:image-tag] in what I've read and see in the
> > > source.
> >
> > Ya, I've noticed that. I think the reason for that is that ref.name
> > being in the format "NAME:TAG" is a OCI convention that umoci doesn't
> > _actually_ really need to care about. umoci cares about what directory
> > the image is in (image-path) and what the ref.name should be set to
> > (image-tag). Whether image-tag contains a ":" is irrelevant to umoci
> > since it just copies whatever comes after the first ":" into ref.name.
> >
> > Importantly, image-path is never encoded in the JSON anywhere, so it's
> > not equivalent to the container name.
> >
> > >
> > > Are you saying that what is written into th oci spec json file should be
> > > different ? and that encoding what you want in the image-path portion
> > > of the umoci --image parameter makes it pass through correctly ?
> >
> > Ya. I can get containers that have a "nice" ref.name encoded in them by
> > doing:
> >
> > OCI_IMAGE_TAG = "$image_name:${IMAGE_BASENAME}:latest"
> >
> > Which then sets `ref.name = "${IMAGE_BASENAME}:latest"` and everything
> > is happy with this. My suggestion is that the default behavior should
> > be to encode a nice ref.name like this. Otherwise, tools can (and do)
> > weird things when loading an OCI image. For example, in the current
> > implementation, `podmain load -i <IMAGE>` will import the container
> > _name_ as `latest`, then give it a `latest` tag since none was
> > specified, so you end up with a `podman run localhost/latest:latest`
> > which is just weird (and IMHO, a bad experience).
>
> Finally back to this.
>
> Can you send me a link to the spec you are reading ? Because from
> what I've been reading; ref.name has no semantic restrictions within
> the specified grammar. I just want to be sure that I'm not missing
> something.
The OCI spec doesn't really seem to concern itself with the concept of
image labels (e.g. "latest") at all. It's silent on them, so this is
just an emperical observation that behavior of importing images
created by the oci classes is terrible, unless you use a tool that
forces you to explicitly specify the destination name (like skopeo
copy)
>
> I'm of course talking about: org.opencontainers.image.ref.name
> and I'm using the oci image-spec repository.
Yep
>
> That lack of semantics means we are just chasing whatever tools
> have decided is their default, and there is no right choice IMHO.
Maybe, but again, the default experience of `podman load`, which is
IMHO the one we probably _really_ care about, is terrible.
Based on the extra work the OCI class is doing to construct a
directory name that looks like an image name, I had sort of assumed
that it was trying (and failing) to encode the image name into the
image so it would load nicely. Otherwise all the logic to construct
the directory name could just be simplified to use a fixed name
(like... "image") because the name of the directory has no affect on
the name of the image. Maybe I'm reading too much into the tea leaves
here :)
>
> It (ref.name) is most often just a tag (in the OCI examples), which is
> why I was just setting it to the tag, and not containing the image
> name in it at all, and also why I always check the variable to be
> empty before assigning a simple default .. there's no way to get
> it right for all consumers.
>
> Whatever is inheriting the class can tune it for the intended
> container runtime. That being said, I'm ok with a bit more complexity
> in the class to avoid everyone carrying more information than required
> outside of the class.
>
> i.e. I don't really want to change the default, or more specifically force
> the <image name>:<tag>, since with the proposed change, there's no
> way to pass in an OCI_IMAGE_TAG and not have it somehow modified.
> But I also don't need to compose some of the values each time, if there's
> a single variable it is easier to override and make sure it is consistently
> used (like your patch using the local variable image_spec).
>
> In this case, is something breaking if you make sure your tag is
> encoded as "${IMAGE_BASENAME}:latest" before letting the image
> class do its work ? From a glance, it will be consistently composed
> as "$image_name:${IMAGE_BASENAME}:latest" and then passed
> via --image.
Ya, this works fine and I'll do that. Figured I'd at least make the
case that the default is perhaps not particularly great.... It was
really confusing to me (as a new person to meta-virt) why it was
generating such bad names when I imported the images. If we don't
change it, perhaps a least a warning against using `podman load` to
load images in the README?
>
> I'm building some oci-images now and having a look at the different
> runtimes and copies of the images .. to see what happens with the
> ref.name. and I'm going to experiment with some ways we can hint
> about the format as an input.
>
> Bruce
>
>
> >
> > >
> > > Bruce
> > >
> > > > + fi
> > > >
> > > > if [ -n "$new_image" ]; then
> > > > bbdebug 1 "OCI: umoci init --layout $image_name"
> > > > umoci init --layout $image_name
> > > > - umoci new --image $image_name:${OCI_IMAGE_TAG}
> > > > - umoci unpack --rootless --image $image_name:${OCI_IMAGE_TAG}
> > > > $image_bundle_name
> > > > + umoci new --image $image_spec
> > > > + umoci unpack --rootless --image $image_spec $image_bundle_name
> > > > else
> > > > # todo: create a different tag, after checking if the passed
> > > > one exists
> > > > true
> > > > @@ -52,58 +54,58 @@ IMAGE_CMD:oci() {
> > > > bbdebug 1 "OCI: cp -r ${IMAGE_ROOTFS}/* $image_bundle_name/rootfs/"
> > > > cp -r ${IMAGE_ROOTFS}/* $image_bundle_name/rootfs
> > > >
> > > > - bbdebug 1 "OCI: umoci repack --image $image_name:${OCI_IMAGE_TAG}
> > > > $image_bundle_name"
> > > > - umoci repack --image $image_name:${OCI_IMAGE_TAG}
> > > > $image_bundle_name
> > > > + bbdebug 1 "OCI: umoci repack --image $image_spec
> > > > $image_bundle_name"
> > > > + umoci repack --image $image_spec $image_bundle_name
> > > >
> > > > bbdebug 1 "OCI: configuring image"
> > > > if [ -n "${OCI_IMAGE_LABELS}" ]; then
> > > > for l in ${OCI_IMAGE_LABELS}; do
> > > > - bbdebug 1 "OCI: umoci config --image
> > > > $image_name:${OCI_IMAGE_TAG} --config.label $l"
> > > > - umoci config --image $image_name:${OCI_IMAGE_TAG}
> > > > --config.label $l
> > > > + bbdebug 1 "OCI: umoci config --image $image_spec
> > > > --config.label $l"
> > > > + umoci config --image $image_spec --config.label $l
> > > > done
> > > > fi
> > > > if [ -n "${OCI_IMAGE_ENV_VARS}" ]; then
> > > > for l in ${OCI_IMAGE_ENV_VARS}; do
> > > > - bbdebug 1 "umoci config --image
> > > > $image_name:${OCI_IMAGE_TAG} --config.env $l"
> > > > - umoci config --image $image_name:${OCI_IMAGE_TAG}
> > > > --config.env $l
> > > > + bbdebug 1 "umoci config --image $image_spec --config.env $l"
> > > > + umoci config --image $image_spec --config.env $l
> > > > done
> > > > fi
> > > > if [ -n "${OCI_IMAGE_PORTS}" ]; then
> > > > for l in ${OCI_IMAGE_PORTS}; do
> > > > - bbdebug 1 "umoci config --image
> > > > $image_name:${OCI_IMAGE_TAG} --config.exposedports $l"
> > > > - umoci config --image $image_name:${OCI_IMAGE_TAG}
> > > > --config.exposedports $l
> > > > + bbdebug 1 "umoci config --image $image_spec
> > > > --config.exposedports $l"
> > > > + umoci config --image $image_spec --config.exposedports $l
> > > > done
> > > > fi
> > > > if [ -n "${OCI_IMAGE_RUNTIME_UID}" ]; then
> > > > - bbdebug 1 "umoci config --image $image_name:${OCI_IMAGE_TAG}
> > > > --config.user ${OCI_IMAGE_RUNTIME_UID}"
> > > > - umoci config --image $image_name:${OCI_IMAGE_TAG} --config.user
> > > > ${OCI_IMAGE_RUNTIME_UID}
> > > > + bbdebug 1 "umoci config --image $image_spec --config.user
> > > > ${OCI_IMAGE_RUNTIME_UID}"
> > > > + umoci config --image $image_spec --config.user
> > > > ${OCI_IMAGE_RUNTIME_UID}
> > > > fi
> > > > if [ -n "${OCI_IMAGE_WORKINGDIR}" ]; then
> > > > - bbdebug 1 "umoci config --image $image_name:${OCI_IMAGE_TAG}
> > > > --config.workingdir ${OCI_IMAGE_WORKINGDIR}"
> > > > - umoci config --image $image_name:${OCI_IMAGE_TAG}
> > > > --config.workingdir ${OCI_IMAGE_WORKINGDIR}
> > > > + bbdebug 1 "umoci config --image $image_spec
> > > > --config.workingdir ${OCI_IMAGE_WORKINGDIR}"
> > > > + umoci config --image $image_spec --config.workingdir
> > > > ${OCI_IMAGE_WORKINGDIR}
> > > > fi
> > > > if [ -n "${OCI_IMAGE_STOPSIGNAL}" ]; then
> > > > - bbdebug 1 "umoci config --image $image_name:${OCI_IMAGE_TAG}
> > > > --config.stopsignal ${OCI_IMAGE_STOPSIGNAL}"
> > > > - umoci config --image $image_name:${OCI_IMAGE_TAG}
> > > > --config.stopsignal ${OCI_IMAGE_STOPSIGNAL}
> > > > + bbdebug 1 "umoci config --image $image_spec
> > > > --config.stopsignal ${OCI_IMAGE_STOPSIGNAL}"
> > > > + umoci config --image $image_spec --config.stopsignal
> > > > ${OCI_IMAGE_STOPSIGNAL}
> > > > fi
> > > > if [ -n "${OCI_IMAGE_OS}" ]; then
> > > > - bbdebug 1 "umoci config --image $image_name:${OCI_IMAGE_TAG}
> > > > --os ${OCI_IMAGE_OS}"
> > > > - umoci config --image $image_name:${OCI_IMAGE_TAG} --os
> > > > ${OCI_IMAGE_OS}
> > > > + bbdebug 1 "umoci config --image $image_spec --os
> > > > ${OCI_IMAGE_OS}"
> > > > + umoci config --image $image_spec --os ${OCI_IMAGE_OS}
> > > > fi
> > > >
> > > > - bbdebug 1 "umoci config --image $image_name:${OCI_IMAGE_TAG}
> > > > --architecture ${OCI_IMAGE_ARCH}"
> > > > - umoci config --image $image_name:${OCI_IMAGE_TAG} --architecture
> > > > ${OCI_IMAGE_ARCH}
> > > > + bbdebug 1 "umoci config --image $image_spec --architecture
> > > > ${OCI_IMAGE_ARCH}"
> > > > + umoci config --image $image_spec --architecture ${OCI_IMAGE_ARCH}
> > > > # NOTE: umoci doesn't currently expose setting the architecture
> > > > variant,
> > > > # so if you need it use sloci instead
> > > > if [ -n "${OCI_IMAGE_SUBARCH}" ]; then
> > > > bbnote "OCI: image subarch is set to: ${OCI_IMAGE_SUBARCH}, but
> > > > umoci does not"
> > > > bbnote " expose variants. use sloci instead if this is
> > > > important"
> > > > fi
> > > > - umoci config --image $image_name:${OCI_IMAGE_TAG}
> > > > --config.entrypoint ${OCI_IMAGE_ENTRYPOINT}
> > > > + umoci config --image $image_spec --config.entrypoint
> > > > ${OCI_IMAGE_ENTRYPOINT}
> > > > if [ -n "${OCI_IMAGE_ENTRYPOINT_ARGS}" ]; then
> > > > - umoci config --image $image_name:${OCI_IMAGE_TAG} --config.cmd
> > > > "${OCI_IMAGE_ENTRYPOINT_ARGS}"
> > > > + umoci config --image $image_spec --config.cmd
> > > > "${OCI_IMAGE_ENTRYPOINT_ARGS}"
> > > > fi
> > > > - umoci config --image $image_name:${OCI_IMAGE_TAG} --author
> > > > ${OCI_IMAGE_AUTHOR_EMAIL}
> > > > + umoci config --image $image_spec --author ${OCI_IMAGE_AUTHOR_EMAIL}
> > > >
> > > > # make a tar version of the image direcotry
> > > > # 1) image_name.tar: compatible with oci tar format, blobs and
> > > > rootfs
> > > > --
> > > > 2.34.1
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > > --
> > > - Thou shalt not follow the NULL pointer, for chaos and madness await
> > > thee at its end
> > > - "Use the force Harry" - Gandalf, Star Trek II
>
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#8404):
https://lists.yoctoproject.org/g/meta-virtualization/message/8404
Mute This Topic: https://lists.yoctoproject.org/mt/101756743/21656
Group Owner: [email protected]
Unsubscribe: https://lists.yoctoproject.org/g/meta-virtualization/unsub
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
