Maven assembly of WAR + Docker image #2937
Conversation
(cherry picked from commit acd39e8)
(cherry picked from commit 6852f4b)
(cherry picked from commit cc4b9fe)
(cherry picked from commit 5f2e1e2)
(cherry picked from commit f3a722c)
(cherry picked from commit ec3fcdf)
(cherry picked from commit 58dbfa4)
(cherry picked from commit ecccc12)
(cherry picked from commit 98febea)
(cherry picked from commit bfe3efe)
(cherry picked from commit 7d0f02f)
(cherry picked from commit 6785c18)
(cherry picked from commit 50915fa)
(cherry picked from commit 168fd77)
(cherry picked from commit fbcdd54)
(cherry picked from commit 7e0af37)
(cherry picked from commit 46e27f1)
(cherry picked from commit 4b5fc6c)
latest changes from `master` @ `$PROJECT_HOME/restcomm/configuration/config-scripts/as7-config-scripts/restcomm/autoconfig.d/`
Moving them into this GH repo, so they can be included in the docker image build. As we are moving towards using the docker image as our binary (instead of just the WAR file), it makes sense to keep all those components that are used to build the image in the same repository anyway.
Major differences are two: 1. restcomm binary is NOT retrieved from Box, rather it is expected to be available in the local filesystem. This is because this `Dockerfile` will be used as part of the maven build, so the WAR file will (just) have been created anyway. 2. The Dockerfile now comes in 2 variants: one is `Dockerfile_standalone`, which includes *just* restcomm-connect (i.e. no mediaserver, no rvd, no webrtc-ui). As such, any mediaserver, etc. files are removed from the Dockerfile_standalone. On the other hand, the `Dockerfile_all-in-one` is a more straightforward port from `Restcomm-Docker` repo and basically just includes change (1) above and some reordering (to make continuous rebuilds of docker image faster, leveraging cached layers.
In a nutshell, the `standalone` profile includes just restcomm-connect. The `all-in-one` includes RC + RMS + RVD + WebRTC UI, just like it used to be before introducing these 2 profiles. (Eventually, we should be getting rid of this second one, but for the moment, for backwards compatibility and for moving forward with a smoother migration process, we should keep it around and just focus on improving the WAR + docker image generation on that part (in maven build, instead of `ant` + separate restcomm-docker repo).
the plugin is used in each of the 2 maven profiles (`standalone` and `all-in-one`) to pick up the corresponding Dockerfile (`Dockerfile_standalone` and `Dockerfile_all-in-one`) and build the appropriate docker image (`restcomm/restcomm` and `restcomm/connect` respectively). For the moment - and this is perhaps a point for discussion - the plugin's `build` goal is bound to the `pre-integration-test` phase, as the fabric8 philosophy suggests you build a docker image (which is the resulting binary artifact from your maven build) and then you `start` it - still within the `pre-integration-test` phase - (together with all of its dependencies, e.g. database docker containers, 3rd party dependencies, etc.) so that you can then run the `verify` phase tests against the running system. There is also a binding to the plugin's `stop` goal, in the `post-integration-phase`, which tears down the above setup. I am leaving some of the above bindings commented out, on purpose, so you, oh dear reviewer, can get a better idea of how this works, in case you're not familiar. Whether we choose to adopt this setup (which in my previous experience is pretty damn awesome!! ) largely depends however on what tests we already run in the `verify` phase and whether we would care to move some to other phases - or even other jenkins pipeline stages. If we don't want to adopt the above approach, then we can consider binding the docker image creation in some other phase of mvn lifecycle.
(due to previous merge conflicts) i.e. this change was introduced in 4b5fc6c, but the cherry pick didn't bring it along due to some merge conflict.
(cherry picked from commit 5004ef1)
This way the `ADD` command in the Dockerfile can both copy the file AND unpack it in one go ; )
this is almost certainly not needed (especially so in the mid/long-term), but i am adding it for backwards compatibility reasons, and to make for a smoother migration
these were also being excluded in the [build.xml](https://github.com/RestComm/Restcomm-Connect/blob/master/release/build.xml#L198)
Manually uploaded media server 6.0.23 in our own CXS Nexus releases repo and fixed artifact ID. Also added some missing files, which are needed for the all-in-one docker image bundle.
Restcomm does not yet fully support it
gsaslis
changed the title
| <goals> | ||
| <!-- "build" should be used to create the images with the | ||
| artifact --> | ||
| <goal>build</goal> |
There was a problem hiding this comment.
again i dont see the "build" goal being hooked to pre-integration-test. i think "package" would be better
There was a problem hiding this comment.
@jaimecasero that is a fair point.
btw, do you agree that it should be bound to some phase, or would you prefer that we added the plugin and then explicitly invoke the docker:build maven target when we want to build a docker image?
There was a problem hiding this comment.
im looking into hook docker image generation into some phase, so its part of "regular" build, and invoked with regular maven goals
| </properties> | ||
| <profiles> | ||
| <profile> | ||
| <id>all-in-one</id> |
There was a problem hiding this comment.
what happens on maven:deploy when both profiles are enabled. Will both binaries will get deployed into nexus with different classifier?
There was a problem hiding this comment.
@jaimecasero with regards to the docker images, that is not an issue, because we have restcomm/restcomm and restcomm/connect images.
What would you propose for WAR binaries?
There was a problem hiding this comment.
I guess you mean ZIP binaries... The assemble ZIP files should be deployed in nexus as attachment of project with specific classifier. But since you are using same project coordiante to create slightly different ZIP binaries, then assembly classifier is required to be different...
Honestly i tend to think these are really two different maven artifacts (all-in-one vs standalone),which should be modelled as separate maven modules with diff coordiantes(artifactid). In terms of zip versus docker, im fine sharing same project module, as their are semantically the same binary file (same features delivered) but with different flavors (ZIP versus Docker image). Also, we dont want to deploy docker image into Nexus as maven dependency,but be deployed in Nexus docker registry as docker image...
Again, im not sure about valid usage of profiles here...
There was a problem hiding this comment.
the separate module sounds like a valid approach, but we already have separate modules for as7 (and wildfly is coming up) and the approach of a separate module for each combination of (standalone vs. all-in-one) * (as7 vs. wildfly) is already starting to sound like too much overhead... ?
I would happily let you choose on this one and I can adapt accordingly, so we can get this merged in.
| <artifactId>sip-servlets-as7</artifactId> | ||
| <version>${sipservletapi.version}</version> | ||
| <type>zip</type> | ||
| <classifier>assembly</classifier> |
There was a problem hiding this comment.
classifier is the same as previous assembly, will this create collision if both profiles are enabled?
There was a problem hiding this comment.
i get your point, but - to make sure I did get it right - this is not the right place to add that comment, right?
(i mean this right here is referring to the sip-servlets dependency, not the artifact generation... )
There was a problem hiding this comment.
right, this is not right place sorry, i guess you got the right point anyway..
|
@gsaslis i dont think we will be able to merge this into master with all the wildfly10 stuff, specially all the changes in the pom files... |
|
@jaimecasero i agree. this is what i tried to highlight on my opening PR comment above. i followed this approach primarily so i can cherry pick your commits and you can keep track of what's included. |
|
@gsaslis wildfly10 new module is not really any issue. But along with the new module there were lots of changes to the existing pom files to remove tomcat depends and bring widlfly. Those changes in core project pom files to accomodate wildfly10 qare the ones i would try to exlucude from this PR... |
|
@jaimecasero ah, i see, so you're saying i should reverse commit this: a4df2e0 |
| <assembly xmlns="http://maven.apache.org/ASSEMBLY/2.0.0" | ||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://maven.apache.org/ASSEMBLY/2.0.0 http://maven.apache.org/xsd/assembly-2.0.0.xsd"> | ||
| <id>assembly</id> |
There was a problem hiding this comment.
this is the "right" place i think. this id is same as standalone one, and could create potential collision
There was a problem hiding this comment.
this should be addressed with 71fd01f
|
@jaimecasero reverse committed with fe1e250 |
the bindings to the `pre-integration-test` and `post-integration-test` phases only really make sense if/when we want to start some containers during the `verify` phase. I am keeping those execution bindings in, so we have the hooks ready. However, you can *safely remove* the `start` and `stop` executions of the fabric8 docker maven plugin with no problems.
gvagenas
force-pushed
the
feature/mavenized_assembly
branch
from
0128dcd to
1898ff3
Compare
gvagenas
force-pushed
the
master
branch
2 times, most recently
from
9469191 to
c67f142
Compare
gvagenas
force-pushed
the
feature/mavenized_assembly
branch
from
1898ff3 to
1ab97c4
Compare
3 participants
What this PR does / why we need it:
Largely, this PR includes cherry-picked work from the
restcomm1657branch, where there is an ongoing effort to move fromanttomavenfor the generation of the WAR artifacts.On top of that, this PR introduces 2 extra features:
restcomm/restcomm), as well as the "standalone" (restcomm/connect), ported over from the Restcomm-Docker repository.With this, that repository largely (if not entirely) becomes deprecated and reduces our tech debt.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #2474
Special notes for your reviewer:
restcomm1657).restcomm/connectdocker image has already been tested with the new mediaserver docker image, and we can get a simple+1234call running correctly. If there is a more extensive list of tests you would like to run, happy to help with that.