ARTEMIS-3042 Add docker multistage build #4307

SamTV12345 · 2022-12-06T21:29:50Z

Added multistage docker image build

This adds the possibility to create an artemis image with just the command:

docker build -f Docker-alpine -t artemis_alpine .

Furthermore it has the smallest attack surface compared to the other images as it is based on alpine and only contains the jre, bash and libaio for running the application.

When releasing a new version simply adjust the arg value at the top and run above command to build a more recent version.

jbertram · 2022-12-06T21:36:41Z

How is this related to https://issues.apache.org/jira/browse/ARTEMIS-4045 (i.e. the Jira referenced in the PR title)?

SamTV12345 · 2022-12-06T21:44:11Z

Sorry my bad. Didn't get that the number refers to a jira ticket.

jbertram · 2022-12-06T22:11:40Z

Is this related to https://issues.apache.org/jira/browse/ARTEMIS-3042? The PR's title seems to correspond. If it is related please include the Jira in the commit message. If not, please create a Jira for this change and include it in the commit message. You can read more details about how to format commit messages in the hacking guide.

SamTV12345 · 2022-12-07T05:47:02Z

Thanks for the hint. I added a message according to your guidelines. This pr solves exactly that issue.

SamTV12345 · 2022-12-07T19:26:42Z

If there are any further questions I am more than welcome to answer them.

jbertram · 2022-12-07T19:44:11Z

It's not clear to me how your changes are directly related to https://issues.apache.org/jira/browse/ARTEMIS-3042 aside from just the wording. Your commit is just adding a new Dockerfile which is similar to the three other Dockerfiles which are already present. These three Dockerfiles were also already present when ARTEMIS-3042 was opened originally. The Jira description even says, "The current docker image build is not really user friendly or convenient at all." Therefore, adding another version of what already exists doesn't seem to help resolve the issue. Can you shed any light on this?

jbertram · 2022-12-07T19:45:50Z

To be clear, I am happy to merge your commit eventually. I just want the Jira/commit association to be clear for posterity's sake.

artemis-docker/docker-run.sh

jbertram · 2022-12-07T19:47:06Z

artemis-docker/prepare-docker.sh

@@ -187,4 +187,4 @@ cp ./docker-run.sh "$ARTEMIS_DIST_DIR/docker"
 echo "Docker file support files at:"
 tree "$ARTEMIS_DIST_DIR/docker"

-next_step
+next_step


This change also isn't necessary.

jbertram

Please remove the unnecessary changes. Thanks!

SamTV12345 · 2022-12-07T20:33:37Z

It's not clear to me how your changes are directly related to https://issues.apache.org/jira/browse/ARTEMIS-3042 aside from just the wording. Your commit is just adding a new Dockerfile which is similar to the three other Dockerfiles which are already present. These three Dockerfiles were also already present when ARTEMIS-3042 was opened originally. The Jira description even says, "The current docker image build is not really user friendly or convenient at all." Therefore, adding another version of what already exists doesn't seem to help resolve the issue. Can you shed any light on this?

A docker multistage build contains one or multiple steps of building an image and one final image that is used during runtime. A multistage build has the advantage that some components are only needed during build time leading to a smaller footprint. It also has the charm that I don't need to know anything about building this image. Everything is handled during build process.

Before my pr you had to follow the guides, run the script, cd into the _TMP/artemis/version folder and start the build process. This can be teadious and result in user errors: At first I forgot to change to that directory I had to read through the proprietary instructions to this project.

Now you can start directly with building. Just use the Dockerfile as you would with any other Dockerfile and execute docker build -t myartemis -f Dockerfile-alpine . You don't need to go through any instructions and have a standardized process of building images.

If you agree I would remove the other Dockerfiles as they are not needed anymore and may confuse the end user.

SamTV12345 · 2022-12-07T20:40:04Z

The other two files are solved. Seems Windows/IntelliJ doesn't add a newline to the file.

SamTV12345 · 2022-12-08T14:02:57Z

Seems like the failing test is not the cause of my commit. The other pr is also failing.

SamTV12345 · 2022-12-09T14:17:45Z

When will this be merged? Is there something to do from my side?

jbertram · 2022-12-09T21:04:04Z

At this point two main things are not clear in your commit:

How to use your new Docker file.
How your new Docker file fits with the other Docker files already available.

To resolve # 1 you should probably just add some documentation. You could do this, for example, in the artemis-docker/readme.md.

