Skip to content

Conversation

@fuweid
Copy link
Contributor

@fuweid fuweid commented Jun 4, 2019

Ⅰ. Describe what this PR did

  1. add rollback for copy command
  2. add debug log
  3. support to mount /dev/, /proc folder

Ⅱ. Does this pull request fix one issue?

none

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

has cases

Ⅳ. Describe how to verify it

wait for CI

Ⅴ. Special notes for reviews

fuweid added 2 commits June 4, 2019 11:28
The /dev/ /tmp /proc are default mount point from oci spec. They are not
in the container mount points. When we pass the /dev/terminal-log as
dest, it will mount after the /dev, which handed by runC in the special
namespace. But when we try to mount for copy, we don't have /dev folder
or sub folder, we need to create parent folder for the target or empty
file.

Signed-off-by: Wei Fu <[email protected]>
@fuweid fuweid requested a review from ZYecho June 4, 2019 03:31
@pouchrobot pouchrobot added areas/cli kind/bug This is bug report for project size/L labels Jun 4, 2019
@codecov
Copy link

codecov bot commented Jun 4, 2019

Codecov Report

Merging #2890 into master will decrease coverage by 0.1%.
The diff coverage is 35.55%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2890      +/-   ##
=========================================
- Coverage   68.91%   68.8%   -0.11%     
=========================================
  Files         291     291              
  Lines       18230   18249      +19     
=========================================
- Hits        12563   12557       -6     
- Misses       4215    4236      +21     
- Partials     1452    1456       +4
Flag Coverage Δ
#criv1alpha2_test 38.33% <0%> (-0.07%) ⬇️
#integration_test_0 36.1% <0%> (-0.05%) ⬇️
#integration_test_1 35.69% <35.55%> (-0.01%) ⬇️
#integration_test_2 36.03% <0%> (+0.01%) ⬆️
#integration_test_3 35.68% <24.44%> (ø) ⬆️
#node_e2e_test 34.11% <0%> (-0.12%) ⬇️
#unittest 28.03% <0%> (-0.04%) ⬇️
Impacted Files Coverage Δ
daemon/mgr/container_copy.go 58.08% <35.13%> (-1.92%) ⬇️
daemon/mgr/container_storage.go 60.03% <37.5%> (-0.43%) ⬇️
pkg/streams/utils.go 82.14% <0%> (-7.15%) ⬇️
cri/ocicni/netns.go 58.1% <0%> (-2.71%) ⬇️
cri/ocicni/cni_manager.go 59.25% <0%> (-1.86%) ⬇️
ctrd/supervisord/daemon.go 49.32% <0%> (-1.36%) ⬇️
daemon/mgr/container.go 59.72% <0%> (-0.63%) ⬇️
cri/v1alpha2/cri.go 69.06% <0%> (-0.26%) ⬇️
ctrd/container.go 54.3% <0%> (+0.38%) ⬆️
apis/server/utils.go 75% <0%> (+3.84%) ⬆️
... and 1 more

Copy link
Contributor

@ZYecho ZYecho left a comment

Choose a reason for hiding this comment

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

LGTM

@ZYecho ZYecho merged commit 86433c9 into AliyunContainerService:master Jun 4, 2019
@fuweid fuweid deleted the me-enhance-cp branch June 4, 2019 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

areas/cli kind/bug This is bug report for project size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants