Copy goal for copying files from container to host #1405

mabrarov · 2020-11-13T14:14:32Z

Copy goal for copying files & directories from container to host. Refer to issue #752.

Things which are missing due to I have few time to work on them:

Unit tests for CopyMojo class.
Example different than implementation of docker builder pattern and / or example copying directory (i.e. not just a single file).
Documentation for the copy goal.

I appreciate any help with making this pull request complete.

Fixes #752

codecov · 2020-11-13T14:18:46Z

Codecov Report

Merging #1405 (90ab423) into master (5b91720) will increase coverage by 0.88%.
The diff coverage is 81.56%.

@@             Coverage Diff              @@
##             master    #1405      +/-   ##
============================================
+ Coverage     59.25%   60.14%   +0.88%     
- Complexity     2004     2068      +64     
============================================
  Files           163      165       +2     
  Lines          9047     9358     +311     
  Branches       1366     1414      +48     
============================================
+ Hits           5361     5628     +267     
- Misses         3198     3236      +38     
- Partials        488      494       +6

Impacted Files	Coverage Δ	Complexity Δ
...rc/main/java/io/fabric8/maven/docker/LogsMojo.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...c/main/java/io/fabric8/maven/docker/StartMojo.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...8/maven/docker/assembly/DockerAssemblyManager.java	`16.41% <0.00%> (-0.71%)`	`11.00 <0.00> (ø)`
...o/fabric8/maven/docker/service/ArchiveService.java	`23.52% <0.00%> (-3.14%)`	`1.00 <0.00> (ø)`
.../io/fabric8/maven/docker/service/QueryService.java	`21.21% <0.00%> (-0.67%)`	`4.00 <0.00> (ø)`
.../io/fabric8/maven/docker/service/WatchService.java	`3.79% <0.00%> (ø)`	`1.00 <0.00> (ø)`
...ven/docker/access/hc/DockerAccessWithHcClient.java	`24.25% <3.33%> (-0.97%)`	`26.00 <0.00> (ø)`
.../docker/config/handler/property/ValueProvider.java	`83.17% <44.44%> (-3.56%)`	`12.00 <1.00> (+1.00)`	⬇️
...rc/main/java/io/fabric8/maven/docker/SaveMojo.java	`83.33% <50.00%> (-3.51%)`	`37.00 <1.00> (-1.00)`
...rc/main/java/io/fabric8/maven/docker/StopMojo.java	`50.52% <75.00%> (-9.96%)`	`21.00 <5.00> (-12.00)`
... and 15 more

sonarcloud · 2020-12-17T05:05:44Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

84.7% Coverage
0.0% Duplication

mabrarov · 2020-12-17T05:08:44Z

I squashed commits related to the code changes for copy goal. Documentation and additional examples are the only things which are still pending. Though... I feel that unit tests for CopyMojo class could be enhanced too.

rohanKanojia · 2021-03-07T05:42:07Z

@mabrarov : polite ping, Could you please rebase your branch against master? Is this PR ready? Do you have some time to add examples / docs ?

mabrarov · 2021-03-07T11:39:03Z

@rohanKanojia,

Could you please rebase your branch against master?

Done.

Is this PR ready?

Documentation is missing. It is in progress (was on hold due to the lack of time to work on it). Give me a week to complete this part of the work.

Do you have some time to add examples

This pull request includes it/builder test which itself is a good example. I'm looking to add an example which is different than docker builder pattern, e.g. running tests in container and copying results of tests from container to host (refer to #752). Unfortunately, I cannot promise that I'll be able to add this part of the work soon, i.e. help is appreciated.

rhuss · 2021-03-07T18:21:15Z

Thanks for the PR ! Unfortunately I don't have much time anymore to help except some sporadic reviews (on a month-based scale), but happy to help to integrate it as I think this covers a potential useful use case.

src/main/java/io/fabric8/maven/docker/config/CopyConfiguration.java

rohanKanojia · 2021-03-20T10:19:15Z

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