To resolve # 2 you might also just add documentation. However, if the approach you've taken in this new Docker file is superior to the approach offered by the existing Docker files then it may be worth updating those to use your new approach so that users don't need to run prepare-docker.sh manually but they still have the option of using CentOS & Ubuntu with JDK or JRE.

SamTV12345 · 2022-12-09T22:51:01Z

I'm done. I adapted the README.md to reflect the changes in the build process and added the multistage capability to every Dockerfile.

SamTV12345 · 2022-12-12T17:41:10Z

I guess this pr can be merged.

jbertram · 2022-12-12T17:46:40Z

I'm still in the process of testing all the changes locally. I have other priorities on my plate at the moment so progress has stopped at the moment.

@SamTV12345, is there any particular rush to get this merged? If so, how come?

jbertram · 2022-12-12T17:47:08Z

@clebertsuconic, can you take a look at this as well since I believe you originally created the Docker files.

SamTV12345 · 2022-12-12T18:09:08Z

Thanks for reporting back. I'd like to get this pr merged as soon as possible so I can move on to the next ticket. I guess it doesn't make much sense that I create a lot of pr's that don't get merged at the end.
But I can understand that you have other priorities. I'll wait till I hear back from you.

jbertram · 2022-12-12T18:55:01Z

I'd like to get this pr merged as soon as possible so I can move on to the next ticket.

Is this PR a dependency for another task you're working on?

I think most folks work on tasks in parallel. Once they send a PR they move on to their next task knowing that the PR will be reviewed and merged asynchronously.

I guess it doesn't make much sense that I create a lot of pr's that don't get merged at the end.

Have you created other PRs that haven't been merged? I looked through the list of open and closed PRs and I don't see any others from you. Perhaps you used a different account when you created those?

ViliusS · 2023-04-27T06:19:21Z

I think Docker multistage builds are great. I don't need to configure anything on the host machine and can simply press "build" in Jenkins.

I don't argue that, but as I said, such images are great only for final deployment or local development.

Other than that you can still always customize the Dockerfile by cloning your own Dockerfile before docker build and replace the standard build with your own.

I know that, however this PR changes the purpose of these Dockerfiles. Previously they contained only configuration needed for the runtime and served more as a tool to build your own custom images. Now they contain all the build flow inside, hence they now serve just as an easier way to run Artemis.

ViliusS · 2023-04-27T11:16:30Z

I gave this more thought today. In theory, if the requirement is to just conveniently allow users to build docker image and run it locally there is a docker-compose exactly for that purpose. All prepare/run steps can be automated via Docker Compose YAML file.
It also allows for better build and runtime separation.

SamTV12345 · 2023-04-27T14:39:34Z

The question is how minimal is the produced image? Could you create a docker-compose with a few steps that would satisfy the requirement for different base images? If we talk about a docker-compose yaml you also need suitable tooling in form of a docker compose cli plugin or v1 docker-compose. If you could easily switch between base images and select them by a build arg that would indeed be interesting. I am unfortunately currently pretty involved in other FOSS projects so if you could please go through creating said yaml I should be able to adapt the scripts and add the alpine image which was one of the reasons this pr exists.

amarkevich · 2023-05-31T19:21:04Z

Minor suggestion:

apt install tree curl -y

can be replaced with existing tools:

tree -> ls -l (easy one)
curl -> wget (more complex)

SamTV12345 · 2023-05-31T19:42:45Z

Minor suggestion:

apt install tree curl -y

can be replaced with existing tools:

tree -> ls -l (easy one)

curl -> wget (more complex)

This is performed in the builder section. This section is executed one time at the beginning of the pipeline. Any subsequent pipeline runs in the future will have this command cached and it will never be rerun as it is done just at the beginning. I tuned the Dockerfile to cache the things that are static like apt packages and add the dynamic parts of the app e.g. artemis at the end.

amarkevich · 2023-06-01T09:24:52Z

command cached

anyway 'tree' can be safely removed #4453

SamTV12345 · 2023-06-01T21:08:15Z

command cached

anyway 'tree' can be safely removed #4453

Removed.

amarkevich · 2023-06-01T22:21:55Z

artemis-docker/prepare-docker.sh

@@ -181,10 +163,9 @@ if [ ! -d "${ARTEMIS_DIST_DIR}/docker" ]; then
  mkdir "${ARTEMIS_DIST_DIR}/docker"
 fi

-cp ./Dockerfile-* "$ARTEMIS_DIST_DIR/docker"
 cp ./docker-run.sh "$ARTEMIS_DIST_DIR/docker"

 echo "Docker file support files at:"
 tree "$ARTEMIS_DIST_DIR/docker"


