Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolves "Alpine version of container #32" #40

Conversation

gabrielsclimaco
Copy link

Creates Dockerfile for an Alpine version of RethinkDB.

The result image is almost 4x smaller then original RethinkDB image:
image

resolve #32

@gabrielsclimaco gabrielsclimaco changed the title Resolves #32 Alpine version of container Resolves "Alpine version of container #32" Aug 31, 2017
@gabrielsclimaco gabrielsclimaco force-pushed the 32-alpine-version-of-container branch from 404f8f9 to ca57a86 Compare August 31, 2017 18:05
@@ -0,0 +1,15 @@
# Only alpine version that suports rethinkdb is edge
FROM alpine:edge

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a maintainer line. Right now, all Dockerfiles list @dalanmiller as maintainer (example: https://github.com/rethinkdb/rethinkdb-dockerfiles/blob/master/jessie/2.3.6/Dockerfile#L3), but, per docker-library/official-images#3332 (comment), we should probably change these to @AtnNn. (For now, I'd stick with copying the @dalanmiller credit, to make one-shot updating everything simpler.)

# Only alpine version that suports rethinkdb is edge
FROM alpine:edge

ENV RETHINKDB_VERSION 2.3.6-r1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency and compatibility with the images for other distros, this should be RETHINKDB_PACKAGE_VERSION, as it is in the Debian dockerfiles (see https://github.com/rethinkdb/rethinkdb-dockerfiles/blob/master/jessie/2.3.6/Dockerfile#L10 for an example).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you want to pin a version? Current release on Alpine 3.7 is -r4


WORKDIR /data

EXPOSE 28015 29015 8080
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer it if the EXPOSE directive were moved to come after CMD, with the comment stating which ports are which, the way it is in the other Dockerfiles.

@@ -0,0 +1,15 @@
# Only alpine version that suports rethinkdb is edge
FROM alpine:edge
Copy link
Collaborator

@stuartpb stuartpb Aug 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's no stable version of Alpine that supports RethinkDB, then we probably shouldn't be packaging for it yet (at least, not as the new base for the official image). The few Dockerfiles in the official library that are packaged for Alpine (by a cursory sampling of images I'd expect to see migrated, it would seem that the announcement of increased forward adoption of Alpine a year and a half ago has been greatly exaggerated) don't do it without pinning a specific release, and edge is considered unsuitable for production.

Alternately, this could be listed as a Dockerfile for Alpine Edge, but it would need to go in a subdirectory for Alpine release versions (just as the repo is currently divided into separate directories for different Debian and Ubuntu releases). If doing this, I'd introduce a top-level distro directory level to the hierarchy (ie. moving the repo's jessie directory to debian/jessie) - I skipped this when originally creating the repo because, at the time I originally devised the current hierarchy, the intersection of "distros with official Docker images" and "distros that RethinkDB upstream builds packages for" was exclusively limited to the Debian / Ubuntu non-conflicting-named-release family.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually Alpine 3.7 stable ships RethinkDB, no need for edge.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I noted 26 days ago, Alpine's actually been packaging RethinkDB since 3.5.

@Vanuan
Copy link

Vanuan commented Sep 6, 2017

@stuartpb Could you speak up in this thread: docker-library/official-images#3332
Do you know if @AtnNn is still around?

@stuartpb
Copy link
Collaborator

stuartpb commented Sep 6, 2017

I don't know the current status of the upstream RethinkDB maintainership: I've got a PR in the works that'll raise this question directly.

@stuartpb
Copy link
Collaborator

To answer my question about Edge above, it looks like Alpine upstream's been packaging this since 3.5 in April: https://pkgs.alpinelinux.org/packages?name=rethinkdb

On that basis, I'll probably push a few PRs that addresses the points I raised in my review above in a little bit:

  • I'll do a commit that refactors the repository layout to have a per-distro layer (this tree is starting to get crowded anyway, and it'll make tooling a little more future-flexible).
  • I'll do a commit that replaces the MAINTAINER line in all Dockerfiles with some kind of LABEL entry, per MAINTAINER having been deprecated since Docker 1.13. (I'll leave @dalanmiller as the listed maintainer, per RethinkDB 2.3.6, w/ culling of less than latest docker-library/official-images#3530, and add myself to the list, since doing this pretty much brings me back to being a "maintainer" in practice)
  • I'll submit a PR to the Docker Library upstream that updates the entry to include debian tags as separate (for legacy use), and changes the existing tags to use Alpine. (In any other context, changing the implementation of published tags like this would be grossly irresponsible, but in the Docker world, it's par for the course, so whatever), as well as reflecting myself as a maintainer.

