Skip to content

Add direct unpack support for overlay and aufs#2913

Closed
Random-Liu wants to merge 1 commit intocontainerd:masterfrom
Random-Liu:direct-unpack
Closed

Add direct unpack support for overlay and aufs#2913
Random-Liu wants to merge 1 commit intocontainerd:masterfrom
Random-Liu:direct-unpack

Conversation

@Random-Liu
Copy link
Copy Markdown
Member

@Random-Liu Random-Liu commented Jan 4, 2019

For #2886.

This PR:

  1. Added direct unpack support for overlay and aufs;
  2. Remove the .wh..wh.plnk related logic. It is not defined in the OCI spec; it shouldn't be included in docker image any more; and Docker overlay2 doesn't handle it.
  3. Update some unit test cases correspondingly, please check this comment to see why the test update is needed.

Time consumed to pull k8s.gcr.io/kube-cross:v1.10.3-1 on my machine:

  • Containerd HEAD: 40.347s
  • Containerd HEAD + this change: 35.65s

Signed-off-by: Lantao Liu lantaol@google.com

@Random-Liu Random-Liu modified the milestones: 1.2.1, 1.3 Jan 4, 2019
Random-Liu
Random-Liu commented Jan 4, 2019
View reviewed changes
archive/tar_opts_unix.go Outdated
Copy link
Copy Markdown
Member Author

@Random-Liu Random-Liu Jan 4, 2019

Choose a reason for hiding this comment

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

Do we need something like this https://github.com/moby/moby/blob/de640c9f4932d851316a0a72470c4d3446f6f5ac/pkg/archive/archive_linux.go#L80? @AkihiroSuda

Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Jan 5, 2019

Choose a reason for hiding this comment

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

yes but L96, not L80

Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Jan 5, 2019

Choose a reason for hiding this comment

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

Or we can just add godoc to suggest client implementors not to use WithConvertWhiteout when running in userns.

Random-Liu
Random-Liu commented Jan 4, 2019
View reviewed changes
vendor/github.com/containerd/aufs/aufs.go Outdated
Copy link
Copy Markdown
Member Author

@Random-Liu Random-Liu Jan 4, 2019

Choose a reason for hiding this comment

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

I'll make this change to containerd/aufs once this PR is ok to most people. :)

@Random-Liu Random-Liu force-pushed the direct-unpack branch 3 times, most recently from d9b1f9c to 96dcd69 Compare January 5, 2019 00:56
AkihiroSuda
AkihiroSuda reviewed Jan 5, 2019
View reviewed changes
archive/tar_opts_unix.go Outdated
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Jan 5, 2019

Choose a reason for hiding this comment

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

needs https://github.com/containerd/containerd/pull/2913/files#r245435103

AkihiroSuda
AkihiroSuda reviewed Jan 5, 2019
View reviewed changes
archive/tar.go Outdated
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Jan 5, 2019

Choose a reason for hiding this comment

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

Can we split this to a new function in the global scope

@Random-Liu Random-Liu force-pushed the direct-unpack branch 2 times, most recently from a1596a6 to a361eb9 Compare January 7, 2019 07:47
@Random-Liu
Add direct unpack support for overlay and aufs
0ed1a6d 
Signed-off-by: Lantao Liu <lantaol@google.com>
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 7, 2019

Codecov Report

Merging #2913 into master will increase coverage by 0.12%.
The diff coverage is 50.79%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2913      +/-   ##
==========================================
+ Coverage   43.89%   44.02%   +0.12%     
==========================================
  Files         101      104       +3     
  Lines       10800    10887      +87     
==========================================
+ Hits         4741     4793      +52     
- Misses       5328     5362      +34     
- Partials      731      732       +1
Flag Coverage Δ
#linux 47.62% <54.31%> (+0.12%) ⬆️
#windows 41.18% <20.45%> (+0.11%) ⬆️
Impacted Files Coverage Δ
archive/tar_opts_windows.go 0% <ø> (ø) ⬆️
diff/apply/apply.go 0% <0%> (ø)
diff/apply/apply_linux.go 54.34% <54.34%> (ø)
archive/tar.go 48.85% <64.1%> (+5.06%) ⬆️
archive/tar_opts_linux.go 71.42% <71.42%> (ø)
archive/tar_opts.go 50% <80%> (+21.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5a7946...0ed1a6d. Read the comment docs.

This was referenced Jan 9, 2019
Merged
Closed
Merged
@estesp estesp mentioned this pull request Feb 22, 2019
Closed
@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented Jun 16, 2019

needs rebase

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Aug 13, 2019

I'm going to carry this one. Closing so I can re-open as draft to run tests

@dmcgowan dmcgowan closed this Aug 13, 2019
@Random-Liu
Copy link
Copy Markdown
Member Author

Random-Liu commented Aug 13, 2019

@dmcgowan SG.

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

Reviewers

@AkihiroSuda AkihiroSuda AkihiroSuda left review comments

Assignees

@dmcgowan dmcgowan

@AkihiroSuda AkihiroSuda

Labels

None yet

Projects

None yet

Milestone

1.3

Development

Successfully merging this pull request may close these issues.

4 participants

@Random-Liu @codecov-io @AkihiroSuda @dmcgowan