Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Implement containerd shim v2 API for Kata Containers #485

Closed
gnawux opened this issue Jul 16, 2018 · 17 comments
Closed

Implement containerd shim v2 API for Kata Containers #485

gnawux opened this issue Jul 16, 2018 · 17 comments
Assignees

Comments

@gnawux
Copy link
Member

gnawux commented Jul 16, 2018

In the arch committee meeting last week, Michael Crosby introduced the containerd shim API v2. After the meeting, read related documents, patches, and discussed with @bergwolf @lifupan and related folks. And I think we could begin to implement the kata shim for containerd v2 API now.

Current kata & containerd working pattern

Right now, containerd and runtime work in the following pattern:

containerd-kata-shimv1

Whenever containerd tries to create a container,

  • It creates a containerd-shim
  • The containerd-shim call runtime cli -- the cli is specified by the runtime=kata flag, and the cli should be compatible with runc
  • The containerd-shim served at the address specified by containerd, and feedback events to containerd’s listening address

Though the containerd-shim designed to be an replaceable component

  • There still has to be one containerd shim per container (or process?)

Benefits from shim v2 API

With the proposed shim v2 API, the most significant change is

  • The shim will write the serving address back to containerd through stdout, instead of specified by containerd

As a result

  • We may replace the containerd-shim with a customized shim, and don’t need to implement a runc compatible CLI for containerd-shim.
  • We may use a single shim for a couple of containers, such as one shim per pod

Then the architecture may change to the follows:

containerd-shimv2-per-pod

  • One shim for containerd per pod
  • No proxies and kata-shim processes any more

And in v2 API, stats function is moved to shim, which makes the shim more self-contained.

Where we start

@WeiZhang555
Copy link
Member

This is a good proposal, and simplifies the architecture a lot!

One question: I think we need to keep the compatibility with older containerd-shim, we can't only support containerd-shim V2 because it's so new.

@bergwolf
Copy link
Member

@WeiZhang555 I agree we should keep the runc compatible cli as long as possible. For this new shim, IMO it should have its own repo so that in future we can push it to containerd to be a first class citizen there.

@sboeuf
Copy link

sboeuf commented Jul 16, 2018

