feature: add pouch kill functionality #1485

xiechengsheng · 2018-06-07T10:24:26Z

Signed-off-by: xiechengsheng XIE1995@whut.edu.cn

Ⅰ. Describe what this PR did

add pouch kill functionality

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

$ pouch ps
Name            ID       Status          Created          Image                                            Runtime
foo             c926cf   Up 5 seconds    6 seconds ago    registry.hub.docker.com/library/busybox:latest   runc
$ pouch kill foo
foo
$ pouch ps -a
Name            ID       Status                     Created          Image                                            Runtime
foo             c926cf   Exited (137) 9 seconds     25 seconds ago   registry.hub.docker.com/library/busybox:latest   runc

Ⅴ. Special notes for reviews

NONE

codecov-io · 2018-06-07T10:49:49Z

Codecov Report

Merging #1485 into master will increase coverage by 0.02%.
The diff coverage is 47.78%.

@@            Coverage Diff             @@
##           master    #1485      +/-   ##
==========================================
+ Coverage   40.83%   40.85%   +0.02%     
==========================================
  Files         254      256       +2     
  Lines       16601    16714     +113     
==========================================
+ Hits         6779     6829      +50     
- Misses       8954     9007      +53     
- Partials      868      878      +10

Impacted Files	Coverage Δ
cli/kill.go	`0% <0%> (ø)`
cli/main.go	`0% <0%> (ø)`	⬆️
apis/server/container_bridge.go	`83.69% <100%> (+0.8%)`	⬆️
client/container_kill.go	`100% <100%> (ø)`
apis/server/router.go	`91.36% <100%> (+0.06%)`	⬆️
ctrd/container.go	`50.31% <55.81%> (+0.13%)`	⬆️
daemon/mgr/container.go	`49.06% <71.42%> (+0.25%)`	⬆️
apis/server/utils.go	`59.52% <0%> (-4.77%)`	⬇️
... and 1 more

allencloud · 2018-06-11T09:24:54Z

apis/server/container_bridge.go

