Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

copy command: bug fix and enhance #2890

Merged
merged 2 commits into from
Jun 4, 2019
Merged

copy command: bug fix and enhance #2890

merged 2 commits into from
Jun 4, 2019

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

Signed-off-by: Wei Fu <fuweid89@gmail.com>
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 <fuweid89@gmail.com>
@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