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

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

Merged

Conversation

rohanKanojia
Copy link

  • Added Docs for docker:copy
  • Added a copy-from-container sample which contains profiles for
    copying file and directory to localhost from container

@@ -0,0 +1,89 @@
<project xmlns = "http://maven.apache.org/POM/4.0.0"
Copy link
Owner

@mabrarov mabrarov Mar 28, 2021

Choose a reason for hiding this comment

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

Project in "samples" directory use different formatting of this header:

Suggested change
<project xmlns = "http://maven.apache.org/POM/4.0.0"
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>io.fabric8.dmp.samples</groupId>
<artifactId>dmp-sample-parent</artifactId>
<version>0.34-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Copy link
Owner

Choose a reason for hiding this comment

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

It looks like this file doesn't follow formatting of existing POMs from the "samples" directory, where 2 space indentation is used (while this file uses 3 spaces).

<groupId>io.fabric8.dmp.samples</groupId>
<artifactId>dmp-sample-parent</artifactId>
<version>0.34-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
Copy link
Owner

Choose a reason for hiding this comment

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

"../pom.xm" is default value for relativePath ("http://maven.apache.org/POM/4.0.0" XML schema):

      <xs:element name="relativePath" minOccurs="0" type="xs:string" default="../pom.xml">
        <xs:annotation>
          <xs:documentation source="version">4.0.0</xs:documentation>
          <xs:documentation source="description">
            The relative path of the parent &lt;code&gt;pom.xml&lt;/code&gt; file within the check out.
            The default value is &lt;code&gt;../pom.xml&lt;/code&gt;.
            ...
          </xs:documentation>
        </xs:annotation>
      </xs:element>

so I'd prefer to omit it. Unfortunately, it looks like all POMs in "samples" directory have this excessive item, so let's be consistence.

<artifactId>dmp-sample-copy-from-container</artifactId>
<profiles>
<profile>
<id>Copy-File</id>
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like the most of maven profiles of projects located in "sample" directory use lower-case names:

Suggested change
<id>Copy-File</id>
<id>copy-file</id>

<artifactId>docker-maven-plugin</artifactId>
<executions>
<execution>
<id>copy-from-container</id>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<id>copy-from-container</id>
<id>copy-file-from-container</id>

<configuration>
<images>
<image>
<name>busybox:latest</name>
Copy link
Owner

Choose a reason for hiding this comment

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

I believe we can omit default tag:

Suggested change
<name>busybox:latest</name>
<name>busybox</name>

<entries>
<entry>
<containerPath>/etc/hosts</containerPath>
<hostDirectory>/tmp/</hostDirectory>
Copy link
Owner

Choose a reason for hiding this comment

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