+	if sigStr := req.FormValue("signal"); sigStr != "" {
+		var err error
+		if sig, err = signal.ParseSignal(sigStr); err != nil {
+			return err


I think here we need to return a status code of 400 bad request. WDYT? @xiechengsheng

ok, will add http 400 err here.

allencloud · 2018-06-11T09:26:53Z

client/container_kill.go

@@ -0,0 +1,20 @@
+package client


Could you add a mock test for this API?

allencloud · 2018-06-11T09:27:35Z

client/container_kill.go

+	}
+	ensureCloseReader(resp)
+
+	return err


Here, please explicitly return nilrather than return err.

allencloud · 2018-06-11T09:28:46Z

daemon/mgr/container.go

+	}
+
+	if sig != 0 && !signal.ValidSignalForPlatform(syscall.Signal(sig)) {
+		return fmt.Errorf("signal: %d is not suppoerted", sig)


fmt.Errorf("signal %d is not supported", sig)

allencloud · 2018-06-11T09:29:21Z

daemon/mgr/container.go

+	}
+
+	if !c.IsRunning() {
+		return fmt.Errorf("container: %s is not running, can not execute kill command", c.ID)


please remove : in the error message.

allencloud · 2018-06-14T01:51:21Z

@xiechengsheng
CI fails with:

----------------------------------------------------------------------
FAIL: /go/src/github.com/alibaba/pouch/test/api_container_kill_test.go:92: APIContainerKillSuite.TestKillContainerWithInvalidSignal
/go/src/github.com/alibaba/pouch/test/api_container_kill_test.go:112:
    CheckRespStatus(c, resp, 204)
/go/src/github.com/alibaba/pouch/test/util_api.go:25:
    c.Assert(resp.StatusCode, check.Equals, status, check.Commentf("Error:%s", got.Message))
... obtained int = 500
... expected int = 204
... Error:process id fa1602b47d813085d88165f71bcf854abda35163c2d450000ac1156348ab4d6b: not found

allencloud · 2018-06-14T01:52:23Z

apis/swagger.yml

+          default: "SIGKILL"
+      responses:
+        204:
+          description: "no error"


You add status code 400 in code, then here we need to add 400 as well in swagger. @xiechengsheng

Signed-off-by: xiechengsheng <XIE1995@whut.edu.cn>

xiechengsheng · 2018-06-14T03:22:01Z

OK, I will fix this ASAP.

allencloud · 2018-06-14T05:22:56Z

LGTM, thank you very much for this excellent work. @xiechengsheng

xiechengsheng · 2018-06-14T06:30:16Z

Wow, merged so quickly. 😄

wangforthinker · 2019-05-05T06:12:40Z

Could we continue to push this feature? @fuweid @xiechengsheng

pouchrobot added kind/feature size/XL labels Jun 7, 2018

allencloud reviewed Jun 11, 2018

View reviewed changes

client/container_kill.go

@@ -0,0 +1,20 @@

package client

Copy link

Collaborator

allencloud Jun 11, 2018

Choose a reason for hiding this comment

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

Could you add a mock test for this API?

allencloud reviewed Jun 11, 2018

View reviewed changes

xiechengsheng force-pushed the add-kill branch 3 times, most recently from 1d99e08 to b50b084 Compare June 13, 2018 15:11

allencloud reviewed Jun 14, 2018

View reviewed changes

feature: add pouch kill functionality

975585d

Signed-off-by: xiechengsheng <XIE1995@whut.edu.cn>

xiechengsheng force-pushed the add-kill branch from b50b084 to 975585d Compare June 14, 2018 03:21

allencloud merged commit 6cf387a into AliyunContainerService:master Jun 14, 2018

pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jun 14, 2018

pouchrobot mentioned this pull request Jun 14, 2018

Weekly Report of Pouch from 2018-06-07 to 2018-06-14 #1528

Closed

xiechengsheng mentioned this pull request Jun 15, 2018

Revert "feature: add pouch kill functionality" #1538

Merged

fuweid mentioned this pull request Mar 4, 2019

[Proposal] Support to send signal to container main process in pouch. #2733

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: add pouch kill functionality #1485

feature: add pouch kill functionality #1485

xiechengsheng commented Jun 7, 2018

codecov-io commented Jun 7, 2018 •

edited

Loading

allencloud Jun 11, 2018 •

edited

Loading

xiechengsheng Jun 12, 2018

allencloud Jun 11, 2018

allencloud Jun 11, 2018 •

edited

Loading

allencloud Jun 11, 2018

allencloud Jun 11, 2018

allencloud commented Jun 14, 2018

allencloud Jun 14, 2018

xiechengsheng commented Jun 14, 2018

allencloud commented Jun 14, 2018

xiechengsheng commented Jun 14, 2018

wangforthinker commented May 5, 2019

feature: add pouch kill functionality #1485

feature: add pouch kill functionality #1485

Conversation

xiechengsheng commented Jun 7, 2018

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov-io commented Jun 7, 2018 • edited Loading

Codecov Report

allencloud Jun 11, 2018 • edited Loading

Choose a reason for hiding this comment

xiechengsheng Jun 12, 2018

Choose a reason for hiding this comment

allencloud Jun 11, 2018

Choose a reason for hiding this comment

allencloud Jun 11, 2018 • edited Loading

Choose a reason for hiding this comment

allencloud Jun 11, 2018

Choose a reason for hiding this comment

allencloud Jun 11, 2018

Choose a reason for hiding this comment

allencloud commented Jun 14, 2018

allencloud Jun 14, 2018

Choose a reason for hiding this comment

xiechengsheng commented Jun 14, 2018

allencloud commented Jun 14, 2018

xiechengsheng commented Jun 14, 2018

wangforthinker commented May 5, 2019

codecov-io commented Jun 7, 2018 •

edited

Loading

allencloud Jun 11, 2018 •

edited

Loading

allencloud Jun 11, 2018 •

edited

Loading