fixes: top options can't work

[lifubang]

Signed-off-by: Lifubang lifubang@acmcoder.com

Ⅰ. Describe what this PR did

1. When pouch top --help:
   top command is to display the running processes of a container.You can add options just like using Linux ps command.
   But when I run: pouch top redisps -aux
   It returns:
   Error: unknown shorthand flag: 'a' in -aux

2. in https://github.com/alibaba/pouch/blob/master/docs/api/HTTP_API.md
   Display the running processes of a container
   POST /containers/{id}/top

But in https://github.com/alibaba/pouch/blob/master/client/container_top.go#L19
and https://github.com/alibaba/pouch/blob/master/apis/server/router.go#L57
The type is get.

And, in https://github.com/alibaba/pouch/blob/master/apis/server/container_bridge.go#L323
It uses req.Form.Get

Is post? or get?

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Yes, you already had test cases.

Ⅳ. Describe how to verify it

pouch top redisps -aux

Ⅴ. Special notes for reviews

Make pouch top options work
API use get method

[codecov]

Codecov Report

❗ No coverage uploaded for pull request head (topoptions@c5f707f). Click here to learn what that means.
The diff coverage is n/a.

[pouchrobot]

We found this is your first time to contribute to Pouch, @lifubang
👏 We really appreciate it.
Just remind that you have read the contribution guide: https://github.com/alibaba/pouch/blob/master/CONTRIBUTING.md
If you didn't, you should do that first. If done, welcome again and please enjoy hacking! 🍻

[CLAassistant]

All committers have signed the CLA.

[allencloud]

Thanks a lot for your contribution. @lifubang
I think it is a GET method, like https://github.com/moby/moby/blob/master/api/swagger.yaml#L4957 . And we wish that API could be compatible with Moby's.

[lifubang]

So, change to get?

[allencloud]

So, change to get?

I think we need to update the swagger.yml to make method to be get. Then @pouchrobot will automatically update the docs without interaction of humans. @lifubang

[allencloud]

Thanks a lot for this update. While for other commands like create, run and so on, we use flagSet.SetInterspersed(false)in

func (cc *CreateCommand) addFlags() {
	flagSet := cc.cmd.Flags()
	flagSet.SetInterspersed(false)

	c := addCommonFlags(flagSet)
	cc.container = c
}

How about making pouchd use the unified way of flagSet.SetInterspersed(false)?

[lifubang]

I have finished.

[allencloud]

Do you use wechat or DingTalk? Please add me as a friend with account 15869045568. Thanks a lot. @lifubang

[lifubang]

OK, wait a minute.

[allencloud]

Thanks again for the fix. While when I was checking the cli integration test, I found that there is no integration test for the args of pouch top command. Could you add one to lock down the functionality in test case? @lifubang

[allencloud]

Thanks for the test cased added. I feel that we are almost there. Currently there is still some minor issue in code style:

#!/bin/bash -eo pipefail
make check
gometalinter
gometalinter --config .gometalinter.json ./...
test/api_container_top_test.go:1::warning: file is not goimported (goimports)
Makefile:148: recipe for target 'gometalinter' failed
make: *** [gometalinter] Error 1
Exited with code 2

Is the goimport failing or something esle? @lifubang

[codecov]

Codecov Report

Merging #2252 into master will decrease coverage by 1.25%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2252      +/-   ##
==========================================
- Coverage   66.77%   65.52%   -1.26%     
==========================================
  Files         208      208              
  Lines       16923    16924       +1     
==========================================
- Hits        11301    11090     -211     
- Misses       4262     4508     +246     
+ Partials     1360     1326      -34

Flag	Coverage Δ
#criv1alpha1test	32.74% <0%> (+0.07%)	⬆️
#criv1alpha2test	100% <ø> (+63.99%)	⬆️
#integrationtest	39.45% <100%> (-0.04%)	⬇️
#nodee2etest	33.56% <0%> (+0.12%)	⬆️
#unittest	23.76% <0%> (-0.01%)	⬇️

Impacted Files	Coverage Δ
apis/server/container_bridge.go	79.41% <100%> (+0.07%)	⬆️
cri/v1alpha2/server.go	42.4% <0%> (-36.81%)	⬇️
storage/quota/quota.go	12.02% <0%> (-36.07%)	⬇️
storage/quota/prjquota.go	12.58% <0%> (-12.59%)	⬇️
daemon/mgr/container_storage.go	50.93% <0%> (-8.18%)	⬇️
cri/v1alpha2/cri_wrapper.go	53.6% <0%> (-7.61%)	⬇️
apis/server/utils.go	61.9% <0%> (-4.77%)	⬇️
cri/stream/remotecommand/httpstream.go	43.52% <0%> (-3.11%)	⬇️
cri/v1alpha2/cri_utils.go	87.55% <0%> (-3.08%)	⬇️
cri/v1alpha2/cri.go	64.77% <0%> (-1.95%)	⬇️
... and 8 more

[allencloud]

I am afraid that CLI integration test fails:

----------------------------------------------------------------------
FAIL: /home/travis/gopath/src/github.com/alibaba/pouch/test/cli_top_test.go:72: PouchTopSuite.TestTopContainerWithOptions
/home/travis/gopath/src/github.com/alibaba/pouch/test/cli_top_test.go:84:
    c.Fatalf("unexpected output %s expected %s", out, expectString)
... Error: unexpected output USERPID%CPU%MEMVSZRSSTTYSTATSTARTTIMECOMMANDroot211170.40.012364?Ss04:060:00top expected UIDPID%CPU
----------------------------------------------------------------------

@lifubang PTAL.

[allencloud]

LGTM