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.

Sign up for GitHub

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

support container to container copy #10728

Closed
wants to merge 3 commits into from
Closed

support container to container copy #10728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c31cc84
support container to container copy
Jun 19, 2021
f0a0b12
Merge branch 'containers:master' into c2c-copy
infiniteregrets Jun 25, 2021
67cbb74
fix bugs
Jun 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 80 additions & 1 deletion cmd/podman/containers/cp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,14 @@ func cp(cmd *cobra.Command, args []string) error {
return err
}

if len(sourceContainerStr) > 0 {
if len(sourceContainerStr) > 0 && len(destContainerStr) > 0 {
return copyContainerToContainer(sourceContainerStr, sourcePath, destContainerStr, destPath)
} else if len(sourceContainerStr) > 0 {
return copyFromContainer(sourceContainerStr, sourcePath, destPath)
}

return copyToContainer(destContainerStr, destPath, sourcePath)

}

// containerMustExist returns an error if the specified container does not
Expand Down Expand Up @@ -113,6 +116,82 @@ func doCopy(funcA func() error, funcB func() error) error {
return errorhandling.JoinErrors(copyErrors)
}

func copyContainerToContainer(fromContainer string, sourcePath string, toContainer string, destPath string) error {
if err := containerMustExist(fromContainer); err != nil {
return err
}

if err := containerMustExist(toContainer); err != nil {
return err
}

fromContainerInfo, err := registry.ContainerEngine().ContainerStat(registry.GetContext(), fromContainer, sourcePath)
if err != nil {
return errors.Wrapf(err, "%q could not be found on container %s", sourcePath, fromContainer)
}

var toContainerBaseName string
Copy link
Contributor

Choose a reason for hiding this comment

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

can you change to toContainerBaseName := "" removes an unnecessary line.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I am seeing you are getting an ineffectual assignment error here. Where are you using toContainerBaseName after setting it?

Copy link
Author

Choose a reason for hiding this comment

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

can you change to toContainerBaseName := "" removes an unnecessary line.

sure, will make the changes once I figure everything out

Also, I am seeing you are getting an ineffectual assignment error here. Where are you using toContainerBaseName after setting it?

I am not completely sure tbh, I had to do _ = toContainerBaseName to get rid of that unused variable error

Copy link
Contributor

Choose a reason for hiding this comment

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

ok @infiniteregrets you're getting that error because you set the base name and never actually use it. the _ = is just a clever way to trick golang. You should use that name somewhere in your copy code.

Copy link
Author

Choose a reason for hiding this comment

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

@cdoern I am using it?

if toContainerInfo != nil {
			toContainerBaseName = filepath.Base(toContainerInfo.LinkTarget)
		} else {
			toContainerBaseName = filepath.Base(destPath)
		}

This looks like a shadowing issue? Im not sure but I'll try to figure it. Thanks!

_ = toContainerBaseName
toContainerInfo, err := registry.ContainerEngine().ContainerStat(registry.GetContext(), toContainer, destPath)
if err != nil {
if strings.HasSuffix(destPath, "/") {
return errors.Wrapf(err, "%q could not be found on container %s", destPath, toContainer)
}
if toContainerInfo != nil {
toContainerBaseName = filepath.Base(toContainerInfo.LinkTarget)
} else {
toContainerBaseName = filepath.Base(destPath)
}

parentDir, err := containerParentDir(toContainer, destPath)
if err != nil {
return errors.Wrapf(err, "could not determine parent dir of %q on container %s", destPath, toContainer)
}
toContainerInfo, err = registry.ContainerEngine().ContainerStat(registry.GetContext(), toContainer, parentDir)
if err != nil {
return errors.Wrapf(err, "%q could not be found on container %s", destPath, toContainer)
}
} else {
toContainerBaseName = filepath.Base(toContainerInfo.LinkTarget)
}

if fromContainerInfo.IsDir && !toContainerInfo.IsDir {
return errors.New("destination must be a directory when copying a directory")
}

fromContainerTarget, toContainerTarget := fromContainerInfo.LinkTarget, toContainerInfo.LinkTarget
if !toContainerInfo.IsDir {
toContainerTarget = filepath.Dir(destPath)
}
reader, writer := io.Pipe()

fromContainerCopy := func() error {
Copy link
Contributor

@cdoern cdoern Jun 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

potentially could consolidate toContainerCopy and fromContainercopy into a function you call twice that takes specific parameters. Seems slightly redundant to have nearly identical code twice.

Copy link
Author

Choose a reason for hiding this comment

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

there might be some changes to the functions, so depending on what the final outcome looks like i'll refactor the code

defer writer.Close()
copyFunc, err := registry.ContainerEngine().ContainerCopyToArchive(registry.GetContext(), fromContainer, fromContainerTarget, writer)
if err != nil {
return err
}
if err := copyFunc(); err != nil {
return errors.Wrap(err, "error copying from container")
}
return nil
}

toContainerCopy := func() error {
defer reader.Close()
copyFunc, err := registry.ContainerEngine().ContainerCopyFromArchive(registry.GetContext(), toContainer, toContainerTarget, reader)
if err != nil {
return err
}
if err := copyFunc(); err != nil {
return errors.Wrap(err, "error copying to container")
}
return nil
}

return doCopy(fromContainerCopy, toContainerCopy)
}

// copyFromContainer copies from the containerPath on the container to hostPath.
func copyFromContainer(container string, containerPath string, hostPath string) error {
if err := containerMustExist(container); err != nil {
Expand Down
12 changes: 0 additions & 12 deletions pkg/copy/parse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,6 @@ func ParseSourceAndDestination(source, destination string) (string, string, stri
sourceContainer, sourcePath := parseUserInput(source)
destContainer, destPath := parseUserInput(destination)

numContainers := 0
if len(sourceContainer) > 0 {
numContainers++
}
if len(destContainer) > 0 {
numContainers++
}

if numContainers != 1 {
return "", "", "", "", errors.Errorf("invalid arguments %q, %q: exactly 1 container expected but %d specified", source, destination, numContainers)
}

if len(sourcePath) == 0 || len(destPath) == 0 {
return "", "", "", "", errors.Errorf("invalid arguments %q, %q: you must specify paths", source, destination)
}
Expand Down