Runtime v2 (shim API) by crosbymichael · Pull Request #2434 · containerd/containerd

crosbymichael · 2018-07-03T16:31:55Z

This implements the shimv2 API proposed in #2426. The tests are using the new io.containerd.oci.v1 runtime under the new TaskManger implementing the v2 APIs.

Runtime v2

Runtime v2 introduces a first class shim API for runtime authors to integrate with containerd.
The shim API is minimal and scoped to the execution lifecycle of a container.

Binary Naming

Users specify the runtime they wish to use when creating a container.
The runtime can also be changed via a container update.

> ctr run --runtime io.containerd.runc.v1

When a user specifies a runtime name, io.containerd.runc.v1, they will specify the name and version of the runtime.
This will be trasnlated by containerd into a binary name for the shim.

io.containerd.runc.v1 -> containerd-shim-runc-v1

containerd keeps the containerd-shim-* prefix so that users can ps aux | grep containerd-shim to see running shims on their system.

Shim Authoring

This section is dedicated to runtime authors wishing to build a shim.
It will detail how the API works and different considerations when building shim.

Commands

Container information is provided to a shim in two ways.
The OCI Runtime Bundle and on the Create rpc request.

`start`

Each shim MUST implement a start subcommand.
This command will launch new shims.
The start command MUST accept the following flags:

-namespace the namespace for the container
-id the id of the container
-address the address of the containerd's main socket
-publish-binary the binary path to publish events back to containerd

The start command, as well as all binary calls to the shim, has the bundle for the container set as the cwd.

The start command MUST return an address to a shim for containerd to issue API requests for container operations.

The start command can either start a new shim or return an address to an existing shim based on the shim's logic.

`delete`

Each shim MUST implement a delete subcommand.
This command allows containerd to delete any container resources created, mounted, and/or run by a shim when containerd can no longer communicate over rpc.
This happens if a shim is SIGKILL'd with a running container.
These resources will need to be cleaned up when containerd looses the connection to a shim.
This is also used when containerd boots and reconnects to shims.
If a bundle is still on disk but containerd cannot connect to a shim, the delete command is invoked.

The delete command MUST accept the following flags:

-namespace the namespace for the container
-id the id of the container
-address the address of the containerd's main socket
-publish-binary the binary path to publish events back to containerd

The delete command will be executed in the container's bundle as its cwd.

Host Level Shim Configuration

containerd does not provide any host level configuration for shims via the API.
If a shim needs configuration from the user with host level information across all instances, a shim specific configuration file can be setup.

Container Level Shim Configuration

On the create request, there is a generic *protobuf.Any that allows a user to specify container level configuration for the shim.

message CreateTaskRequest {
	string id = 1;
	...
	google.protobuf.Any options = 10;
}

A shim author can create their own protobuf message for configuration and clients can import and provide this information is needed.

I/O

I/O for a container is provided by the client to the shim via fifo on Linux, named pipes on Windows, or log files on disk.
The paths to these files are provided on the Create rpc for the initial creation and on the Exec rpc for additional processes.

message CreateTaskRequest {
	string id = 1;
	bool terminal = 4;
	string stdin = 5;
	string stdout = 6;
	string stderr = 7;
}

message ExecProcessRequest {
	string id = 1;
	string exec_id = 2;
	bool terminal = 3;
	string stdin = 4;
	string stdout = 5;
	string stderr = 6;
}

Containers that are to be launched with an interactive terminal will have the terminal field set to true, data is still copied over the files(fifos,pipes) in the same way as non interactive containers.

Root Filesystems

The root filesytems for the containers is provided by on the Create rpc.
Shims are responsible for managing the lifecycle of the filesystem mount during the lifecycle of a container.

message CreateTaskRequest {
	string id = 1;
	string bundle = 2;
	repeated containerd.types.Mount rootfs = 3;
	...
}

The mount protobuf message is:

message Mount {
	// Type defines the nature of the mount.
	string type = 1;
	// Source specifies the name of the mount. Depending on mount type, this
	// may be a volume name or a host path, or even ignored.
	string source = 2;
	// Target path in container
	string target = 3;
	// Options specifies zero or more fstab style mount options.
	repeated string options = 4;
}

