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

allow better control of assembly process/export #64

Closed
jgangemi opened this issue Dec 6, 2014 · 11 comments
Closed

allow better control of assembly process/export #64

jgangemi opened this issue Dec 6, 2014 · 11 comments
Assignees
Labels
Milestone

Comments

@jgangemi
Copy link
Collaborator

jgangemi commented Dec 6, 2014

split off from issue #62, relevant parts below.


BTW, in other issue #53 Thomas has problems with the UID of the exported directory. I tried to changed from ADD to COPY but this didn't helped.

My idea is to use an extra configuration (like 'assemblyUser') and then do the required 'chown' on our own by inserting it into the on-the-fly generated Dockerfile. Since this is yet another assembly related paremeter, what do you think about combining this into extra configuration section like in

<build>
   ...
   <assembly>
      <descriptor>src/.../assembly.xml</descriptor>
      <directory>/maven</directory>
      <export>true</export>
      <uid>www-data</uid>
   </assembly>
</build>

For backwards compatibility one would still support exportDir, assemblyDescriptor and assemblyDescriptorRef for some time ?


i would change uid to user b/c that's what docker calls it and i'd probably call directory basedir b/c that's more maven-ish


The stuff with support for Dockerfiles can be found here #13

@rhuss
Copy link
Collaborator

rhuss commented Dec 8, 2014

The current working suggestion is:

<build>
   ...
   <assembly>
      <descriptor>src/resources/docker/assembly.xml</descriptor>
      <directory>/maven</directory>
      <export>true</export>
      <uid>www-data</uid>
   </assembly>
</build>

and keeping extra volumes to export outside this configuration section. Depending on the export flag, the assembly directory is added to the volume list (true) or not (false).

@jgangemi
Copy link
Collaborator Author

jgangemi commented Dec 8, 2014

given that i'm about 1/2 to 2/3 of the way through this i thought i should get some feedback on where i'm headed...

a new, top level build element has been added to allow for a configurable (default) source/target directories.

<configuration>
  <sourceDirectory>src/main/docker</sourceDirectory>
  <outputDirectory>target/docker</outputDirectory>
  ...
</configuration>

most of the time i'm sure this will never get used directly but i know there are ppl out there that dislike the default maven dir structure (i used to be one of them) and this allows more flexibility. it also allows for what's to follow.


if you are happy w/ what the assembly defaults

<assembly>
  <basedir>/maven</basedir>
  <export>true</export>
</assembly>

and you don't want to use one of the predefined desriptorRefs, as long as the descriptor lives dockerSourceDirectory, it doesn't have to be specified (and the file name doesn't matter) b/c the assembly plugin will now scan that directory looking for xml files. if you want to specify the descriptor name, you can

<assembly>
  ...
  <descriptor>docker-assembly.xml</descriptor>
</assembly>

if you want to specify a location outside of dockerSourceDirectory, you can do that too

<assembly>
  ...
  <descriptor>/path/to/descriptor</descriptor>
</assembly>

or if you want the to use a predefined ref

<assembly>
  ...
  <descriptorRef>project</descriptorRef>
</assembly>

the flexibility is there. if you don't want any assembly to occur (see #66), you can do this

<from>...</from>
...
<command>...</command>
<assemble>false</assemble>

and create a container w/o any artifacts from the project at all.


you can now define the location of a docker file like so:

...
<image>
  <build>   
    <dockerfile>container/Dockerfile</dockerfile>
  </build>
</image>

if you define a dockerfile element, with the exception of the assembly block, all other fields will be ignored. this seems to be the way other plugins work with regard to different options that can negate each other, so i don't think there needs to be complicated validation logic that checks if you're using a Dockerfile and have a command element in the configuration.

if the path to the file is relative, it will search dockerSourceDirectory, otherwise it will attempt to resolve out the path. any files that are located inside the directory will automatically become part of the build.

for now, it will be up to the user to handle adding an 'ADD' line to the dockerfile to pull in the the project artifact. i have some ideas on using the resources plugin to do property substitution, etc but i'm taking the approach that if you're using a Dockerfile you should be able to build it directly using docker as well.

in terms of the assembly block, if you're happy using all the defaults (project archive in to be added lives in 'maven' and you are letting the assembly scan for the descriptor) it does not have to be specified. otherwise, you can specify the assembly block w/ either descriptor or descriptorRef element in addition to defining the addFrom directory you want the project archive to end up in before the docker build occurs, eg:

<assembly>
  <descriptorRef>project</descriptorRef>
  <addFrom>include</addFrom>
</assembly>

like above, if a dockerfile is in use, any other elements defined in the assembly block will be ignored.

i thought about using a separate xml block for this but that would just mean more xml and if you're happy w/ what the defaults are, you can really pair down the maven configuration if you're using a Dockerfile. this also means user is responsible for making sure the project's artifact makes its way into the resulting image, but i think that's an acceptable trade-off for now b/c using a raw Dockerfile should be considered an advanced configuration that requires more from the user.


other tidbits...

now that a user can be specified as part of the build, Dockerfile generation can be tweaked slightly to run the USER command before the ADD/COPY so the files get created as the correct user. if for some reason files need to be owned by multiple users, a Dockerfile would have to be used.

thinking about it now, maybe there should be two user type elements - one in the assembly to indicate who the files should be owned by and one in the build element to indicate who the command should be run as.

<command>some command</command>
<user>tomcat</user>
<assembly>
  ...
  <owner>bob</owner>
</assembly>

i think that's everything :)

@rhuss
Copy link
Collaborator

rhuss commented Dec 8, 2014

Wow, lots of stuff ;-)