+        for (ImageConfiguration imageConfiguration : imageConfigurations) {
+            CopyConfiguration copyConfiguration = imageConfiguration.getCopyConfiguration();
+            if (isEmpty(copyConfiguration)) {
+                continue;


I think we should log here too so that user can find out that he/she is not providing CopyConfiguration

Added debug log in 1b775a7. It's assumed that it's OK if copy configuration is missing, because user may have multiple images configured but wants to copy just from one of them. Please, let me know if you disagree and want to have info or warn log for this case.

Thanks a lot. Looks good. I noticed this when I was trying out your PR and didn't configure any copyConfiguration. I was assuming that you implemented it like this:

mvn docker:copy -Ddocker.copy.containerPath=/src -Ddocker.copy.hostDir=/target

But I noticed later that we need to specify copy related configuration in <image>

This is still possible with external configuration (check "Activating property configuration externally" section specifically). Here is example for the integration test from this pull request:

mvn -q clean install -DskipTests \ && mvn -q -f it/builder/builder-image/pom.xml package \ && mvn -f it/builder/builder-image/pom.xml docker:copy \ -Ddocker.imagePropertyConfiguration=override \ -Ddocker.createContainers \ -Ddocker.copy.entries.1.containerPath=//dmp-it-builder-app.tar.gz \ -Ddocker.copy.entries.1.hostDirectory=target \ && tar -tvf it/builder/builder-image/target/dmp-it-builder-app.tar.gz

Expected output looks like:

[INFO] Scanning for projects... [INFO] [INFO] ---------< io.fabric8.dmp.itests:dmp-it-builder-builder-image >--------- [INFO] Building dmp-it-builder-builder-image 0.34-SNAPSHOT [INFO] --------------------------------[ pom ]--------------------------------- [INFO] [INFO] --- docker-maven-plugin:0.34-SNAPSHOT:copy (default-cli) @ dmp-it-builder-builder-image --- [INFO] DOCKER> Global docker.imagePropertyConfiguration=override property activates property configuration for image [INFO] DOCKER> Copying /dmp-it-builder-app.tar.gz from 050943faf3b8 container into D:\Users\Marat\Documents\work\java\docker-maven-plugin\it\builder\builder-image\target host directory [INFO] Expanding: C:\Users\Marat\AppData\Local\Temp\docker-copy-7878239238909315602.tar into D:\Users\Marat\Documents\work\java\docker-maven-plugin\it\builder\builder-image\target [INFO] ------------------------------------------------------------------------ [INFO] BUILD SUCCESS [INFO] ------------------------------------------------------------------------ [INFO] Total time: 2.996 s [INFO] Finished at: 2021-03-23T20:42:59+03:00 [INFO] ------------------------------------------------------------------------ -rwxr-xr-x root/root 88976 1970-01-01 03:00 hello

This command:

mvn -f it/builder/builder-image/pom.xml docker:copy \ -Ddocker.imagePropertyConfiguration=override \ -Ddocker.createContainers \ -Ddocker.copy.entries.1.containerPath=//dmp-it-builder-app.tar.gz \ -Ddocker.copy.entries.1.hostDirectory=target

can be simplified till:

mvn -f it/builder/builder-image/pom.xml docker:copy \ -Ddocker.imagePropertyConfiguration=override \ -Ddocker.createContainers \ -Ddocker.copy.entries..containerPath=//dmp-it-builder-app.tar.gz \ -Ddocker.copy.entries..hostDirectory=target

Note that I use //dmp-it-builder-app.tar.gz instead of /dmp-it-builder-app.tar.gz, because I use Git Bash on Windows. On Linux you can use /dmp-it-builder-app.tar.gz.

Building of docker image and copying from the built image (from a temporary container created from the built image) can be combined into a single maven run:

mvn -f it/builder/builder-image/pom.xml clean package docker:copy \ -Ddocker.imagePropertyConfiguration=override \ -Ddocker.createContainers \ -Ddocker.copy.entries.1.containerPath=//dmp-it-builder-app.tar.gz \ -Ddocker.copy.entries.1.hostDirectory=target

You can copy from arbitrary image as well:

$ mvn -f it/builder/builder-image/pom.xml docker:copy \ -Ddocker.imagePropertyConfiguration=only \ -Ddocker.name=alpine \ -Ddocker.createContainers \ -Ddocker.copy.entries.1.containerPath=//etc/os-release \ -Ddocker.copy.entries.1.hostDirectory=target \ && cat it/builder/builder-image/target/os-release ... NAME="Alpine Linux" ID=alpine VERSION_ID=3.12.1 PRETTY_NAME="Alpine Linux v3.12" HOME_URL="https://alpinelinux.org/" BUG_REPORT_URL="https://bugs.alpinelinux.org/"

Nice, Thanks a lot for your comment. I think it would be nice if we could add all these configuration modes as documentation about docker:copy goal in https://github.com/fabric8io/docker-maven-plugin/tree/master/src/main/asciidoc/inc

Please note that you still need a POM with configuration of d-m-p having at least one image, because d-m-p external configuration feature can only extend / override that image configuration, but cannot create a new image configuration.
For example, this command doesn't work (doesn't do anything):

$ mvn -f it/builder/pom.xml docker:copy \ -Ddocker.imagePropertyConfiguration=only \ -Ddocker.name=alpine \ -Ddocker.createContainers \ -Ddocker.copy.entries.1.containerPath=//etc/os-release \ -Ddocker.copy.entries.1.hostDirectory=target

because the referenced POM - it/builder/pom.xml - has no image configuration for d-m-p.

src/main/java/io/fabric8/maven/docker/config/CopyConfiguration.java

it/builder/builder-image/.gitignore

it/builder/builder-image/CMakeLists.txt

rohanKanojia · 2021-03-20T10:48:52Z

Hi, I was trying to test copy goal. But somehow I'm not able to get it working. I'm trying with a configuration like this(I copied it from your integration test):

                        <plugin>
                          <groupId>io.fabric8</groupId>
                          <artifactId>docker-maven-plugin</artifactId>
                          <version>0.34-SNAPSHOT</version>
                          <executions>
                            <execution>
                              <id>copy-from-builder</id>
                              <phase>generate-resources</phase>
                              <goals>
                                <goal>start</goal>
                                <goal>copy</goal>
                                <goal>stop</goal>
                              </goals>
                              <configuration>
                                <images>
                                  <image>
                                    <name>busybox:latest</name>
                                    <copy>
                                      <entries>
                                        <entry>
                                          <containerPath>/etc/hostname</containerPath>
                                          <hostDirectory>/home/rohaan/Downloads/</hostDirectory>
                                        </entry>
                                      </entries>
                                    </copy>
                                  </image>
                                </images>
                                <createContainers>true</createContainers>
                              </configuration>
                            </execution>
                          </executions>  
                        </plugin>

I'm trying to copy /etc/hostname file from container to my Downloads folder:

meetup-app : $ docker run -it busybox:latest  /bin/sh
/ # cat /etc/hostname 
53369ce74e83

When I run mvn clean install I can see copy goal being executed. I can also see the file getting created, but the file is empty:

meetup-app : $ ls ~/Downloads/hostname 
/home/rohaan/Downloads/hostname
meetup-app : $ cat ~/Downloads/hostname

Could you please review and check what I'm doing wrong?

mabrarov · 2021-03-21T13:09:37Z

@rohanKanojia,

Regarding your test - there is a chance that /etc/hostname file is empty at the time copy goal is executed, because Docker may modify /etc/hostname file after container start. Could you please provide the full maven project to reproduce the issue you faced? Is it possible to implement some pause after start of container and before invocation of copy goal (like wait option with some timeout)?

<createContainers>true</createContainers>

is definitely wrong (should be removed) because it leads to copy goal creating container before copying from it, instead of using container created by start goal.

rohanKanojia · 2021-03-23T06:54:52Z

ohk, It seems to be working after removing <createContainers>true</createContainers> from plugin configuration. Thanks a lot

rohanKanojia · 2021-03-25T17:56:17Z

@mabrarov : polite ping, Do you have some time to add docs/sample? If not, I can try submitting a PR to your fork.

rohanKanojia · 2021-03-27T15:14:33Z

@mabrarov : I've created a PR for docs+sample: mabrarov#1 . Would appreciate if you could review it. I think we can then proceed with merge.

mabrarov · 2021-03-31T01:04:09Z

Thanks to @rohanKanojia I was able to complete this PR - documentation and examples are in place now and I consider this PR ready for review and merging. Please note that when I was writing documentation, then I fixed one error and implemented pulling of images when createContainers=true and imagePullPolicy allows (and image is missing) or requires pulling image.

rohanKanojia · 2021-03-31T06:29:19Z

@mabrarov: Thanks a lot for your work! Could you please add a line to doc/changelog.md regarding this change? I think we can proceed with merge after that

I'm planning to cut 0.35.0 this week and it would be awesome if this change could make into it.

mabrarov · 2021-03-31T12:45:45Z

Added new copy goal into "doc/changelog.md". Waiting for 0.35.0 release.

mabrarov · 2021-03-31T14:19:38Z

@rohanKanojia,

I'm new to GitHub Actions and CircleCI, but it feels like we need to add Windows and Linux tests for (builds with) Maven Wrapper, where mvnw.cmd (Windows) and ./mvnw (Linux) are used instead of mvn, because I found that Maven Wrapper files were broken in this repository.

rohanKanojia · 2021-03-31T14:39:38Z

@mabrarov : Hello, sorry for naive question. What's the difference in running maven goals using mvn as compared to these wrappers?

mabrarov · 2021-03-31T14:47:38Z

@rohanKanojia,

There is no difference b/w Maven and Maven Wrapper in terms of execution of goals.

My idea is like this: if we have Maven Wrapper (and we already have it, even without this PR), then it should work. Unfortunately, Maven Wrapper files were broken, which means that either nobody uses Maven Wrapper in this repository or nobody tests Maven Wrapper. Thus we need to either remove (probably unused) Maven Wrapper (which I personally like, because it helps with quick build where you don't need to install anything except JDK) or implement tests for Maven Wrapper. The simplest test for Maven Wrapper is building the project with it :)

rohanKanojia · 2021-03-31T14:51:28Z

@mabrarov : I see. I don't have any problem in adding additional Github/CircleCI workflows for fixed maven wrapper. You can go ahead and create a PR for this if you have time. If not, you can create an issue and I'll try to pick it up in the coming holidays.

mabrarov · 2021-04-01T12:53:20Z

@rohanKanojia,

Here is the new feature request about Maven Wrapper: #1450

rohanKanojia · 2021-04-01T15:08:03Z

@mabrarov : I've created #1451 for adding github action workflows for maven wrappers. Would appreciate if you could review it..

rohanKanojia · 2021-04-01T15:09:58Z

@mabrarov : Thanks a lot for your PR! Could you please resolve this minor conflict? I think we can proceed with merge then

Signed-off-by: Marat Abrarov <abrarov@gmail.com>

+ Added Docs for `docker:copy` + Added a `copy-from-container` sample which contains profiles for copying file and directory to localhost from container Signed-off-by: Rohan Kumar <rohaan@redhat.com>

…l documentation Signed-off-by: Marat Abrarov <abrarov@gmail.com>

Signed-off-by: Marat Abrarov <abrarov@gmail.com>

rohanKanojia requested a review from rhuss November 13, 2020 14:22

mabrarov mentioned this pull request Nov 13, 2020

Need capability to copy files from running container to host #752

Closed

mabrarov force-pushed the copy_mojo branch from d5b1bb9 to 30ba0f9 Compare December 3, 2020 07:43

mabrarov force-pushed the copy_mojo branch 2 times, most recently from 5b6329e to f6fbddd Compare December 17, 2020 05:03

mabrarov force-pushed the copy_mojo branch from f6fbddd to 96eace7 Compare March 7, 2021 11:36

mabrarov force-pushed the copy_mojo branch from 96eace7 to c5ff939 Compare March 7, 2021 12:12

rohanKanojia added this to the 0.35.0 milestone Mar 7, 2021

rhuss added the WIP Work in Progress label Mar 7, 2021

rhuss added documentation missing update changelog labels Mar 7, 2021

mabrarov force-pushed the copy_mojo branch from c5ff939 to 8e2cc92 Compare March 7, 2021 23:16

rohanKanojia self-requested a review March 12, 2021 17:44

rohanKanojia reviewed Mar 20, 2021

View reviewed changes

mabrarov force-pushed the copy_mojo branch from 8e2cc92 to 1e9b689 Compare March 23, 2021 11:44

mabrarov force-pushed the copy_mojo branch 4 times, most recently from 894ac7e to 2cae86d Compare March 31, 2021 00:14

mabrarov force-pushed the copy_mojo branch from 2cae86d to 8606a72 Compare March 31, 2021 00:49

rohanKanojia added update changelog and removed WIP Work in Progress documentation missing update changelog labels Mar 31, 2021

mabrarov force-pushed the copy_mojo branch from 436b4b1 to c25e4cc Compare March 31, 2021 12:44

rohanKanojia removed the update changelog label Mar 31, 2021

mabrarov mentioned this pull request Apr 1, 2021

CI builds with Maven Wrapper to ensure that Maven Wrapper files and configuration are correct #1450

Closed

rohanKanojia approved these changes Apr 1, 2021

View reviewed changes

mabrarov and others added 4 commits April 2, 2021 01:04

Support of building on Windows OS

5af48e7

Signed-off-by: Marat Abrarov <abrarov@gmail.com>

copy goal for copying files / directories from container to host.

f8e8852

Signed-off-by: Marat Abrarov <abrarov@gmail.com>

Add Docs for Copy Goal and Sample for File/Directory Copy

c723560

+ Added Docs for `docker:copy` + Added a `copy-from-container` sample which contains profiles for copying file and directory to localhost from container Signed-off-by: Rohan Kumar <rohaan@redhat.com>

Formatting changes of example for copy goal and extension of copy goa…

c672044

…l documentation Signed-off-by: Marat Abrarov <abrarov@gmail.com>

mabrarov force-pushed the copy_mojo branch 3 times, most recently from 1730cf5 to 78fe4c9 Compare April 1, 2021 22:33

Fixed Maven Wrapper (https://github.com/takari/maven-wrapper).

90ab423

Signed-off-by: Marat Abrarov <abrarov@gmail.com>

mabrarov force-pushed the copy_mojo branch from 78fe4c9 to 90ab423 Compare April 1, 2021 22:35

rohanKanojia merged commit b141bce into fabric8io:master Apr 2, 2021

rohanKanojia mentioned this pull request Apr 2, 2021

Fix #1450: CI builds with Maven Wrapper to ensure that Maven Wrapper files and configuration are correct #1451

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Copy goal for copying files from container to host #1405

Copy goal for copying files from container to host #1405

mabrarov commented Nov 13, 2020 •

edited by rohanKanojia

codecov bot commented Nov 13, 2020 •

edited

sonarcloud bot commented Dec 17, 2020

mabrarov commented Dec 17, 2020 •

edited

rohanKanojia commented Mar 7, 2021

mabrarov commented Mar 7, 2021

rhuss commented Mar 7, 2021

rohanKanojia Mar 20, 2021

mabrarov Mar 23, 2021

rohanKanojia Mar 23, 2021 •

edited

mabrarov Mar 23, 2021 •

edited

rohanKanojia Mar 23, 2021

mabrarov Mar 27, 2021 •

edited

rohanKanojia commented Mar 20, 2021

mabrarov commented Mar 21, 2021 •

edited

rohanKanojia commented Mar 23, 2021

rohanKanojia commented Mar 25, 2021

rohanKanojia commented Mar 27, 2021

mabrarov commented Mar 31, 2021 •

edited

rohanKanojia commented Mar 31, 2021

mabrarov commented Mar 31, 2021

mabrarov commented Mar 31, 2021 •

edited

rohanKanojia commented Mar 31, 2021

mabrarov commented Mar 31, 2021

rohanKanojia commented Mar 31, 2021

mabrarov commented Apr 1, 2021

rohanKanojia commented Apr 1, 2021

rohanKanojia commented Apr 1, 2021

Copy goal for copying files from container to host #1405

Copy goal for copying files from container to host #1405

Conversation

mabrarov commented Nov 13, 2020 • edited by rohanKanojia

codecov bot commented Nov 13, 2020 • edited

Codecov Report

sonarcloud bot commented Dec 17, 2020

mabrarov commented Dec 17, 2020 • edited

rohanKanojia commented Mar 7, 2021

mabrarov commented Mar 7, 2021

rhuss commented Mar 7, 2021

rohanKanojia Mar 20, 2021

Choose a reason for hiding this comment

mabrarov Mar 23, 2021

Choose a reason for hiding this comment

rohanKanojia Mar 23, 2021 • edited

Choose a reason for hiding this comment

mabrarov Mar 23, 2021 • edited

Choose a reason for hiding this comment

rohanKanojia Mar 23, 2021

Choose a reason for hiding this comment

mabrarov Mar 27, 2021 • edited

Choose a reason for hiding this comment

rohanKanojia commented Mar 20, 2021

mabrarov commented Mar 21, 2021 • edited

rohanKanojia commented Mar 23, 2021

rohanKanojia commented Mar 25, 2021

rohanKanojia commented Mar 27, 2021

mabrarov commented Mar 31, 2021 • edited

rohanKanojia commented Mar 31, 2021

mabrarov commented Mar 31, 2021

mabrarov commented Mar 31, 2021 • edited

rohanKanojia commented Mar 31, 2021

mabrarov commented Mar 31, 2021

rohanKanojia commented Mar 31, 2021

mabrarov commented Apr 1, 2021

rohanKanojia commented Apr 1, 2021

rohanKanojia commented Apr 1, 2021

mabrarov commented Nov 13, 2020 •

edited by rohanKanojia

codecov bot commented Nov 13, 2020 •

edited

mabrarov commented Dec 17, 2020 •

edited

rohanKanojia Mar 23, 2021 •

edited

mabrarov Mar 23, 2021 •

edited

mabrarov Mar 27, 2021 •

edited

mabrarov commented Mar 21, 2021 •

edited

mabrarov commented Mar 31, 2021 •

edited

mabrarov commented Mar 31, 2021 •

edited