As said during the Arch committee meeting, I would like to see the Kata implementation of the containerd-shim v2 as a separate directory (different repo might come further down the road, but right now, let's keep this simple). The same way we have the cli directory for our OCI implementation (matching runc as much as possible), we would have a new containerd-shim-v2 directory providing the Kata implementation of this shim v2. Most of the implementation will follow what frakti is actually leveraging (stateless Kata API), to be more efficient.
@gnawux let us know who will work on that, and how we can help with this (if there are some areas where the work can be split).

@crosbymichael
Copy link

Looks good!

@WeiZhang555
Copy link
Member

And as @crosbymichael described in the Arch Committee meeting, containerd-shim-v2 isn't aware of POD concept, so the containerd-shim should be one per container, so I think the final arch should be:

image

Is this right? @gnawux

@gnawux
Copy link
Member Author

gnawux commented Jul 18, 2018

@WeiZhang555 we investigated the APIs and code in the PR and has the following conclusion -- though containerd will request shim service for every container, it does not expect there is a new shim socket or process for the container. As a result, we think it could be implemented as one shim per pod. Let's just try to prove it with a prototype.

@bergwolf
Copy link
Member

@WeiZhang555 Per discussions in containerd/containerd/issues/2426, containerd-shim-v2 start can return the unix socket address of a previously started shim. That's how one shim per pod is supposed to be implemented.

@WeiZhang555
Copy link
Member

Oh, cool, this sounds a little hacky but workable.

@crosbymichael
Copy link

Yes, that is correct. We updated the v2 API to introduce a start command for the shims. This way the shims can configure and launch new shims how they need to. It also allows this start command to inspect the spec, look at annotations for pods, and return an existing shim instead of creating a new one. If one does not already exist, it can start one.

@crosbymichael
Copy link

I pushed an README this morning for shim authoring. Please take a look and provide any feedback if you have questions or if something does not make sense.

https://github.com/containerd/containerd/pull/2434/files#diff-9bb937a499e2e6fdc4536481d6315c50

@gnawux
Copy link
Member Author

gnawux commented Jul 18, 2018

thank you @crosbymichael. One more question -- is it essential for a v2shim commandline to implement the "no action" behavior

	default:
		client := NewShimClient(ctx, service, signals)
		return client.Serve()

or it's ok to support start and delete only?

@crosbymichael
Copy link

If you want to split it into multiple binaries then that is fine, all we care about is that start returns a socket to the API and delete lets us call for a cleanup if a shim dies and we can no longer communicate to it.

@resouer
Copy link

resouer commented Aug 9, 2018

[Update] A temporary branch has been created in develop repo here.

Will send out PRs to upstream when core apis are ready so you can set up basic tests.

@grahamwhaley
Copy link
Contributor

Nice @resouer . Just for ref, link to branch itself is here I think? (link above goes to closed PRs in that repo afaict).

@resouer
Copy link

resouer commented Aug 9, 2018

@grahamwhaley Oh, yes. The link I posted seems to be the merged PRs to that branch :/

@Ace-Tang
Copy link
Contributor

Ace-Tang commented Sep 3, 2018

Looking forward to the implement !

lifupan added a commit to hyperhq/kata-runtime that referenced this issue Sep 3, 2018
regroup the imported modules according the projects
they belonged to.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
@gnawux
Copy link
Member Author

gnawux commented Sep 3, 2018

@Ace-Tang PR #572 has already been there under review. Comments are welcome

lifupan added a commit to hyperhq/kata-runtime that referenced this issue Sep 4, 2018
In order to get rid of the confusion with shim's api
version, rename kata shimv2's shim to containerd-shim-kata-v2,
here the "v2" means the api version instead of the binary
shim's version. For the cri plugin, please set runtime_type
as: io.containerd.kata.v2

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Sep 7, 2018
regroup the imported modules according the projects
they belonged to.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Sep 7, 2018
In order to get rid of the confusion with shim's api
version, rename kata shimv2's shim to containerd-shim-kata-v2,
here the "v2" means the api version instead of the binary
shim's version. For the cri plugin, please set runtime_type
as: io.containerd.kata.v2

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Sep 17, 2018
regroup the imported modules according the projects
they belonged to.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Sep 17, 2018
In order to get rid of the confusion with shim's api
version, rename kata shimv2's shim to containerd-shim-kata-v2,
here the "v2" means the api version instead of the binary
shim's version. For the cri plugin, please set runtime_type
as: io.containerd.kata.v2

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Sep 17, 2018
If the networkNs hasn't been created, created it here.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Sep 17, 2018
kata has moved running network hooks from virtcontainer
to cli, thus it's needed to running those hooks in shimv2
also.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Sep 19, 2018
regroup the imported modules according the projects
they belonged to.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Sep 19, 2018
If the networkNs hasn't been created, created it here.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to lifupan/kata-runtime that referenced this issue Nov 16, 2018
Replace the vci APIs with the sandbox apis.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 17, 2018
refactor the codes based on the katautils pkg, and
rename the directory as containerd-shim-v2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 17, 2018
Replace the vci APIs with the sandbox apis.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 17, 2018
refactor the codes based on the katautils pkg, and
rename the directory as containerd-shim-v2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 17, 2018
Replace the vci APIs with the sandbox apis.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to lifupan/kata-runtime that referenced this issue Nov 19, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 19, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 19, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 19, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to lifupan/kata-runtime that referenced this issue Nov 20, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to lifupan/kata-runtime that referenced this issue Nov 20, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to lifupan/kata-runtime that referenced this issue Nov 22, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to lifupan/kata-runtime that referenced this issue Nov 22, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to lifupan/kata-runtime that referenced this issue Nov 22, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to lifupan/kata-runtime that referenced this issue Nov 22, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to lifupan/kata-runtime that referenced this issue Nov 22, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 22, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 22, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 22, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 27, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 27, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 28, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 28, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
lifupan added a commit to hyperhq/kata-runtime that referenced this issue Nov 28, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
teawater pushed a commit to teawater/runtime that referenced this issue Nov 30, 2018
Add the Makefile target of building shimv2.

Fixes: kata-containers#485

Signed-off-by: fupan <[email protected]>
zklei pushed a commit to zklei/runtime that referenced this issue Jun 13, 2019
This version of vsock fully supports Go 1.12 runtime network poller
integration, and is complaint with net.Listener and net.Conn as
checked by golang.org/x/net/nettest.

Fixes kata-containers#485.

Signed-off-by: Matt Layher <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants