Add ExecSync. #72
Add ExecSync. #72
Conversation
Random-Liu
force-pushed
the
add-exec-sync
branch
from
a37bddd to
499b691
Compare
Random-Liu
force-pushed
the
add-exec-sync
branch
3 times, most recently
from
9f57c33 to
4facac4
Compare
pkg/metadata/container.go
Outdated
| @@ -73,6 +74,11 @@ type ContainerMetadata struct { | |||
| // into the store directly. | |||
| // TODO(random-liu): Reset this field to false during state recoverry. | |||
| // TODO(random-liu): Remove following fields after switching to new containerd | ||
| // client. | ||
| // Not including them in unit test now because they will be removed soon. | ||
| // Spec is the oci runtime spec used to run the container. |
There was a problem hiding this comment.
fields, them, they.. but only Spec is there...was this supposed to cover more fields?
There was a problem hiding this comment.
Initially there are more. Will change to field.
pkg/server/container_execsync.go
Outdated
| if err != nil { | ||
| return nil, fmt.Errorf("an error occurred when try to find container %q: %v", r.GetContainerId(), err) | ||
| } | ||
| id := r.GetContainerId() |
There was a problem hiding this comment.
move this assignment up and use id as param to .Get()
There was a problem hiding this comment.
Changed to id := meta.ID.
The metadata store may support truncated index soon, so get the id from metadata.
| } | ||
| exitCode, err := waitContainerExec(cancel, events, id, resp.Pid, r.GetTimeout()) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to wait for exec in container %q to finish: %v", id, err) |
There was a problem hiding this comment.
this return eats the unknownExitCode.. What's that for if we are going to nil it here?
There was a problem hiding this comment.
Good catch! We do have some special logic here in dockershim. Let me check.
There was a problem hiding this comment.
If containerd dies during waiting, the exec process may still be running, so I think we should return error instead of 255 exit code without error.
I still keep the unknownExitCode, because:
- We may still need it when adding the recovery logic after daemon restart.
- Just in case we use the 0 exit code by mistake.
There was a problem hiding this comment.
ok.. will watch for the recovery logic code to see unknownExitCode in action then :-)
Random-Liu
force-pushed
the
add-exec-sync
branch
from
4facac4 to
ae0252c
Compare
Signed-off-by: Lantao Liu <[email protected]>
Random-Liu
force-pushed
the
add-exec-sync
branch
from
ae0252c to
9b79201
Compare
Add ExecSync.
Re-implement "No HTTP2" support using "GODEBUG=http2client=0"
3 participants
Add simple
ExecSyncbased on current containerd api.Some TODOs in the future which will be addressed in future PRs:
ExecSynctimeout support, including timeout waiting and graceful termination after timeout.Signed-off-by: Lantao Liu [email protected]