its still there

What exactly is still there? tree has been removed.

'tree' command in L188 will cause build error since not available in build stage. I propose to remove it like done in https://github.com/apache/activemq-artemis/pull/4453/files#diff-09d0fd7b380274547af0cdaf1cb41c22b3990dfed9848b560e9d9f77b97fb204L188
or replace with 'ls -l'

ViliusS · 2023-06-02T05:39:19Z

The question is how minimal is the produced image? Could you create a docker-compose with a few steps that would satisfy the requirement for different base images? If we talk about a docker-compose yaml you also need suitable tooling in form of a docker compose cli plugin or v1 docker-compose. If you could easily switch between base images and select them by a build arg that would indeed be interesting. I am unfortunately currently pretty involved in other FOSS projects so if you could please go through creating said yaml I should be able to adapt the scripts and add the alpine image which was one of the reasons this pr exists.

I didn't had much time lately, but I will try to come up with YAML file which does the same as this PR this week.

Produced image won't change because this doesn't depend on a tooling. Compose plugin is installed by default with Docker Desktop, so it's not a problem either.

SamTV12345 · 2023-06-02T20:59:33Z

I removed the leftover tree command. I maybe can ditch all these sh files and build it with docker-compose. @ViliusS I'll try that today. Maybe I find a clever way to build that with docker-compose

SamTV12345 · 2023-06-02T22:29:35Z

The question is how minimal is the produced image? Could you create a docker-compose with a few steps that would satisfy the requirement for different base images? If we talk about a docker-compose yaml you also need suitable tooling in form of a docker compose cli plugin or v1 docker-compose. If you could easily switch between base images and select them by a build arg that would indeed be interesting. I am unfortunately currently pretty involved in other FOSS projects so if you could please go through creating said yaml I should be able to adapt the scripts and add the alpine image which was one of the reasons this pr exists.

I didn't had much time lately, but I will try to come up with YAML file which does the same as this PR this week.

Produced image won't change because this doesn't depend on a tooling. Compose plugin is installed by default with Docker Desktop, so it's not a problem either.

It's implemented. It is now save to review and hopefully merge.

ViliusS · 2023-06-11T09:35:44Z

I've went through couple of different YAML and bash solution iterations today. I have also tested the solution provided in this PR. I still have to conclude that at least for us multistage builds give more headache than it solves.

My main concern, as mentioned previously, is that Artemis preparation process will be integrated into Docker image which doesn't allow for easy customization. For example previously we had such Jenkins CI pipeline:

      stage('prepare configuration') {
        steps {
          sh "./prepare-docker.sh --from-release --artemis-version $ARTEMIS_VERSION"
          sh "........ our modifications and custom configuration goes here........."
        }
      }
      stage('build image') {
        steps {
          script {
            docker.withRegistry(env.DOCKER__REPOSITORY, env.DOCKER_LOGIN) {
              sh '''
                cd _TMP_/artemis/$ARTEMIS_VERSION
                docker build -f ./docker/Dockerfile-ubuntu-17-jre -t $BUILD_IMAGE $BUILD_DIR
                docker push $BUILD_IMAGE
                rm -rf _TMP_
              '''
            }
          }
        }
      }

If this PR is ought to be accepted we won't be able to modify Artemis image as easily anymore. We would need to download official docker file, modify it by using "patch" or similar tools and only then build our image. This means we would need to maintain our own Dockerfile which looses the purpose of having official Docker file altogether. It also integrates preparation process into build process which lowers visibility in Jenkins (or any other CI system) UI.