Shims are responsible for mounting the filesystem into the rootfs/ directory of the bundle.
Shims are also responsible for unmounting of the filesystem.
During a delete binary call, the shim MUST ensure that filesystem is also unmounted.
Filesystems are provided by the containerd snapshotters.

Other

Unsupported rpcs

If a shim does not or cannot implement an rpc call, it MUST return a github.com/containerd/containerd/errdefs.ErrNotImplemented error.

ttrpc

ttrpc is the only currently supported protocol for shims.
It works with standard protobufs and GRPC services as well as generating clients.
The only difference between grpc and ttrpc is the wire protocol.
ttrpc removes the http stack in order to save memory and binary size to keep shims small.
It is recommended to use ttrpc in your shim but grpc support is also in development.

Closes #2426

stevvooe · 2018-07-03T17:56:41Z

client.go

mlaventure

Looks good so far, don't have the time to finish goign through it all atm, but I'll try to get another pass at it.

mlaventure · 2018-07-03T17:20:21Z

client.go

mlaventure · 2018-07-03T17:21:09Z

cmd/containerd-shim-oci-v1/main.go

containerd-shim-oci-v1

mlaventure · 2018-07-03T17:24:06Z

container_linux_test.go

Not handled anymore?

I looked at the test and it really wasn't testing much, killing the shim then killing the process. I don't think it was a valid test or it was and changed sometime in the past. Felt safe to remove.

It checks that the container process (redis) is correctly cleaned up if the shim crash/is sigkilled (e.g. by oom).
It doesn't actually kill the process, it checks that it died.

ok, my bad, i'll fix it. i missed the kill 0

Seems this test is still removed in the latest commit? or did I miss a move somewhere else?

ok, so I researched this more and the test is only valid for v1 when container was still running and didn't have to reconnect to a shim again. The reason is that it sets an exit handler that does the "cleanup on dead shim" code. However, this only runs if the shim process that containerd is doing a waidpid on returns and checks the shim pid.

I think its safe to remove this as it doesn't really work, it only works in the tests because the daemon isn't restarted. If we want to keep it, i can do some refactoring but it will take a little bit of work to make this work across daemon reboots with running shims. I can do that in a followup PR.

Makes sense.

I haven't yet got the time to compile and play around with the new API to see how it actually behave.

Having a way to start a "cleanup shim" in such scenario may be a cleaner solution for v2 though (and if that fails, things are left to the user to clean).

mlaventure · 2018-07-03T17:27:31Z

metrics/cgroups/metrics.go

No timeout / cancel / Sync ? We could end up with several go routine getting the stats on the same task, no?

they are sync'd in the caller, hard to have a timeout for this because under load, collection could be slower

oh, right. I missed the waitgroup, my bad.

mlaventure · 2018-07-03T17:33:24Z

runtime/v1/linux/proc/init.go

nit, but could we bundle exported variables together?

mlaventure · 2018-07-03T17:37:40Z

runtime/v2/bundle.go

why the separation between the base dir and the last dir in the path?

base should always be created while mkdir should return an exists error if something is already there

What is the reason behind doing them in 2 loops if the remove all happens on error anyway? Also what does it mean if Mkdir returns an exist on the second part, seems odd that it would both return an exists error and delete the directory.

mlaventure · 2018-07-03T18:03:36Z

runtime/v2/oci/service_linux.go

cleanup stdin if created?

mlaventure · 2018-07-03T18:03:48Z

runtime/v2/oci/service_linux.go

missing cleanup of in/outw

jterry75 · 2018-07-04T22:13:21Z

@crosbymichael - This is a really good start. I can rebase my windows side changes on this and resubmit

codecov-io · 2018-07-05T20:09:16Z

Codecov Report

Merging #2434 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2434      +/-   ##
==========================================
- Coverage   44.76%   44.73%   -0.04%     
==========================================
  Files          92       93       +1     
  Lines        9483     9490       +7     
==========================================
  Hits         4245     4245              
- Misses       4555     4562       +7     
  Partials      683      683

