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.

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

feature: add exec inspect api in daemon side #875

Merged

Conversation

allencloud
Copy link
Collaborator

@allencloud allencloud commented Mar 12, 2018

Signed-off-by: Allen Sun [email protected]

Ⅰ. Describe what this PR did

This PR implements GET /exec/{id}/json just in daemon side.

I have found that in moby api v1.24, the daemon side has returned a full type of ContainerExecInspect , but in client side of engine-api, it only takes advantages of four fields:

// ContainerExecInspect holds information returned by exec inspect.
type ContainerExecInspect struct {
	ExecID      string
	ContainerID string
	Running     bool
	ExitCode    int
}

Since there is possibility that user would call this API several times, so I removed execConfig.exitCh in struct ContainerExecConfig.

So in this PR, I only add these four fields in daemon side.

Ⅱ. Does this pull request fix one issue?

fixes #864

Ⅲ. Describe how you did it

none

Ⅳ. Describe how to verify it

none

Ⅴ. Special notes for reviews

none

@HusterWan
Copy link
Contributor

@allencloud what's status of this PR, can you rebase this pr? thanks a lot

@yyb196
Copy link
Collaborator

yyb196 commented Apr 9, 2018

@allencloud We should fix and merge this pr ASAP, our deployment system relay on this interface to check the result of pouch exec, in the health checking case.

@allencloud
Copy link
Collaborator Author

@yyb196
Thanks for your feedback. Actually this pr is dependent on ctrd which is need to be refactored. I will get a way to figure this out ASAP.

@allencloud allencloud force-pushed the add-exec-inpsect branch 17 times, most recently from 13bf84d to 4f1affa Compare April 9, 2018 14:00
@pouchrobot
Copy link
Collaborator

ping @allencloud

CI fails according integration system.
Please refer to the CI failure Details button to corresponding test, and update your PR to pass CI.

If this is flaky test, welcome to track this with profiling an issue.

build url: https://travis-ci.org/alibaba/pouch/builds/364133108
build duration: 1571s

@YaoZengzeng
Copy link
Contributor

LGTM, but we need to make some change for exec of cri manager.

I'll fix it soon :) @allencloud

@YaoZengzeng YaoZengzeng merged commit bc01dc6 into AliyunContainerService:master Apr 10, 2018
@allencloud allencloud deleted the add-exec-inpsect branch April 10, 2018 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature required] need API /exec/{name:.*}/json
5 participants