feat(oci): manifest/config updates to support containerd

[vdice]

Updates per #1857

• Adds artifactType: application/vnd.bytecodealliance.component.v1+wasm to the OCI manifest (feat(oci): manifest/config updates to support containerd #1882 (comment))
• Adds the locked spin config data as a separate layer
• Updates the OCI manifest config to represent the structure expected by the spec
  • ⚠️ This is a breaking change: older Spin clients won't be able to run Spin apps published by clients with this updated logic.
  • Currently this config uses defaults and we don't yet attempt to push any data specific to a Spin app. Open to thoughts on what we might want to include.
• Updates the wasm layer media type to application/vnd.bytecodealliance.wasm.component.layer.v0+wasm (see feat(oci): manifest/config updates to support containerd #1882 (comment))
  • ⚠️ This is a breaking change: older Spin clients won't be able to run Spin apps published by clients with this updated logic.
  • Note that the corresponding upstream PR is still in flight -- is it too early to update the media type?
• Adds a notion of the 'legacy' wasm layer media type such that new Spin clients can pull Spin apps published by older clients

[vdice]

I'm happy with these changes if @jsturtevant is. We'd like to get them in before a forthcoming 2.0 release as they represent breaking changes. My only reservation is around updating the wasm layer media type while its canonical value seems to still be under discussion. Thoughts?

[jsturtevant]

My only reservation is around updating the wasm layer media type while its canonical value seems to still be under discussion. Thoughts?

For the same reasons in #1882 (comment), runwasi and containerd are not currently opinionated about the layer media types. Runwasi, will grab any layers that are not image types and pass them to the runtime, with the media type. This may change in the future but for now while the eco system is still stabilizing I think it might be ok to keep them layers in the way you have them now in order to not break existing users.

In the WARG/runwasi the plan is to start to use application/vnd.bytecodealliance.wasm.component.layer.v0+wasm. Once we have a 1.0 for WASI and specific way to use this via containerd we will get to a canonical value. Till then I think it this will be in flux, and as mentioned runwasi shims will use the OS (p1/p2) to do most of the distinction between the two formats.

[vdice]

This may change in the future but for now while the eco system is still stabilizing I think it might be ok to keep them layers in the way you have them now in order to not break existing users.

Thanks @jsturtevant; I've kept it at the current value for now and created another PR for Spin to futureproof this a bit so that when we do update the media type it won't necessarily be a breaking change.

[jsturtevant]

great! give me a day or so and I will give this a go with deislabs/containerd-wasm-shims#164

[vdice]

@jsturtevant thank you. We have some urgency to get breaking changes merged before this week's Spin 2.0 release, so do let us know if anything else crops up when integrating with the shim. I'm hoping any other required changes will be additive/non-breaking.

[radu-matei]

End-to-end testing this branch looks great, including running existing apps from the registry that were previously pushed with Spin V1.

[jsturtevant]

I was able to validate this with the latest runwasi in deislabs/containerd-wasm-shims#164

spin registry push docker.io/jsturtevant/spin-wasm-shim:latest-2.0

 sudo ./ctr run --net-host --rm --runtime=io.containerd.spin.v1 docker.io/
jsturtevant/spin-wasm-shim:latest-2.0 testwasm shim.wasm

Serving http://0.0.0.0:3000
Available Routes:
  hello: http://0.0.0.0:3000/hello
  go-hello: http://0.0.0.0:3000/go-hello
Hello, world! You should see me in pod logs

[jsturtevant]

One minor but user experience improvement would be to provide a default entry point https://github.com/containerd/runwasi/blob/2fb77889d5d7b0e590a574cfbc44089b54036811/crates/oci-tar-builder/src/bin.rs#L61C39-L61C39

if this isn't set then when running with ctr need to provide one:

which is the shim.wasm in the command sudo ./ctr run --net-host --rm --runtime=io.containerd.spin.v1 docker.io/ jsturtevant/spin-wasm-shim:latest-2.0 testwasm shim.wasm

otherwise end up with

time="2023-10-30T22:55:30.029486688Z" level=error msg="on non-Windows, at least one process arg entry is required"

time="2023-10-30T22:55:30.029555687Z" level=error msg="failed to initialize container process: missing args in the process spec"

[radu-matei]

@jsturtevant -- what exactly would be the meaning for the "default entrypoint" for a Spin app, which can have multiple components (which means no one wasm component)?

The OCI loader from Spin loads the locked application file and knows which layers are the wasm components, and so it doesn't need any default entrypoint.

[jsturtevant]

This value is passed as arg0 in wasmtime shim. Maybe it isn't used spin shim?

[radu-matei]

My assumption is that it wouldn't be used in the Spin shim, as the information about components is part of the locked app manifest -- if that's not the case, it's something we can explore in detail as a follow-up for this PR?

[jsturtevant]

Yes, I don't think it is blocking. I was using the applications name in runwasi. I am afk, but can look what happens in the docker container scenario later

[jsturtevant]

Spin currently ignores this value, runwasi parses it and sends to the shim via RuntimeContext but it is ignored by spin's shim: https://github.com/deislabs/containerd-wasm-shims/blob/6ca8df8247d94cf1dc6ce5e529b00ee728aee4ec/containerd-shim-spin-v1/src/engine.rs#L123

The reason this is working when packages as a container is because / is the command is set on the yaml: https://github.com/deislabs/containerd-wasm-shims/blob/6ca8df8247d94cf1dc6ce5e529b00ee728aee4ec/README.md?plain=1#L95

In the case of a shim such as wasmtime/wasmedge it makes sense to have pass the entrypoint info as arg0 and we even do some additional parsing to allow for entering other functions in the wasm module https://github.com/containerd/runwasi/blob/642cafacde77d25762c8c0c0ca78ff1010caff16/crates/containerd-shim-wasm/src/container/context.rs#L17-L25

So in this case we can continue to use / in the command for the pod yaml or you could set this value in the config (maybe image name?) and users wouldn't need to set the command to / in the yaml. In the future maybe it can be useful otherwise we can revisit this in the next iteration of the config support in containerd.

[vdice]

Thank you for all of the great context @jsturtevant. Much appreciated. Tracking in #2000.