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

Fixes copyArchiveToContainerCmdImpl fail when hostResource has an emtty dir which gets added as last entry to tar #574

Merged
merged 2 commits into from
May 9, 2016

Conversation

denlap007
Copy link
Contributor

@denlap007 denlap007 commented May 8, 2016

This is a simple fix for CopyArchiveToContainerCmdImpl which fails with com.github.dockerjava.api.exception.BadRequestException: Unable to perform tar on host resource /path/to/resource, when the resourceHost is a dir that contains an empty dir which will be added as the last entry to the generated .tar file.

Steps to reproduce

  1. Create a local dir with the following structure:
|--local_dir
   |--a (empty dir)
   |--b (dir with file)
       |--b.txt (a file)

Name the dirs in the local dir exactly as shown above (preserve names a and b, the file name or content is irrelevant).

  1. Now we will use the CopyArchiveToContainerCmd with the docker client t o copy the local dir into a container. Run the program:
package fixForCopyArchiveToContainerCmdImpl;

import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.api.command.CreateContainerResponse;
import com.github.dockerjava.core.DockerClientBuilder;

public class Main {
    public static void main(String[] args) {
        // create docker client
        DockerClient docker = DockerClientBuilder.getInstance("unix:///var/run/docker.sock").build();
        // create a test container
        CreateContainerResponse container = docker.createContainerCmd("progrium/busybox:latest")
                .withCmd("/bin/sh", "-c", "while true; do sleep 9999; done")
                .exec();
        // start the container
        docker.startContainerCmd(container.getId()).exec();
        // copy data from local dir to container /home path
        String localDir = "/home/data";
        docker.copyArchiveToContainerCmd(container.getId())
                .withHostResource(localDir)
                .withRemotePath("/home")
                .exec();
    }
}

Then, the output will be: com.github.dockerjava.api.exception.BadRequestException: Unable to perform tar on host resource /path/to/resource

In oder to generate the tar file, all files under the local dir must be added to the tarArchiveOutputStream as tarArchiveEntries. The exception happens when the last entry added to the tarArchiveOutputStream is an empty dir.

Notice
In order to view the order with which the files and folders are added to the tarArchiveOutputStream you may add print messages to: com.github.dockerjava.core.util.TarDirWalker class. Specifically, to methods: preVisitDirectory(), visitFile(), e.g.

@Override
    public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)  {
        System.out.println("@preVisitDirectory, path: " + dir.toString());
...
}
 @Override
    public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
        System.out.println("@visitFile, path: " + file.toString());
...
}

If in any case, the empty folder is not the last entry added to tarArchiveOutputStream the exception won't be reproduced.


This change is Reviewable

…ty dir which gets added as last entry to tar

CopyArchiveToContainerCmdImpl FAILS with com.github.dockerjava.api.exception.BadRequestException: Unable to perform tar on host resource /path/to/resource, if the resourceHost is a dir that containes an empty dir which will be added as the last entry to the generated .tar file.
@marcuslinke
Copy link
Contributor

@denlap007 Thanks for finding this. It would be nice to have a test case which reproduces exactly the scenario you described above. Would it be possible to add an appropriate test to CopyArchiveToContainerCmdImplTest please?

@denlap007
Copy link
Contributor Author

denlap007 commented May 9, 2016

Hi, I created a Test Case which reproduces the scenario described above. Shall I post it here or make a new pull request?

@marcuslinke
Copy link
Contributor

Its OK to add a new commit to this PR. Thanks.

@codecov-io
Copy link

Current coverage is 23.80%

Merging #574 into master will decrease coverage by -<.01%

  1. File ...rResultCallback.java (not in diff) was modified. more
    • Misses +1
    • Partials 0
    • Hits 0
@@             master       #574   diff @@
==========================================
  Files           296        296          
  Lines          6129       6131     +2   
  Methods           0          0          
  Messages          0          0          
  Branches        532        533     +1   
==========================================
  Hits           1459       1459          
- Misses         4585       4587     +2   
  Partials         85         85          

Powered by Codecov. Last updated by 1ab6e63...d2b2a2d

@marcuslinke marcuslinke added this to the 3.0.0-RC6 milestone May 9, 2016
@marcuslinke marcuslinke merged commit d2b2a2d into docker-java:master May 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants