-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add Native Docker config - Ansible free #1
base: master
Are you sure you want to change the base?
Conversation
Fix github url strings (org edx -> openedx)
…label fix: Remove MAINTAINER label
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.
@zubairshakoorarbisoft & @jmbowman Based on theses changes I don't believe you realized this Dockerfile is what we build in GoCD and ship to edx.org EKS clusters. I'm all for getting the Ansible bits out of devstack but could we make a separate Dockerfile for devstack or extend the original dockerfile to add the devstack bits?
cc @jdmulloy & @nadeemshahzad
COPY /manage.py /edx/bin/manage.edx_notes_api | ||
|
||
# Expose canonical Discovery port | ||
EXPOSE 18281 |
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.
Why did we switch from 8120 to 18281?
ENV EDXNOTES_CONFIG_ROOT /edx/etc | ||
ENV DJANGO_SETTINGS_MODULE notesserver.settings.yaml_config | ||
#install supervisor and deps in its virtualenv | ||
RUN . ${SUPERVISOR_VENV_BIN}/activate && \ |
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.
I think it's considered poor practice to run supervisor in a docker container, is there a reason it's needed?
|
||
# libssl-dev; # mysqlclient wont install without this. | ||
ENV HOME /root | ||
ENV PATH "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin" |
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.
What do we use snap bin for?
# Expose canonical Discovery port | ||
EXPOSE 18281 | ||
|
||
FROM app as prod |
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.
I'm confused why we want a prod multistage image here.
# this prevents the image cache from busting unless the dependencies have changed. | ||
COPY requirements/base.txt /edx/app/notes/requirements/base.txt | ||
COPY requirements/pip.txt /edx/app/notes/requirements/pip.txt | ||
ENV DJANGO_SETTINGS_MODULE "notesserver.settings.devstack" |
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.
I believe this dockerfile is used in the stage, prod and edge EKS clusters so it's not appropriate to hardcode a devstack file here.
ISSUE: openedx#303
This PR is part of effort aimed at removing Ansible based configurations and replacing them with Dockerfile. Currently Devstack Docker images are built using Ansible based configurations in the configurations repository. Through this effort we will make sure that the Repo has its own Dockerfile which has all the necessary configurations to setup small production and dev environments.
Steps to run this Image with Devstack:
Build the Image locally first using the target dev i.e. docker build -t image-name-of-choice --target dev .
After the image is built successfully go to the docker compose file of devstack and replace the existing insights image with the one that you built without changing any other configurations there.
Run make
dev.up.edx_notes_api
in the terminal while in the devstack directory.