[ 
https://issues.apache.org/jira/browse/QPID-8352?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17784004#comment-17784004
 ] 

ASF GitHub Bot commented on QPID-8352:
--------------------------------------

gemmellr commented on code in PR #225:
URL: https://github.com/apache/qpid-broker-j/pull/225#discussion_r1386428084


##########
doc/developer-guide/src/main/markdown/release-instructions.md:
##########
@@ -183,3 +183,5 @@ Sources are kept in a Git repository. Thus a git client is 
required.
 14. Remove the previous release binaries from 
<https://dist.apache.org/repos/dist/release/qpid/broker-j>
     when a new one is announced.
 15. Update jenkins jobs if required.
+16. Docker images can be build and pushed to the container registry according 
to the qpid-docker 
[README.md](https://github.com/apache/qpid-broker-j/tree/main/qpid-docker#readme).
+16. Docker images can be build and pushed to the container registry according 
to the qpid-docker 
[README.md](https://github.com/apache/qpid-broker-j/tree/main/qpid-docker#readme).

Review Comment:
   Duplicate line



##########
qpid-docker/README.md:
##########
@@ -0,0 +1,198 @@
+## Docker Image Example
+
+This is an example on how a Docker Image For Apache Qpid Broker-J based on 
Eclipse Temurin JRE image can be created.
+
+## Building Container Image
+
+Build the parent project using command
+
+```
+mvn clean install -DskipTests=true
+```
+
+Navigate to the module 'qpid-docker':
+
+```
+cd qpid-docker
+```
+
+Execute command
+
+```
+docker-build.sh --local-dist-path <PATH_TO_LOCAL_QPID_DISTRIBUTION>
+```
+
+This will copy all the files necessary to build the pre-configured Docker 
image and provide you with additional 
+instructions. Follow these instructions to finish building the image you want 
based on one of the provided Docker file 
+or even one of your own.
+
+If you would rather use an official Apache release in your image rather than a 
local release then run the following 
+command from the qpid-docker directory where <QPID_VERSION> is the release 
version you wish to use (e.g. 9.1.0):
+
+```
+docker-build.sh --qpid-version <QPID_VERSION>
+```
+
+This will download the Qpid Broker-J release and copy all the files necessary 
to build the pre-configured Docker image
+and provide you with additional instructions. Follow these instructions to 
finish building the image you want based on 
+the provided Docker file or even one of your own.
+
+### Container Structure
+
+Broker-J files are copied to the folder /qpid-broker-j \
+This folder belongs to user qpid, which is part of the root group. Java 
process is executed under the qpid user as well.
+
+### Running the Container
+
+Container can be started using following command:
+```
+docker run -d -p 5672:5672 -p 8080:8080 --name qpid <IMAGE_NAME>
+```
+There are two ports exposed: 5672 for AMQP connections and 8080 for HTTP 
connections.
+
+There are following environment variables available when running the container:
+
+| Environment Variable | Description                                           
                       |
+|----------------------|------------------------------------------------------------------------------|
+| JAVA_GC              | JVM Garbage Collector parameters, default value 
"-XX:+UseG1GC"               |
+| JAVA_MEM             | JVM memory parameters, default value "-Xmx300m 
-XX:MaxDirectMemorySize=200m" |
+| JAVA_OPTS            | Further JVM parameters, empty by default              
                       |
+
+#### Container Volume
+
+Before starting the container a volume should be created:

Review Comment:
   Same comment as the main docs



##########
qpid-docker/Containerfile:
##########
@@ -0,0 +1,79 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+ARG OS_NAME=alpine

Review Comment:
   I would default to ubuntu, same way e.g temurin itself does as visible below.



##########
qpid-docker/README.md:
##########
@@ -0,0 +1,198 @@
+## Docker Image Example
+
+This is an example on how a Docker Image For Apache Qpid Broker-J based on 
Eclipse Temurin JRE image can be created.
+
+## Building Container Image
+
+Build the parent project using command
+
+```
+mvn clean install -DskipTests=true
+```
+
+Navigate to the module 'qpid-docker':
+
+```
+cd qpid-docker
+```
+
+Execute command
+
+```
+docker-build.sh --local-dist-path <PATH_TO_LOCAL_QPID_DISTRIBUTION>
+```
+
+This will copy all the files necessary to build the pre-configured Docker 
image and provide you with additional 
+instructions. Follow these instructions to finish building the image you want 
based on one of the provided Docker file 
+or even one of your own.
+
+If you would rather use an official Apache release in your image rather than a 
local release then run the following 
+command from the qpid-docker directory where <QPID_VERSION> is the release 
version you wish to use (e.g. 9.1.0):
+
+```
+docker-build.sh --qpid-version <QPID_VERSION>
+```
+
+This will download the Qpid Broker-J release and copy all the files necessary 
to build the pre-configured Docker image
+and provide you with additional instructions. Follow these instructions to 
finish building the image you want based on 
+the provided Docker file or even one of your own.
+
+### Container Structure
+
+Broker-J files are copied to the folder /qpid-broker-j \
+This folder belongs to user qpid, which is part of the root group. Java 
process is executed under the qpid user as well.
+
+### Running the Container
+
+Container can be started using following command:
+```
+docker run -d -p 5672:5672 -p 8080:8080 --name qpid <IMAGE_NAME>
+```
+There are two ports exposed: 5672 for AMQP connections and 8080 for HTTP 
connections.
+
+There are following environment variables available when running the container:
+
+| Environment Variable | Description                                           
                       |
+|----------------------|------------------------------------------------------------------------------|
+| JAVA_GC              | JVM Garbage Collector parameters, default value 
"-XX:+UseG1GC"               |
+| JAVA_MEM             | JVM memory parameters, default value "-Xmx300m 
-XX:MaxDirectMemorySize=200m" |
+| JAVA_OPTS            | Further JVM parameters, empty by default              
                       |
+
+#### Container Volume
+
+Before starting the container a volume should be created:
+
+```
+docker volume create --driver local --opt 
device=<PATH_TO_FOLDER_ON_HOST_MACHINE> --opt type=none --opt o=bind qpid_volume
+```
+
+Then container using created volume can be started as follows:
+
+```
+docker run -d -p 5672:5672 -p 8080:8080 -v qpid_volume:/qpid-broker-j/work 
--name qpid <IMAGE_NAME>
+```
+or
+```
+podman run -d -p 5672:5672 -p 8080:8080 -v qpid_volume:/qpid-broker-j/work:Z 
--name qpid <IMAGE_NAME>
+```
+When container is started for the first time, configuration files become 
available in the mapped folder being persisted 
+after subsequent restarts.
+
+### Broker Users
+
+Default configuration provides two preconfigured broker users:
+- admin (password 'admin')
+- guest (password 'guest')

Review Comment:
   The hard coded user+pass could be a bit annoying. Could they be updateable 
via env variable and either passed as config properties when starting the 
broker, or manipulate the config file when copying it?
   
   Does it really need 2 users by default if others can be added by management 
or custom config?



##########
qpid-docker/README.md:
##########
@@ -0,0 +1,198 @@
+## Docker Image Example
+
+This is an example on how a Docker Image For Apache Qpid Broker-J based on 
Eclipse Temurin JRE image can be created.
+
+## Building Container Image
+
+Build the parent project using command

Review Comment:
   Only someone wanting a locally altered non-release broker/image should ever 
need to do this. Normally folks should just be using a release. Instructions 
should be updated to prioritise release-based builds, then mention custom local 
builds after that.



##########
qpid-docker/README.md:
##########
@@ -0,0 +1,198 @@
+## Docker Image Example
+
+This is an example on how a Docker Image For Apache Qpid Broker-J based on 
Eclipse Temurin JRE image can be created.
+
+## Building Container Image
+
+Build the parent project using command
+
+```
+mvn clean install -DskipTests=true
+```
+
+Navigate to the module 'qpid-docker':
+
+```
+cd qpid-docker
+```
+
+Execute command
+
+```
+docker-build.sh --local-dist-path <PATH_TO_LOCAL_QPID_DISTRIBUTION>
+```
+
+This will copy all the files necessary to build the pre-configured Docker 
image and provide you with additional 
+instructions. Follow these instructions to finish building the image you want 
based on one of the provided Docker file 
+or even one of your own.
+
+If you would rather use an official Apache release in your image rather than a 
local release then run the following 
+command from the qpid-docker directory where <QPID_VERSION> is the release 
version you wish to use (e.g. 9.1.0):
+
+```
+docker-build.sh --qpid-version <QPID_VERSION>
+```
+
+This will download the Qpid Broker-J release and copy all the files necessary 
to build the pre-configured Docker image
+and provide you with additional instructions. Follow these instructions to 
finish building the image you want based on 
+the provided Docker file or even one of your own.
+
+### Container Structure
+
+Broker-J files are copied to the folder /qpid-broker-j \
+This folder belongs to user qpid, which is part of the root group. Java 
process is executed under the qpid user as well.
+
+### Running the Container
+
+Container can be started using following command:
+```
+docker run -d -p 5672:5672 -p 8080:8080 --name qpid <IMAGE_NAME>
+```
+There are two ports exposed: 5672 for AMQP connections and 8080 for HTTP 
connections.
+
+There are following environment variables available when running the container:
+
+| Environment Variable | Description                                           
                       |
+|----------------------|------------------------------------------------------------------------------|
+| JAVA_GC              | JVM Garbage Collector parameters, default value 
"-XX:+UseG1GC"               |
+| JAVA_MEM             | JVM memory parameters, default value "-Xmx300m 
-XX:MaxDirectMemorySize=200m" |
+| JAVA_OPTS            | Further JVM parameters, empty by default              
                       |
+
+#### Container Volume
+
+Before starting the container a volume should be created:
+
+```
+docker volume create --driver local --opt 
device=<PATH_TO_FOLDER_ON_HOST_MACHINE> --opt type=none --opt o=bind qpid_volume
+```
+
+Then container using created volume can be started as follows:
+
+```
+docker run -d -p 5672:5672 -p 8080:8080 -v qpid_volume:/qpid-broker-j/work 
--name qpid <IMAGE_NAME>
+```
+or
+```
+podman run -d -p 5672:5672 -p 8080:8080 -v qpid_volume:/qpid-broker-j/work:Z 
--name qpid <IMAGE_NAME>
+```
+When container is started for the first time, configuration files become 
available in the mapped folder being persisted 
+after subsequent restarts.
+
+### Broker Users
+
+Default configuration provides two preconfigured broker users:
+- admin (password 'admin')
+- guest (password 'guest')
+
+User 'admin' has read and write access to all broker objects.
+User 'guest' has read-only access to all broker objects, can not send or 
consume messages.
+
+Further broker users as well as other broker objects (queues, exchanges, 
keystores, truststore, ports etc.)
+can be created via HTTP management interface. Description of the broker REST 
API can be found in broker book (chapter 6.3).
+
+To change user password following command can be used:
+
+```
+curl -d '{"password": "<NEW_PASSWORD>"}' 
http://admin:admin@localhost:8080/api/latest/user/plain/<USERNAME>
+```
+
+## Broker Customization
+
+To customize broker before building the container image, its configuration 
files may be edited to start broker with
+queues, exchanges, users or other objects.
+
+The file config.json contains definitions of the broker objects and references 
a file containing definitions 
+of virtualhost objects (exchanges and queues).
+
+It may be helpful first to create broker objects needed via broker web GUI or 
via REST API, and then investigate the 
+configuration files and copy the appropriate definitions to the configuration 
files used for container image creation.

Review Comment:
   I'd expect detail around this to be in/referenced-by the user manual bits as 
commented previously.
   
   This still seems tailored to only customising the default image config 
before creation though and building it with this module. Users should just have 
a way they can supply replacement config files to be used by the existing image 
on first start, not need to track down the source release archive for the image 
they start with in order to get this module and modify it and build a new image.
   
   E.g the activemq-artemis image lets people provide override files by 
providing them in a directory they will be copied from, over top of the default 
files that were just created: 
https://github.com/apache/activemq-artemis/blob/2.31.2/artemis-docker/docker-run.sh#L38-L40.
 That way users can use the existing image but configure things how they like. 
That feature was contributed by a user in 
https://github.com/apache/activemq-artemis/pull/4575 after the first Artemis 
image was published, which behaved a lot like this one currently would seem to.



##########
qpid-docker/docker-build.sh:
##########
@@ -0,0 +1,175 @@
+#!/bin/sh
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from_local_dist=
+from_release=
+local_dist_path=
+qpid_version=
+MY_NAME=$(basename "$0")
+
+print_help()
+{
+  cat << END_OF_HELP
+Usage: $MY_NAME [OPTION]...
+
+ options:
+
+  --local-dist-path  Path to the local Apache Qpid Broker-J distribution
+  --qpid-version     Apache Qpid Broker-J version

Review Comment:
   The release based option should go first, its the normal use case. The 
description could be clearer, something like "Apache Qpid Broker-J release 
version to build with" and "Path to the local Apache Qpid Broker-J distribution 
to build with"
   
   Using "release" within the actual option name for release-version builds 
rather than simply "qpid-version" would also help make it more obvious that its 
a choice of 'release vs local-build' distribution.



##########
qpid-docker/Containerfile:
##########
@@ -0,0 +1,79 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+ARG OS_NAME=alpine
+
+#
+# Builder images
+#
+
+# Alpine
+FROM docker.io/library/eclipse-temurin:17-jre-alpine AS builder-alpine
+RUN adduser -u 1001 -G root qpid --disabled-password
+
+# Ubuntu
+FROM docker.io/library/eclipse-temurin:17-jre AS builder-ubuntu
+RUN useradd -u 1001 -G root qpid
+
+#
+# Final image
+#
+FROM builder-${OS_NAME}
+
+ARG BROKER_VERSION="unknown"
+
+# Labels
+LABEL description="Apache Qpid Broker-J ${BROKER_VERSION}"
+LABEL io.k8s.display-name="qpid-broker-j ${BROKER_VERSION}"
+LABEL io.k8s.description="Apache Qpid Broker-J ${BROKER_VERSION} using BDB as 
a message storage"
+LABEL maintainer="Apache Qpid Team, us...@qpid.apache.org"
+LABEL name="Apache Qpid Broker-J"
+LABEL summary="Apache Qpid Broker-J ${BROKER_VERSION} using BDB as a message 
storage"

Review Comment:
   I would drop the "using BDB as a message storage" and leave simply "Apache 
Qpid Broker-J ${BROKER_VERSION}" since there are no longer a multitude of 
variant images at this point, and it may not actually match a custom config 
used anyway.



##########
doc/java-broker/src/docbkx/Java-Broker-Docker.xml:
##########
@@ -0,0 +1,141 @@
+<?xml version="1.0"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<chapter xmlns="http://docbook.org/ns/docbook"; version="5.0" 
xml:id="Java-Broker-Docker">
+    <title>Docker Images</title>
+
+    <section xml:id="Java-Broker-Docker-Docker-Image">
+
+        <title>Docker Image</title>
+
+        <para>
+            By default broker images is built with BDB message storage.
+        </para>
+
+    </section>
+
+    <section xml:id="Java-Broker-Docker-Running-The-Container">
+
+        <title>Running the Container</title>
+
+        <section xml:id="Java-Broker-Docker-Container-Start">
+
+            <title>Container Start</title>
+
+            <para>
+                Before starting the container a volume should be created:
+            </para>
+
+            <para>
+                <programlisting>
+                    docker volume create --driver local --opt 
device=&lt;PATH_TO_FOLDER> --opt type=none --opt o=bind qpid_volume
+                </programlisting>
+            </para>
+
+            <para>
+                Then container using created volume can be started:
+            </para>
+
+            <para>
+                <programlisting>
+                    docker run -d -p 5672:5672 -p 8080:8080 -v 
qpid_volume:/qpid-broker-j/work --name qpid &lt;IMAGE_NAME>
+                </programlisting>
+            </para>
+
+            <para>
+                or
+            </para>
+
+            <para>
+                <programlisting>
+                    podman run -d -p 5672:5672 -p 8080:8080 -v 
qpid_volume:/qpid-broker-j/work:Z --name qpid &lt;IMAGE_NAME>
+                </programlisting>
+            </para>
+
+            <para>
+                There are two ports exposed: 5672 for AMQP connections and 
8080 for HTTP connections.
+            </para>
+
+            <para>
+                There are following environment variables available when 
running the container:
+            </para>
+
+            <table>
+                <title>Environment Variables</title>
+                <tgroup cols="2">
+                    <colspec colnum="1" colname="variable" colwidth="1*"/>
+                    <colspec colnum="2" colname="description" colwidth="1*"/>
+                    <thead>
+                        <row>
+                            <entry>Environment Variable</entry>
+                            <entry>Description</entry>
+                        </row>
+                    </thead>
+                    <tbody>
+                        <row>
+                            <entry>JAVA_GC</entry>
+                            <entry>JVM Garbage Collector parameters, default 
value "-XX:+UseG1GC"</entry>
+                        </row>
+                        <row>
+                            <entry>JAVA_MEM</entry>
+                            <entry>JVM memory parameters, default value 
"-Xmx300m -XX:MaxDirectMemorySize=200m"</entry>
+                        </row>
+                        <row>
+                            <entry>JAVA_OPTS</entry>
+                            <entry>Further JVM parameters, default value is an 
empty string</entry>
+                        </row>
+                    </tbody>
+                </tgroup>
+            </table>
+
+        </section>
+
+    </section>
+
+    <section xml:id="Java-Broker-Docker-Broker-Users">
+
+        <title>Broker Users</title>
+
+        <para>
+            Container has two preconfigured broker users:
+        </para>
+
+        <para>
+            <itemizedlist>
+                <listitem><para>admin (password 'admin')</para></listitem>
+                <listitem><para>guest (password 'guest')</para></listitem>
+            </itemizedlist>
+        </para>
+
+        <para>
+            User 'admin' has read and write access to all broker objects. User 
'guest' has read-only access to all broker
+            objects, can not send or consume messages.
+        </para>
+
+        <para>
+            Further broker users as well as other broker objects (queues, 
exchanges, keystores, truststore, ports etc.)
+            can be created via HTTP management interface.
+        </para>

Review Comment:
   Should be possible to easily supply the config, letting them configure the 
broker any way they like. Not covered by these docs?



##########
doc/java-broker/src/docbkx/Java-Broker-Docker.xml:
##########
@@ -0,0 +1,141 @@
+<?xml version="1.0"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<chapter xmlns="http://docbook.org/ns/docbook"; version="5.0" 
xml:id="Java-Broker-Docker">
+    <title>Docker Images</title>
+
+    <section xml:id="Java-Broker-Docker-Docker-Image">
+
+        <title>Docker Image</title>
+
+        <para>
+            By default broker images is built with BDB message storage.
+        </para>
+
+    </section>
+
+    <section xml:id="Java-Broker-Docker-Running-The-Container">
+
+        <title>Running the Container</title>
+
+        <section xml:id="Java-Broker-Docker-Container-Start">
+
+            <title>Container Start</title>
+
+            <para>
+                Before starting the container a volume should be created:

Review Comment:
   Maybe mention why, or what happens if they dont. I rarely create volumes 
before starting a [throwaway] container.





> Official Docker image for Broker-J
> ----------------------------------
>
>                 Key: QPID-8352
>                 URL: https://issues.apache.org/jira/browse/QPID-8352
>             Project: Qpid
>          Issue Type: Improvement
>          Components: Broker-J
>            Reporter: Chris O'Brien
>            Priority: Minor
>
> Currently there is no official Docker image for Broker-J.
> It would be great if one was provided, as there are more than a few people 
> interested in running Broker-J in a container, shown by the handful of 
> inflexible and un-maintained Dockerfiles/images for Broker-J floating around 
> GitHub/Docker Hub.
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

Reply via email to