-
Notifications
You must be signed in to change notification settings - Fork 539
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
make dockerfile more configurable #254
base: main
Are you sure you want to change the base?
make dockerfile more configurable #254
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
RUN apt-get update && apt-get install -y git && \ | ||
ARG UBUNTU_VERSION | ||
ARG RELEASE_REV=master | ||
RUN apt-get update && apt-get install -y software-properties-common git sudo && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
14.04 needs software-properties-common
, and 16.04
needs sudo
mcrouter/scripts/docker/Dockerfile
Outdated
ARG UBUNTU_VERSION | ||
ARG RELEASE_REV=master | ||
RUN apt-get update && apt-get install -y software-properties-common git sudo && \ | ||
add-apt-repository ppa:george-edison55/cmake-3.x && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed for cmake >= 3.0
./install_ubuntu_14.04.sh $MCROUTER_DIR && \ | ||
./clean_ubuntu_14.04.sh $MCROUTER_DIR && rm -rf $MCROUTER_DIR/repo && \ | ||
./install_ubuntu_${UBUNTU_VERSION}.sh $MCROUTER_DIR && \ | ||
{ ./clean_ubuntu_${UBUNTU_VERSION}.sh $MCROUTER_DIR || true ; } && rm -rf $MCROUTER_DIR/repo && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no clean script for 16.04, but we can just try to call out to clean and accept and move on if it does not exist or fails.
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
f4a8dbf
to
e2e6aa8
Compare
@andreazevedo not sure if you are the right person to ping on this. The change to the Dockerfile would help us build mcrouter into containers using appropriately tagged release version and also selectively for different versions of either ubuntu 14.04 or 16.04 Thanks in advance. |
hello come across this error below: it looks like I use ubuntu:14.04 did you ever see this error before and how to fix it |
@hzluyang I am able to build using a released version without any errors (based on #251 (comment), that is also a recommended approach?
this builds everything,
I guess part of the benefit of this Hope this helps. |
e2e6aa8
to
ebe5f43
Compare
ebe5f43
to
956e37b
Compare
Could #241 be integrated into this PR also? |
👋 Am new here 😸
Was trying to build containers for
mcrouter
using the existing Dockerfile.Since we ideally want to build from a
release
version, and potentially build for either ubuntu14.04
or16.04
(the supported options in the Dockerfile), these changes just make the Dockerfile a little moreconfigurable
Specifically you can call out to
docker build
like this.for
Ubuntu 16.04
docker build --build-arg UBUNTU_VERSION=16.04 --build-arg RELEASE_REV=release-37-0
and
for
Ubuntu 14.04
docker build --build-arg UBUNTU_VERSION=14.04 --build-arg RELEASE_REV=release-37-0
The adjustable variables are
UBUNTU_VERSION
with a default of14.04
RELEASE_REV
with a default ofmaster
Also just doing
docker build
with the existingDockerfile
and no arguments, will build with the defaultubuntu:14.04
andmaster
branch (the current default)@andreazevedo (or is there another person I could ask to review?)