[WIP]Fix the --runtime option of container create to support path #2411

yankay · 2023-08-02T10:15:17Z

Description

Unlike ctr , the nerdctl does not support the --runtime flag while creating a container with path

Steps to reproduce the issue

# Copy runc to another path
cp  /usr/local/bin/containerd-shim-runc-v2 /tmp/haha

# create a container with the path
nerdctl container create --runtime /tmp/haha docker.m.daocloud.io/library/nginx:alpine

ctr -n k8s.io container ls
CONTAINER                                                           IMAGE                                        RUNTIME
8e7e104ba64f9aa8d320fe1972d4d62634d920151612c4f0e0fc0fb12b51ace6    docker.m.daocloud.io/library/nginx:alpine    io.containerd.runc.v2

After the fix.

# create a container with the path
root@kay201:~/oss/nerdctl# ./_output/nerdctl container create --runtime /tmp/haha docker.m.daocloud.io/library/nginx:alpine
ctr -n k8s.io container ls
95fadeb4c7e1aadf13698fa4abb1c8d34b16e23444fe9c37beb84a97cb31037f
CONTAINER                                                           IMAGE                                        RUNTIME
95fadeb4c7e1aadf13698fa4abb1c8d34b16e23444fe9c37beb84a97cb31037f    docker.m.daocloud.io/library/nginx:alpine    /tmp/haha

The runtime flag with path can be configed successfully.

And it can run sucessfuly:

root@kay201:~/oss/nerdctl# ./_output/nerdctl run --runtime /tmp/haha docker.m.daocloud.io/library/nginx:alpine
/docker-entrypoint.sh: /docker-entrypoint.d/ is not empty, will attempt to perform configuration
/docker-entrypoint.sh: Looking for shell scripts in /docker-entrypoint.d/
/docker-entrypoint.sh: Launching /docker-entrypoint.d/10-listen-on-ipv6-by-default.sh
10-listen-on-ipv6-by-default.sh: info: Getting the checksum of /etc/nginx/conf.d/default.conf

AkihiroSuda

This doesn't work for --runtime=/usr/local/bin/crun, --runtime=runsc, etc.

fahedouch · 2023-08-06T11:57:54Z

pkg/cmd/container/run_runtime.go

 			if !strings.HasPrefix(runtimeStr, "io.containerd.runc.") {
 				if cgroupManager == "systemd" {
 					logrus.Warnf("cannot set cgroup manager to %q for runtime %q", cgroupManager, runtimeStr)
 				}
 				runtimeOpts = nil
 			}
-		} else {
+		} else if filepath.IsAbs(runtimeStr) {
 			// runtimeStr is a runc binary


why runtimeStr is a runc binary here?

To support --runtime=/usr/local/bin/crun, --runtime=runsc, etc.
(runc-compatible third-party runtime binaries are called "runc binary" here)

--runtime=runsc is not absolute path

fahedouch · 2023-08-07T07:34:35Z

test is failing

test target: "nerdctl"
=== RUN   TestComposeUpDetailedError
    compose_up_test.go:54: assertion failed: 
        Command:  /usr/local/bin/nerdctl --namespace=nerdctl-test compose -f /tmp/nerdctl-compose-test3932546953/docker-compose.yaml up -d
        ExitCode: 0
        Stdout:   
        Stderr:   time="2023-08-07T07:04:15Z" level=info msg="Creating network nerdctl-compose-test3932546953_default"
        time="2023-08-07T07:04:15Z" level=info msg="Ensuring image ghcr.io/stargz-containers/alpine:3.13-org"
        time="2023-08-07T07:04:15Z" level=info msg="Creating container nerdctl-compose-test3932546953-foo-1"
        
        
        Failures:
        ExitCode was 0 expected 1
        Expected stderr to contain "failed to resolve runtime path: invalid runtime name invalid"

Signed-off-by: Kay Yan <kay.yan@daocloud.io>

fahedouch · 2023-08-08T20:34:38Z

do we have the same behavior as docker ?

AkihiroSuda · 2023-08-08T22:43:05Z

do we have the same behavior as docker ?

No, Docker needs explicit configuration in daemon.json.

For nerdctl, we support strings like

“io.containerd.runc.v2”
“runc”, “crun”, “runsc” etc.
“/usr/local/bin/runc” etc.

AkihiroSuda · 2023-08-08T22:45:26Z

pkg/cmd/container/run_runtime.go

@@ -47,6 +48,9 @@ func generateRuntimeCOpts(cgroupManager, runtimeStr string) ([]containerd.NewCon
 				}
 				runtimeOpts = nil
 			}