@stuartpb stuartpb mentioned this pull request Dec 26, 2017
# Only alpine version that suports rethinkdb is edge
FROM alpine:edge

ENV RETHINKDB_VERSION 2.3.6-r1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you want to pin a version? Current release on Alpine 3.7 is -r4

@stuartpb stuartpb mentioned this pull request Jan 6, 2018
@zaherg
Copy link

zaherg commented May 18, 2018

@mterron both Alpine latest and 3.7 now include rethinkdb package by default so it can be installed directly now from alpine repo https://pkgs.alpinelinux.org/packages?name=rethinkdb&branch=v3.7

@daveisfera
Copy link

I'd recommend building from scratch instead of installing a package that limits you to the version available in the OS repos:
#32 (comment)

@zaherg
Copy link

zaherg commented May 18, 2018

@daveisfera I tried your script (a few months ago) and it fails to build an image with rethinkdb 2.3.6 but with 2.3.5 it works nice.

meanwhile, with the alpine package, I can successfully run 2.3.6

@daveisfera
Copy link

This commit added patch -fp1 and that's not a valid command on Alpine, so that's why it fails:
rethinkdb/rethinkdb@41e11c7#diff-2f99a770a02afe004be6e92b2cc92161R126

@zaherg
Copy link

zaherg commented May 18, 2018

@daveisfera yet the one from the package manager work.

trust me, I agree with your approach, but some people just want an image that works.

@daveisfera
Copy link

That works until 2.4 comes out or 2.3.7 is released and the Alpine package isn't updated.

I'm guessing that the Alpine package built because it uses the original 2.3.6 version and not the one with the 2.3.6-windows version with the problematic commit that I mentioned. I opened a RethinkDB bug because I believe that's going to cause problems with future Alpine builds as well.

@daveisfera
Copy link

Here's a patch to workaround that issue:

