CreateTaskRequest options switch missing case for *runctypes.CreateOptions about gvisor-containerd-shim HOT 7 CLOSED

PR: #20

from gvisor-containerd-shim.

@jmillikin-stripe Are you using gvisor-containerd-shim with Kubernetes + containerd? Or just containerd?

With "Kubernetes + containerd", CreateOptions should never be used.
With just containerd, I don't think containerd client options are designed for non-runc runtimes right now, e.g. https://github.com/containerd/containerd/blob/master/task_opts_unix.go#L31

from gvisor-containerd-shim.

I'm using it with containerd directly, not via Kubernetes or CRI.

It works fine except for this one particular missing type assertion, and the referenced PR does work for the subset of CreateOptions that gVisor supports.

from gvisor-containerd-shim.

@jmillikin-stripe can you share your config.toml? Also, what exactly are you doing to get this error? I suppose you are creating a container via the containerd API rather than CRI?

from gvisor-containerd-shim.

@Random-Liu I think the relevant code is here since it's 1.2.x. In this case there aren't any runtime checks since it wasn't implemented yet. I feel like we should probably handle this case.
https://github.com/containerd/containerd/blob/release/1.2/task_opts_unix.go

from gvisor-containerd-shim.

I'm using the containerd API directly. The config.toml file has no effect on this behavior; the reproduction is in the following client code:

	var stdout, stderr bytes.Buffer
	task, err := container.NewTask(
		containerCtx,
		cio.NewCreator(
			cio.WithFIFODir(fifoDir),
			cio.WithStreams(
				nil, // stdin
				&stdout,
				&stderr,
			),
		),
		containerd.WithNoNewKeyring,
	)

The containerd.WithNoNewKeyring option is an optimization for runc containers. If set, the shim's creation request options will be set to a *runctypes.CreateOptions. I would like it if I could set that option unconditionally and let the shim ignore unknown options, but the failing type check here prevents that.

from gvisor-containerd-shim.

I'm using it with containerd directly, not via Kubernetes or CRI.

No wonder. That's what I guessed. :)

I think the relevant code is here since it's 1.2.x. In this case there aren't any runtime checks since it wasn't implemented yet. I feel like we should probably handle this case.

I'm fine with handling that :)
Just want to confirm this is caused by a directly use with containerd, not a bug in Kubernetes integration.

from gvisor-containerd-shim.