Fix #1197: Unable to build dockerFile without pushing it to docker server (DockerInDocker) #1297

rohanKanojia · 2019-11-21T15:05:50Z

with docker.skip.build.image flag we can skip pushing created docker archive to docker daemon.

rohanKanojia · 2019-11-21T15:10:06Z

However, when I try to load image using this tarball. I get some error:

~/work/repos/dmp-demo-project : $ docker load --input ./target/docker/test-db/tmp/docker-build.tar                                                            
open /var/lib/docker/tmp/docker-import-295740741/repositories: no such file or directory

Is this desired behavior?

rhuss · 2019-11-21T15:58:22Z

Is this desired behavior?

Yes, I think so. It's really that this tar is picked up e.g. by another build mechansim like Kaniko to build the real image. However, I think it would be important to implement -Ddocker.saveBuildArchive=target/docker-build.tar as well to copy over the tar to a different location so that it can be picked up from there without the need to dive into the target directory. Should be either an absolute path or, if relative, should be relative to the project directory.

rhuss · 2019-11-21T16:04:08Z

Actually thinking again, I think that a single argument should be sufficient:

buildArchiveOnly (property: docker.buildArchiveOnly) for creating only the archive with Dockerfile which needs to be post-processed to create an image. The value should point to the path where to create the archive. If no value is given, the archive is created below target/docker as usual but no image is created.

wdyt ?

rohanKanojia · 2019-11-21T16:21:35Z

@rhuss : okay, let me do the changes as you asked.

rohanKanojia · 2019-11-21T17:31:59Z

Here is what I interpreted:

Remove docker.skip.build.image
Add a new flag docker.buildArchiveOnly which would contain the path to where tarball should be saved/copied. I have one doubt here; If no value is given, the archive is created below target/docker as usual but no image is created. But I thought no value would be the case when we want to POST the tarball to docker daemon. Am I missing something here?

rhuss · 2019-11-21T18:40:24Z

I hope we can detect if the option is given but without value. I'm not 100% if this is possible. Ideally we should also interprete the special values of true and false.

So my suggestion would be:

-Ddocker.buildArchiveOnly=/path/to/archive will create the build tar with name /path/to/archive and then stop without actually building
-Ddocker.buildArchiveOnly, -Ddocker.buildArchiveOnly=true will just don't build the image (but don't copy over the archive to another place, just leaving it in target.
-Ddocker.buildArchiveOnly=false just build as usual. This is the default but might be useful if you want to override this field in sub-module pom.

@rohanKanojia wdyt ? I think this will nicely cover this feature with a single config option but still staying flexible and intuitive.

rohanKanojia · 2019-11-21T18:42:11Z

cool 👍 , let me implement this in next update

rohanKanojia · 2019-11-21T20:46:20Z

Here is how it looks now:

Using docker.buildArchiveOnly=/new/path
Using docker.buildArchiveOnly=true
Using normal docker:build

marcsaegesser · 2019-12-06T21:44:52Z

I hadn't noticed that the discussion had moved over here. Sorry for delay responding.

I cloned this repo, built this branch and did some testing. So far things look good. I had to add -Ddocker.skip.tag=true when using -Ddocker.buildArchiveOnly or the build failed when trying to tag a non-existent image. Should docker.buildArchiveOnly imply docker.skip.tag?

I've updated one of my CI/CD pipelines to use this version of the plugin and then used Kaniko to create the Docker image in an environment that had no Docker daemon.

rhuss

looks good to me with one minor change request.

rhuss · 2019-12-20T13:56:15Z

src/main/java/io/fabric8/maven/docker/service/BuildService.java

+     * @return tarball for docker image
+     * @throws MojoExecutionException in case any exception comes during building tarball
+     */
+    public File buildArchive(ImageConfiguration imageConfiguration, BuildContext buildContext, String buildArchiveOnlyFlagOrBuildArchiveLocation)


Can we make the last argument just a archivePath and if not-null, store the archive there ? The caller should then prepare that parameter based on the options given. That way this method becomes much more reusable and decoupled from its actual, Maven specific usage.

rhuss · 2019-12-20T13:57:15Z

src/main/java/io/fabric8/maven/docker/service/BuildService.java