The final word will be on Artemis maintainers, by my proposal is to leave current Docker files as their are. If building Artemis images for fast local development is really a problem (which I don't think it is to run 2 commands instead of 1), then have separate Docker files. Or better yet, have official Docker image uploaded to Docker Hub for easy run with default settings. That way Docker files can stay in the repo for those wanting to modify the image.

On a side note, what I would really want to see happening is proper customization via environment variables or Docker entrypoint as mentioned in ARTEMIS-3042.

SamTV12345 · 2023-06-11T16:09:34Z

What exactly is this step sh "........ our modifications and custom configuration goes here........." and can't you simply do this before the jar is put into the container? The final build step is simply take the compiled jar with modifications if you like and put it into the various docker images.

ViliusS · 2023-06-11T20:20:24Z

What exactly is this step sh "........ our modifications and custom configuration goes here........." and can't you simply do this before the jar is put into the container? The final build step is simply take the compiled jar with modifications if you like and put it into the various docker images.

Modifications to web console for example.

In the past this was also the place where we applied our own patches before building Artemis from source.

SamTV12345 · 2023-06-11T20:28:01Z

Well that should be possible without a problem. You do your changes in the jenkins pipeline first, then compile it on the machine with mvn clean install or a similar command and then simply copy it to ./artemis-docker/target/apache-artemis*-bin/*SNAPSHOT . After that just docker-compose -f artemis-docker/docker-compose-final.yml up -d and you are done.

ViliusS · 2023-06-12T05:38:37Z

Well that should be possible without a problem. You do your changes in the jenkins pipeline first, then compile it on the machine with mvn clean install or a similar command and then simply copy it to ./artemis-docker/target/apache-artemis*-bin/*SNAPSHOT . After that just docker-compose -f artemis-docker/docker-compose-final.yml up -d and you are done.

As I said multiple times before, we can't without modifying provided Dockerfile. For modification to happen tarball needs to be downloaded and extracted first.

clebertsuconic · 2023-06-13T19:35:53Z

@SamTV12345 @jbertram what is really the advantage of using this? just to avoid the prepare phase?

This is doing a build all over again?

The ./prepare.sh and build seemed a lot simpler to me.

if you really wanted to have this committed, we could create a new folder ./artemis-distribution-multistage ... or move all the docker possibilities under a single directory.

although it gets complicated with competing things.. we already have the ./artemis-image being a different OCI image by @gtully

clebertsuconic · 2023-06-13T20:32:55Z

@ViliusS do you understand the advantage of multistage builds?

@SamTV12345 I'm trying to understand the difference here? what's the issue with ./prepare.sh?

again.. this seems moot by ./artemis-image ... did you look at it?

clebertsuconic · 2023-06-13T20:54:41Z

is this making a docker official build any easier? is that the reason behind this?

clebertsuconic · 2023-06-13T21:02:21Z

it would be nice if we could chat somewhere? Slack at Apache?

at the end of build it just hangs as it's running the brokers

SamTV12345 · 2023-06-13T21:19:51Z

is this making a docker official build any easier? is that the reason behind this?

This turbocharges new builds. You can build all platforms in next to no time and push it to Dockerhub. It streamlines the build process. Before that you had to go through the steps in the ./prepare.sh. Then go to the sub directory, then start building dockerfiles one after the other. If I want to start the whole build that would mean 1 x prepare.sh, 5-6 times Dockerfiles with the right tags. Non parallel builds. With this approach I can build all 6 images in around 5 minutes with minimum resource usage of the maven repository.
This docker compose also allows for non contributors/people new to the project build images with their known tooling docker and docker-compose.
Last but not least you can simple select an image with docker composer service up.

SamTV12345 · 2023-06-13T21:22:09Z

it would be nice if we could chat somewhere? Slack at Apache?

at the end of build it just hangs as it's running the brokers

You can add my developer email [email protected] and we can go through the build process. Without any screenshots/output its hard to evaluate.

ViliusS · 2023-06-13T22:16:45Z

@ViliusS do you understand the advantage of multistage builds?

If you are asking in general, then yes. It's just a mechanism to separate build stage from the runtime.

In this particular case I see no advantage, sorry. These dockerfiles previously were used just for runtime, by packing downloaded already built releases. So the separation was already there.

clebertsuconic · 2023-06-14T14:32:05Z

@SamTV12345 I had everything ready on my build locally, building the image for me would been a couple seconds... and this made everything from scratch, including downloading the entire maven artifacts somewhere.

I wouldn't have a problem if this was an additional option to build the image, meaning if I want to use the "regular" way we could just do what we have been doing so far. in that sense if you could reuse the docker files as they are now, and just call the "multi-stage" with docker compose build as an option.. then I would be ok... as long as it is well documented.

This adds the possibility to create an artemis image with just the docker compose build command. First the image is downloaded in an Eclipse Temurin installation and compiled with a mounted volume. After that a second docker compose build command can be used to put the build artifact into the different containers. Thus activemq-artemis is only built once resulting in a very fast compile time.

SamTV12345 · 2023-10-16T11:32:28Z

@clebertsuconic I rebased the commits. What should be done for this pr to be merged?

SamTV12345 changed the title ~~ARTEMIS 4045 - Added multistage docker build for an alpine image.~~ Added multistage docker build for an alpine image. Dec 6, 2022

SamTV12345 force-pushed the main branch from 0a42347 to 0dff428 Compare December 7, 2022 05:45

SamTV12345 changed the title ~~Added multistage docker build for an alpine image.~~ ARTEMIS-3042 Add docker multistage build Dec 7, 2022

jbertram reviewed Dec 7, 2022

View reviewed changes

artemis-docker/docker-run.sh Outdated Show resolved Hide resolved

jbertram reviewed Dec 7, 2022

View reviewed changes

jbertram requested changes Dec 7, 2022

View reviewed changes

SamTV12345 force-pushed the main branch 2 times, most recently from 3914b7f to 2ac4f0e Compare December 7, 2022 20:39

jbertram approved these changes Dec 8, 2022

View reviewed changes

SamTV12345 force-pushed the main branch from 6f4226d to 7f35dff Compare December 9, 2022 22:48

SamTV12345 requested a review from jbertram December 10, 2022 14:14

jbertram requested a review from clebertsuconic December 12, 2022 17:47

SamTV12345 force-pushed the main branch from 2b34b8e to ceb9725 Compare June 1, 2023 21:07

amarkevich reviewed Jun 1, 2023

View reviewed changes

SamTV12345 force-pushed the main branch from ceb9725 to a83f292 Compare June 2, 2023 20:03

SamTV12345 force-pushed the main branch from a83f292 to 95f18c3 Compare June 2, 2023 22:28

SamTV12345 force-pushed the main branch from 95f18c3 to f7f1eec Compare October 16, 2023 11:26

ARTEMIS-3042 Add docker multistage build #4307

Are you sure you want to change the base?

ARTEMIS-3042 Add docker multistage build #4307

Conversation

SamTV12345 commented Dec 6, 2022 • edited Loading

jbertram commented Dec 6, 2022 • edited Loading

SamTV12345 commented Dec 6, 2022

jbertram commented Dec 6, 2022

SamTV12345 commented Dec 7, 2022

SamTV12345 commented Dec 7, 2022

jbertram commented Dec 7, 2022

jbertram commented Dec 7, 2022

jbertram Dec 7, 2022

Choose a reason for hiding this comment

jbertram left a comment

Choose a reason for hiding this comment

SamTV12345 commented Dec 7, 2022

SamTV12345 commented Dec 7, 2022

SamTV12345 commented Dec 8, 2022

SamTV12345 commented Dec 9, 2022

jbertram commented Dec 9, 2022 • edited Loading

SamTV12345 commented Dec 9, 2022

SamTV12345 commented Dec 12, 2022

jbertram commented Dec 12, 2022

jbertram commented Dec 12, 2022 • edited Loading

SamTV12345 commented Dec 12, 2022

jbertram commented Dec 12, 2022

ViliusS commented Apr 27, 2023 • edited Loading

ViliusS commented Apr 27, 2023

SamTV12345 commented Apr 27, 2023

amarkevich commented May 31, 2023 • edited Loading

SamTV12345 commented May 31, 2023

amarkevich commented Jun 1, 2023

SamTV12345 commented Jun 1, 2023

amarkevich Jun 1, 2023

Choose a reason for hiding this comment

SamTV12345 Jun 2, 2023

Choose a reason for hiding this comment

amarkevich Jun 2, 2023

Choose a reason for hiding this comment

ViliusS commented Jun 2, 2023 • edited Loading

SamTV12345 commented Jun 2, 2023

SamTV12345 commented Jun 2, 2023

ViliusS commented Jun 11, 2023

SamTV12345 commented Jun 11, 2023 • edited Loading

ViliusS commented Jun 11, 2023

SamTV12345 commented Jun 11, 2023

ViliusS commented Jun 12, 2023

clebertsuconic commented Jun 13, 2023

clebertsuconic commented Jun 13, 2023

clebertsuconic commented Jun 13, 2023

clebertsuconic commented Jun 13, 2023

SamTV12345 commented Jun 13, 2023

SamTV12345 commented Jun 13, 2023

ViliusS commented Jun 13, 2023

clebertsuconic commented Jun 14, 2023 • edited Loading

SamTV12345 commented Oct 16, 2023

SamTV12345 commented Dec 6, 2022 •

edited

Loading

jbertram commented Dec 6, 2022 •

edited

Loading

jbertram commented Dec 9, 2022 •

edited

Loading

jbertram commented Dec 12, 2022 •

edited

Loading

ViliusS commented Apr 27, 2023 •

edited

Loading

amarkevich commented May 31, 2023 •

edited

Loading

ViliusS commented Jun 2, 2023 •

edited

Loading

SamTV12345 commented Jun 11, 2023 •

edited

Loading

clebertsuconic commented Jun 14, 2023 •

edited

Loading