On a first glimpse:

  • Isn't the absence of an <assembly> section enough for not using an assembly ? Why do you need a dedicated <assemble>false</assemble> ?
  • Working with absolute paths is a no-go. We shouldn't even consider this, because this leads to non-portable builds for sure. Frankly, I'm not a fan of an extra directory level and would leave it simple so that every relative path is relative to the pom.xml it is specified in. I know, the fixed directory layout of maven is blessing and curse in one, but that's the maven way, and I think we should get over it. IMO its more confusing for people when deviating from the Maven path, regardless whether we like it or not.IMO the biggest achievment of Maven is, that if you have arranged with Maven (it takes time, you don't like, you learned to cope with it), then you love it because every Maven project looks the same. It might be restrictive creativity and hurts aesthetic feelings, but it is the same everywhere. And a relative path is relative to the directory the pom.xml lives in. That's it and I would like to keep it that way without any emergency exit. I don't think its too much overhead to specify the relative path for every file (Dockerfile, assembly.xml, anything else ?). That's my my point ;-)
  • Instead of an Dockerfile, I would like to configure the directory where the Dockerfile is in, so that supporting files can be referenced directly from within the Dockerfile. E.g a <dockerDirectory> or something like this (mayeb even only <docker> ?) would be nice. A Dockerfile is always named Dockerfile.
  • Variable substitution would be super cool, because that is rarely supported by other plugins and really quite useful.I would even give up the possibility to directly use the Dockerfile with the docker CLI (which BTW won't work if you use an assembly anyways, because this would have to be also added dynamically).
  • Sorry, I didn't get <addFrom> (but it is late, too ;-). What it is for ?

Complete agreement for:

  • <owner> is a good idea to distinguish it with <user> (or if we want to go nerdy, maybe even <chown>).
  • The existance of a <dockerDir> (or whatever) will ignore other specs (except assembly), no errors thrown.

These are my points ;)

One important thing: I reviewed you pull request, made some changes (essentially minor, however some refactorings nevertheless), so I would like to push this tomorrow in extra branch and I would be awesome if you could rebase on this.

@jgangemi
Copy link
Collaborator Author

jgangemi commented Dec 8, 2014

lol - let's see...

the absence of an assembly block means use all the default values so i'm not sure how you can indicate 'no assembly' otherwise w/o some other explicit element b/c assembly == null means use default values.

i don't think it should be up to us to decide if someone wants to make a non-portable build. if they want to use a full path, that's their business but i'm not going to argue the point either way. as an aside, i think just by the nature of how maven works, everything is relative to the ${project.dir} if no leading / is found.

having said that, i still the idea of using src/main/docker as the default directory b/c it means if the user is fine w/ the assembly defaults, as long as their assembly file lives in src/main/docker, they don't have to specify any assembly configuration and the build section for the image only contains the docker configuration.

if you're still not sold, perhaps as a compromise we could leave using a default directory as an explicit option that you enable instead of having it be implicit. i know specifying the following

<assembly>
  <descriptor>src/main/docker/assembly.xml</descriptor>
</assembly>

might not seem like a lot, but i think the less xml that needs to be defined, the better.

i went back and forth between the path to the Dockerfile itself and the dir it lived in, so i'll change it to be the dir.

i realize there is a chicken and egg issue if you use an assembly and a Dockerfile, so the idea to allow the container to be built independent of the plugin may not be practice.

addFrom is meant to be a way to override the maven directory the project artifact gets placed into by the assembly and is referenced by the ADD command in the Dockerfile the plugin creates.

the idea behind this is if you are using a Dockerfile and you already are including files, eg:

/container/
  Dockerfile
  include/
    - file1
    - file2

you can do <addFrom>include</addFrom> and the assembly plugin will put the artifact in that location and you don't need to make any modification to the Dockerfile to explicity put ADD maven <container_path> in their Dockerfile.