+		} else if filepath.IsAbs(runtimeStr) {
+			// runtimeStr is absolute path to runtime binary


Runc-compatible binary should be assumed unless the binary name is composed of reversed domains

We can also consider “CSV” values like “type=runc,path=foo”, “type=containerd,name=com.example.foo.v1”, “type=containerd,path=containerd-shim-foo-v1”

fahedouch · 2023-08-09T09:53:32Z

do we have the same behavior as docker ?

No, Docker needs explicit configuration in daemon.json.

For nerdctl, we support strings like

“io.containerd.runc.v2”

“runc”, “crun”, “runsc” etc.

“/usr/local/bin/runc” etc.

I mean this behavior

root@kay200:~# docker container create --runtime com.example.sample.v1 docker.m.daocloud.io/nginx:alpine
Error response from daemon: Unknown runtime specified com.example.sample.v1

fahedouch · 2023-10-20T14:11:46Z

@yankay PTAL

AkihiroSuda requested changes Aug 2, 2023

View reviewed changes

yankay changed the title ~~Fix the --runtime option of container create is ignored~~ [WIP]Fix the --runtime option of container create is ignored Aug 3, 2023

yankay closed this Aug 4, 2023

yankay reopened this Aug 4, 2023

yankay force-pushed the fix-runtime-option-is-ignored branch 4 times, most recently from a1dfb02 to e7b91a5 Compare August 4, 2023 10:06

fahedouch reviewed Aug 6, 2023

View reviewed changes

yankay force-pushed the fix-runtime-option-is-ignored branch 4 times, most recently from 5218242 to 2c8aa16 Compare August 7, 2023 06:40

yankay force-pushed the fix-runtime-option-is-ignored branch from 2c8aa16 to 88a0813 Compare August 7, 2023 07:56

yankay changed the title ~~[WIP]Fix the --runtime option of container create is ignored~~ [WIP ]Fix the --runtime option of container create to support path Aug 7, 2023

yankay force-pushed the fix-runtime-option-is-ignored branch from 88a0813 to 9bbe3ea Compare August 7, 2023 09:44

yankay changed the title ~~[WIP ]Fix the --runtime option of container create to support path~~ [WIP]Fix the --runtime option of container create to support path Aug 7, 2023

yankay force-pushed the fix-runtime-option-is-ignored branch 3 times, most recently from d78d883 to ef8f1c9 Compare August 8, 2023 08:41

Fix the --runtime option of container create is ignored

ef8f1c9

Signed-off-by: Kay Yan <kay.yan@daocloud.io>

AkihiroSuda reviewed Aug 8, 2023

View reviewed changes

AkihiroSuda marked this pull request as draft August 8, 2023 22:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP]Fix the --runtime option of container create to support path #2411

[WIP]Fix the --runtime option of container create to support path #2411

yankay commented Aug 2, 2023 •

edited

AkihiroSuda left a comment •

edited

fahedouch Aug 6, 2023

AkihiroSuda Aug 7, 2023

fahedouch Aug 7, 2023

fahedouch commented Aug 7, 2023

fahedouch commented Aug 8, 2023

AkihiroSuda commented Aug 8, 2023

AkihiroSuda Aug 8, 2023

AkihiroSuda Aug 8, 2023

fahedouch commented Aug 9, 2023 •

edited

fahedouch commented Oct 20, 2023

[WIP]Fix the --runtime option of container create to support path #2411

Are you sure you want to change the base?

[WIP]Fix the --runtime option of container create to support path #2411

Conversation

yankay commented Aug 2, 2023 • edited

Description

Steps to reproduce the issue

After the fix.

AkihiroSuda left a comment • edited

Choose a reason for hiding this comment

fahedouch Aug 6, 2023

Choose a reason for hiding this comment

AkihiroSuda Aug 7, 2023

Choose a reason for hiding this comment

fahedouch Aug 7, 2023

Choose a reason for hiding this comment

fahedouch commented Aug 7, 2023

fahedouch commented Aug 8, 2023

AkihiroSuda commented Aug 8, 2023

AkihiroSuda Aug 8, 2023

Choose a reason for hiding this comment

AkihiroSuda Aug 8, 2023

Choose a reason for hiding this comment

fahedouch commented Aug 9, 2023 • edited

fahedouch commented Oct 20, 2023

yankay commented Aug 2, 2023 •

edited

AkihiroSuda left a comment •

edited

fahedouch commented Aug 9, 2023 •

edited