--- a/mk/support/pkg/v8.sh
+++ b/mk/support/pkg/v8.sh
@@ -6,7 +6,7 @@

 pkg_install-include () {
     pkg_copy_src_to_build
-    in_dir "$build_dir" patch -fp1 < "$pkg_dir"/patch/v8_2-HandleScope-protected.patch
+    in_dir "$build_dir" patch -p1 < "$pkg_dir"/patch/v8_2-HandleScope-protected.patch
     
     rm -rf "$install_dir/include"
     mkdir -p "$install_dir/include"
@@ -29,7 +29,7 @@
    
 pkg_install () {
     pkg_copy_src_to_build
-    in_dir "$build_dir" patch -fp1 < "$pkg_dir"/patch/v8_2-HandleScope-protected.patch
+    in_dir "$build_dir" patch -p1 < "$pkg_dir"/patch/v8_2-HandleScope-protected.patch
     sed -i.bak '/unittests/d;/cctest/d' "$build_dir/build/all.gyp" # don't build the tests
     mkdir -p "$install_dir/lib"
     if [[ "$OS" = Darwin ]]; then

And then a Dockerfile to use that patch and some other improvements on top of my original version:

FROM alpine:3.7

ENV RETHINKDB_VERSION 2.3.6

COPY v8_no_force.patch /tmp
     
RUN mkdir /tmp/build_rethinkdb && cd /tmp/build_rethinkdb \
    && apk add --no-cache curl --virtual .fetch-deps \
    && curl -sSLO https://download.rethinkdb.com/dist/rethinkdb-$RETHINKDB_VERSION.tgz \
    && tar xf rethinkdb-$RETHINKDB_VERSION.tgz \
    && cd rethinkdb-$RETHINKDB_VERSION \
    && for f in libressl-all musl-fixes-all paxmark-x86_64; do echo $f; curl -sSLO https://git.alpinelinux.org/cgit/aports/tree/community/rethinkdb/$f.patch?h=3.7-stable; patch -p1 -i $f.patch?h=3.7-stable; done \
    && patch -p1 -i /tmp/v8_no_force.patch \
    && apk del .fetch-deps \
    && apk add --no-cache protobuf libressl libcurl boost libexecinfo \
    && apk add --no-cache bash python2 linux-headers bsd-compat-headers m4 paxmark protobuf-dev icu-dev libressl-dev curl-dev boost-dev libexecinfo-dev --virtual .build-deps \
    && apk add --no-cache g++ make --virtual .build-deps2 \
    && ./configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var --dynamic all --with-system-malloc \
    && export CXXFLAGS="-fno-delete-null-pointer-checks" && export LDFLAGS="-lexecinfo" \
    && num_cpus=$(cat /proc/self/status | awk '$1 == "Cpus_allowed_list:" { print $2 }' | tr , '\n' | awk -F'-' '{ if (NF == 2) count += $2 - $1 + 1; else count += 1 } END { print count }') \
    && make -j ${num_cpus} && make -j ${num_cpus} install \
    && apk del .build-deps && apk del .build-deps2 \
    && rm -rf /tmp/build_rethinkdb

VOLUME ["/data"]

WORKDIR /data

CMD ["rethinkdb", "--bind", "all"]

#   process cluster webui
EXPOSE 28015 29015 8080

@daveisfera
Copy link

Apparently, some of the files take a lot of memory to compile and when I use more than 1 cpu on my laptop, it fails when it runs out of memory, so I'd take out the num_cpus stuff so it just does it 1 at a time and reduces the memory requirements.

@daveisfera
Copy link

Here's a version that keeps the patch local instead of downloading it from the Alpine packaging repo:

FROM alpine:3.7

ENV RETHINKDB_VERSION 2.3.6

COPY v8_no_force.patch libressl-all.patch /tmp/

RUN mkdir /tmp/build_rethinkdb && cd /tmp/build_rethinkdb \
    && apk add --no-cache curl --virtual .fetch-deps \
    && curl -sSLO https://download.rethinkdb.com/dist/rethinkdb-$RETHINKDB_VERSION.tgz \
    && apk del .fetch-deps \
    && tar xf rethinkdb-$RETHINKDB_VERSION.tgz \
    && cd rethinkdb-$RETHINKDB_VERSION \
    && for f in /tmp/*.patch; do echo $f; patch -p1 -i $f || exit; done \
    && apk add --no-cache protobuf libressl libcurl boost libexecinfo \ 
    && apk add --no-cache bash python2 linux-headers bsd-compat-headers m4 paxmark protobuf-dev icu-dev libressl-dev curl-dev boost-dev libexecinfo-dev --virtual .build-deps \
    && apk add --no-cache g++ make --virtual .build-deps2 \
    && ./configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var --dynamic all --with-system-malloc \
    && export CXXFLAGS="-fno-delete-null-pointer-checks" && export LDFLAGS="-lexecinfo" \
    && make && make install \
    && apk del .build-deps && apk del .build-deps2 \
    && rm -rf /tmp/build_rethinkdb
    
VOLUME ["/data"]

WORKDIR /data

CMD ["rethinkdb", "--bind", "all"]

#   process cluster webui
EXPOSE 28015 29015 8080

I also submitted a PR with the LibreSSL patch ( rethinkdb/rethinkdb#6671 ) and there's a link to the patch there. Once that's merged, it would no longer be needed in the Dockerfile.

@stuartpb
Copy link
Collaborator

stuartpb commented Oct 3, 2018

Closing in favor of #45.

@stuartpb stuartpb closed this Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alpine version of container
6 participants