Skip to content
This repository has been archived by the owner on Apr 20, 2022. It is now read-only.

Add containerd shim v2 support. #13

Merged
merged 4 commits into from Jan 30, 2019
Merged

Add containerd shim v2 support. #13

merged 4 commits into from Jan 30, 2019

Conversation

Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented Jan 28, 2019 •
edited

For #1.

The shim v2 binary name is containerd-shim-runsc-v1.

Containerd 1.2

To use containerd-shim-runsc-v1:

[plugins.cri.containerd.runtimes.runsc]
  runtime_type = "io.containerd.runsc.v1"

A config.toml can be put in the runtime root (/run/containerd/runsc by default) to set runtime-handler specific options for containerd-shim-runsc-v1.
An example of /run/containerd/runsc/config.toml:

[runsc_config]
  debug = "true"
  debug-log = "/tmp/runsc-logs/runsc.log.%ID%.%TIMESTAMP%.%COMMAND%"
  user-log = "/tmp/runsc-logs/runsc.log.%ID%.user"

Containerd 1.3+

The containerd config to use containrd-shim-runsc-v1 is the same with containerd 1.2.

Containerd 1.3+ explicitly added config file support to configure runtime-handler specific options:

[plugins.cri.containerd.runtimes.runsc.options]
  TypeUrl = "io.containerd.runsc.v1.options"
  ConfigPath = "/somewhere/config.toml"

I'll send a follow-up PR to update the document.

@Random-Liu
Update vendors
9bb537a 
Signed-off-by: Lantao Liu <lantaol@google.com>
@Random-Liu
Add containerd shim v2 support.
935512f 
Signed-off-by: Lantao Liu <lantaol@google.com>
@Random-Liu
Copy link
Member Author

Please note that this PR also ported the fixes containerd/containerd@18f57e2#diff-2775e018eedd9942bc5c2c3c7f9039a8 and containerd/containerd@18f57e2#diff-2775e018eedd9942bc5c2c3c7f9039a8.

@Random-Liu Random-Liu mentioned this pull request Jan 28, 2019
Merged
@Random-Liu Random-Liu force-pushed the support-shim-v2-api branch 6 times, most recently from 8a8b7c7 to 09ea670 Compare January 28, 2019 23:29
@Random-Liu
Add test and doc for containerd-shim-runsc-v1.
5ccc68b 
Signed-off-by: Lantao Liu <lantaol@google.com>
ianlewis
ianlewis approved these changes Jan 29, 2019
View reviewed changes
Copy link
Contributor

@ianlewis ianlewis left a comment

Choose a reason for hiding this comment

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

LGTM
Please address comments as best as possible before merging

docs/runtime-handler-shim-v2-quickstart.md Outdated
@@ -0,0 +1,187 @@
# Runtime Handler Quickstart (Shim V2)

This document describes how to install and run the `containerd-shim-runsc-v1`
Copy link
Contributor

Choose a reason for hiding this comment

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

This document describes how to install and run containerd-shim-runsc-v1

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

docs/runtime-handler-shim-v2-quickstart.md

[embedmd]:# (../test/e2e/shim-install.sh shell /{ # Step 1\(dev\)/ /^}/)
```shell
{ # Step 1(dev): Build and install gvisor-containerd-shim and containerd-shim-runsc-v1
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make this use a release when we have it available?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, added a TODO.

pkg/v2/service.go
}
cmd := exec.Command(self, args...)
cmd.Dir = cwd
cmd.Env = append(os.Environ(), "GOMAXPROCS=2")
Copy link
Contributor

Choose a reason for hiding this comment

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

This should have a comment as to why it's here.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is from the containerd-shim, and this is to minimize the resource usage of the shim.

I want to avoid unnecessary diff with upstream containerd-shim :)

pkg/v2/service.go
default:
return nil, errors.Errorf("unsupported option type")
}
if path != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are you setting the default path to the shim config?

Copy link
Member Author

Choose a reason for hiding this comment

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

The default is empty string, means no special config.

Copy link
Contributor

Choose a reason for hiding this comment

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

The description says it's /run/containerd/runsc but I missed where this was being set.

Is the config.toml that gets set there per container or per sandbox?

pkg/v2/service.go
}

func (s *service) Stats(ctx context.Context, r *taskAPI.StatsRequest) (*taskAPI.StatsResponse, error) {
// TODO(random-liu): Use `runsc events --stats`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's have an issue for this in gVisor (I believe it's not supported per container yet)

Copy link
Member Author

Choose a reason for hiding this comment

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

#15

test/e2e/shim-install.sh Outdated
@@ -6,22 +6,20 @@ set -ex

# Build gvisor-containerd-shim
if [ "${INSTALL_LATEST}" === "1" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

This should just be a == I think. Currently it shows an error in travis

if [ "${INSTALL_LATEST}" == "1" ]; then

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

test/e2e/shim-install.sh
}
else
{ # Step 1(dev): Build and install gvisor-containerd-shim and containerd-shim-runsc-v1
make
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a good way to check as part of the test that it's actually using the v2 shim as opposed to the v1 without just trusting that the config.toml is correct?

Maybe just something like this?

ps aux | grep containerd-shim-runsc-v1

Not sure it needs to be in the quick start, but it would be nice to have as part of the test script.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@Random-Liu
Copy link
Member Author

Merging based on #13 (review).

@Random-Liu Random-Liu merged commit 35db607 into google:master Jan 30, 2019
@Random-Liu Random-Liu deleted the support-shim-v2-api branch January 30, 2019 02:51
@Random-Liu Random-Liu mentioned this pull request Feb 26, 2019
Closed
@cmichelQT cmichelQT mentioned this pull request Feb 27, 2020
Open
@thaJeztah thaJeztah mentioned this pull request Apr 2, 2021
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ianlewis ianlewis ianlewis approved these changes

Assignees

@ianlewis ianlewis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Random-Liu @ianlewis