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

support RuntimeClass.handler, this will useful like nvidia is not the… #350

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

zhangguanzhang
Copy link
Contributor

@zhangguanzhang zhangguanzhang commented Apr 13, 2024

Fixes #111 . default-runtime is runc, You cannot use RuntimeClass.handler to specify that the pod uses a non-runc runtime.:

# curl -s --unix-socket /var/run/docker.sock http://./v1.42/info | jq .Runtimes
{
  "io.containerd.runc.v2": {
    "path": "runc"
  },
  "nvidia": {
    "path": "nvidia-container-runtime"
  },
  "runc": {
    "path": "runc"
  }
}

before

apiVersion: node.k8s.io/v1
kind: RuntimeClass
metadata:
  name: nvidia
handler: docker
---
apiVersion: v1
kind: Pod
metadata:
....
    spec:
      runtimeClassName: nvidia

RuntimeClass.handler=docker will used runc

docker inspect xx | grep Runtime:
            "Runtime": "runc",

change handler: docker to handler: nvidia will not work, but after this pr:

apiVersion: node.k8s.io/v1
kind: RuntimeClass
metadata:
  name: nvidia
handler: nvidia # <----
---
apiVersion: v1
kind: Pod
metadata:
....
    spec:
      runtimeClassName: nvidia

will handle the RuntimeClass.handler

docker inspect xx | grep Runtime:
            "Runtime": "nvidia",

work with crictl https://github.com/containerd/containerd/blob/main/docs/cri/crictl.md#run-a-pod-sandbox-using-a-config-file

$ cat sandbox-config.json 
{
    "metadata": {
        "name": "nginx-sandbox",
        "namespace": "default",
        "attempt": 1,
        "uid": "hdishd83djaidwnduwk28bcsb"
    },
    "linux": {
    }
}
$ crictl  runp --runtime runc111 sandbox-config.json 
E0420 22:22:04.460269    2703 remote_runtime.go:193] "RunPodSandbox from runtime service failed" err="rpc error: code = Unknown desc = failed to get sandbox runtime: no runtime for \"runc111\" is configured"
FATA[0000] run pod sandbox: rpc error: code = Unknown desc = failed to get sandbox runtime: no runtime for "runc111" is configured
$ crictl  runp --runtime runc sandbox-config.json 
e919f7425bc569ad823da6ffc799bac1318c0b1a1bef7568362ce7f1f10710af

@zhangguanzhang
Copy link
Contributor Author

@neersighted @corhere PTAL

Copy link
Collaborator

@corhere corhere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! cc @nwneisen

@nwneisen
Copy link
Collaborator

@zhangguanzhang Please take a look at why CI is failing

@zhangguanzhang
Copy link
Contributor Author

@zhangguanzhang Please take a look at why CI is failing

I found the reason and am modifying it locally. Also, can you help me look at another PR first? #349

@zhangguanzhang
Copy link
Contributor Author

zhangguanzhang commented Apr 20, 2024

@zhangguanzhang Please take a look at why CI is failing

https://github.com/containerd/containerd/blob/main/docs/cri/crictl.md#run-a-pod-sandbox-using-a-config-file

I used the crictl create test on containerd with a non-existent sandbox ID and found an error. I thought the CreateContainer should have called the RunPodSandbox before it

$ crictl create fakeID container-config.json sandbox-config.json 
E0420 23:37:56.581510    2865 remote_runtime.go:319] "CreateContainer in sandbox from runtime service failed" err="rpc error: code = NotFound desc = failed to find sandbox id \"aaa\": not found" podSandboxID="fakeID"
FATA[0000] creating container: rpc error: code = NotFound desc = failed to find sandbox id "fakeID": not found

@zhangguanzhang
Copy link
Contributor Author

@corhere @nevalla
My local code tests are normal and I have added corresponding unit tests. PTAL

@zhangguanzhang zhangguanzhang force-pushed the runtimeclass branch 6 times, most recently from 4917a42 to 999da7a Compare April 21, 2024 10:39
@cncal
Copy link
Contributor

cncal commented Apr 24, 2024

This feature is exactly what I want. Greate job. Hope this PR would be merged ASSP.

@nwneisen
Copy link
Collaborator

LGTM

@nwneisen nwneisen merged commit 57af35d into Mirantis:master Apr 24, 2024
11 checks passed
@zhangguanzhang zhangguanzhang deleted the runtimeclass branch April 24, 2024 04:17
@jandubois
Copy link

@nwneisen Will this PR be cherry-picked into release/0.3 and included in the next release?

Not pushing for it, I just want to know if it makes sense to wait for a release, or if I should build from master for now.

@neersighted
Copy link
Contributor

This is a new feature that I don't think is appropriate for cherry-pick into an existing branch. We're working on a 0.4 very shortly.

@jandubois
Copy link

We're working on a 0.4 very shortly.

Any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for alternative runtimes
6 participants