Add support for OS Features in the format#16
Conversation
dmcgowan
force-pushed
the
add-os-features
branch
from
77bf3ea to
3f1935b
Compare
|
Does the format already support (non-OS) (I don't think either are currently used for anything serious, but if we're going to support one we should probably support the other too.) |
Which format are you referring to? In OCI there are non-OS |
dmcgowan
force-pushed
the
add-os-features
branch
from
3f1935b to
1dc1164
Compare
|
The corresponding part in the spec is (I believe) https://github.com/opencontainers/image-spec/blob/92353b0bee778725c617e7d57317b568a7796bd0/image-index.md#image-index-property-descriptions
|
I'd like to disallow them and I don't want to have URL-encoding later. Let me check with OCI folks. |
|
Marking this one as ready, OCI issue is there for discussion but the format is owned by containerd |
|
As asked over in opencontainers/image-spec#1237, what's the use case? Is there a particular "feature" you're wanting to put in here / use? |
platforms.go
Outdated
| if platform.OSVersion != "" { | ||
| OSAndVersion := fmt.Sprintf(osAndVersionFormat, platform.OS, platform.OSVersion) | ||
| osOptions := platform.OSVersion | ||
| for _, feature := range platform.OSFeatures { |
There was a problem hiding this comment.
Do we consider platforms to be equal if the features are in a different order?
There was a problem hiding this comment.
It should be considered an unordered set, so yes. We can make the order deterministic here.
|
@tianon firstly to indicate support for erofs from the runtime and requirement for erofs support in the image |
dmcgowan
force-pushed
the
add-os-features
branch
2 times, most recently
from
062cfd0 to
9e8e537
Compare
|
Curious; was there a specific reason for using I recall when we were discussing the addition of os-version, that I considered ways to expand that; #6 (comment)
One thing we should look at (and perhaps as part of the OCI?) is to define limits; I recall a discussion on our Slack where Docker Hub failed to process some images, because some tool (I think it was https://github.com/unikraft/kraftkit) used the Unfortunately, I don't have a reference to that image itself, but I found the content of the field; ["CONFIG_ARCH_X86_64=y","CONFIG_CROSS_COMPILE=","CONFIG_DEBUG_SYMBOLS_LVL3=y","CONFIG_HAVE_APIC=y","CONFIG_HAVE_BOOTENTRY=y","CONFIG_HAVE_INTCTLR=y","CONFIG_HAVE_LIBC=y","CONFIG_HAVE_MMIO=y","CONFIG_HAVE_NW_STACK=y","CONFIG_HAVE_PCI=y","CONFIG_HAVE_SCHED=y","CONFIG_HAVE_TIME=y","CONFIG_HOST_ARCH=x86_64","CONFIG_HZ=100","CONFIG_KVM_BOOT_PROTO_MULTIBOOT=y","CONFIG_KVM_DEBUG_SERIAL_CONSOLE=y","CONFIG_KVM_DEBUG_VGA_CONSOLE=y","CONFIG_KVM_KERNEL_SERIAL_CONSOLE=y","CONFIG_KVM_KERNEL_VGA_CONSOLE=y","CONFIG_KVM_SERIAL_BAUD_115200=y","CONFIG_KVM_VMM_QEMU=y","CONFIG_LIBLWIP=y","CONFIG_LIBMUSL=y","CONFIG_LIBMUSL_AIO=y","CONFIG_LIBMUSL_CONF=y","CONFIG_LIBMUSL_CRYPT=y","CONFIG_LIBMUSL_CTYPE=y","CONFIG_LIBMUSL_DIRENT=y","CONFIG_LIBMUSL_ENV=y","CONFIG_LIBMUSL_ERRNO=y","CONFIG_LIBMUSL_EXIT=y","CONFIG_LIBMUSL_FCNTL=y","CONFIG_LIBMUSL_FENV=y","CONFIG_LIBMUSL_FORCE_THREAD=y","CONFIG_LIBMUSL_INTERNAL=y","CONFIG_LIBMUSL_IPC=y","CONFIG_LIBMUSL_LDSO=y","CONFIG_LIBMUSL_LEGACY=y","CONFIG_LIBMUSL_LINUX=y","CONFIG_LIBMUSL_LOCALE=y","CONFIG_LIBMUSL_LOCALE_LEGACY=y","CONFIG_LIBMUSL_MALLOC=y","CONFIG_LIBMUSL_MATH=y","CONFIG_LIBMUSL_MISC=y","CONFIG_LIBMUSL_MMAN=y","CONFIG_LIBMUSL_MQ=y","CONFIG_LIBMUSL_MULTIBYTE=y","CONFIG_LIBMUSL_NETWORK=y","CONFIG_LIBMUSL_PASSWD=y","CONFIG_LIBMUSL_PRNG=y","CONFIG_LIBMUSL_PROCESS=y","CONFIG_LIBMUSL_REGEX=y","CONFIG_LIBMUSL_SCHED=y","CONFIG_LIBMUSL_SEARCH=y","CONFIG_LIBMUSL_SELECT=y","CONFIG_LIBMUSL_SETJMP=y","CONFIG_LIBMUSL_SIGNAL=y","CONFIG_LIBMUSL_STAT=y","CONFIG_LIBMUSL_STDIO=y","CONFIG_LIBMUSL_STDLIB=y","CONFIG_LIBMUSL_STRING=y","CONFIG_LIBMUSL_TEMP=y","CONFIG_LIBMUSL_TERMIOS=y","CONFIG_LIBMUSL_THREAD=y","CONFIG_LIBMUSL_TIME=y","CONFIG_LIBMUSL_UNISTD=y","CONFIG_LIBNGINX=y","CONFIG_LIBNGINX_HTTP=y","CONFIG_LIBNGINX_HTTP_ACCESS=y","CONFIG_LIBNGINX_HTTP_AUTOINDEX=y","CONFIG_LIBNGINX_HTTP_BROWSER=y","CONFIG_LIBNGINX_HTTP_CHARSET=y","CONFIG_LIBNGINX_HTTP_EMPTY_GIF=y","CONFIG_LIBNGINX_HTTP_FASTCGI=y","CONFIG_LIBNGINX_HTTP_GEO=y","CONFIG_LIBNGINX_HTTP_GRPC=y","CONFIG_LIBNGINX_HTTP_LIMIT_CONN=y","CONFIG_LIBNGINX_HTTP_LIMIT_REQ=y","CONFIG_LIBNGINX_HTTP_MAP=y","CONFIG_LIBNGINX_HTTP_MEMCACHED=y","CONFIG_LIBNGINX_HTTP_MIRROR=y","CONFIG_LIBNGINX_HTTP_POSTPONE=y","CONFIG_LIBNGINX_HTTP_PROXY=y","CONFIG_LIBNGINX_HTTP_REFERER=y","CONFIG_LIBNGINX_HTTP_SCGI=y","CONFIG_LIBNGINX_HTTP_SPLIT_CLIENTS=y","CONFIG_LIBNGINX_HTTP_SSI=y","CONFIG_LIBNGINX_HTTP_STUB_STATUS=y","CONFIG_LIBNGINX_HTTP_UPSTREAM=y","CONFIG_LIBNGINX_HTTP_UPSTREAM_HASH=y","CONFIG_LIBNGINX_HTTP_UPSTREAM_IP_HASH=y","CONFIG_LIBNGINX_HTTP_UPSTREAM_KEEPALIVE=y","CONFIG_LIBNGINX_HTTP_UPSTREAM_LEAST_CONN=y","CONFIG_LIBNGINX_HTTP_UPSTREAM_RANDOM=y","CONFIG_LIBNGINX_HTTP_UPSTREAM_ZONE=y","CONFIG_LIBNGINX_HTTP_USERID=y","CONFIG_LIBNGINX_HTTP_UWSGI=y","CONFIG_LIBNGINX_HTTP_V2=y","CONFIG_LIBNGINX_MAIN_FUNCTION=y","CONFIG_LIBPOSIX_ENVIRON=y","CONFIG_LIBPOSIX_ENVIRON_ENVP0=PATH=/bin","CONFIG_LIBPOSIX_ENVIRON_ENVP0_NOTEMPTY=y","CONFIG_LIBPOSIX_ENVIRON_ENVP10=","CONFIG_LIBPOSIX_ENVIRON_ENVP11=","CONFIG_LIBPOSIX_ENVIRON_ENVP12=","CONFIG_LIBPOSIX_ENVIRON_ENVP13=","CONFIG_LIBPOSIX_ENVIRON_ENVP14=","CONFIG_LIBPOSIX_ENVIRON_ENVP15=","CONFIG_LIBPOSIX_ENVIRON_ENVP1=","CONFIG_LIBPOSIX_ENVIRON_ENVP2=","CONFIG_LIBPOSIX_ENVIRON_ENVP3=","CONFIG_LIBPOSIX_ENVIRON_ENVP4=","CONFIG_LIBPOSIX_ENVIRON_ENVP5=","CONFIG_LIBPOSIX_ENVIRON_ENVP6=","CONFIG_LIBPOSIX_ENVIRON_ENVP7=","CONFIG_LIBPOSIX_ENVIRON_ENVP8=","CONFIG_LIBPOSIX_ENVIRON_ENVP9=","CONFIG_LIBPOSIX_EVENT=y","CONFIG_LIBPOSIX_FUTEX=y","CONFIG_LIBPOSIX_LIBDL=y","CONFIG_LIBPOSIX_PROCESS=y","CONFIG_LIBPOSIX_PROCESS_CLONE=y","CONFIG_LIBPOSIX_PROCESS_INIT_PIDS=y","CONFIG_LIBPOSIX_PROCESS_MAX_PID=31","CONFIG_LIBPOSIX_PROCESS_PIDS=y","CONFIG_LIBPOSIX_SOCKET=y","CONFIG_LIBPOSIX_SYSINFO=y","CONFIG_LIBPOSIX_TIME=y","CONFIG_LIBPOSIX_USER=y","CONFIG_LIBPOSIX_USER_GID=0","CONFIG_LIBPOSIX_USER_GROUPNAME=root","CONFIG_LIBPOSIX_USER_UID=0","CONFIG_LIBPOSIX_USER_USERNAME=root","CONFIG_LIBRAMFS=y","CONFIG_LIBSYSCALL_SHIM=y","CONFIG_LIBSYSCALL_SHIM_LEGACY_VERBOSE=y","CONFIG_LIBSYSCALL_SHIM_NOWRAPPER=y","CONFIG_LIBUKALLOC=y","CONFIG_LIBUKALLOCBBUDDY=y","CONFIG_LIBUKARGPARSE=y","CONFIG_LIBUKBOOT=y","CONFIG_LIBUKBOOT_BANNER_POWEREDBY=y","CONFIG_LIBUKBOOT_INITBBUDDY=y","CONFIG_LIBUKBOOT_INITSCHEDCOOP=y","CONFIG_LIBUKBOOT_MAXNBARGS=60","CONFIG_LIBUKBUS=y","CONFIG_LIBUKBUS_PCI=y","CONFIG_LIBUKBUS_PLATFORM=y","CONFIG_LIBUKCPIO=y","CONFIG_LIBUKDEBUG=y","CONFIG_LIBUKDEBUG_ENABLE_ASSERT=y","CONFIG_LIBUKDEBUG_PRINTK=y","CONFIG_LIBUKDEBUG_PRINTK_ERR=y","CONFIG_LIBUKDEBUG_PRINT_SRCNAME=y","CONFIG_LIBUKDEBUG_PRINT_TIME=y","CONFIG_LIBUKDEBUG_REDIR_PRINTD=y","CONFIG_LIBUKINTCTLR=y","CONFIG_LIBUKINTCTLR_MAX_HANDLERS_PER_IRQ=8","CONFIG_LIBUKINTCTLR_XPIC=y","CONFIG_LIBUKLIBID=y","CONFIG_LIBUKLIBID_INFO=y","CONFIG_LIBUKLIBID_INFO_COMPILEDATE=y","CONFIG_LIBUKLIBID_INFO_LIB_COMPILER=y","CONFIG_LIBUKLIBID_INFO_UKFULLVERSION=y","CONFIG_LIBUKLIBPARAM=y","CONFIG_LIBUKLOCK=y","CONFIG_LIBUKLOCK_MUTEX=y","CONFIG_LIBUKLOCK_RWLOCK=y","CONFIG_LIBUKLOCK_SEMAPHORE=y","CONFIG_LIBUKMMAP=y","CONFIG_LIBUKMPI=y","CONFIG_LIBUKMPI_MBOX=y","CONFIG_LIBUKNETDEV=y","CONFIG_LIBUKNETDEV_DISPATCHERTHREADS=y","CONFIG_LIBUKNETDEV_MAXNBQUEUES=1","CONFIG_LIBUKSCHED=y","CONFIG_LIBUKSCHEDCOOP=y","CONFIG_LIBUKSCHED_TCB_INIT=y","CONFIG_LIBUKSGLIST=y","CONFIG_LIBUKSIGNAL=y","CONFIG_LIBUKSTREAMBUF=y","CONFIG_LIBUKSWRAND=y","CONFIG_LIBUKSWRAND_CHACHA=y","CONFIG_LIBUKSWRAND_INITIALSEED_TIME=y","CONFIG_LIBUKTIMECONV=y","CONFIG_LIBVFSCORE=y","CONFIG_LIBVFSCORE_AUTOMOUNT_ROOTFS=y","CONFIG_LIBVFSCORE_PIPE_SIZE_ORDER=16","CONFIG_LIBVFSCORE_ROOTFS=initrd","CONFIG_LIBVFSCORE_ROOTFS_INITRD=y","CONFIG_LIBVIRTIO_BUS=y","CONFIG_LIBVIRTIO_MMIO=y","CONFIG_LIBVIRTIO_NET=y","CONFIG_LIBVIRTIO_PCI=y","CONFIG_LIBVIRTIO_RING=y","CONFIG_LLVM_TARGET_ARCH=","CONFIG_LWIP_AUTOIFACE=y","CONFIG_LWIP_DHCP=y","CONFIG_LWIP_DNS=y","CONFIG_LWIP_DNS_MAX_SERVERS=2","CONFIG_LWIP_DNS_TABLE_SIZE=32","CONFIG_LWIP_HEAP=y","CONFIG_LWIP_ICMP=y","CONFIG_LWIP_IPV4=y","CONFIG_LWIP_IP_REASS_MAX_PBUFS=10","CONFIG_LWIP_LOOPBACK=y","CONFIG_LWIP_NETIF_EXT_STATUS_CALLBACK=y","CONFIG_LWIP_NETIF_STATUS_PRINT=y","CONFIG_LWIP_NUM_TCPCON=64","CONFIG_LWIP_NUM_TCPLISTENERS=64","CONFIG_LWIP_SOCKET=y","CONFIG_LWIP_STACKTHREAD_MBOX_SIZE=256","CONFIG_LWIP_STACKTHREAD_MBOX_SIZE_256=y","CONFIG_LWIP_TCP=y","CONFIG_LWIP_TCP_KEEPALIVE=y","CONFIG_LWIP_TCP_MSS=1460","CONFIG_LWIP_TCP_RECVMBOX_FACTOR=2","CONFIG_LWIP_THREADS=y","CONFIG_LWIP_UDP=y","CONFIG_LWIP_UDP_RECVMBOX_FACTOR=2","CONFIG_LWIP_UKNETDEV=y","CONFIG_LWIP_UKNETDEV_SCRATCH=64","CONFIG_LWIP_UNIKRAFT21X=y","CONFIG_LWIP_WND_SCALE=y","CONFIG_MARCH_X86_64_GENERIC=y","CONFIG_OPTIMIZE_COMPRESS=y","CONFIG_OPTIMIZE_NOOMITFP=y","CONFIG_OPTIMIZE_PERF=y","CONFIG_PLAT_KVM=y","CONFIG_STACK_SIZE_PAGE_ORDER=4","CONFIG_UKPLAT_LCPU_MAXCOUNT=1","CONFIG_UKPLAT_MEMREGION_MAX_COUNT=128","CONFIG_UK_ARCH=x86_64","CONFIG_UK_CODENAME=Pandora","CONFIG_UK_DEFNAME=nginx-qemu-x86_64-initrd","CONFIG_UK_FULLVERSION=0.15.0~fb9fdf3","CONFIG_UK_NAME=nginx","CONFIG_VIRTIO_DEVICE=y","CONFIG_VIRTIO_MMIO_MAX_DEV_CMDLINE=4"] |
Hi @tianon, the idea is to use this field ( Is that correct? @dmcgowan Or I wonder if |
I do think using the os.Features is the cleanest approach. This PR is only proposing we support a subset of the possible string characters for os.Features so that we can represent it as a string in config, logs, platform output. It is entirely containerd specific. We are not trying to force the use of os.Features in this PR, but it will be good to help prove out and optimize around erofs. |
|
So the idea is that image publishers will re-pack their images with erofs instead of tar and publish both variants under a single image index? As an image publisher, I don't see a strong reason for me to do that, but it's conceivable that other publishers feel differently. The general version of the problem being solved here is one that we already have today in the form of images that contain zstd layers (and runtimes that do not support zstd yet) - it's even conceivable that you'd have an image with one layer that's a IMO, it would make more sense from an OCI image-spec perspective to pack that into The way I'd probably suggest handling this case from the OCI image-spec is a generic annotation that lists all layer |
dmcgowan
force-pushed
the
add-os-features
branch
2 times, most recently
from
61e830d to
e6ed81a
Compare
Updates the os part of the format to include features after the os version. The guarantees that the format may fully represent the platform structure. Signed-off-by: Derek McGowan <derek@mcg.dev>
Ensure the formatting output is consistent Signed-off-by: Derek McGowan <derek@mcg.dev>
dmcgowan
force-pushed
the
add-os-features
branch
from
e6ed81a to
2474351
Compare
|
Updated and now green after CI updates This change is only adding to our formatting and its ok to support a limited set in the formatting. Our use of os.Features can be used to make recommendations to OCI, but any specific objections to features is not a reason to hold off on this PR. |
There was a problem hiding this comment.
The general version of the problem being solved here is one that we already have today in the form of images that contain zstd layers (and runtimes that do not support zstd yet) - it's even conceivable that you'd have an image with one layer that's a
.tar.gz, one that's a.tar.zstd, and one that's erofs (especially in the case of base images you didn't create as part of your build, so only your "new" layers would be erofs).IMO, it would make more sense from an OCI image-spec perspective to pack that into
annotationsthanos.features-- the fact that mounting an erofs layer requires specific OS features is not really a requirement of the image, but of the runtime based on the types of the layers and the runtime's choice of how to mount/handle them. It's also conceivable that a runtime might implement erofs entirely in-runtime and unpack it just like it does a tar, for example.
Just express my own perspective: sure, {tar.gz,tar.zstd,tar,erofs} layers can be combined in a single container image (considering if we really need such hybird images) and it can work. but from the generic point of view, not all potential mediatype (even potential valid customized layer mediatypes) can allow such arbitary combination, and if we consider all possible combinations, the order of magnitude is 2^n, which is huge.
we could provide another list in the manifest but that list doesn't mean more compared to just scan the individual layers to find these, so I agree that we need to give a particular name somewhere to limit a specific capability and fix the range of possiblities.
os.features seems already a user-specific field by definition, so personally I think: now, it's okay for containerd defines its own way to generate and parse this (like Wei said, pouchcontainer already used that) and it doesn't conflict with any written rule; on the other hand, it looks clean too, so I approve Derek's proposal.
My overall thought is still that we need to move forward in some way, rather than pace back and forth -- it's already 2026 and maybe years later, no human developer anymore.
|
LGTM! nydus also uses |
|
To give some context on other projects that have tried solving similar issues:
None of this is to discourage the containerd team from working on different alternatives. I don't think there's any consensus in the community yet for OCI to try to standardize things. And so I think it's good for containerd to try out their own implementation to see if there's agreement from the community. That said, anything we release here is going to get adopted not just in the containerd code, but also in images that are pushed to registries. And so the ecosystem will be locked into supporting that until those images eventually fade from existence (which as Docker schema v1 shows, can be a rather long time). If there's a way to gate any usage of this behind an experimental flag, that would improve our flexibility to change our direction later. |
| } | ||
|
|
||
| // Limit to 4 elements to prevent unbounded split | ||
| parts := strings.SplitN(specifier, "/", 4) |
There was a problem hiding this comment.
this limit may no longer be appropriate
| if osVer == nil { | ||
| return specs.Platform{}, fmt.Errorf("%q is an invalid OS component of %q: OSAndVersion specifier component must match %q: %w", part, specifier, osAndVersionRe.String(), errInvalidArgument) | ||
| // First element is <os>[(<OSVersion>[+<OSFeature>]*)] | ||
| osOptions := osRe.FindStringSubmatch(part) |
There was a problem hiding this comment.
nit... wdyt osOptions vs osSelectors
| case 2: | ||
| // In this case, we treat as a regular OS[(OSVersion)]/arch pair. We don't care | ||
| // about whether or not we know of the platform. | ||
| p.Architecture, p.Variant = normalizeArch(parts[1], "") |
There was a problem hiding this comment.
variant.. vs options/selectors, are these two cases potentially at issue now where sometimes the parts are variants and others are options/selectors.. what happens if it's variants and options?
Actually before This PR just raises another usage of Even someday, OCI could finally get in agreement with something like this or other alternatives (I'm pretty doubtful since everyone goes into slightly different direction; it seems to not get any improvement for almost a decade and either of them really moves forward even slightly), I think the final change is still minor too (the case is rather different from Docker schema v1 I think). |
13 participants
Updates the os part of the format to include features after the os version. The guarantees that the format may fully represent the platform structure.
I propose we consider including the features in the format for 1.0 release, which we are trying to finalize.
Should we consider "/" or "+" as supported parts of OS features here? Right now the spec does not specify but we can define our own limitations. We could support it either by updating the parser (less backwards compatible) or URL encoding (ugly, but could be an edge case so who cares).