Implementation of POST /images/load endpoint #627

leonsabr · 2016-07-06T11:31:40Z

This is an implementation of POST /images/load endpoint.

A few words about testing. I added a small tarball (20 kB) with a single image named "docker-java/load:1.0". Tests try to load this tarball into docker, check if expected image appears, and remove it in @AfterMethod. Test for netty implementation is the same as for jaxrs/jersey implementation.

This change is

KostyaSha · 2016-07-06T11:33:31Z

About tar, maybe create it with text instructions in container and then copy from container (command already exists)?

KostyaSha · 2016-07-06T11:34:55Z

src/test/java/com/github/dockerjava/core/command/LoadImageCmdImplTest.java

+
+        final Image image = findImageWithId(expectedImageId, dockerClient.listImagesCmd().exec());
+
+        assertNotNull(image, "Can't find expected image after loading from a tar archive");


please use hamcrest matchers

KostyaSha · 2016-07-06T11:36:23Z

in general lgtm beside comments

KostyaSha · 2016-07-06T11:47:35Z

Or even you can generate tar with apache.io tar, should be not so much lines of code!

leonsabr · 2016-07-06T11:48:24Z

About tar, maybe create it with text instructions in container and then copy from container (command already exists)?

Please, give me more details. This image tarball should have a strict format.

KostyaSha · 2016-07-06T11:53:45Z

Sorry, right name of library https://commons.apache.org/proper/commons-compress/tar.html that would work for creating tar.

Not sure about The layer.tar file contains aufs style .wh..wh.aufs files and directories for storing attribute changes and deletions. How you created it now?

KostyaSha · 2016-07-06T11:56:58Z

And one more idea, could you check how docker tests this case? https://github.com/docker/docker/search?utf8=%E2%9C%93&q=layer.tar
I usually picking their tests into docker-java.
PS. if there is no way to create tar programmatically than it would be ok to keep it as binary in repo.

lanwen · 2016-07-06T12:16:46Z

src/test/java/com/github/dockerjava/netty/exec/LoadImageCmdExecTest.java

