feature: enable managing more containers for some commands #1357

xiechengsheng · 2018-05-19T11:01:03Z

enable managing more containers for some commands

Ⅰ. Describe what this PR did

Some commands just support managing one container/image, which is inconvenient for users to use. This pr enables managing one or more containers for some client commands and fixes little bugs in commands that support manage more than one container by now.

Ⅱ. Does this pull request fix one issue?

NONE.

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

$ pouch ps -a
Name            ID       Status                  Created        Image                                            Runtime
my_container2   0a83b7   Stopped (0) 3 seconds   1 minute ago   registry.hub.docker.com/library/busybox:latest   runc
my_container1   2aa41e   Stopped (0) 3 seconds   1 minute ago   registry.hub.docker.com/library/busybox:latest   runc
$ pouch start my_container1 my_container2
my_container1
my_container2
$ pouch ps 
Name            ID       Status         Created        Image                                            Runtime
my_container2   0a83b7   Up 8 seconds   1 minute ago   registry.hub.docker.com/library/busybox:latest   runc
my_container1   2aa41e   Up 9 seconds   1 minute ago   registry.hub.docker.com/library/busybox:latest   runc

More test codes are in test folder.

Ⅴ. Special notes for reviews

codecov-io · 2018-05-19T11:26:46Z

Codecov Report

Merging #1357 into master will decrease coverage by 0.07%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1357      +/-   ##
==========================================
- Coverage   17.34%   17.26%   -0.08%     
==========================================
  Files         189      189              
  Lines       11832    11886      +54     
==========================================
  Hits         2052     2052              
- Misses       9632     9686      +54     
  Partials      148      148

Impacted Files	Coverage Δ
cli/rmi.go	`0% <0%> (ø)`	⬆️
cli/pause.go	`0% <0%> (ø)`	⬆️
cli/restart.go	`0% <0%> (ø)`	⬆️
cli/start.go	`0% <0%> (ø)`	⬆️
cli/rm.go	`0% <0%> (ø)`	⬆️
cli/unpause.go	`0% <0%> (ø)`	⬆️

allencloud · 2018-05-20T04:35:19Z

Could you help to review this? @ZouRui89 Thanks a lot.

ZouRui89 · 2018-05-21T01:48:15Z

cli/pause.go


 	"github.com/spf13/cobra"
 )

 // pauseDescription is used to describe pause command in detail and auto generate command doc.
-var pauseDescription = "Pause a running container object in Pouchd. " +
+var pauseDescription = "Pause one or more running containers object in Pouchd. " +
 	"when pausing, the container will pause its running but hold all the relevant resource." +
 	"This is useful when you wish to pause a container for a while and to restore the running status later." +


Please change the corresponding amount here. Thx.

ZouRui89 · 2018-05-21T01:56:36Z

cli/pause.go

