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

Better automate deploy #762

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,8 @@ src/sous_chef/static

# Ignore generated pdf files.
src/*.pdf

# the magic docker-compose override file is for overriding settings locally
# it should not be a part of the repo. If you want per-environment settings,
# use an explicit `docker-compose -f docker-compose.yml -f docker-compose.environment.yml`
docker-compose.override.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

@kousu If we are using docker-compose we should follow their conventions, and by default docker-compose command will read the configurations files "docker-compose.yml" and "docker-compose.override.yml". The main idea is to have in the "docker-compose.yml" the configurations that remain the same in all environements, use "docker-compose.override.yml" to the development environment that is used more frequently by developers and have separated configurations files for other environments like staging or prod.
For more detailed information check https://docs.docker.com/compose/extends/#example-use-case

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the link. Having the laser focus there really helps move the debate forward, and I agree that you're doing it just as they suggest.

I still think it's wrong. docker-compose.yml can have dev settings in it by default which are then overridden by docker-compose.prod.yml. "override" says to me "these are settings which are custom to this specific instance, which I need in order"; if they meant "override" to always mean "dev" then they should have called it "dev". Emphatically I think that name means they must not be stored as part of the codebase, and not be in the repo, be .gitignore'd.

Copy link
Contributor

@lingxiaoyang lingxiaoyang Jul 11, 2017

Choose a reason for hiding this comment

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

@kousu I agree with you personally. I don't like the use of the word "override" of docker-compose either and it brought me some troubles and confusion... but I think this is their convention according to their doc, so I would follow the usage of "override" so that we work on the same page with docker-experienced developers.

I think it is important to mention the doc link somewhere in the project, in order to help developers like me and @kousu who don't know this convention of docker-compose a priori.

28 changes: 25 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,35 @@ FROM python:3.5
ENV PYTHONUNBUFFERED 1
RUN mkdir /code
WORKDIR /code
ADD requirements.txt /code/
COPY requirements.txt /code/
COPY . /code/

# install underlying debian dependencies
RUN apt-get update
RUN apt-get install curl gettext -y
RUN curl -sL https://deb.nodesource.com/setup_8.x | bash -
RUN apt-get install nodejs build-essential -y
RUN apt-get install binutils libproj-dev gdal-bin -y
RUN apt-get clean

# install python dependencies
RUN pip3 install -r requirements.txt

# install javascript dependencies
RUN npm install gulp -g
ADD . /code/
RUN apt-get clean

# compile front-end "assets" (this should be unnecessary since they're basically just files, but hey, it's 2017)
# the 'rebuild node-sass' is a patch over a glitch which showed up recently;
# the tip that fixed it is from https://github.com/sass/node-sass/issues/1387#issuecomment-185451183
# perhaps it is not truly necessary.
run mkdir /code/src/sous_chef/static/
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is not truly necessary we should not include it, so we need to check it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is resolved in #775.

RUN python3 src/manage.py collectstatic --noinput
WORKDIR /code/tools/gulp
RUN npm install # gulp dependencies
RUN gulp
WORKDIR /code/

# the default entrypoint for the container
# when this ends, the container ends, then restarts
# can be overridden in docker-compose.yml files
CMD python3 src/manage.py runserver 0.0.0.0:8000
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not use this server by default, this is a development server that should be used only for development if we are going to use this image in production, we should use by default gunicorn and override it with the development server in the docker-compose.override.yml configuration file, or not have any entry point at all.

Copy link
Author

Choose a reason for hiding this comment

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

I thought you must have an entry point. Without one, the container shuts down immediately.
I would love it if docker let you just bring up a container with no master process. It lets you docker exec arbitary side commands in a single container, so why do any of them need to be key?

Copy link
Contributor

Choose a reason for hiding this comment

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

44 changes: 27 additions & 17 deletions INSTALL.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,33 @@ For older Windows versions, you may have to use **Docker Toolbox** (https://www.

```
$> git clone https://github.com/savoirfairelinux/sous-chef
$> docker-compose build
$> cd sous-chef
$> docker-compose up
```

After this, test it worked by going to http://localhost:8000.

**Notice**: if you see the error "Can't connect to database" in `souschef_web_1`, try Ctrl+C and re-run `docker-compose up`.

(Optional) Running docker-compose with production settings:
To make changes to the running image, you could re-run it with `docker-compose up --build`, but to speed up development
you can instead loop-mount the `sous-chef/` directory into the container by running

```
$> docker-compose -f docker-compose.yml -f docker-compose.dev.yml up
```

Similarly, you could use the production settings file that uses `gunicorn` to better handle load:

```
$> docker-compose -f docker-compose.yml -f docker-compose.prod.yml up
```

## Django initialization

Unfortunately, the bulk of the Django configuration cannot happen until the containers are already built
and running, because it needs to talk to the database which is in a different container, managed by docker-compose.
So after you do the first build, you need to manually do some more steps.

In your console:

```
Expand All @@ -58,29 +71,26 @@ $> python3 manage.py createsuperuser
$> python3 manage.py loaddata sample_data
```

## Generate Django assets using gulp
## (Optional) Generate frontend assets

### Run gulp
You do not need to run these commands to deploy Sous-Chef, but if during development
you make changes to anything in the frontend you will need to know at least gulp.

**From container:**
[gulp](http://gulpjs.com/) is a javascript-based build system.
We use it to compile and optimize files from `src/frontend/` to `src/sous_chef/static/`.
We specifically use it to compile scss to css, javascript to minified javascript,
and images to further-compressed images.

```
$> docker-compose exec web sh -c "cd tools/gulp && npm install --unsafe-perm && gulp"
```

Or **from host machine:**
To run gulp, use:

```
$> cd tools/gulp && npm install --unsafe-perm && gulp

# If you don't have the gulp command, try:
$> cd tools/gulp && node node_modules/gulp/bin/gulp.js
$> docker-compose exec web sh -c "cd tools/gulp && gulp"
```

Please rest assured that the "unsafe-perm" option will not bring any security risk to sous-chef. The Node.js packages that we are installing here are only used for generating static files, such as images, CSS, JavaScript, etc., and will never be executed from external.

If you have an error with this command, try deleting the folder `tools/gulp/node_modules` completely and rerun it. If the problem still exists, please let us know.
If you have an error with this command, try deleting the folder `tools/gulp/node_modules` completely and rerun it. If the problem still exists, please let us know, including the versions of node, npm, and gulp that you have.

[collectstatic](https://docs.djangoproject.com/en/1.11/ref/contrib/staticfiles/#collectstatic) is similar, but built into Django.
We are not using it to manage our own static files, but some of our Django dependencies might need this to be run.
Copy link
Contributor

Choose a reason for hiding this comment

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

@kousu Good work! We keep this file.


## Connection to application

Expand Down
10 changes: 10 additions & 0 deletions docker-compose.dev.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# use as `docker-compose -f docker-compose.yml -f docker-compose.dev.yml up`
# this causes the the local folder to be loop-mounted into the container
# so that you can edit code with your normal editors and have the changes
# immediately be reflected in the running container, without a `docker-compose build`.

version: '3'
services:
web:
volumes:
- .:/code
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this file and we should keep using docker-compose.override.yml because for developers is easier to do docker-compose up -d than docker-compose -f docker-compose.yml -f docker-compose.dev.yml

Copy link
Contributor

Choose a reason for hiding this comment

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

Resolved in #775.

8 changes: 0 additions & 8 deletions docker-compose.override.yml

This file was deleted.

4 changes: 3 additions & 1 deletion docker-compose.prod.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
version: '3'
services:
web:
command: /bin/bash -c "cd tools/gulp && gulp && cd ../../src && python manage.py collectstatic --noinput && /usr/local/bin/gunicorn sous_chef.wsgi:application -w 2 -b :8000"
environment:
- SOUSCHEF_ENVIRONMENT_NAME=PROD
command: bash -c "cd src/ && /usr/local/bin/gunicorn sous_chef.wsgi:application -w 2 -b :8000"
2 changes: 2 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ services:
- backend

web:
ports:
- "127.0.0.1:8000:8000"
environment:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this binding why you have added it?

Copy link
Author

Choose a reason for hiding this comment

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

I moved this binding from docker-compose.override.yml, and added the fix for #761 at the same time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems to me that it's needed? I didn't see in #775.

Copy link
Contributor

@erozqba erozqba Jul 11, 2017

Choose a reason for hiding this comment

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

@lingxiaoyang if we are only using this binding in development mode, in the docker-compose.override.yml we don't relly need to take care of security and promiscuous ports because we are using the development server anyway.

@lingxiaoyang @kousu but we should change the binding in the nginx container to add the IP
127.0.0.1:80:80

Copy link
Contributor

Choose a reason for hiding this comment

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

@erozqba Got your idea.

- SOUSCHEF_ENVIRONMENT_NAME=DEV
- SOUSCHEF_VERSION=v1.0-beta
Expand Down