+
+    @Test
+    public void loadImageFromTar() throws Exception {
+        final Path imageTarFile = Paths.get("src/test/resources/testLoadImageFromTar/image.tar");


please use CurrentThread classloader to get input stream - as of on different os this path may be changed

That load from relative path, it related to maven plugin that executes it from root of project. So classloaders unrelated. Ideally you should put test resources under class_name/test_name and then load from classloader this test resource.

Please give me an example of under class_name/test_name rule? It looks like directories in src/test/resources/ violates this rule. Should I use src/test/resources/LoadImageCmdImplTest/loadImageFromTar/image.tar? What if one resource is used by several classes?

Then you can make public getter but place it in one place. The issue with placing just in resources that it will be like a trash can and after test deletions/refactorings they will be forgotten.
@leonsabr will search for examples bit later.

leonsabr · 2016-07-06T12:57:09Z

In order to get this tarball I manually did the following:

Created a simple Dockerfile which produces a 2-layer image.
docker build -t docker-java/load:1.0 .
docker save docker-java/load:1.0 > image.tar

As I see, Docker guys have tarballs in repository:

$ find docker -type f -name "*.tar"
docker/integration-cli/fixtures/load/emptyLayer.tar
docker/pkg/archive/testdata/broken.tar
docker/pkg/tarsum/testdata/46af0962ab5afeb5ce6740d4d91652e69206fc991fd5328c1a94d364ad00e457/layer.tar
docker/pkg/tarsum/testdata/511136ea3c5a64f264b78b5433614aec563103b4d4702f3ba7d4d2698e22c158/layer.tar
docker/pkg/tarsum/testdata/collision/collision-0.tar
docker/pkg/tarsum/testdata/collision/collision-1.tar
docker/pkg/tarsum/testdata/collision/collision-2.tar
docker/pkg/tarsum/testdata/collision/collision-3.tar
docker/pkg/tarsum/testdata/xattr/layer.tar

As I understand you suggest to have text files in resources, then in runtime create a tarball according to format? It is tricky because tarball contains layer tarballs inside. So you need to create correct layer tarballs, then put them in the image tarball.

By the way, I can do a smaller tarball, with one layer, a few kB.

KostyaSha · 2016-07-06T13:06:05Z

Or we can pick some their tarball..

KostyaSha · 2016-07-06T13:07:09Z

So ok to have tarball in repo. But please place under corresponding class name

leonsabr · 2016-07-06T15:45:29Z

What was changed in second commit:

Hamcrest matchers
Smaller tarball 12 kB
TestResources class which provides access to the tarball's Path. Tarball is placed in resources/api/images/load/image.tar directory. What do you think about such approach?

I will squash commits for cleaner history later.

KostyaSha · 2016-07-12T08:31:38Z

src/main/java/com/github/dockerjava/api/DockerClient.java

@@ -90,6 +91,8 @@

    CreateImageCmd createImageCmd(@Nonnull String repository, @Nonnull InputStream imageStream);

+    LoadImageCmd loadImageCmd(@Nonnull InputStream imageStream);


When this feature appeared in docker API? Could you place javadoc with since (example could be found through search) ?

KostyaSha · 2016-07-12T08:34:45Z

LGTM, javadoc with API version would be welcome.

leonsabr · 2016-07-14T17:32:45Z

It was tricky but I found the first reference of this endpoint in docs/reference/api/docker_remote_api_v1.7.md. Now this docs are removed from the Docker website.

I squashed commits, so now all changes are in a single commit.

KostyaSha · 2016-07-14T17:37:31Z

@leonsabr thanks! it was enough to check latest 3-4 api versions and mark with latest known. I don't think that anybody using 1.8 :)

KostyaSha · 2016-08-14T22:57:45Z

@leonsabr 1.12 build fails with

Tests run: 323, Failures: 1, Errors: 0, Skipped: 2, Time elapsed: 556.507 sec <<< FAILURE! - in TestSuite
(com.github.dockerjava.netty.exec.LoadImageCmdExecTest)  Time elapsed: 0.152 sec  <<< FAILURE!
io.netty.handler.codec.CorruptedFrameException: invalid JSON received at byte position 0: 4c6f6164656420696d6167653a20646f636b65722d6a6176612f6c6f61643a312e300a
Results :
Failed tests: 
  LoadImageCmdExecTest.loadImageFromTar » CorruptedFrame invalid JSON received a...

Plus i found that it has verbose output #664

KostyaSha · 2016-08-14T23:02:55Z

src/main/java/com/github/dockerjava/core/RemoteApiVersion.java

@@ -23,6 +23,11 @@
    private static final Pattern VERSION_REGEX = Pattern.compile("v?(\\d+)\\.(\\d+)");

    /**
+     * Online documentation is not available anymore.
+     */
+    public static final RemoteApiVersion VERSION_1_7 = RemoteApiVersion.create(1, 7);


sinde docker 1.7 or 1.7 API?

KostyaSha reviewed Jul 6, 2016
View reviewed changes

lanwen reviewed Jul 6, 2016
View reviewed changes

KostyaSha reviewed Jul 12, 2016
View reviewed changes

Implement POST /images/load endpoint

a58de04

leonsabr force-pushed the images_load branch from bd4a71d to a58de04 Compare July 14, 2016 17:27

KostyaSha merged commit d803c46 into docker-java:master Jul 14, 2016

KostyaSha added this to the 3.0.1 milestone Jul 14, 2016

leonsabr deleted the images_load branch July 15, 2016 09:50

KostyaSha added the API/1.17 label Aug 14, 2016

KostyaSha reviewed Aug 14, 2016
View reviewed changes

KostyaSha added API/<1.17 and removed API/1.17 labels Aug 14, 2016

panuse pushed a commit to TuKangTech/docker-java that referenced this pull request Aug 20, 2017

Implement POST /images/load endpoint (docker-java#627)

51a4a76

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of POST /images/load endpoint #627

Implementation of POST /images/load endpoint #627

leonsabr commented Jul 6, 2016 •

edited by marcuslinke

Loading

KostyaSha commented Jul 6, 2016

KostyaSha Jul 6, 2016

KostyaSha commented Jul 6, 2016

KostyaSha commented Jul 6, 2016 •

edited

Loading

leonsabr commented Jul 6, 2016

KostyaSha commented Jul 6, 2016

KostyaSha commented Jul 6, 2016

lanwen Jul 6, 2016

KostyaSha Jul 6, 2016

leonsabr Jul 6, 2016

KostyaSha Jul 6, 2016

leonsabr commented Jul 6, 2016

KostyaSha commented Jul 6, 2016

KostyaSha commented Jul 6, 2016

leonsabr commented Jul 6, 2016

KostyaSha Jul 12, 2016

KostyaSha commented Jul 12, 2016

leonsabr commented Jul 14, 2016

KostyaSha commented Jul 14, 2016

KostyaSha commented Aug 14, 2016 •

edited

Loading

KostyaSha Aug 14, 2016

KostyaSha Aug 14, 2016


		final Image image = findImageWithId(expectedImageId, dockerClient.listImagesCmd().exec());

		assertNotNull(image, "Can't find expected image after loading from a tar archive");

		@@ -90,6 +91,8 @@

		CreateImageCmd createImageCmd(@Nonnull String repository, @Nonnull InputStream imageStream);

		LoadImageCmd loadImageCmd(@Nonnull InputStream imageStream);

Implementation of POST /images/load endpoint #627

Implementation of POST /images/load endpoint #627

Conversation

leonsabr commented Jul 6, 2016 • edited by marcuslinke Loading

KostyaSha commented Jul 6, 2016

Choose a reason for hiding this comment

KostyaSha commented Jul 6, 2016

KostyaSha commented Jul 6, 2016 • edited Loading

leonsabr commented Jul 6, 2016

KostyaSha commented Jul 6, 2016

KostyaSha commented Jul 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leonsabr commented Jul 6, 2016

KostyaSha commented Jul 6, 2016

KostyaSha commented Jul 6, 2016

leonsabr commented Jul 6, 2016

Choose a reason for hiding this comment

KostyaSha commented Jul 12, 2016

leonsabr commented Jul 14, 2016

KostyaSha commented Jul 14, 2016

KostyaSha commented Aug 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leonsabr commented Jul 6, 2016 •

edited by marcuslinke

Loading

KostyaSha commented Jul 6, 2016 •

edited

Loading

KostyaSha commented Aug 14, 2016 •

edited

Loading