it's possible this may be a flawed idea - i need to experiment a little w/ the assembly plugin to see if this will actually work but i think it should.

i'll incorporate all comments into an initial PR and we can go from there.

w.r.t. the other pull requuest, at this point what exactly do you want me to do there? this work should be its own PR so if you're happy w/ things as they stand, i'm not going to argue against any changes you've made so why not just push those changes into master?

@rhuss
Copy link
Collaborator

rhuss commented Dec 9, 2014

IMO, at least the descriptor or the descriptorRef should be provided for an assembly (I don't like the idea of having a 'default' assembly name because this really makes it hard to understand what's going on when reading the configuration alone because it references implicitly an external file).

I heavily argue for not having an (implicite) assembly by default, but I can live with a configurable Docker top-dir.

If I understand you right, instead of

<configuration>
  <image>
     ...
     <build>
        <assembly>
            <descriptor>src/main/docker/assembly.xml</descriptor>
        </assembly>
     </build>
  </image>
</configuration>

you suggest

<configuration>
  <dockerDir>src/main/docker</dockerDir>
  <image>
     ...
     <build>
        <assembly>
            <descriptor>assembly.xml</descriptor>
        </assembly>
     </build>
  </image>
</configuration>

where in this case dockerDir could be omitted because it is the default ?


wrt/ addFrom : I don't think that we need this (as you probably already have noticed, I'm big fan of avoiding configuration options if they either can be realized otherwise or provide only a marginal value. The less, the better ;-). My idea is, if you are using an explicit Dockerfile and an assembly, can reference files relative to its location as usual with ADD and COPY and the assembly is simply added on-the-fly with an COPY directive right before the command or at the beginning of the Dockerfile (one would still have to discuss where to place it, but probably at the beginning of the Dockerfile right after FROM and MAINTAINER would be ok). Maybe one could 'invent' a special pseudo-Dockerfile command like MAVEN_COPY_ASSEMBLY to let the user specify the location where to put the COPY /maven, EXEC chown user.

I think that's simple enough that it can be understood even without documentation.


Evolving on this idea as a next step could be an docker:export goal or something like this, which creates a tar including the resolved Dockerfile with all other files which then can be used to re-create the image with the Docker CLI. Or one could define a new artefact type, too (though I have to look up, how this could work), so if one would use <packaging>docker</packaging> then this tar file containing Dockefile et al. could easily deployed to a maven registry with mvn clean install deploy

But that's the next story .... ;-)


For the pull request: I pushed branch 65-pr-jgangemi to github and will merge this into master right before the release. If you are working on another PR here, it would be nice, if you could rebase on this branch, not master, before creating the PR so that I then can merge both to master later on without much conflicts.


Yup, quite some stuff ;-)

FYI, I created an channel #docker-maven-plugin @ freenode and hang around there. Alternatively, we can also try https://gitter.im/rhuss/docker-maven-plugin , I'm there, too.

@jgangemi
Copy link
Collaborator Author

i'll circle back on this later tonight/tomorrow - had something come up.

@jgangemi
Copy link
Collaborator Author

i'm back working on this again...hopefully it won't take me long to make these changes.

on a separate, but related note: isn't the volumes change in #65 a breaking change? if yes, why don't we just ditch exportDir and the others as part of this?

i hung out on the irc channel today - i'll try again tomorrow.

@rhuss
Copy link
Collaborator

rhuss commented Dec 16, 2014

Yes, it is a breaking change (I documented this here) so feel free to drop the support for the old stuff already and change the configuration as discussed.

Normally, I wouldn't be that aggressive, but for 0.10.6 we will force the user to change a bit anyways:

  • API version v1.15 mandatory
  • <volumes> directive

Please add to this document for helping people in migration.

And yes, we are still in pre-1.0 mode ;-)

@jgangemi
Copy link
Collaborator Author

i knew i saw it some place, i just couldn't remember where. i'll be sure to make changes there as well when i do doc updates.

@rhuss
Copy link
Collaborator

rhuss commented Dec 22, 2014

I want to do a release before christmas, so I wonder whether we can include your changes already. Do you think you can make it until tomorrow ?

Maybe you can push you current state anyways, since I have some time until wednesday and could do the integration ?

No rush, though. There will be a 0.11.1 release after this anyways ;-) (However, having all already known fundamental config changes in one release would be nice, but that's not mandatory, of course ....)

@jgangemi jgangemi self-assigned this Feb 19, 2015
@jgangemi jgangemi added this to the 0.11.0 milestone Feb 19, 2015
@jgangemi
Copy link
Collaborator Author

this was merged, so closing...

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

No branches or pull requests

2 participants