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

Fixing Issues for the Ubuntu Builds in Docker #16

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

rainmanh
Copy link

@rainmanh rainmanh commented Jun 5, 2017

  • Fixed Supervisord daemon issue on Xenian (Ubuntu 16.04)
  • Fixed redis log file permissions for Xenian (Ubuntu 16.04)
  • Fixed Supervisord daemon issue on Trusty (Ubuntu 14.04)
  • Fixing Arakoon installation issue on Xenian (Ubuntu 16.04)

@rainmanh rainmanh force-pushed the gonzalo_docker_201706 branch from c53e33d to 3ff68bb Compare June 5, 2017 18:46
@rainmanh
Copy link
Author

rainmanh commented Jun 5, 2017

+1

@@ -52,8 +52,9 @@ RUN echo "deb http://apt.openvstorage.com unstable main" > /etc/apt/sources.list
libxio0 libxio0-dbg libxio0-dbgsym \
libxio-dev libxio-dev-dbgsym && \
apt-get install -y libev4 && \
cd /root && apt-get download --allow-unauthenticated arakoon && \
cd /tmp && apt-get download --allow-unauthenticated arakoon && \
Copy link
Member

Choose a reason for hiding this comment

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

/tmp is shared between all builds on the internal build system, so better not use it. The downloaded deb is removed a couple of lines further, so nothing is left behind.

Copy link
Author

Choose a reason for hiding this comment

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

@dejonghb , there is a bug in Xenial (ubuntu 16.04 ) permissions related and using root as a download directory does not work.
id you do not like tmp , it would be better to allocate some other tmp directory.

Copy link
Member

Choose a reason for hiding this comment

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

No problem to use something else than /root. The problem is the jenkins builds share /tmp on the buildslave so either create a unique directory in /tmp first and use that (and cleanup further on) or use something else inside the container if /root does not work.

ADD supervisord.conf /etc/supervisor/supervisord.conf
ADD docker-entrypoint.sh /sbin/docker-entrypoint.sh

ENTRYPOINT ["/sbin/docker-entrypoint.sh"]
#ENTRYPOINT ["/sbin/docker-entrypoint.sh"]
Copy link
Member

Choose a reason for hiding this comment

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

Again this is needed for the internal build system, so it needs to be kept in here.

Copy link
Author

Choose a reason for hiding this comment

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

(same comment as below)

##apt-get install -qq -y alba arakoon

# finally execute the command the user requested
exec "$@"
Copy link
Member

Choose a reason for hiding this comment

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

This exec is required as well.

Copy link
Author

@rainmanh rainmanh Jun 6, 2017

Choose a reason for hiding this comment

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

True!
Will correct this one.

@rainmanh rainmanh force-pushed the gonzalo_docker_201706 branch from 48ef066 to f4c4071 Compare June 6, 2017 12:45
@rainmanh rainmanh force-pushed the gonzalo_docker_201706 branch from f4c4071 to 98d8d6e Compare June 6, 2017 13:09
@rainmanh
Copy link
Author

rainmanh commented Jun 6, 2017

@dejonghb

  • Preserved exec "$@" as user will pass parameters to the container.
  • Temporary Directory created for Xenial

@@ -43,6 +43,9 @@ RUN groupadd -g 1002 jenkins && \
useradd jenkins -s /bin/bash -g jenkins -m -u ${UID} -g 1002 -d /home/jenkins && \
echo "jenkins ALL=NOPASSWD: ALL" >/etc/sudoers.d/jenkins

#Creating TMP Directoy To Avoid permission Issues -- Bug in Xenial
RUN mkdir /opt/tmp_docker; chmod -R 777 /opt/tmp_docker
Copy link
Member

Choose a reason for hiding this comment

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

don't make this an extra step (layer) but create the directory below (around line 55) where it's used (and cleanup afterwards)

@@ -52,8 +55,9 @@ RUN echo "deb http://apt.openvstorage.com unstable main" > /etc/apt/sources.list
libxio0 libxio0-dbg libxio0-dbgsym \
libxio-dev libxio-dev-dbgsym && \
apt-get install -y libev4 && \
cd /root && apt-get download --allow-unauthenticated arakoon && \
cd /opt/tmp_docker && apt-get download --allow-unauthenticated arakoon && \
Copy link
Member

Choose a reason for hiding this comment

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

see above: mkdir /opt/tmp_docker && cd /opt/tmp_docker && apt-get download...

Copy link
Member

Choose a reason for hiding this comment

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

to check: is an apt-get install arakoon not possible these days instead of the download/install sequence hack?

dpkg -i --ignore-depends=libgflags2,libsnappy1 arakoon_*_amd64.deb && \
#chown _apt arakoon_*_amd64.deb && \
rm arakoon_*_amd64.deb && \
Copy link
Member

Choose a reason for hiding this comment

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

"rm -rf /opt/tmp_docker" to clean-up

@@ -17,10 +17,5 @@ then
[ -d /home/jenkins/.ssh ] && chown ${UID} /home/jenkins/.ssh
fi

# update alba & arakoon packages to latest/greatest
Copy link
Member

Choose a reason for hiding this comment

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

keep this the same as for 14.04 (couldn't do this at the time this Dockerfile was created)

# To preserve the logs for redis
RUN chown -R redis:redis /var/log/redis/

# Cleaning behind
Copy link
Member

Choose a reason for hiding this comment

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

not needed if /opt/tmp_docker is cleaned up after use

RUN sed -i "s/daemonize yes/daemonize no/g" /etc/redis/redis.conf

# To preserve the logs for redis
RUN chown -R redis:redis /var/log/redis/
Copy link
Member

Choose a reason for hiding this comment

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

add to RUN above to avoid extra docker layer; these 2 are redis customisations that go together anyway
(even better to move those 2 steps into the whole install step above)

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.

2 participants