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 stop command and recover container #30

Merged
merged 1 commit into from
Nov 2, 2017
Merged

feature: add stop command and recover container #30

merged 1 commit into from
Nov 2, 2017

Conversation

skoowoo
Copy link
Contributor

@skoowoo skoowoo commented Nov 1, 2017

"./pouch stop ID" be used to stop a running container. When restart the
pouchd, will recover these running containers.

Signed-off-by: skoo87 marckywu@gmail.com

Copy link
Collaborator

@allencloud allencloud left a comment

Choose a reason for hiding this comment

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

I left some comments. PTAL

}

func (s *Server) stopContainer(ctx context.Context, resp http.ResponseWriter, req *http.Request) error {
t, _ := strconv.Atoi(req.FormValue("t"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not think we can ignore the error here. Here is my reason:
This function is handling restful API. If a user uses http pure request "t=asdfg" which is invalid to access daemon, we had better throw out the error rather than make this move on.

}

// name is the container's prefix of the id.
if !ok {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can eliminate the if with an else with the above if.

@skoowoo
Copy link
Contributor Author

skoowoo commented Nov 1, 2017

Ok, I updated again. @allencloud

@allencloud
Copy link
Collaborator

Another thing, from the very beginning, is it necessary for us to take #32 into consideration?
Need some thoughts input. @skoo87

@skoowoo
Copy link
Contributor Author

skoowoo commented Nov 1, 2017

About the status code, I agree with you. @allencloud

"./pouch stop ID" be used to stop a running container. When restart the
pouchd, will recover these running containers.

Signed-off-by: skoo87 <marckywu@gmail.com>
@yyb196
Copy link
Collaborator

yyb196 commented Nov 2, 2017

LGTM

@skoowoo skoowoo merged commit a055655 into AliyunContainerService:master Nov 2, 2017
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.

3 participants