Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Provide urpc alternative to OCI #238

Open
fvoznika opened this issue May 29, 2019 · 4 comments
Open

Provide urpc alternative to OCI #238

fvoznika opened this issue May 29, 2019 · 4 comments
Assignees
@zkoopmans
Labels
area: integration Issue related to third party integrations priority: p2 Normal priority type: enhancement New feature or request

Comments

@fvoznika
Copy link
Member

Using OCI to create and manage containers require many process invocations to start a Pod. It also requires many satellite processes to be running: one runsc wait process for every container, another runsc exec for every exec invocation, runsc events --stats to capture stats. Using processes also makes it hard to distinguish between command errors and errors coming from the container, e.g. runsc exec may fail because there is an invalid parameter, or because the application being executed failed with an invalid parameter.

Many of these problems go away if we had an RPC interface that gvisor-containerd-shim can call directly. The proposal is to create a client library that talks directly to the sandbox using the urpc endpoint that is already exposed. This would be a full replacement of OCI, requiring no extra process invocation to create and manage pod, including terminal support.

gvisor-containerd-shim would use this new API exclusively, leaving the OCI implementation to be used by docker and other users of runsc. Multi-container support, and gVisor specific annotations can be removed from the command-line interface.
@fvoznika fvoznika added type: enhancement New feature or request priority: p2 Normal priority labels May 29, 2019
@kevinGC
Copy link
Collaborator

kevinGC commented May 30, 2019

I agree that it'd be cleaner for gvisor-containerd-shim to talk via its own interface to the sandbox, and to clean up the gVisor-specific annotations in runsc. I have two questions:

  1. The gVisor-specific annotations are very useful for debugging. The --debug-log*, --network, and --strace flags for example are very useful to be able to toggle when testing (either running runsc manually or by specifying them in /etc/docker/daemon.json). If these are going away, what will be the similarly easy to use successor?
  2. Is it correct to assume that this would only reduce the number of processes when using Kubernetes? In the non-Kubernetes case (e.g. using docker or runsc directly), we'd still be using the OCI interface, as those cases don't use gvisor-containerd-shim.

@fvoznika
Copy link
Member Author

  1. Yes, these are important and we'll need to continue to support them. The annotations that can go away are the ones like "io.kubernetes.cri.container-type" and others for mounts that we'll be adding shortly.
  2. Correct, docker (and other users) will continue to use the OCI interface.

@ianlewis
Copy link
Contributor

I think it's worth while to have commands that are accessible via the command line so I don't think those should be removed, but it does make sense for the shim to use an rpc interface to talk to the sandbox directly without having to rely on various satellite invocations of runsc (which are effectively rpc proxies to the sandbox anyway).

@Random-Liu Random-Liu mentioned this issue Oct 5, 2019
Open
@fvoznika fvoznika mentioned this issue Mar 12, 2020
Open
@ianlewis
Copy link
Contributor

ianlewis commented Apr 7, 2020

Some operations like creating and deleting sandboxes have sandbox-wide logic. One example is calculating the sandbox process' OOMScore which looks at each container's oom_score_adj on creation or deletion of a container in the sandbox.

gvisor/runsc/container/container.go

Line 1090 in ca90dad

func adjustSandboxOOMScoreAdj(s *sandbox.Sandbox, rootDir string, destroy bool) error {

Synchronization will need to be done on the urpc client side since the sandbox process won't have permissions to perform operations like setting the gofer's oom_score_adj.

copybara-service bot pushed a commit that referenced this issue Apr 7, 2020
@ianlewis @gvisor-bot
Update TODO to #238
2f1b72e 
Move TODO to #238 so that proper synchronization of operations is handled
when we create the urpc client.

Issue #238
Fixes #512

PiperOrigin-RevId: 305345002
@copybara-service copybara-service bot mentioned this issue Apr 7, 2020
Merged
copybara-service bot pushed a commit that referenced this issue Apr 8, 2020
@ianlewis @gvisor-bot
Update TODO to #238
5802051 
Move TODO to #238 so that proper synchronization of operations is handled
when we create the urpc client.

Issue #238
Fixes #512

PiperOrigin-RevId: 305383924
@ianlewis ianlewis mentioned this issue Jul 17, 2020
Open
@ianlewis ianlewis added the area: integration Issue related to third party integrations label Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zkoopmans zkoopmans

Labels
area: integration Issue related to third party integrations priority: p2 Normal priority type: enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ianlewis @kevinGC @fvoznika @zkoopmans