NIFIREG-252: adding mavendocker build profile #245
Conversation
|
@kevdoran could you please take a look? / Give it a run? |
|
creates the docker image locally |
|
reviewing... |
There was a problem hiding this comment.
Thanks for the contribution @ekovacs!
Overall this looks good to me.
I had one inline suggestion about disabling a plugin execution under certain conditions, but that is more of a nice-to-have than a deal-breaker.
At some point I would like to try to combine this with the nifi-registry-docker module, especially to de-duplicate items such as the shell scripts that get added to the image, but we can save that for a future improvement to the project.
Ran the build and the output is working for me. This will help with testing, esp. integration tests with nifi.
Thanks!
| <plugin> | ||
| <artifactId>exec-maven-plugin</artifactId> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <executions> | ||
| <execution> | ||
| <id>Docker integration tests</id> | ||
| <phase>integration-test</phase> | ||
| <goals> | ||
| <goal>exec</goal> | ||
| </goals> | ||
| <configuration> | ||
| <arguments> | ||
| <argument>${project.version}-dockermaven</argument> | ||
| <argument>${project.version}</argument> | ||
| </arguments> | ||
| <executable>${project.basedir}/integration-test.sh</executable> | ||
| </configuration> | ||
| </execution> | ||
| </executions> | ||
| </plugin> |
There was a problem hiding this comment.
We should disable executing this plugin when -DskipTests is used with the mvn build.
(It occurs to me that this would be a nice enhancement for nifi as well, which uses a similar mechanism for testing the dockermaven result.)
There was a problem hiding this comment.
updated PR with skipping on -DskipTests
now this will be printed, if skipTests is set:
[INFO] --- exec-maven-plugin:1.3.2:exec (Docker integration tests) @ dockermaven ---
[INFO] skipping execute as per configuraion
also, please note that this test would only run, when integration-test or later maven lifecycle phase (eg.: verify) is specified.
also fixing port greping in test
at first, i put so i put it under the root directly, and specified i could have moved the existing |
|
Yeah, good points. I would support moving nifi-registry-core/nifi-registry-docker to a top level module that is responsible for both dockerhub and dockermaven. Do you think we should do that now or save that improvement for later? |
currently If it is OK with you, i'd rather file a separate Jira / PR for that move & mavenization, to keep this PR small & single-work-item-focused. |
|
+1 for relocating it and making it a maven project, as well as for doing that as a separate jira/pr. I will re-review this PR as-is. |
just created a follow up JIRA about it: |
skip integration test if -DskipTests is set This closes #245. Signed-off-by: Kevin Doran <kdoran@apache.org>
No description provided.