We need to cover in documentation that hostDirectory can be relative path. In case hostDirectory is not an absolute path (java.io.File#isAbsolute) or is null ( is not defined), then it's considered relative path and the base directory for that relative path is the current project base directory (org.apache.maven.project.MavenProject#getBasedir). null or undefined hostDirectory is treated like an empty string, i.e. leads to usage of base directory of maven project.

For these samples - to make them less OS specific and to avoid spoiling filesystem of host OS with results of build - I'd recommend to use project's build directory as value of hostDirectory, like:

Suggested change
<hostDirectory>/tmp/</hostDirectory>
<hostDirectory>${project.build.directory}</hostDirectory>

In another example we could use relative directory, like:

Suggested change
<hostDirectory>/tmp/</hostDirectory>
<hostDirectory>target</hostDirectory>

</build>
</profile>
<profile>
<id>Copy-Directory</id>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<id>Copy-Directory</id>
<id>copy-directory</id>

<entries>
<entry>
<containerPath>/dev</containerPath>
<hostDirectory>/tmp/</hostDirectory>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<hostDirectory>/tmp/</hostDirectory>
<hostDirectory>target</hostDirectory>

@mabrarov
Copy link
Owner

mabrarov commented Mar 28, 2021

Thank you for your help.

Here are my ideas / plans:

  1. I definitely need help with samples and documentation, because I'm already late with my promises :) I don't want to be a blocker here.
  2. I like this PR and going to review / test it ASAP.
  3. I believe I will want to extend / modify changes coming with this PR - if this will happen then I plan to merge this PR and implement additional changes on top of it (don't want to lose history or authoring of changes).

## How to Build?
You can compile the project as usual maven command:
```
mvn clean install
Copy link
Owner

Choose a reason for hiding this comment

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

There is no need of using install goal (spoiling local maven repository):

Suggested change
mvn clean install
mvn clean package

<relativePath>../pom.xml</relativePath>
</parent>
<modelVersion>4.0.0</modelVersion>
<artifactId>dmp-sample-copy-from-container</artifactId>
Copy link
Owner

Choose a reason for hiding this comment

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

Let's use pom packaging

Suggested change
<artifactId>dmp-sample-copy-from-container</artifactId>
<artifactId>dmp-sample-copy-from-container</artifactId>
<packaging>pom</packaging>

to avoid warnings like:

[INFO] --- maven-jar-plugin:2.4:jar (default-jar) @ dmp-sample-copy-from-container ---
[WARNING] JAR will be empty - no content was marked for inclusion!

## Copying File from Container to Local Host
We have a `Copy-File` profile which copies `/etc/hosts` file from container to your `/tmp` directory. It has executions set up for `start`,`copy`,`stop` goals so they will be activated when you build using the specified profile. In order to run it, use this command:
```
mvn clean install -PCopy-File
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
mvn clean install -PCopy-File
$ mvn clean package -P copy-file

```
Once command finishes successfully, try checking your `/tmp` directory to see file got created or not. If everything goes okay, you should be able to see output like this:
```
copy-from-container : $ ls /tmp/
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
copy-from-container : $ ls /tmp/
$ ls target
hosts
$ cat target/hosts

```
mvn clean install -PCopy-File
```
Once command finishes successfully, try checking your `/tmp` directory to see file got created or not. If everything goes okay, you should be able to see output like this:
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Once command finishes successfully, try checking your `/tmp` directory to see file got created or not. If everything goes okay, you should be able to see output like this:
Once command finishes successfully, try checking "target" directory to see if the file got created or not. If everything goes okay, you should be able to see output like this:

<id>copy-from-container</id>
<phase>generate-resources</phase>
<goals>
<goal>start</goal>
Copy link
Owner

Choose a reason for hiding this comment

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

This profile could use <createContainers>true</createContainers>, i.e. could demonstrate copying from temporary (created, but not started) container instead of copying from the started container (like it's done in Copy-File profile).

ff02::2 ip6-allrouters
172.17.0.3 80dd81263592
```

Copy link
Owner

Choose a reason for hiding this comment

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

We could add a note about usage of d-m-p external configuration, like:

Docker Maven Plugin external configuration can be used to define entries which need to be coped, e.g.:

$ mvn clean package -Pcopy-file \
    -Ddocker.imagePropertyConfiguration=override \
    -Ddocker.name=alpine \
    -Ddocker.copy.entries.1.containerPath=/etc/os-release \
    -Ddocker.copy.entries.1.hostDirectory=target \
    -Ddocker.copy.entries.2.containerPath=/etc/hostname \
    -Ddocker.copy.entries.2.hostDirectory=target
...
$ ls target
hostname  os-release
$ cat target/hostname
f1387e0c6253
$ cat target/os-release
NAME="Alpine Linux"
ID=alpine
VERSION_ID=3.13.2
PRETTY_NAME="Alpine Linux v3.13"
HOME_URL="https://alpinelinux.org/"
BUG_REPORT_URL="https://bugs.alpinelinux.org/"

@@ -0,0 +1,33 @@
[[docker:copy]]
Copy link
Owner

Choose a reason for hiding this comment

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

Without changes in other files, like "src/main/asciidoc/index.adoc", this file doesn't look being picked and included into the generated documentation.

@rohanKanojia rohanKanojia force-pushed the copy_mojo_docs_and_sample branch 3 times, most recently from 1bc02d8 to 107b715 Compare March 28, 2021 17:24
```

## Copying Directory from Container to Local Host
We have a `copy-directory` profile which copies `/dev` directory from container to your `/tmp` directory. It has `createContainers=true` with which plugin copies from a temporary(created, but not started) container instead of copying from the standard container. In order to run it, use this command:
Copy link
Owner

Choose a reason for hiding this comment

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

/tmp directory is not used by this example anymore, does it?

@mabrarov
Copy link
Owner

Per requirements of d-m-p commits in PRs should be signed-off by the author. Could you please sign-off your commit(s) and update (force push) this PR?

@rohanKanojia
Copy link
Author

sure, let me do that.

Once command finishes successfully, try checking your `target` directory to see file got created or not. If everything goes okay, you should be able to see output like this:
```
copy-from-container : $ ls target
dev hosts
Copy link
Owner

Choose a reason for hiding this comment

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

hosts file looks excessive, because clean goal should remove it (if it is created by previous command - by mvn clean package -Pcopy-file):

Suggested change
dev hosts
dev


This is a plain maven application used to demonstrate how to copy files or directories from container using `docker:copy` goal

## How to Build?
Copy link
Owner

@mabrarov mabrarov Mar 30, 2021

Choose a reason for hiding this comment

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

To be honest I don't see a need in this section, because package does nothing for this sample. I'd omit it to reduce this README till essential part.

@@ -0,0 +1,64 @@
# Docker Maven Plugin Copy Sample

This is a plain maven application used to demonstrate how to copy files or directories from container using `docker:copy` goal
Copy link
Owner

Choose a reason for hiding this comment

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

"maven application" -> "maven project"?

+ 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>

This is a plain maven project used to demonstrate how to copy files or directories from container using `docker:copy` goal

## Copying File from Container to Local Host
Copy link
Owner

Choose a reason for hiding this comment

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

I don't know if d-m-p has any standard for Markdown formatting, but I appreciate if we could use the following rules:

  1. Keep one empty line before bock of code (if it's not the first line of file).
  2. Keep one empty line before (if it's not the first line of file) and after each header.
  3. File ends with one empty line.
Suggested change
## Copying File from Container to Local Host
## Copying File from Container to Local Host
We have a `copy-file` profile which copies `/etc/hosts` file from container to your project build directory.
It has executions set up for `start`, `copy` and `stop` goals, so they will be activated when you build using the specified profile.
In order to run it, use this command:
```
$ mvn clean package -Pcopy-file
```
Once command finishes successfully, try checking your `target` directory to see if the file got created or not.
If everything goes okay, you should be able to see output like this:
```
$ ls target
hosts
$ cat target/hosts
127.0.0.1 localhost
::1 localhost ip6-localhost ip6-loopback
fe00::0 ip6-localnet
ff00::0 ip6-mcastprefix
ff02::1 ip6-allnodes
ff02::2 ip6-allrouters
172.17.0.3 80dd81263592
```
Docker Maven Plugin external configuration can be used to define entries which need to be coped, e.g.:
```
$ mvn clean package -Pcopy-file \
-Ddocker.imagePropertyConfiguration=override \
-Ddocker.name=alpine \
-Ddocker.copy.entries.1.containerPath=/etc/os-release \
-Ddocker.copy.entries.1.hostDirectory=target \
-Ddocker.copy.entries.2.containerPath=/etc/hostname \
-Ddocker.copy.entries.2.hostDirectory=target
...
$ ls target
hostname os-release
$ cat target/hostname
f1387e0c6253
$ cat target/os-release
NAME="Alpine Linux"
ID=alpine
VERSION_ID=3.13.2
PRETTY_NAME="Alpine Linux v3.13"
HOME_URL="https://alpinelinux.org/"
BUG_REPORT_URL="https://bugs.alpinelinux.org/"
```
## Copying Directory from Container to Local Host
We have a `copy-directory` profile which copies `/dev` directory from container to your `target` directory.
It has `createContainers=true` with which plugin copies from a temporary (created, but not started) container instead of copying from the standard container (e.g. created and started by `start` goal).
In order to run it, use this command:
...

This is a plain maven project used to demonstrate how to copy files or directories from container using `docker:copy` goal

## Copying File from Container to Local Host
We have a `copy-file` profile which copies `/etc/hosts` file from container to your project build directory. It has executions set up for `start`,`copy`,`stop` goals so they will be activated when you build using the specified profile. In order to run it, use this command:
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
We have a `copy-file` profile which copies `/etc/hosts` file from container to your project build directory. It has executions set up for `start`,`copy`,`stop` goals so they will be activated when you build using the specified profile. In order to run it, use this command:
We have a `copy-file` profile which copies `/etc/hosts` file from container to your project build directory.
It has executions set up for `start`, `copy` and `stop` goals, so they will be activated when you build using the specified profile.
In order to run it, use this command:

```

## Copying Directory from Container to Local Host
We have a `copy-directory` profile which copies `/dev` directory from container to your `target` directory. It has `createContainers=true` with which plugin copies from a temporary(created, but not started) container instead of copying from the standard container. In order to run it, use this command:
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
We have a `copy-directory` profile which copies `/dev` directory from container to your `target` directory. It has `createContainers=true` with which plugin copies from a temporary(created, but not started) container instead of copying from the standard container. In order to run it, use this command:
We have a `copy-directory` profile which copies `/dev` directory from container to your `target` directory.
It has `createContainers=true` with which plugin copies from a temporary (created, but not started) container instead of copying from the standard container (e.g. created and started by `start` goal).
In order to run it, use this command:

@@ -0,0 +1,87 @@
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
Copy link
Owner

Choose a reason for hiding this comment

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

I believe that we need to add copy-from-container module into "samples/pom.xml" POM (into the modules section), don't we?

@mabrarov mabrarov changed the base branch from copy_mojo to copy_mojo_docs March 30, 2021 15:37
@mabrarov
Copy link
Owner

@rohanKanojia,

I'm going to merge this PR into copy_mojo_docs branch, then extend / adjust documentation and samples there and then merge (fast-forward) copy_mojo_docs branch into copy_mojo branch. Please, let me know if you have concerns and thank you for driving this.

@rohanKanojia
Copy link
Author

oh, Sorry I was busy with work so couldn't update your recent comments. Shall I address your comments or do you plan to fix them yourself? I'm okay with whatever way you prefer.

@mabrarov
Copy link
Owner

I plan to fix them myself.

@rohanKanojia
Copy link
Author

ohk, I guess you can merge it then

@mabrarov mabrarov merged commit 544a7b2 into mabrarov:copy_mojo_docs Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants