Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding containerd config options for the shim #573

Open
jsturtevant opened this issue Apr 22, 2024 · 9 comments
Open

Adding containerd config options for the shim #573

jsturtevant opened this issue Apr 22, 2024 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@jsturtevant
Copy link
Contributor

jsturtevant commented Apr 22, 2024

Consider adding containerd config options for the shim to enable feature code paths. Adding containerd config options for the shim would allow for on node configuration and debugging. It could also provide a path for turning on and off newly release / unstable features w/o recompilation.

Originally from spinkube/containerd-shim-spin#79 (comment)

@Mossaka
Copy link
Member

Mossaka commented May 1, 2024

I have a couple questions for this ask:

  1. What kind of options does containerd support to parse? I saw this section of the doc mentions that you can add options to shim runtime (e.g. through [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.<runtime>.options]) but isn't clear on what options are allowed and how will these options being feed to the shim.
  2. What options do we (runwasi) want to support? I would imagine that there is a diff among the set of options that are allowed for different shims (e.g. wasmtime shim vs. spin shim). Or do these options largely open to interpretation for each shim implementaion?
    To elaborate a bit more. I think options like "enable logs" or "enable tracing" are kind of common across all shims, so perhaps that's something runwasi should provide? On the other hand, options like "disable wasmtime precompilation" is a wasmtime shim specific option, which should not be of a concern of runwasi.
  3. Do users have to restart containerd everytime to enable / disable an option for a shim?
    • Follow up on the question above, I think it's best to let runtime-class-manager operator (e.g. the design doc by @devigned ) to mutate the configurations for each shim.

@Mossaka Mossaka added the enhancement New feature or request label May 1, 2024
@jprendes
Copy link
Collaborator

jprendes commented May 1, 2024

IIUC, a shim can define its own options as a protobuf message
https://github.com/containerd/containerd/blob/main/core/runtime/v2/README.md#container-level-shim-configuration

@kate-goldenring
Copy link
Contributor

As @jprendes mentioned, it looks like the options set in the containerd config should be passed in the CreateTaskRequest message under the options field. This is received during create request (issued by containerd) in runwasi. To test out configuring options with containerd, I added an option to the config.toml for the Spin shim and restarted the containerd service:

 [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.spin]
    runtime_type = "io.containerd.spin.v2"

  [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.spin.options]
    Foo = "bar"

I then built the shim locally with a print statement here in runwasi:

    fn task_create(&self, req: CreateTaskRequest) -> Result<CreateTaskResponse> {
        log::info!("task_create called with options: {:?}", req.options);

I expected the option to be passed by containerd and logged but there are still not options set:

May 17 16:39:35 kagold-ThinkPad-X1-Carbon-6th containerd[738]: time="2024-05-17T23:39:35.20109351Z" level=info msg="task_create;"
May 17 16:39:35 kagold-ThinkPad-X1-Carbon-6th containerd[738]: time="2024-05-17T23:39:35.201106634Z" level=info msg="task_create called with options: MessageField(None)"

Does anyone have any recommendations for how to approach this? runc passes the following options:

          [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
            BinaryName = ""
            CriuImagePath = ""
            CriuPath = ""
            CriuWorkPath = ""
            IoGid = 0
            IoUid = 0
            NoNewKeyring = false
            NoPivotRoot = false
            Root = ""
            ShimCgroup = ""
            SystemdCgroup = true

@Mossaka
Copy link
Member

Mossaka commented May 18, 2024

I expected the option to be passed by containerd and logged but there are still not options set:

How did you run the shim? Did you use ctr or kubectl? It might be that containerd only applies configuration to the shim only via CRI (see https://github.com/containerd/containerd/blob/main/docs/man/containerd-config.toml.5.md#multiple-runtimes)

@kate-goldenring
Copy link
Contributor

Thats a good call out. I'll try again from a Kubernetes context

@jsturtevant
Copy link
Contributor Author

  • What options do we (runwasi) want to support? I would imagine that there is a diff among the set of options that are allowed for different shims (e.g. wasmtime shim vs. spin shim). Or do these options largely open to interpretation for each shim implementaion?
    To elaborate a bit more. I think options like "enable logs" or "enable tracing" are kind of common across all shims, so perhaps that's something runwasi should provide? On the other hand, options like "disable wasmtime precompilation" is a wasmtime shim specific option, which should not be of a concern of runwasi.

One set of options we should consider is how to turn on various runtime options as mentioned in spinkube/containerd-shim-spin#115 (comment)

@kate-goldenring
Copy link
Contributor

When using CRI, I was able to see that runwasi receives the options 🎉 So, we'd need to parse the received object and decide where to pass it forward.

@Mossaka
Copy link
Member

Mossaka commented Nov 19, 2024

Can we close this issue now since @kate-goldenring has tested that the containerd config could be passed in the CreateTaskRequest message under the options field?

@kate-goldenring
Copy link
Contributor

@Mossaka we still are not passing these options to the shim in any way. I think this is a good issue to keep open as we discuss what/how we'd want to pass containerd options to the shim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: In Progress
Development

No branches or pull requests

4 participants