Flag	Coverage Δ
#linux	`48.96% <0%> (-0.02%)`	⬇️
#windows	`41.03% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
sys/reaper.go	`0% <ø> (ø)`	⬆️
sys/reaper_linux.go	`0% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 02579c8...d53a96f. Read the comment docs.

dmcgowan · 2018-07-06T01:28:29Z

runtime/v2/bundle.go

wrapping this could be useful, I don't think the message includes the path

dmcgowan · 2018-07-06T01:52:35Z

runtime/v2/shim.go

Why is this command not given the bundle id anywhere?

its not needed and comes on the Create request

What does this exec with the delete arg intended to do?

dmcgowan · 2018-07-06T02:10:48Z

runtime/v2/util.go

Currently name will never be absolute, but is it worth considering a case where name is in the same directory as self but not in the lookup path? In that case the name might be better found at os.Join(os.Dir(self), name). Unless there is a plan here to document setting PATH on containerd for the runtime binary installation location.

i think for now, we say that it must be in PATH. i'll remove this abs code

ehazlett · 2018-07-06T16:40:13Z

runtime/v2/shim.go

nit: either Wrap or add the ID?

ehazlett · 2018-07-06T16:43:49Z

runtime/v2/shim.go

Should we return or log this error?

ehazlett · 2018-07-06T16:45:29Z

runtime/v2/shim.go

include the ID?

ehazlett · 2018-07-06T16:54:54Z

runtime/v2/shim.go

Do we need to check the StatusUnknown task type or have a runtime.UnknownStatus?

dmcgowan · 2018-07-09T21:00:52Z

services/tasks/local.go

The formatting here is off

Should we add a default case and explanation for not using now?

dmcgowan · 2018-07-13T22:00:54Z

runtime/v2/binary.go

Should this be trimmed? Also wondering if we should just take the stdout rather than combined output.

You previously expressed that this extra connect could cause some additional time, although mostly negligible. Could we still pass in the socket and if the returned address doesn't match just remove it and create a new connection, or would that end up being even longer?

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

dmcgowan

LGTM

stevvooe · 2018-07-17T20:51:05Z

runtime/v2/runc/options/oci.proto

+option go_package = "github.com/containerd/containerd/runtime/v2/runc/options;options";
+
+message Options {
+	bool no_pivot_root = 1;


I know this isn't popular, but let's document the intent of these fields.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

stevvooe · 2018-07-17T20:52:56Z

runtime/v2/task/shim.proto

+// each container and allows reattaching to the IO and receiving the exit status
+// for the container processes.
+service Task {
+	rpc State(StateRequest) returns (StateResponse);


It would be good to document what each method does.

stevvooe · 2018-07-17T20:53:46Z

I don't see anything wildly controversial here. It would be good to document the methods and fields used in GRPC.

crosbymichael · 2018-07-17T20:57:53Z

@stevvooe I was going to do documentation in a separate PR. I'll take care of proto comments there as well. I wanted to make a document for shim authors, "Building a shim 101"

Random-Liu · 2018-07-18T02:42:12Z

@crosbymichael Will take a look at this PR tomorrow. Sorry for the delay.

This readme specifies shim api and authoring docs. Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

crosbymichael · 2018-07-18T16:09:56Z

Update the description and added a readme for shim authors.

Runtime v2

Runtime v2 introduces a first class shim API for runtime authors to integrate with containerd.
The shim API is minimal and scoped to the execution lifecycle of a container.

Binary Naming

Users specify the runtime they wish to use when creating a container.
The runtime can also be changed via a container update.

> ctr run --runtime io.containerd.runc.v1

When a user specifies a runtime name, io.containerd.runc.v1, they will specify the name and version of the runtime.
This will be trasnlated by containerd into a binary name for the shim.

io.containerd.runc.v1 -> containerd-shim-runc-v1

containerd keeps the containerd-shim-* prefix so that users can ps aux | grep containerd-shim to see running shims on their system.

Shim Authoring

This section is dedicated to runtime authors wishing to build a shim.
It will detail how the API works and different considerations when building shim.

Commands

Container information is provided to a shim in two ways.
The OCI Runtime Bundle and on the Create rpc request.

`start`

Each shim MUST implement a start subcommand.
This command will launch new shims.
The start command MUST accept the following flags:

-namespace the namespace for the container
-id the id of the container
-address the address of the containerd's main socket
-publish-binary the binary path to publish events back to containerd

The start command, as well as all binary calls to the shim, has the bundle for the container set as the cwd.

The start command MUST return an address to a shim for containerd to issue API requests for container operations.

The start command can either start a new shim or return an address to an existing shim based on the shim's logic.

`delete`

Each shim MUST implement a delete subcommand.
This command allows containerd to delete any container resources created, mounted, and/or run by a shim when containerd can no longer communicate over rpc.
This happens if a shim is SIGKILL'd with a running container.
These resources will need to be cleaned up when containerd looses the connection to a shim.
This is also used when containerd boots and reconnects to shims.
If a bundle is still on disk but containerd cannot connect to a shim, the delete command is invoked.

The delete command MUST accept the following flags:

-namespace the namespace for the container
-id the id of the container
-address the address of the containerd's main socket
-publish-binary the binary path to publish events back to containerd

The delete command will be executed in the container's bundle as its cwd.

Host Level Shim Configuration

containerd does not provide any host level configuration for shims via the API.
If a shim needs configuration from the user with host level information across all instances, a shim specific configuration file can be setup.

Container Level Shim Configuration

On the create request, there is a generic *protobuf.Any that allows a user to specify container level configuration for the shim.

message CreateTaskRequest {
	string id = 1;
	...
	google.protobuf.Any options = 10;
}

A shim author can create their own protobuf message for configuration and clients can import and provide this information is needed.

I/O

I/O for a container is provided by the client to the shim via fifo on Linux, named pipes on Windows, or log files on disk.
The paths to these files are provided on the Create rpc for the initial creation and on the Exec rpc for additional processes.

message CreateTaskRequest {
	string id = 1;
	bool terminal = 4;
	string stdin = 5;
	string stdout = 6;
	string stderr = 7;
}

message ExecProcessRequest {
	string id = 1;
	string exec_id = 2;
	bool terminal = 3;
	string stdin = 4;
	string stdout = 5;
	string stderr = 6;
}

Containers that are to be launched with an interactive terminal will have the terminal field set to true, data is still copied over the files(fifos,pipes) in the same way as non interactive containers.

Root Filesystems

The root filesytems for the containers is provided by on the Create rpc.
Shims are responsible for managing the lifecycle of the filesystem mount during the lifecycle of a container.

message CreateTaskRequest {
	string id = 1;
	string bundle = 2;
	repeated containerd.types.Mount rootfs = 3;
	...
}

The mount protobuf message is:

message Mount {
	// Type defines the nature of the mount.
	string type = 1;
	// Source specifies the name of the mount. Depending on mount type, this
	// may be a volume name or a host path, or even ignored.
	string source = 2;
	// Target path in container
	string target = 3;
	// Options specifies zero or more fstab style mount options.
	repeated string options = 4;
}

Shims are responsible for mounting the filesystem into the rootfs/ directory of the bundle.
Shims are also responsible for unmounting of the filesystem.
During a delete binary call, the shim MUST ensure that filesystem is also unmounted.
Filesystems are provided by the containerd snapshotters.

Other

Unsupported rpcs

If a shim does not or cannot implement an rpc call, it MUST return a github.com/containerd/containerd/errdefs.ErrNotImplemented error.

ttrpc

ttrpc is the only currently supported protocol for shims.
It works with standard protobufs and GRPC services as well as generating clients.
The only difference between grpc and ttrpc is the wire protocol.
ttrpc removes the http stack in order to save memory and binary size to keep shims small.
It is recommended to use ttrpc in your shim but grpc support is also in development.

Random-Liu · 2018-07-18T18:57:18Z

The api LGTM overall. One thing is that can we include required events? e.g. TaskExit (required), OOMEvents (optional/required?) etc.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

crosbymichael · 2018-07-18T19:13:08Z

@Random-Liu ok, updated the README with events.

thaJeztah · 2018-07-18T22:52:00Z

it's merged! 🎉 🙌👏👏

stevvooe reviewed Jul 3, 2018

View reviewed changes

client.go Outdated

Copy link

Member

stevvooe Jul 3, 2018

Choose a reason for hiding this comment

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

linux

mlaventure reviewed Jul 3, 2018

View reviewed changes

ehazlett force-pushed the shimv2 branch from 9aba29e to a42b130 Compare July 3, 2018 19:30

jterry75 mentioned this pull request Jul 4, 2018

WIP: Refactors the linux runtime to be generic and introduces the windows runtime side-by-side #2427

Closed

ehazlett force-pushed the shimv2 branch from 272ec84 to e52f795 Compare July 5, 2018 15:35

crosbymichael force-pushed the shimv2 branch 5 times, most recently from e3c9050 to b238640 Compare July 5, 2018 19:53

crosbymichael force-pushed the shimv2 branch from b238640 to d8f0e8e Compare July 5, 2018 20:15

dmcgowan reviewed Jul 6, 2018

View reviewed changes

runtime/v2/bundle.go Outdated

Copy link

Member

dmcgowan Jul 6, 2018

Choose a reason for hiding this comment

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

wrapping this could be useful, I don't think the message includes the path

dmcgowan reviewed Jul 6, 2018

View reviewed changes

ehazlett reviewed Jul 6, 2018

View reviewed changes

crosbymichael force-pushed the shimv2 branch 2 times, most recently from 263acdb to 7976ef8 Compare July 9, 2018 18:21

crosbymichael changed the title ~~[wip] Runtime v2 (shim API)~~ Runtime v2 (shim API) Jul 9, 2018

crosbymichael force-pushed the shimv2 branch 2 times, most recently from aeb0941 to 578358e Compare July 9, 2018 20:57

dmcgowan reviewed Jul 9, 2018

View reviewed changes

crosbymichael force-pushed the shimv2 branch 4 times, most recently from 8deea2c to 9a1e7b9 Compare July 11, 2018 19:13

containerd deleted a comment from crosbymichael Jul 11, 2018

crosbymichael force-pushed the shimv2 branch from 9a1e7b9 to cf332e0 Compare July 12, 2018 18:42

crosbymichael force-pushed the shimv2 branch from dae3af8 to be9840f Compare July 13, 2018 17:05

dmcgowan reviewed Jul 13, 2018

View reviewed changes

gnawux mentioned this pull request Jul 16, 2018

Implement containerd shim v2 API for Kata Containers kata-containers/runtime#485

Closed

crosbymichael force-pushed the shimv2 branch 3 times, most recently from bb10512 to 20cf7ef Compare July 16, 2018 20:01

crosbymichael added 2 commits July 17, 2018 10:21

Runtime v2

da1b547

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

Add shim start for shim creation

7e49c60

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

crosbymichael force-pushed the shimv2 branch from 20cf7ef to 7e49c60 Compare July 17, 2018 14:21

dmcgowan approved these changes Jul 17, 2018

View reviewed changes

stevvooe reviewed Jul 17, 2018

View reviewed changes

Add oom events to shim

fd31052

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

stevvooe reviewed Jul 17, 2018

View reviewed changes

Add README.md to runtime v2

026b7d9

This readme specifies shim api and authoring docs. Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

Add comments for oci protos

d53a96f

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

crosbymichael force-pushed the shimv2 branch from f15b41a to d53a96f Compare July 18, 2018 19:12

dmcgowan merged commit 94cfce6 into containerd:master Jul 18, 2018

Ace-Tang mentioned this pull request Oct 26, 2018

feature: support multiple containerd-shim AliyunContainerService/pouch#2363

Closed

ehazlett mentioned this pull request Nov 27, 2018

cr: support checkpoint/restore without image #2830

Merged

fuweid mentioned this pull request Dec 27, 2018

[proposal] add Add/Delete methods in PlatformRuntime interface #2888

Closed

liangxianlong mentioned this pull request Jan 10, 2019

some problems when use "docker stats" with kata-runtime moby/moby#38329

Closed

2 tasks

BetaXOi mentioned this pull request Jan 14, 2019

Any plans to support containerd shim v2? moby/moby#38559

Closed

gnawux mentioned this pull request Feb 13, 2019

Creating container DO NOT need runtime anymore in kata-shim-v2? kata-containers/runtime#1170

Closed

Conversation

crosbymichael commented Jul 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Runtime v2

Binary Naming

Shim Authoring

Commands

start

delete

Host Level Shim Configuration

Container Level Shim Configuration

I/O

Root Filesystems

Other

Unsupported rpcs

ttrpc

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlaventure left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jterry75 commented Jul 4, 2018

Uh oh!

codecov-io commented Jul 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crosbymichael commented Jul 3, 2018 •

edited

Loading

`start`

`delete`

codecov-io commented Jul 5, 2018 •

edited

Loading

Random-Liu commented Jul 18, 2018 •

edited

Loading