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

feature: populate empty volumes when creating container #1745

Merged
merged 1 commit into from
Jul 19, 2018

Conversation

shaloulcy
Copy link
Contributor

@shaloulcy shaloulcy commented Jul 17, 2018

Signed-off-by: Eric Li lcy041536@gmail.com

Ⅰ. Describe what this PR did

if the volume is empty and image path is not empty, we will populate the volume with the image data

Ⅱ. Does this pull request fix one issue?

fixes #1739

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Signed-off-by: Eric Li <lcy041536@gmail.com>
@codecov-io
Copy link

codecov-io commented Jul 17, 2018

Codecov Report

Merging #1745 into master will decrease coverage by 0.01%.
The diff coverage is 43.24%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1745      +/-   ##
==========================================
- Coverage   14.14%   14.13%   -0.02%     
==========================================
  Files         281      281              
  Lines       56728    56797      +69     
==========================================
- Hits         8027     8026       -1     
- Misses      47777    47838      +61     
- Partials      924      933       +9
Impacted Files Coverage Δ
daemon/mgr/container.go 51.43% <43.24%> (-1.03%) ⬇️
pkg/meta/store.go 54.76% <0%> (-4.77%) ⬇️
daemon/containerio/cio.go 58.73% <0%> (-4.77%) ⬇️
daemon/containerio/container_io.go 53.59% <0%> (-4.42%) ⬇️
daemon/mgr/container_utils.go 54.97% <0%> (-1.74%) ⬇️
daemon/mgr/system.go 73.27% <0%> (-1.73%) ⬇️
daemon/logger/jsonfile/utils.go 71.54% <0%> (-1.63%) ⬇️
ctrd/image.go 81.18% <0%> (+1.98%) ⬆️

@YaoZengzeng
Copy link
Contributor

I've tested this PR on my local machine, it works fine.

@allencloud
Copy link
Collaborator

I think this could definitely add an integration test. WDYT? @shaloulcy @YaoZengzeng

@shaloulcy
Copy link
Contributor Author

@YaoZengzeng Are you interested to add an integration test? thanks~

@YaoZengzeng
Copy link
Contributor

@shaloulcy After this PR being merged, I'll write a test for it.

@rudyfly
Copy link
Collaborator

rudyfly commented Jul 19, 2018

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jul 19, 2018
@rudyfly rudyfly merged commit f251319 into AliyunContainerService:master Jul 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
areas/storage kind/feature LGTM one maintainer or community participant agrees to merge the pull reuqest. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature request] If the mount destination is not empty, we should see all the content after mounting
7 participants