shim: GRPC service #462
Merged
shim: GRPC service #462
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The large diff is from the proto generation |
mlaventure
reviewed
Jan 24, 2017
| } | ||
|
|
||
| message PtyRequest { | ||
| message CreateRequest { | ||
| string id = 1 [(gogoproto.customname) = "ID"]; |
| } | ||
|
|
||
| message ExecRequest { | ||
|
|
| } | ||
|
|
||
| message PtyRequest { | ||
| uint32 pid = 1; | ||
| uint32 width = 2; |
| return nil, err | ||
| } | ||
| runtime := &runc.Runc{ | ||
| Command: r.Runtime, |
There was a problem hiding this comment.
Should we get runtimeArgs too?
| console *runc.Console | ||
| io runc.IO | ||
| runc *runc.Runc | ||
| status int |
| if err := p.runc.Create(context, r.ID, r.Bundle, opts); err != nil { | ||
| return nil, err | ||
| } | ||
| if socket != nil { |
There was a problem hiding this comment.
I'd if r.Terminal for consistency with the above. If the socket is nil at the point with r.Terminal = true, then there's an issue with go-runc or runc
| IO: io, | ||
| NoPivot: r.NoPivot, | ||
| } | ||
| if err := p.runc.Create(context, r.ID, r.Bundle, opts); err != nil { |
There was a problem hiding this comment.
need a defer to kill the process on subsequent errors
| ) | ||
| if r.Terminal { | ||
| if socket, err = runc.NewConsoleSocket(filepath.Join(cwd, "pty.sock")); err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
can we use the errors package since go-runc doesn't?
| } | ||
|
|
||
| func (p *initProcess) Delete(context context.Context) error { | ||
| p.killAll(context) |
There was a problem hiding this comment.
Return here on error? so containerd doesn't delete the state directory?
| go func() { | ||
| defer l.Close() | ||
| if err := server.Serve(l); err != nil { | ||
| l.Close() |
There was a problem hiding this comment.
there's already a defer above for it
stevvooe
reviewed
Jan 24, 2017
| @@ -6,14 +6,49 @@ import "google/protobuf/empty.proto"; | |||
| import "gogoproto/gogo.proto"; | |||
|
|
|||
| service ShimService { | |||
There was a problem hiding this comment.
No need to repeat Service. Generated code will call this service.
|
LGTM after vendor fix. |
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
This also finishes the service implementation of the shim behind GRPC Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
mlaventure
approved these changes
Jan 27, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes the shim work over grpc and allows one shim per container. Before it was one shim per process with each exec having its own.
Please take a look and let me know what you think of this new model. I implemented the new runc logic for terminal creation and hid some of the nasty io handling in the runc lib.
I still need to implement exec fully and do some testing but opening this for an early review.