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

Inherit image configuration from the parent pom #360

Closed
osigida opened this issue Jan 5, 2016 · 7 comments
Closed

Inherit image configuration from the parent pom #360

osigida opened this issue Jan 5, 2016 · 7 comments

Comments

@osigida
Copy link
Contributor

osigida commented Jan 5, 2016

Hi All,

I have a parent pom where a general image configuration is defined in pluginManagement section (all child projects build this image, with very few changes, like java start class)

Sometimes I'd like to build extra image together with one defined in the parent pom.

Is there a way to handle it via configuration inheritance?
Have tried few things but didn't succeeded, only one image defined in child pom was built.

@jgangemi
Copy link
Collaborator

jgangemi commented Jan 5, 2016

you should be able to do this by specifying combine.children="append" as an attribute to the image configuration.

see here: https://maven.apache.org/pom.html under the plugins section.

@osigida
Copy link
Contributor Author

osigida commented Jan 7, 2016

@jgangemi, right, thanks!

However maven is not really predictable in this case

so just for the record

  1. if I have a default configuration in pluginManagement section in paren pom, I just need to declare plugin in build section. and it works fine

  2. if I want to build another image, together with the image declared in parent pom, I need to set <images combine.children="append"> in child pom (in plugin declaration in build section)

  3. if I don't want to build image declared in parent pom but something else, I need to set <images combine.self="override"> in child pom (in plugin declaration in build section)

combine.children="override" and combine.self="append" looks useless and confuses a lot.

@osigida osigida closed this as completed Jan 7, 2016
@jgangemi
Copy link
Collaborator

jgangemi commented Jan 7, 2016

heh - that's not us, that's maven.

i think you can add combine.children="append" to the parent pom and not have to add it to all the children.

@vjkoskela
Copy link

I completely agree that Maven leaves something to be desired in this area. However, what is the plugin's behavior if two image blocks with the name are found? Does it merge these or are they treated separately? -- looking at the code I it looks like they are treated separately.

The ability to append to or override an image configuration (e.g. just its run/env block) would be useful and allow you to work around Maven's all or nothing inheritance. For example:

parent:

<configuration>
  <images>
    <image>
      <name>foo</name>
      <run>
        <env>
          <PORT>80</PORT>
          <USER>root</USER>
          <PASSWORD>secret</PASSWORD>
        </env>
      </run>
    </image>
  </images>
</configuration>

child:

<configuration>
  <images combine.children="append">
    <image>
      <name>foo</name>
      <run>
        <env>
          <PASSWORD>top-secret</PASSWORD>
        </env>
      </run>
    </image>
  </images>
</configuration>

The effective child pom would look like this:

<configuration>
  <images>
    <image>
      <name>foo</name>
      <run>
        <env>
          <PORT>80</PORT>
          <USER>root</USER>
          <PASSWORD>secret</PASSWORD>
        </env>
      </run>
    </image>
    <image>
      <name>foo</name>
      <run>
        <env>
          <PASSWORD>top-secret</PASSWORD>
        </env>
      </run>
    </image>
  </images>
</configuration>

So what would be needed is a way to merge the configurations of image configurations with the same name. For external configuration the plugin already supports resolving configuration from other sources:

https://github.com/fabric8io/docker-maven-plugin/blob/master/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java#L263

https://github.com/fabric8io/docker-maven-plugin/blob/master/src/main/java/io/fabric8/maven/docker/config/ConfigHelper.java#L45

https://github.com/fabric8io/docker-maven-plugin/blob/master/src/main/java/io/fabric8/maven/docker/config/handler/ImageConfigResolver.java#L77

Something similar could be used to match images with the same name and merge the configurations. The precedence would be defined by the order of the images in the list by default or by a new attribute.

In our use case the parent pom contains the default image configuration and child poms need to be able to customize (add/override) some attributes as well as add new images. The latter works great with Maven's append instruction, but the former poses a bit of a problem. What are your thoughts about adding support for merging image configurations with the same name?

@osigida
Copy link
Contributor Author

osigida commented Jan 25, 2017

@vjkoskela it is up to maven and configuration you set for nodes,
try combine.children="merge" or combine.children="override"
also check this: https://maven.apache.org/pom.html
may help to understand how to find the way.

