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.

Sign up for GitHub

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

Jump to bottom

Allow specifying image configuration via Maps. (#360) #1259

Merged
merged 5 commits into from
Oct 8, 2019
Merged

Allow specifying image configuration via Maps. (#360) #1259

merged 5 commits into from
Oct 8, 2019

Conversation

wkritzinger-atlassian
Copy link
Contributor

This allows merging of configuration using the normal Maven behavior.

One such example is the following:

<project>
    <profiles>
        <profile>
            <id>loadbalancer</id>
            <activation>
                <property>
                    <name>loadbalancer.enabled</name>
                </property>
            </activation>
            <build>
                <plugins>
                    <plugin>
                        <groupId>io.fabric8</groupId>
                        <artifactId>docker-maven-plugin</artifactId>
                        <configuration>
                            <imagesMap>
                                <haproxy>
                                    <name>haproxy:2.0-alpine</name>
                                    <run>
                                        <network>
                                            <mode>host</mode>
                                        </network>
                                    </run>
                                </haproxy>
                            </imagesMap>
                        </configuration>
                    </plugin>
                </plugins>
            </build>
        </profile>
        <profile>
            <id>loadbalancer.notlinux</id>
            <activation>
                <os>
                    <name>!linux</name>
                </os>
                <property>
                    <name>loadbalancer.enabled</name>
                </property>
            </activation>
            <properties>
                <haproxy.network.mode>bridge</haproxy.network.mode>
            </properties>
            <build>
                <plugins>
                    <plugin>
                        <groupId>io.fabric8</groupId>
                        <artifactId>docker-maven-plugin</artifactId>
                        <configuration>
                            <imagesMap>
                                <haproxy>
                                    <run>
                                        <ports combine.children="append">
                                            <port>8080:8080</port>
                                        </ports>
                                    </run>
                                </haproxy>
                            </imagesMap>
                        </configuration>
                    </plugin>
                </plugins>
            </build>
        </profile>
        <profile>
            <id>selenium.docker</id>
            <activation>
                <property>
                    <name>selenium.docker</name>
                </property>
            </activation>
            <build>
                <plugins>
                    <plugin>
                        <groupId>io.fabric8</groupId>
                        <artifactId>docker-maven-plugin</artifactId>
                        <configuration>
                            <imagesMap>
                                <selenium>
                                    <name>${webdriver.docker.image}</name>
                                    <run>
                                        <network>
                                            <mode>host</mode>
                                        </network>
                                    </run>
                                </selenium>
                            </imagesMap>
                        </configuration>
                    </plugin>
                </plugins>
            </build>
        </profile>
        <profile>
            <id>selenium.docker.withproxy</id>
            <activation>
                <os>
                    <name>!linux</name>
                </os>
                <property>
                    <name>selenium.docker</name>
                </property>
            </activation>
            <properties>
                <selenium.docker.proxy.ports>7990 7991 7992 7993 7994</selenium.docker.proxy.ports>
                <!-- 'host.docker.internal' is available on Docker for Windows and Mac only. -->
                <selenium.docker.proxy.host>host.docker.internal</selenium.docker.proxy.host>
            </properties>
            <build>
                <plugins>
                    <plugin>
                        <groupId>io.fabric8</groupId>
                        <artifactId>docker-maven-plugin</artifactId>
                        <configuration>
                            <imagesMap>
                                <selenium>
                                    <run>
                                        <run>
                                            <network>
                                                <mode>bridge</mode>
                                            </network>
                                        </run>
                                        <ports combine.children="append">
                                            <port>4444:4444</port>
                                        </ports>
                                    </run>
                                </selenium>
                                <proxy>
                                    <name>alpine/socat</name>
                                    <run>
                                        <entrypoint>
                                            <exec>
                                                <arg>sh</arg>
                                                <arg>-c</arg>
                                                <arg><![CDATA[
                                                        set -- ${selenium.docker.proxy.ports}
                                                        while [ $# -gt 0 ]; do
                                                            socat -d -d TCP-LISTEN:$1,reuseaddr,fork TCP:${selenium.docker.proxy.host}:$1 &
                                                            shift
                                                        done
                                                        exec socat pipe pipe
                                                    ]]></arg>
                                            </exec>
                                        </entrypoint>
                                        <network>
                                            <mode>container</mode>
                                            <name>selenium</name>
                                        </network>
                                        <log>
                                            <prefix/>
                                            <file>
                                                ${project.build.directory}/seleniumDockerLogs/proxy-container.log
                                            </file>
                                        </log>
                                    </run>
                                </proxy>
                            </imagesMap>
                        </configuration>
                    </plugin>
                </plugins>
            </build>
        </profile>
        <profile>
            <id>selenium.docker.debug</id>
            <activation>
                <property>
                    <name>selenium.docker</name>
                    <value>debug</value>
                </property>
            </activation>
            <build>
                <plugins>
                    <plugin>
                        <groupId>io.fabric8</groupId>
                        <artifactId>docker-maven-plugin</artifactId>
                        <configuration>
                            <imagesMap>
                                <selenium>
                                    <run>
                                        <env>
                                            <START_XVFB>true</START_XVFB>
                                            <JAVA_OPTS>-Xmx1024m</JAVA_OPTS>
                                        </env>
                                        <ports combine.children="append">
                                            <port>5900:5900</port>
                                        </ports>
                                    </run>
                                    <name>${webdriver.docker.debug.image}</name>
                                </selenium>
                            </imagesMap>
                        </configuration>
                    </plugin>
                </plugins>
            </build>
        </profile>
    </profiles>
</project>

Configuration with these profiles enabled independently would not be possible without <imagesMap>, as <images> is treated as a list. In Maven, lists can only be overridden, appended or merged, and merging relies on the correct number and position of the elements, neither of which can be controlled if your images only exist in profiles.

Also see #360 (comment)

I could not find existing unit tests for testing the images property on AbstractDockerMojo so I left them out for now. Please let me know if you feel strongly about adding them.

@codecov
Copy link

codecov bot commented Aug 21, 2019

Codecov Report

Merging #1259 into master will decrease coverage by 0.07%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #1259      +/-   ##
============================================
- Coverage     55.53%   55.45%   -0.08%     
  Complexity     1762     1762              
============================================
  Files           156      156              
  Lines          8474     8486      +12     
  Branches       1302     1304       +2     
============================================
  Hits           4706     4706              
- Misses         3314     3326      +12     
  Partials        454      454
Impacted Files Coverage Δ Complexity Δ
...abric8/maven/docker/config/ImageConfiguration.java 58.22% <0%> (-1.52%) 20 <0> (ø)
...va/io/fabric8/maven/docker/AbstractDockerMojo.java 10.71% <0%> (-1.06%) 8 <0> (ø)

2 similar comments
@codecov
Copy link

codecov bot commented Aug 21, 2019

Codecov Report

Merging #1259 into master will decrease coverage by 0.07%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #1259      +/-   ##
============================================
- Coverage     55.53%   55.45%   -0.08%     
  Complexity     1762     1762              
============================================
  Files           156      156              
  Lines          8474     8486      +12     
  Branches       1302     1304       +2     
============================================
  Hits           4706     4706              
- Misses         3314     3326      +12     
  Partials        454      454
Impacted Files Coverage Δ Complexity Δ
...abric8/maven/docker/config/ImageConfiguration.java 58.22% <0%> (-1.52%) 20 <0> (ø)
...va/io/fabric8/maven/docker/AbstractDockerMojo.java 10.71% <0%> (-1.06%) 8 <0> (ø)

@codecov
Copy link

codecov bot commented Aug 21, 2019

Codecov Report

Merging #1259 into master will decrease coverage by 0.07%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #1259      +/-   ##
============================================
- Coverage     55.53%   55.45%   -0.08%     
  Complexity     1762     1762              
============================================
  Files           156      156              
  Lines          8474     8486      +12     
  Branches       1302     1304       +2     
============================================
  Hits           4706     4706              
- Misses         3314     3326      +12     
  Partials        454      454
Impacted Files Coverage Δ Complexity Δ
...abric8/maven/docker/config/ImageConfiguration.java 58.22% <0%> (-1.52%) 20 <0> (ø)
...va/io/fabric8/maven/docker/AbstractDockerMojo.java 10.71% <0%> (-1.06%) 8 <0> (ø)

@codecov
Copy link

codecov bot commented Aug 21, 2019 •
edited
Loading

Codecov Report

Merging #1259 into master will decrease coverage by 0.08%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #1259      +/-   ##
============================================
- Coverage     55.53%   55.44%   -0.09%     
  Complexity     1763     1763              
============================================
  Files           156      156              
  Lines          8474     8487      +13     
  Branches       1302     1304       +2     
============================================
  Hits           4706     4706              
- Misses         3314     3327      +13     
  Partials        454      454
Impacted Files Coverage Δ Complexity Δ
...abric8/maven/docker/config/ImageConfiguration.java 58.22% <0%> (-1.52%) 20 <0> (ø)
...va/io/fabric8/maven/docker/AbstractDockerMojo.java 10.61% <0%> (-1.15%) 8 <0> (ø)

@wkritzinger-atlassian
Allow specifying image configuration via Maps. (fabric8io#360)
37cfcd1 
This allows merging of configuration using the normal Maven behavior.

Signed-off-by: Wolfgang Kritzinger <wkritzinger@atlassian.com>
@wkritzinger-atlassian
Copy link
Contributor Author

Hi @rhuss! Apologies for prodding to get an update here. We're currently running on a fork of this repo and would like to unfork this soon-ish :) Would it be possible for you to have a look at this when it's convenient? Thank you!

@rhuss
Copy link
Collaborator

rhuss commented Oct 7, 2019

Hi @wkritzinger-atlassian sorry for the long delay. First there were vacations and then tons of other work (unfortunately I'm not able to spend much time for d-m-p these days)

I will have a look asap.

rhuss
rhuss reviewed Oct 7, 2019
View reviewed changes
Copy link
Collaborator

@rhuss rhuss left a comment

Choose a reason for hiding this comment

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

On a first look the PR looks good to me. Could you please also up date docs/changelog.md to include this change ?

@wkritzinger-atlassian
Copy link
Contributor Author

@rhuss No worries! I'll update docs/changelog.md 👍

@rhuss rhuss merged commit da1e203 into fabric8io:master Oct 8, 2019
@rhuss
Copy link
Collaborator

rhuss commented Oct 8, 2019

thanks !

@rhuss rhuss mentioned this pull request Jan 9, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhuss rhuss rhuss left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wkritzinger-atlassian @rhuss