Remove reexec usage #535
Remove reexec usage #535
Conversation
Random-Liu
force-pushed
the
remove-reexec
branch
from
5c034e7
to
07a562e
Compare
|
Would it make sense to use https://godoc.org/github.com/containerd/containerd/fs#CopyDir instead? |
|
I think we probably do want the xattrs. It looks like |
pkg/containerd/opts/container.go
Outdated
| @@ -122,7 +122,7 @@ func copyExistingContents(source, destination string) error { | |||
| return errors.Errorf("volume at %q is not initially empty", destination) | |||
| } | |||
|
|
|||
| if err := chrootarchive.NewArchiver(nil).CopyWithTar(source, destination); err != nil { | |||
| if err := copy.DirCopy(source, destination, copy.Content, false); err != nil { | |||
There was a problem hiding this comment.
I believe we can remove copyOwnership() (just below this), it's already done by fs.CopyDir()
Cool. Don't know containerd has such a util function, will use it. |
Signed-off-by: Lantao Liu <lantaol@google.com>
Signed-off-by: Lantao Liu <lantaol@google.com>
Signed-off-by: Lantao Liu <lantaol@google.com>
Random-Liu
force-pushed
the
remove-reexec
branch
from
07a562e
to
8782f18
Compare
|
/test all |
|
/joke |
|
/shrug |
|
@krzyzacy: you can't request testing unless you are a containerd member. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/shrug |
|
/joke |
|
/test all |
|
@Random-Liu: you can't request testing unless you are a containerd member. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/shrug |
|
/joke |
|
/test all |
|
Yeah! I need to make my containerd membership public to let the k8s-ci-bot know it. @krzyzacy /cc @containerd/containerd-maintainers @containerd/containerd-reviewers Make your containerd membership public if you want to trigger test in this repo. :P |
|
/retest |
1 similar comment
|
/retest |
|
@dnephin Are you ok with the new change? |
|
Apply LGTM based on #535 (comment) and #535 (review). /lgtm |
|
@Random-Liu: you cannot LGTM your own PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
For containerd/containerd#1966, #238.
This PR:
github.com/docker/docker/daemon/graphdriver/copyinstead ofgithub.com/docker/docker/pkg/chrootarchive, because we only do directly directory copy, which doesn't have security issue./cc @cpuguy83 @dnephin @ijc @stevvooe
@dnephin I do have one question about
copy.DirCopy. We don't need to copyxattr, right?