You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by ceharris <gi...@git.apache.org> on 2017/12/06 13:01:53 UTC
[GitHub] guacamole-server pull request #126: GUACAMOLE-456: use Docker multi-stage bu...
GitHub user ceharris opened a pull request:
https://github.com/apache/guacamole-server/pull/126
GUACAMOLE-456: use Docker multi-stage build
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/soulwing/guacamole-server master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/guacamole-server/pull/126.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #126
----
commit e4f4761c8781dfc48c7b746cfad22cb8e4f490c7
Author: Carl Harris <ce...@vt.edu>
Date: 2017-12-06T12:57:12Z
GUACAMOLE-456: use Docker multi-stage build
----
---
[GitHub] guacamole-server pull request #126: GUACAMOLE-456: use Docker multi-stage bu...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/126#discussion_r158589014
--- Diff: Dockerfile ---
@@ -76,13 +68,63 @@ COPY src/guacd-docker/bin /opt/guacd/bin/
COPY . "$BUILD_DIR"
# Build guacamole-server from local source
-RUN yum -y install $BUILD_DEPENDENCIES && \
- /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" && \
- rm -Rf "$BUILD_DIR" && \
- yum -y autoremove $BUILD_DEPENDENCIES && \
- yum clean all
+RUN /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" "$PREFIX_DIR"
-# Start guacd, listening on port 0.0.0.0:4822
+# Use same CentOS as the base for the runtime image
+FROM centos:${CENTOS_VERSION}
+
+# Base directory for installed build artifacts.
+# Due to limitations of the Docker image build process, this value is
+# duplicated in an ARG in the first stage of the build. See also the
+# CMD directive at the end of this build stage.
+#
+ARG PREFIX_DIR=/usr/local/guacamole
+
+# Runtime environment
+ENV LC_ALL=en_US.UTF-8
+
+ARG RUNTIME_DEPENDENCIES=" \
+ cairo \
+ dejavu-sans-mono-fonts \
+ freerdp \
+ freerdp-plugins \
+ ghostscript \
+ libjpeg-turbo \
+ libssh2 \
+ liberation-mono-fonts \
+ libtelnet \
+ libvorbis \
+ libvncserver \
+ libwebp \
+ pango \
+ pulseaudio-libs \
+ terminus-fonts \
+ uuid"
+
+# Bring runtime environment up to date and install runtime dependencies
+RUN yum -y update && \
+ yum -y install epel-release && \
+ yum -y install $RUNTIME_DEPENDENCIES && \
+ yum clean all && \
+ rm -rf /var/cache/yum
+
+# Copy build artifacts into this stage
+COPY --from=builder ${PREFIX_DIR} ${PREFIX_DIR}
+
+# Link FreeRDP plugins into proper path
+RUN FREERDP_DIR=$(dirname \
+ $(rpm -ql freerdp-libs | grep 'libfreerdp.*\.so' | head -n1)) && \
+ FREERDP_PLUGIN_DIR="${FREERDP_DIR}/freerdp" && \
+ mkdir -p "$FREERDP_PLUGIN_DIR" && \
+ ln -s "$PREFIX_DIR"/lib/freerdp/*.so "$FREERDP_PLUGIN_DIR"
+
--- End diff --
It does seem a little odd to take care of the linking here. Can the creation of the required links not stay with the build script, with the resulting links considered yet more build artifacts?
---
[GitHub] guacamole-server pull request #126: GUACAMOLE-456: use Docker multi-stage bu...
Posted by ceharris <gi...@git.apache.org>.
Github user ceharris commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/126#discussion_r155232603
--- Diff: Dockerfile ---
@@ -76,13 +68,63 @@ COPY src/guacd-docker/bin /opt/guacd/bin/
COPY . "$BUILD_DIR"
# Build guacamole-server from local source
-RUN yum -y install $BUILD_DEPENDENCIES && \
- /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" && \
- rm -Rf "$BUILD_DIR" && \
- yum -y autoremove $BUILD_DEPENDENCIES && \
- yum clean all
+RUN /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" "$PREFIX_DIR"
-# Start guacd, listening on port 0.0.0.0:4822
+# Use same CentOS as the base for the runtime image
+FROM centos:${CENTOS_VERSION}
+
+# Base directory for installed build artifacts.
+# Due to limitations of the Docker image build process, this value is
+# duplicated in an ARG in the first stage of the build. See also the
+# CMD directive at the end of this build stage.
+#
+ARG PREFIX_DIR=/usr/local/guacamole
+
+# Runtime environment
+ENV LC_ALL=en_US.UTF-8
+
+ARG RUNTIME_DEPENDENCIES=" \
+ cairo \
+ dejavu-sans-mono-fonts \
+ freerdp \
+ freerdp-plugins \
+ ghostscript \
+ libjpeg-turbo \
+ libssh2 \
+ liberation-mono-fonts \
+ libtelnet \
+ libvorbis \
+ libvncserver \
+ libwebp \
+ pango \
+ pulseaudio-libs \
+ terminus-fonts \
+ uuid"
+
+# Bring runtime environment up to date and install runtime dependencies
+RUN yum -y update && \
+ yum -y install epel-release && \
+ yum -y install $RUNTIME_DEPENDENCIES && \
+ yum clean all && \
+ rm -rf /var/cache/yum
+
+# Copy build artifacts into this stage
+COPY --from=builder ${PREFIX_DIR} ${PREFIX_DIR}
+
+# Link FreeRDP plugins into proper path
+RUN FREERDP_DIR=$(dirname \
+ $(rpm -ql freerdp-libs | grep 'libfreerdp.*\.so' | head -n1)) && \
+ FREERDP_PLUGIN_DIR="${FREERDP_DIR}/freerdp" && \
+ mkdir -p "$FREERDP_PLUGIN_DIR" && \
+ ln -s "$PREFIX_DIR"/lib/freerdp/*.so "$FREERDP_PLUGIN_DIR"
+
--- End diff --
This was previously done in the build script, but needs to be deferred until after build artifacts have been copied into the runtime image. Seems small enough to do inline, but could be moved to an external script.
---
[GitHub] guacamole-server pull request #126: GUACAMOLE-456: use Docker multi-stage bu...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/guacamole-server/pull/126
---
[GitHub] guacamole-server pull request #126: GUACAMOLE-456: use Docker multi-stage bu...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/126#discussion_r161844979
--- Diff: Dockerfile ---
@@ -76,13 +68,63 @@ COPY src/guacd-docker/bin /opt/guacd/bin/
COPY . "$BUILD_DIR"
# Build guacamole-server from local source
-RUN yum -y install $BUILD_DEPENDENCIES && \
- /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" && \
- rm -Rf "$BUILD_DIR" && \
- yum -y autoremove $BUILD_DEPENDENCIES && \
- yum clean all
+RUN /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" "$PREFIX_DIR"
-# Start guacd, listening on port 0.0.0.0:4822
+# Use same CentOS as the base for the runtime image
+FROM centos:${CENTOS_VERSION}
+
+# Base directory for installed build artifacts.
+# Due to limitations of the Docker image build process, this value is
+# duplicated in an ARG in the first stage of the build. See also the
+# CMD directive at the end of this build stage.
+#
+ARG PREFIX_DIR=/usr/local/guacamole
+
+# Runtime environment
+ENV LC_ALL=en_US.UTF-8
+
+ARG RUNTIME_DEPENDENCIES=" \
+ cairo \
+ dejavu-sans-mono-fonts \
+ freerdp \
+ freerdp-plugins \
+ ghostscript \
+ libjpeg-turbo \
+ libssh2 \
+ liberation-mono-fonts \
+ libtelnet \
+ libvorbis \
+ libvncserver \
+ libwebp \
+ pango \
+ pulseaudio-libs \
+ terminus-fonts \
+ uuid"
+
+# Bring runtime environment up to date and install runtime dependencies
+RUN yum -y update && \
+ yum -y install epel-release && \
+ yum -y install $RUNTIME_DEPENDENCIES && \
+ yum clean all && \
+ rm -rf /var/cache/yum
+
+# Copy build artifacts into this stage
+COPY --from=builder ${PREFIX_DIR} ${PREFIX_DIR}
+
+# Link FreeRDP plugins into proper path
+RUN FREERDP_DIR=$(dirname \
+ $(rpm -ql freerdp-libs | grep 'libfreerdp.*\.so' | head -n1)) && \
+ FREERDP_PLUGIN_DIR="${FREERDP_DIR}/freerdp" && \
+ mkdir -p "$FREERDP_PLUGIN_DIR" && \
+ ln -s "$PREFIX_DIR"/lib/freerdp/*.so "$FREERDP_PLUGIN_DIR"
+
--- End diff --
> It's a little awkward to copy symlinks. But if you feel really strongly about it, there's certainly a way to get there.
Nope, if there's a technical reason why this should be outside the build script, that's good enough for me.
What's awkward about copying the links, out of curiosity?
> I guess I was thinking that putting these links in place is really just an aspect of installing the build artifacts into the runtime image.
In my view, the creation of the symlinks is analogous to other parts of the install process which are performed by `make install`, and so shouldn't be part of the scope of tasks performed outside the build/install script, however if there are technical reasons that this is best kept within the `Dockerfile`, then it is what it is.
---
[GitHub] guacamole-server pull request #126: GUACAMOLE-456: use Docker multi-stage bu...
Posted by ceharris <gi...@git.apache.org>.
Github user ceharris commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/126#discussion_r162611549
--- Diff: Dockerfile ---
@@ -76,13 +68,63 @@ COPY src/guacd-docker/bin /opt/guacd/bin/
COPY . "$BUILD_DIR"
# Build guacamole-server from local source
-RUN yum -y install $BUILD_DEPENDENCIES && \
- /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" && \
- rm -Rf "$BUILD_DIR" && \
- yum -y autoremove $BUILD_DEPENDENCIES && \
- yum clean all
+RUN /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" "$PREFIX_DIR"
-# Start guacd, listening on port 0.0.0.0:4822
+# Use same CentOS as the base for the runtime image
+FROM centos:${CENTOS_VERSION}
+
+# Base directory for installed build artifacts.
+# Due to limitations of the Docker image build process, this value is
+# duplicated in an ARG in the first stage of the build. See also the
+# CMD directive at the end of this build stage.
+#
+ARG PREFIX_DIR=/usr/local/guacamole
+
+# Runtime environment
+ENV LC_ALL=en_US.UTF-8
+
+ARG RUNTIME_DEPENDENCIES=" \
+ cairo \
+ dejavu-sans-mono-fonts \
+ freerdp \
+ freerdp-plugins \
+ ghostscript \
+ libjpeg-turbo \
+ libssh2 \
+ liberation-mono-fonts \
+ libtelnet \
+ libvorbis \
+ libvncserver \
+ libwebp \
+ pango \
+ pulseaudio-libs \
+ terminus-fonts \
+ uuid"
+
+# Bring runtime environment up to date and install runtime dependencies
+RUN yum -y update && \
+ yum -y install epel-release && \
+ yum -y install $RUNTIME_DEPENDENCIES && \
+ yum clean all && \
+ rm -rf /var/cache/yum
+
+# Copy build artifacts into this stage
+COPY --from=builder ${PREFIX_DIR} ${PREFIX_DIR}
+
+# Link FreeRDP plugins into proper path
+RUN FREERDP_DIR=$(dirname \
+ $(rpm -ql freerdp-libs | grep 'libfreerdp.*\.so' | head -n1)) && \
+ FREERDP_PLUGIN_DIR="${FREERDP_DIR}/freerdp" && \
+ mkdir -p "$FREERDP_PLUGIN_DIR" && \
+ ln -s "$PREFIX_DIR"/lib/freerdp/*.so "$FREERDP_PLUGIN_DIR"
+
--- End diff --
Copying symlinks is awkward because copying from the build image to the runtime image is being performed by the Dockerfile COPY directive. If you specify the full path for the symlink, it copies the link target into destination directory rather than copying the link. It will copy symlinks if you specify the directory containing the links (e.g. in this case '/lib64/freerdp) but that copies everything in the directory in addition to the links that would be placed there by Guacamole.
---
[GitHub] guacamole-server pull request #126: GUACAMOLE-456: use Docker multi-stage bu...
Posted by ceharris <gi...@git.apache.org>.
Github user ceharris commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/126#discussion_r161821557
--- Diff: Dockerfile ---
@@ -76,13 +68,63 @@ COPY src/guacd-docker/bin /opt/guacd/bin/
COPY . "$BUILD_DIR"
# Build guacamole-server from local source
-RUN yum -y install $BUILD_DEPENDENCIES && \
- /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" && \
- rm -Rf "$BUILD_DIR" && \
- yum -y autoremove $BUILD_DEPENDENCIES && \
- yum clean all
+RUN /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" "$PREFIX_DIR"
-# Start guacd, listening on port 0.0.0.0:4822
+# Use same CentOS as the base for the runtime image
+FROM centos:${CENTOS_VERSION}
+
+# Base directory for installed build artifacts.
+# Due to limitations of the Docker image build process, this value is
+# duplicated in an ARG in the first stage of the build. See also the
+# CMD directive at the end of this build stage.
+#
+ARG PREFIX_DIR=/usr/local/guacamole
+
+# Runtime environment
+ENV LC_ALL=en_US.UTF-8
+
+ARG RUNTIME_DEPENDENCIES=" \
+ cairo \
+ dejavu-sans-mono-fonts \
+ freerdp \
+ freerdp-plugins \
+ ghostscript \
+ libjpeg-turbo \
+ libssh2 \
+ liberation-mono-fonts \
+ libtelnet \
+ libvorbis \
+ libvncserver \
+ libwebp \
+ pango \
+ pulseaudio-libs \
+ terminus-fonts \
+ uuid"
+
+# Bring runtime environment up to date and install runtime dependencies
+RUN yum -y update && \
+ yum -y install epel-release && \
+ yum -y install $RUNTIME_DEPENDENCIES && \
+ yum clean all && \
+ rm -rf /var/cache/yum
+
+# Copy build artifacts into this stage
+COPY --from=builder ${PREFIX_DIR} ${PREFIX_DIR}
+
+# Link FreeRDP plugins into proper path
+RUN FREERDP_DIR=$(dirname \
+ $(rpm -ql freerdp-libs | grep 'libfreerdp.*\.so' | head -n1)) && \
+ FREERDP_PLUGIN_DIR="${FREERDP_DIR}/freerdp" && \
+ mkdir -p "$FREERDP_PLUGIN_DIR" && \
+ ln -s "$PREFIX_DIR"/lib/freerdp/*.so "$FREERDP_PLUGIN_DIR"
+
--- End diff --
It's a little awkward to copy symlinks. But if you feel really strongly about it, there's certainly a way to get there.
I guess I was thinking that putting these links in place is really just an aspect of installing the build artifacts into the runtime image.
---
[GitHub] guacamole-server pull request #126: GUACAMOLE-456: use Docker multi-stage bu...
Posted by ceharris <gi...@git.apache.org>.
Github user ceharris commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/126#discussion_r155231763
--- Diff: src/guacd-docker/bin/build-guacd.sh ---
@@ -28,35 +28,22 @@
## The directory which currently contains the guacamole-server source and
## in which the build should be performed.
##
+## @param PREFIX_DIR
+## The directory prefix into which the build artifacts should be installed
+## in which the build should be performed. This is passed to the --prefix
+## option of `configure`.
+##
BUILD_DIR="$1"
-
-##
-## Locates the directory in which the FreeRDP libraries (.so files) are
-## located, printing the result to STDOUT.
-##
-where_is_freerdp() {
- dirname `rpm -ql freerdp-libs | grep 'libfreerdp.*\.so' | head -n1`
-}
--- End diff --
This is now done in the second build stage.
---
[GitHub] guacamole-server pull request #126: GUACAMOLE-456: use Docker multi-stage bu...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/126#discussion_r162736330
--- Diff: Dockerfile ---
@@ -76,13 +68,63 @@ COPY src/guacd-docker/bin /opt/guacd/bin/
COPY . "$BUILD_DIR"
# Build guacamole-server from local source
-RUN yum -y install $BUILD_DEPENDENCIES && \
- /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" && \
- rm -Rf "$BUILD_DIR" && \
- yum -y autoremove $BUILD_DEPENDENCIES && \
- yum clean all
+RUN /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" "$PREFIX_DIR"
-# Start guacd, listening on port 0.0.0.0:4822
+# Use same CentOS as the base for the runtime image
+FROM centos:${CENTOS_VERSION}
+
+# Base directory for installed build artifacts.
+# Due to limitations of the Docker image build process, this value is
+# duplicated in an ARG in the first stage of the build. See also the
+# CMD directive at the end of this build stage.
+#
+ARG PREFIX_DIR=/usr/local/guacamole
+
+# Runtime environment
+ENV LC_ALL=en_US.UTF-8
+
+ARG RUNTIME_DEPENDENCIES=" \
+ cairo \
+ dejavu-sans-mono-fonts \
+ freerdp \
+ freerdp-plugins \
+ ghostscript \
+ libjpeg-turbo \
+ libssh2 \
+ liberation-mono-fonts \
+ libtelnet \
+ libvorbis \
+ libvncserver \
+ libwebp \
+ pango \
+ pulseaudio-libs \
+ terminus-fonts \
+ uuid"
+
+# Bring runtime environment up to date and install runtime dependencies
+RUN yum -y update && \
+ yum -y install epel-release && \
+ yum -y install $RUNTIME_DEPENDENCIES && \
+ yum clean all && \
+ rm -rf /var/cache/yum
+
+# Copy build artifacts into this stage
+COPY --from=builder ${PREFIX_DIR} ${PREFIX_DIR}
+
+# Link FreeRDP plugins into proper path
+RUN FREERDP_DIR=$(dirname \
+ $(rpm -ql freerdp-libs | grep 'libfreerdp.*\.so' | head -n1)) && \
+ FREERDP_PLUGIN_DIR="${FREERDP_DIR}/freerdp" && \
+ mkdir -p "$FREERDP_PLUGIN_DIR" && \
+ ln -s "$PREFIX_DIR"/lib/freerdp/*.so "$FREERDP_PLUGIN_DIR"
+
--- End diff --
OK. Sounds like this really is the best we can do, then. It's unfortunate that multi-stage builds don't provide for isolating `COPY` to only the files affected by that build stage.
---
[GitHub] guacamole-server pull request #126: GUACAMOLE-456: use Docker multi-stage bu...
Posted by ceharris <gi...@git.apache.org>.
Github user ceharris commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/126#discussion_r155231846
--- Diff: src/guacd-docker/bin/build-guacd.sh ---
@@ -28,35 +28,22 @@
## The directory which currently contains the guacamole-server source and
## in which the build should be performed.
##
+## @param PREFIX_DIR
+## The directory prefix into which the build artifacts should be installed
+## in which the build should be performed. This is passed to the --prefix
+## option of `configure`.
+##
BUILD_DIR="$1"
-
-##
-## Locates the directory in which the FreeRDP libraries (.so files) are
-## located, printing the result to STDOUT.
-##
-where_is_freerdp() {
- dirname `rpm -ql freerdp-libs | grep 'libfreerdp.*\.so' | head -n1`
-}
+PREFIX_DIR="$2"
#
# Build guacamole-server
#
cd "$BUILD_DIR"
autoreconf -fi
-./configure
+./configure --prefix="$PREFIX_DIR"
make
make install
ldconfig
-
-#
-# Add FreeRDP plugins to proper path
-#
-
-FREERDP_DIR=`where_is_freerdp`
-FREERDP_PLUGIN_DIR="$FREERDP_DIR/freerdp"
-
-mkdir -p "$FREERDP_PLUGIN_DIR"
-ln -s /usr/local/lib/freerdp/*.so "$FREERDP_PLUGIN_DIR"
-
--- End diff --
This now done in the second build stage.
---