@vjkoskela
Copy link

Thanks @osigida I will take a look again. However, the configuration is stored in a list and Maven merges based on the element name. As a result, the Fabric8 plugin uses image for all elements:

<images>
  <image>
    <name>imageNameA</name>
    ...
  </image>
  <image>
    <name>imageNameB</name>
    ...
  </image>
</images>

Therefore Maven cannot understand if two image blocks are logically related (based on the nested name attribute). From the link you posted:

The default behavior is to merge the content of the configuration element according to element name.

In order to merge elements which represent the same image it should not be a list but a map where the "key" is the name of the image. Maven supports maps as documented below and then I believe we could rely on Maven's merge/override/append semantics to manipulate the xml as desired:

https://maven.apache.org/guides/mini/guide-configuring-plugins.html#Mapping_Maps

For example:

<images>
  <imageNameA>
    ...
  </imageNameA>
  <imageNameB>
    ...
  </imageNameB>
</images>

But this is a major backwards incompatible change to the plugin hence my proposal to perform the merge in the plugin. The example above I posted how Maven would treat multiple image tags with append, below are the results for merge and override:

= Override =

parent:

<configuration>
  <images>
    <image>
      <name>foo</name>
      <run>
        <env>
          <PORT>80</PORT>
          <USER>root</USER>
          <PASSWORD>secret</PASSWORD>
        </env>
      </run>
    </image>
  </images>
</configuration>

child:

<configuration>
  <images combine.children="override">
    <image>
      <name>foo</name>
      <run>
        <env>
          <PASSWORD>top-secret</PASSWORD>
        </env>
      </run>
    </image>
  </images>
</configuration>

The effective child pom would look like this:

<configuration>
  <images>
    <image>
      <name>foo</name>
      <run>
        <env>
          <PASSWORD>top-secret</PASSWORD>
        </env>
      </run>
    </image>
  </images>
</configuration>

= Merge =

parent:

<configuration>
  <images>
    <image>
      <name>foo</name>
      <run>
        <env>
          <PORT>80</PORT>
          <USER>root</USER>
          <PASSWORD>secret</PASSWORD>
        </env>
      </run>
    </image>
  </images>
</configuration>

child:

<configuration>
  <images> <!-- Merge is the default -->
    <image>
      <name>foo</name>
      <run>
        <env>
          <PASSWORD>top-secret</PASSWORD>
        </env>
      </run>
    </image>
  </images>
</configuration>

The effective child pom would look like this:

<configuration>
  <images>
    <image>
      <name>foo</name>
      <run>
        <env>
          <PORT>80</PORT>
          <USER>root</USER>
          <PASSWORD>top-secret</PASSWORD>
        </env>
      </run>
    </image>
  </images>
</configuration>

This works great for a single image, and even multiple images -- however, it assumes that the images are in the same order and ALL images from the parent are declared in the child. Partial image declarations (e.g. override only a subset) or other changes to image declaration order will wreak havoc on the effective configuration.

You can blame Maven for this, but in my opinion if the images were modeled as a map keyed by the image name instead of a list then Maven could merge the correct image configuration together with its existing functionality.

Let me know if I've missed something here.

@jgangemi
Copy link
Collaborator

if you have multiple configuration blocks and they share the same image name and you aren't using any of the combine.children options, maven will merge all those configurations into one.

