Skip to content

Conversation

@mathspanda
Copy link
Contributor

@mathspanda mathspanda commented Nov 7, 2018

Signed-off-by: mathspanda [email protected]

Ⅰ. Describe what this PR did

Some cli option type is value, which is meaningless.

# pouch cli
      --blkio-weight-device value    Block IO weight (relative device weight), need CFQ IO Scheduler enable (default [])
      --device-read-bps value        Limit read rate (bytes per second) from a device (default [])
      --device-read-iops value       Limit read rate (IO per second) from a device (default [])
      --device-write-bps value       Limit write rate (bytes per second) from a device (default [])
      --device-write-iops value      Limit write rate (IO per second) from a device (default [])
      --ulimit value                 Set container ulimit (default [])
# pouchd
      --add-runtime value                   register a OCI runtime to daemon (default [])

In docker:

# docker cli
      --device-read-bps list           Limit read rate (bytes per second) from a device (default [])
      --device-write-bps list          Limit write rate (bytes per second) to a device (default [])
      --device-read-iops list          Limit read rate (IO per second) from a device (default [])
      --device-write-iops list         Limit write rate (IO per second) to a device (default [])
      --ulimit ulimit                  Ulimit options (default [])
# dockerd
      --add-runtime runtime                     Register an additional OCI compatible runtime (default [])

Ⅱ. Does this pull request fix one issue?

issue #2445

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

None

Ⅳ. Describe how to verify it

pouch run --help
pouchd --help

Ⅴ. Special notes for reviews

None

@codecov
Copy link

codecov bot commented Nov 7, 2018

Codecov Report

Merging #2446 into master will increase coverage by 4.21%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2446      +/-   ##
==========================================
+ Coverage   64.66%   68.87%   +4.21%     
==========================================
  Files         276      276              
  Lines       18213    18213              
==========================================
+ Hits        11777    12544     +767     
+ Misses       5163     4248     -915     
- Partials     1273     1421     +148
Flag Coverage Δ
#criv1alpha1test 31.44% <0%> (?)
#criv1alpha2test 35.66% <0%> (-0.12%) ⬇️
#integrationtest 40.21% <0%> (ø) ⬆️
#nodee2etest 32.99% <0%> (+0.01%) ⬆️
#unittest 26.65% <0%> (ø) ⬆️
Impacted Files Coverage Δ
apis/opts/config/blkio.go 69.66% <0%> (ø) ⬆️
apis/opts/config/ulimit.go 0% <0%> (ø) ⬆️
apis/opts/config/runtime.go 66.66% <0%> (ø) ⬆️
cri/stream/portforward/httpstream.go 69.16% <0%> (-6.67%) ⬇️
daemon/mgr/snapshot.go 89.85% <0%> (-4.35%) ⬇️
cri/stream/runtime.go 67.85% <0%> (-2.39%) ⬇️
ctrd/image.go 76.75% <0%> (-2.2%) ⬇️
cri/v1alpha2/cri_wrapper.go 62.4% <0%> (-1.21%) ⬇️
cri/v1alpha2/cri.go 70.03% <0%> (+0.12%) ⬆️
daemon/mgr/container_utils.go 86.7% <0%> (+1.26%) ⬆️
... and 12 more

@allencloud
Copy link
Collaborator

allencloud commented Nov 8, 2018

cri-e2e-test
./hack/testing/run_daemon_cri_e2e.sh v1alpha2
install cni...
hack/install/install_cni.sh: line 26: /home/travis/gopath/src/github.com/containernetworking/plugins/build.sh: No such file or directory
make: *** [cri-e2e-test] Error 1

Please rebase this pr to the latest master, since the master fix this issue. @mathspanda

Copy link
Contributor

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug This is bug report for project size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants