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.

Sign up for GitHub

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

Improve the dockerfile #45

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
29a9245
cleanup apt cache | build without mysql
Oct 13, 2018
126edd2
Fixed the Dockerfile issue
Oct 13, 2018
00ba6da
Improve the dockerfile for mysql-client image
Oct 14, 2018
7d79739
Add SysBench with MySQL support
Oct 15, 2018
88765ff
Add bash support
Oct 15, 2018
3d3bff1
Remove python3 and --no-install-recommends
Oct 15, 2018
eae9346
Reduce docker image layer
Oct 15, 2018
0602944
Bug fix
Oct 15, 2018
dde97f8
Add moreutils package
Oct 19, 2018
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
View file
Open in desktop
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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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

10 changes: 6 additions & 4 deletions mysql-client/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
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
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 I think that adding --no-cache bash will not support all the bash command. What we can do is either installing moreutils package or we can use the base image as ubuntu only. Please accomodate this change it will highy appreciated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added moreutils package

Copy link
Member

Choose a reason for hiding this comment

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

@shashank855 moerutils is necessitated on account of usage of ts in the script. --no-cache shouldn't matter IMO.


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