for example, i have the following profiles which are activated using a dotfile

    <profile>
      <id>application</id>
      <activation>
        <file>
          <exists>.application</exists>
        </file>
      </activation>
      <build>
        <plugins>
          <plugin>
            <artifactId>maven-remote-resources-plugin</artifactId>
            <executions>
              <execution>
                <id>process-remote-resources</id>
                <phase>generate-resources</phase>
              </execution>
            </executions>
          </plugin>
          <plugin>
            <groupId>org.glassfish.hk2</groupId>
            <artifactId>hk2-inhabitant-generator</artifactId>
          </plugin>
          <plugin>
            <groupId>org.codehaus.mojo</groupId>
            <artifactId>build-helper-maven-plugin</artifactId>
            <executions>
              <execution>
                <id>add-application-resources</id>
                <phase>generate-resources</phase>
              </execution>
            </executions>
          </plugin>
          <plugin>
            <groupId>io.fabric8</groupId>
            <artifactId>docker-maven-plugin</artifactId>
            <configuration>
              <images>
                <image>
                  <name>${project.artifactId}:${project.version}</name>
                  <alias>${project.name}</alias>
                  <build>
                    <dockerFileDir>${shared-resources.docker.dir}</dockerFileDir>
                    <assembly>
                      <descriptor>${docker.assembly.descriptor}</descriptor>
                    </assembly>
                    <tags>
                      <tag>latest</tag>
                    </tags>
                  </build>
                  <run>
                    <skip>${docker.disable.run}</skip>
                    <env>
                      <JMX_HOST>docker.local</JMX_HOST>
                      <AWS_ACCESS_KEY_ID>${env.AWS_ACCESS_KEY_ID}</AWS_ACCESS_KEY_ID>
                      <AWS_SECRET_ACCESS_KEY>${env.AWS_SECRET_ACCESS_KEY}</AWS_SECRET_ACCESS_KEY>
                    </env>
                    <ports>
                      <port>8585:8585</port>
                      <port>9000:9000</port>
                    </ports>
                  </run>
                </image>
              </images>
            </configuration>
            <executions>
              <execution>
                <id>build</id>
                <phase>package</phase>
                <goals>
                  <goal>build-nofork</goal>
                </goals>
              </execution>
            </executions>
          </plugin>
        </plugins>
      </build>
    </profile>

    <profile>
      <id>batch-worker</id>
      <activation>
        <file>
          <exists>${basedir}/.batch-worker</exists>
        </file>
      </activation>
      <build>
        <plugins>
          <plugin>
            <groupId>io.fabric8</groupId>
            <artifactId>docker-maven-plugin</artifactId>
            <configuration>
              <images>
                <image>
                  <name>${project.artifactId}:${project.version}</name>
                  <run>
                    <cmd>
                      <arg>&#x002d;&#x002d;batchId ${batch.id} &#x002d;&#x002d;workerTag localTesting</arg>
                    </cmd>
                  </run>
                </image>
              </images>
            </configuration>
          </plugin>
          <plugin>
            <groupId>com.bazaarvoice.maven.plugins</groupId>
            <artifactId>s3-upload-maven-plugin</artifactId>
          </plugin>
        </plugins>
      </build>
    </profile>

    <profile>
      <id>http-server</id>
      <activation>
        <file>
          <exists>.http-server</exists>
        </file>
      </activation>
      <build>
        <plugins>
          <plugin>
            <groupId>io.fabric8</groupId>
            <artifactId>docker-maven-plugin</artifactId>
            <configuration>
              <images>
                <image>
                  <name>${project.artifactId}:${project.version}</name>
                  <run>
                    <ports combine.children="append">
                      <port>8080:8080</port>
                    </ports>
                  </run>
                </image>
              </images>
            </configuration>
          </plugin>
        </plugins>
      </build>
    </profile>

.application is always required and then depending on what type of application i am creating, one of the other two dotfiles is also enabled and the resulting pom has everything merged under whatever image is represented by ${project.artifactId}:${project.version}

wkritzinger-atlassian added a commit to wkritzinger-atlassian/docker-maven-plugin that referenced this issue Aug 21, 2019
This allows merging of configuration using the normal Maven behavior.

Signed-off-by: Wolfgang Kritzinger <wkritzinger@atlassian.com>
wkritzinger-atlassian added a commit to wkritzinger-atlassian/docker-maven-plugin that referenced this issue Aug 21, 2019
This allows merging of configuration using the normal Maven behavior.

Signed-off-by: Wolfgang Kritzinger <wkritzinger@atlassian.com>
wkritzinger-atlassian added a commit to wkritzinger-atlassian/docker-maven-plugin that referenced this issue Oct 8, 2019
Signed-off-by: Wolfgang Kritzinger <wkritzinger@atlassian.com>
rhuss pushed a commit that referenced this issue Oct 8, 2019
* Allow specifying image configuration via Maps. (#360)

This allows merging of configuration using the normal Maven behavior.

Signed-off-by: Wolfgang Kritzinger <wkritzinger@atlassian.com>

* Update changelog.md (#360)

Signed-off-by: Wolfgang Kritzinger <wkritzinger@atlassian.com>
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

3 participants