+        log.info("%s: Created %s in %s", imageConfiguration.getDescription(), dockerArchive.getName(), EnvUtil.formatDurationTill(time));
+
+        // Copy created tarball to directory if specified
+        if (buildArchiveOnlyFlagOrBuildArchiveLocation != null && !buildArchiveOnlyFlagOrBuildArchiveLocation.isEmpty()) {


Can we extract that to a method ? Also, remember, when its empty its the same as false

rhuss · 2019-12-20T13:58:47Z

src/main/java/io/fabric8/maven/docker/BuildMojo.java

@@ -71,7 +77,10 @@ protected void buildAndTag(ServiceHub hub, ImageConfiguration imageConfig)
        ImagePullManager pullManager = getImagePullManager(determinePullPolicy(imageConfig.getBuildConfiguration()), autoPull);
        BuildService buildService = hub.getBuildService();

-        buildService.buildImage(imageConfig, pullManager, buildContext);
+        File buildArchiveFile = buildService.buildArchive(imageConfig, buildContext, buildArchiveOnly);


as mentioned below, the option should be already resolved here to the path (if any) for calling buildArchive

rhuss

Thanks a lot ! The last thing missing would be an update to the documentation, then we are ready to go.

…docker server (DockerInDocker)

rhuss

Thanks ! Some minor formatting issues, but I will fix that later.

let's merge it.

rohanKanojia force-pushed the pr/issue1197 branch from bb83ad5 to 0f61b9c Compare November 21, 2019 15:10

rohanKanojia force-pushed the pr/issue1197 branch from 18bedb5 to e648646 Compare November 21, 2019 20:43

rohanKanojia force-pushed the pr/issue1197 branch from e648646 to dc1a3ca Compare November 21, 2019 20:51

rohanKanojia requested a review from rhuss December 20, 2019 13:33

rhuss requested changes Dec 20, 2019

View reviewed changes

rohanKanojia force-pushed the pr/issue1197 branch from 079a665 to bdf6483 Compare December 20, 2019 16:57

rhuss requested changes Jan 8, 2020

View reviewed changes

rohanKanojia force-pushed the pr/issue1197 branch from bdf6483 to a423664 Compare January 8, 2020 12:53

Fix fabric8io#1197: Unable to build dockerFile without pushing it to …

a423664

…docker server (DockerInDocker)

rhuss approved these changes Jan 8, 2020

View reviewed changes

rhuss merged commit 64aba10 into fabric8io:master Jan 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #1197: Unable to build dockerFile without pushing it to docker server (DockerInDocker) #1297

Fix #1197: Unable to build dockerFile without pushing it to docker server (DockerInDocker) #1297

rohanKanojia commented Nov 21, 2019

rohanKanojia commented Nov 21, 2019

rhuss commented Nov 21, 2019

rhuss commented Nov 21, 2019 •

edited

Loading

rohanKanojia commented Nov 21, 2019

rohanKanojia commented Nov 21, 2019

rhuss commented Nov 21, 2019 •

edited

Loading

rohanKanojia commented Nov 21, 2019

rohanKanojia commented Nov 21, 2019 •

edited

Loading

marcsaegesser commented Dec 6, 2019

rhuss left a comment

rhuss Dec 20, 2019

rhuss Dec 20, 2019

rhuss Dec 20, 2019

rhuss left a comment

rhuss left a comment

Fix #1197: Unable to build dockerFile without pushing it to docker server (DockerInDocker) #1297

Fix #1197: Unable to build dockerFile without pushing it to docker server (DockerInDocker) #1297

Conversation

rohanKanojia commented Nov 21, 2019

rohanKanojia commented Nov 21, 2019

rhuss commented Nov 21, 2019

rhuss commented Nov 21, 2019 • edited Loading

rohanKanojia commented Nov 21, 2019

rohanKanojia commented Nov 21, 2019

rhuss commented Nov 21, 2019 • edited Loading

rohanKanojia commented Nov 21, 2019

rohanKanojia commented Nov 21, 2019 • edited Loading

marcsaegesser commented Dec 6, 2019

rhuss left a comment

Choose a reason for hiding this comment

rhuss Dec 20, 2019

Choose a reason for hiding this comment

rhuss Dec 20, 2019

Choose a reason for hiding this comment

rhuss Dec 20, 2019

Choose a reason for hiding this comment

rhuss left a comment

Choose a reason for hiding this comment

rhuss left a comment

Choose a reason for hiding this comment

rhuss commented Nov 21, 2019 •

edited

Loading

rhuss commented Nov 21, 2019 •

edited

Loading

rohanKanojia commented Nov 21, 2019 •

edited

Loading