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

Improve the dockerfile #45

Merged
merged 9 commits into from
Oct 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 17 additions & 23 deletions mongo-client/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,35 +1,29 @@
FROM ubuntu:16.04

RUN apt-get clean \
RUN apt-get update && apt-get -y --force-yes install \
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the changes @bilal-mk. This changes seems to be good but we tested this Docker file in our own environment and it doesn't work for the purpose. Can you kindly push the changes with the following Docker spec:

FROM ubuntu:16.04

RUN apt-get update -y || true

RUN apt-get install -y --force-yes --no-install-recommends \
    python \
    python-pip \
    make \
    automake \
    libmysqlclient-dev \
    libtool \
    libsasl2-dev \
    libssl-dev \
    libmongoc-dev \
    libbson-dev

RUN apt-get clean \
  && rm -rf /var/lib/apt/lists/* \
  && cp /usr/include/libbson-1.0/* /usr/include/ \
  && cp /usr/include/libmongoc-1.0/* /usr/include/
#  && pip install --upgrade pip 

RUN pip install --upgrade setuptools
RUN  python -m pip install pystrich \
  && python -m pip install pymongo

ADD sysbench-mongo/sysbench /sysbench
WORKDIR /sysbench
RUN ./autogen.sh && ./configure && make

# components for liveness script
ADD liveness/server.py ./

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same is this pull request with reduced the few-layer as part of the Dockerfile best practice. Please share how to properly test this or add those test as part of travis integration test.

Copy link
Member

@ksatchit ksatchit Oct 20, 2018

Choose a reason for hiding this comment

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

Agreed @bilal-mk. Have raised #49 & #50 to track this.

Currently travis performs a build check alone for all the images and doesn't look for existence of packages/artifacts specified in the Dockerfile (saw a few commits pushed as "fixes" for those that broke the build), which is what has necessitated certain manual tests. One way of testing this is to look at the python/bash scripts being pushed into the image & verifying the presence of the binaries &/or auxiliary files used in it in the locally built image. Or maybe simpler, compare the existing docker image layers w/ the one built with changes (docker diff, docker export - or even simply entering the fs)

Amongst the multiple docker images maintained in this repo & litmus - hacktoberfest has helped us refactor the images & its CI to the "extent possible", i.e., w/o breaking prod deployments where they are being used as K8s resources. Of course, further optimizations are possible by refactoring the scripts used in them (which came about before containerizing them!) - but these will be taken up based on priority.

Copy link
Member

@ksatchit ksatchit Oct 20, 2018

Choose a reason for hiding this comment

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

The dockerfile actually looks good. However, In this particular case, the following seem to sporadically cause a skipped install of some packages (though this shouldn't ideally impact us) :

  • the order of execution of apt-get clean/removal of repo refs in apt/lists
  • combining installation of python pip & pip-install of packages into same command.
  • setuptools dependency (typically this is needed only if easy_install is used)

Having said this, it may also depend on the build environment where the build is happening (its n/w connections, proxy settings etc.,).

Copy link
Member

@ksatchit ksatchit Oct 20, 2018

Choose a reason for hiding this comment

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

The travis build has passed(https://api.travis-ci.org/v3/job/443697398/log.txt) & I have tested this on a GCP compute instance (you can guarantee a clean build env here!). So, we should be good to go.

We will revisit this for further optimizations

python \
python-pip \
make \
automake \
libmysqlclient-dev \
libtool \
libsasl2-dev \
libssl-dev \
libmongoc-dev \
libbson-dev \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/* \
&& apt-get update -y || true \
&& apt-get install -y -f \
make \
automake \
libmysqlclient-dev \
libtool \
libsasl2-dev \
libssl-dev \
libmongoc-dev \
libbson-dev

RUN cp /usr/include/libbson-1.0/* /usr/include/ \
&& cp /usr/include/libmongoc-1.0/* /usr/include/
&& cp /usr/include/libbson-1.0/* /usr/include/ \
&& cp /usr/include/libmongoc-1.0/* /usr/include/ \
&& pip install --upgrade pip \
&& /usr/local/bin/pip install pystrich pymongo

ADD sysbench-mongo/sysbench /sysbench
WORKDIR /sysbench
RUN ./autogen.sh && ./configure && make
# components for liveness script
RUN apt-get update && apt-get -y --force-yes install --no-install-recommends \
python3 \
python \
python-pip
RUN pip install --upgrade setuptools
RUN pip install --upgrade pip
RUN python -m pip install pystrich

# components for liveness script
ADD liveness/server.py ./


RUN python -m pip install pymongo

11 changes: 7 additions & 4 deletions mysql-client/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
FROM ubuntu
MAINTAINER OpenEBS
RUN apt-get update
RUN apt-get install -y mysql-client timelimit jq moreutils
FROM alpine

LABEL maintainer="OpenEBS"

Copy link
Contributor

Choose a reason for hiding this comment

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

@bilal-mk In our liveness script we used bash commands and Alpine docker image doesn't have bash installed by default. You will need to add following commands to get bash:
RUN apk update && apk add bash, Otherwise it will throw an error.

RUN apk add --no-cache mysql-client && apk add --no-cache jq && apk add --no-cache bash \
&& apk add --no-cache --repository http://dl-cdn.alpinelinux.org/alpine/edge/testing moreutils

COPY MySQLLoadGenerate.sh mysql-liveness-check.sh /