@@ -22,10 +24,10 @@ type PauseCommand struct {
 func (p *PauseCommand) Init(c *Cli) {
 	p.cli = c
 	p.cmd = &cobra.Command{
-		Use:   "pause CONTAINER",
-		Short: "Pause a running container",
+		Use:   "pause CONTAINER [CONTAINERS]",


Change the value of Use to "pause CONTAINER [CONTAINER...]".

ZouRui89 · 2018-05-21T02:01:46Z

cli/rm.go


 	"github.com/alibaba/pouch/apis/types"

 	"github.com/spf13/cobra"
 )

 var rmDescription = `
-Remove a container object in Pouchd.
+Remove one or more container objects in Pouchd.


Just delete the word object to keep consistency with other commands.

ZouRui89 · 2018-05-21T02:03:24Z

cli/start.go

@@ -29,9 +31,9 @@ func (s *StartCommand) Init(c *Cli) {
 	s.cli = c
 	s.cmd = &cobra.Command{
 		Use:   "start [OPTIONS] CONTAINER",


"start [OPTIONS] CONTAINER [CONTAINER...]"

ZouRui89 · 2018-05-21T02:04:34Z

cli/start.go

 	if s.attach || s.stdin {
+		var wait chan struct{}
+		// We're going to attach to a container, we should make sure we only have one container.


Change it to "If We're going to attach to a container, we should make sure we only have one container."

ZouRui89 · 2018-05-21T02:08:35Z

cli/restart.go

+	if len(errs) > 0 {
+		return errors.New(strings.Join(errs, "\n"))
+	}
+
 	return nil
 }



Change the value of Use to update amount in init function.

ZouRui89 · 2018-05-21T02:14:45Z

Please use "commit -sm" to sign off your commits.

ZouRui89 · 2018-05-21T02:22:26Z

Thx a lot for your work.
The cli part is quite import since it is directly demonstrated to our users.
Please change all the corresponding words in Init function, like the value of Use and any things else related to. Beside, the example part in all the cli files you revised should be also changed, since more than one containers can be operated within one command due to your excellent work. :)

xiechengsheng · 2018-05-21T03:17:26Z

Thanks for your careful review~

ZouRui89 · 2018-05-21T06:16:53Z

test/cli_start_test.go

+	res2 := command.PouchRun("create", "--name", name2, busyboxImage, "top")
+	defer DelContainerForceMultyTime(c, name2)
+	res2.Assert(c, icmd.Success)
+


A for circulation can be used here to enhance the degree of simplicity. Just like the way you do in TestUnpauseMultiContainers.

ZouRui89 · 2018-05-21T06:18:01Z

test/cli_restart_test.go

+	res2 := command.PouchRun("run", "-d", "--cpu-share", "20", "--name", name2, busyboxImage)
+	defer DelContainerForceMultyTime(c, name2)
+	res2.Assert(c, icmd.Success)
+


The same circulation problem as below.

xiechengsheng · 2018-05-21T07:05:54Z

OK, I'll fix this, thanks.

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

ZouRui89 · 2018-05-21T08:00:43Z

LGTM

pouchrobot added areas/cli kind/feature size/XL labels May 19, 2018

xiechengsheng force-pushed the manage-more branch 2 times, most recently from 95942db to 06ff967 Compare May 19, 2018 11:12

allencloud assigned ZouRui89 May 20, 2018

ZouRui89 reviewed May 21, 2018

View reviewed changes

xiechengsheng force-pushed the manage-more branch from 06ff967 to 398fa85 Compare May 21, 2018 03:16

ZouRui89 reviewed May 21, 2018

View reviewed changes

enable managing more containers for some commands

dfb3444

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

xiechengsheng force-pushed the manage-more branch from 398fa85 to dfb3444 Compare May 21, 2018 07:22

ZouRui89 merged commit 1e75a11 into AliyunContainerService:master May 21, 2018

pouchrobot mentioned this pull request May 24, 2018

Weekly Report of Pouch from 2018-05-17 to 2018-05-24 #1408

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: enable managing more containers for some commands #1357

feature: enable managing more containers for some commands #1357

xiechengsheng commented May 19, 2018

codecov-io commented May 19, 2018 •

edited

Loading

allencloud commented May 20, 2018

ZouRui89 May 21, 2018

ZouRui89 May 21, 2018

ZouRui89 May 21, 2018

ZouRui89 May 21, 2018

ZouRui89 May 21, 2018

ZouRui89 May 21, 2018

ZouRui89 commented May 21, 2018

ZouRui89 commented May 21, 2018

xiechengsheng commented May 21, 2018

ZouRui89 May 21, 2018 •

edited

Loading

ZouRui89 May 21, 2018

xiechengsheng commented May 21, 2018

ZouRui89 commented May 21, 2018

feature: enable managing more containers for some commands #1357

feature: enable managing more containers for some commands #1357

Conversation

xiechengsheng commented May 19, 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 May 19, 2018 • edited Loading

Codecov Report

allencloud commented May 20, 2018

ZouRui89 May 21, 2018

Choose a reason for hiding this comment

ZouRui89 May 21, 2018

Choose a reason for hiding this comment

ZouRui89 May 21, 2018

Choose a reason for hiding this comment

ZouRui89 May 21, 2018

Choose a reason for hiding this comment

ZouRui89 May 21, 2018

Choose a reason for hiding this comment

ZouRui89 May 21, 2018

Choose a reason for hiding this comment

ZouRui89 commented May 21, 2018

ZouRui89 commented May 21, 2018

xiechengsheng commented May 21, 2018

ZouRui89 May 21, 2018 • edited Loading

Choose a reason for hiding this comment

ZouRui89 May 21, 2018

Choose a reason for hiding this comment

xiechengsheng commented May 21, 2018

ZouRui89 commented May 21, 2018

codecov-io commented May 19, 2018 •

edited

Loading

ZouRui89 May 21, 2018 •

edited

Loading