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

targetDir in assembly not working #634

Closed
mbacodes opened this issue Dec 6, 2016 · 12 comments
Closed

targetDir in assembly not working #634

mbacodes opened this issue Dec 6, 2016 · 12 comments
Assignees

Comments

@mbacodes
Copy link

mbacodes commented Dec 6, 2016

Description

When trying to set a custom targetDir in the assembly section like described int the docu, it's getting ignored and /maven is used.

Info

  • d-m-p version : 0.18.1
  • Maven version (mvn -v) :
Apache Maven 3.3.9 (bb52d8502b132ec0a5a3f4c09453c07478323dc5; 2015-11-10T17:41:47+01:00)
Maven home: /usr/local/Cellar/maven/3.3.9/libexec
Java version: 1.8.0_65, vendor: Oracle Corporation
Java home: /Library/Java/JavaVirtualMachines/jdk1.8.0_65.jdk/Contents/Home/jre
Default locale: de_DE, platform encoding: UTF-8
OS name: "mac os x", version: "10.11.6", arch: "x86_64", family: "mac"
  • Docker version : 1.12.3
  • Sample project : git@github.com:mbacodes/docker-maven-plugin-test.git
<images>
    <image>
        <name>test/hello-world:latest</name>
        <build>
            <dockerFileDir>hello-world</dockerFileDir>
            <assembly>
                <mode>dir</mode>
                <targetDir>/</targetDir>
                <descriptor>assembly.xml</descriptor>
            </assembly>
        </build>
    </image>
</images>

Setting <targetDir>/foo</targetDir> does not change anything.
The resulting targetDir still is target/docker/test/hello-world/latest/build/maven

@bedag-moo
Copy link

I ran into this as well, and tracked it down to DockerAssemblyManager.createAssemblyArchive, which copies the assembly into the build context before the build context is tarred up for Docker, but specifies a hardcoded directory name:

        assemblyArchiver.createArchive(assembly, ASSEMBLY_NAME, buildMode.getExtension(), source, false);

If a DockerFile is not specified, the directory is renamed when tarring the build context, see DockerAssemblyManager.createDockerFileBuilder:

    if (assemblyConfig != null) {
        builder.add(ASSEMBLY_NAME, "")
               .basedir(assemblyConfig.getTargetDir())
               .assemblyUser(assemblyConfig.getUser())
               .exportTargetDir(assemblyConfig.exportTargetDir());

However, if a DockerFile is specified, a DockerFileBuilder is not created, and the renaming does not occur, leaving the assembly in the wrong place.

I wonder why the plugin does not put the assembly into the correct directory to begin with?

        assemblyArchiver.createArchive(assembly, assemblyConfig.getTargetDir(), buildMode.getExtension(), source, false);

@Madanor
Copy link

Madanor commented Mar 8, 2017

The issue still exists in d-m-p 0.20.0.

pom.xml

...
                            <build>
                                <dockerFileDir>${project.basedir}/src/main/docker</dockerFileDir>
                                <assembly>
                                    <mode>dir</mode>
                                    <targetDir>/</targetDir>
                                    <descriptor>assembly.xml</descriptor>
                                </assembly>
                            </build>
...

The resulting targetDir is always maven
target/docker/.../build/maven/

This targetDir has to be referenced in the Dockerfile and impairs the portability of it. And we use a Dockerfile in the first place to have more freedom and be independent from maven.
COPY maven/*.jar app.jar

@paulwilton
Copy link

also verified this issue exists in latest release when using a Dockerfile

@rhuss
Copy link
Collaborator

rhuss commented Apr 26, 2017

Actual it's true that for the dockerFileMode the config option <targetDir> is ignored.

However, as you have to add the assembly yourself in the Dockerfile anyway with ADD or COPY it shouldn't be the big deal.

So you have to add something like

COPY maven/ /

if you want to have this behaviour. Actual target dir was always meant to be the target directory withing the container which makes no sense if the Dockerfile is not managed by d-m-p but provided externally.

As it says in the documentation:

Any additional files located in the dockerFileDir directory will also be added to the build context as well as any files specified by an assembly. However, you still need to insert ADD or COPY directives yourself into the Dockerfile.

Agreed, this should be more clear as its a bit confusing, I will add to it.

@mbacodes @bedag-moo @Madanor @paulwilton : Considering that this maven/ dir is really only an internal build details specific to d-m-p when you use an assembly, is it still important for you to be able to rename this (temporary) directory?

@paulwilton
Copy link

Yes I think it is - it just seems a bit odd to have the Dockerfile using a 'maven' path in it, when we have separated the Dockerfile out specifically to decouple it from Maven

@rhuss
Copy link
Collaborator

rhuss commented Apr 27, 2017

@paulwilton well, at the end you still refer to a Maven assembly with is configured within Maven. tbh, this was first used as an internal directory, before the Dockerfile support went in.

Maybe it's a good idea to make it configurable so that we could choose a different default (and people could use the config variable to keep their setup). This makes sense also because we support a docker:source goal, which exports the content of the whole context directory (including the Dockerfile). So it's also visible to the outside.

Let me add this and also clarify the documentation more on this topic.

@rhuss rhuss self-assigned this Apr 27, 2017
@Madanor
Copy link

Madanor commented Apr 28, 2017

Yes. It would be great to be able to configure the target path (or source path in the Dockerfile). We'd like to be able to use the Dockerfile with and without Maven. While it works at the moment(COPY maven/ /), it's a little bit confusing and not entirely as expected from the documentation (<targetDir> not evaluated).

@bedag-moo
Copy link

Personally, my complaint is that the documentation seems to imply that targetDir matters, but the code ignores it. This caused a nice debugging session on my end :-(

I don't mind whether you change the documentation or the code as long as the two agree :-)

@rhuss
Copy link
Collaborator

rhuss commented May 1, 2017

@bedag-moo sorry for the bad experience ;-( I completely agree that the documentation at this point is quite thin. The documentation will be changed this week along with a new version. that is the plan.

Thanks for the feedback!

rhuss added a commit to rhuss/docker-maven-plugin that referenced this issue May 3, 2017
this can be used to specify the assembly name which is used also as directory
for holding the assembly files. I.e. the directory which needs to be refered to when
an extrean Dockerfile wants to include an assembly. "targetDir" now defaults to "/" + this name.

Also extended the documentation to clarify the usage.

Fixes fabric8io#634
@rhuss
Copy link
Collaborator

rhuss commented May 3, 2017

@mbacodes @bedag-moo @Madanor @paulwilton : I added now a new option "name" to the assembly config which allows you to specify the original source directory. Also worked on the docs
and updated https://github.com/rhuss/docker-maven-plugin/blob/pr/assembly-name/src/main/asciidoc/inc/build/_overview.adoc and https://github.com/rhuss/docker-maven-plugin/blob/pr/assembly-name/src/main/asciidoc/inc/build/_assembly.adoc

Is this understandable or should I add still some more explanations ?

@rhuss
Copy link
Collaborator

rhuss commented May 3, 2017

Just merged in to master. Happy to pickup still any feedback here.

@Madanor
Copy link

Madanor commented May 4, 2017

@rhuss That looks good to me. Thank you.

rgbj pushed a commit to rgbj/docker-maven-plugin that referenced this issue Jun 21, 2017
this can be used to specify the assembly name which is used also as directory
for holding the assembly files. I.e. the directory which needs to be refered to when
an extrean Dockerfile wants to include an assembly. "targetDir" now defaults to "/" + this name.

Also extended the documentation to clarify the usage.

Fixes fabric8io#634
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants