-
Notifications
You must be signed in to change notification settings - Fork 2
Conversation
server { | ||
listen *:80; | ||
|
||
server_name {{ OPENCRAFT_DOMAIN_NAME }} www.{{ OPENCRAFT_DOMAIN_NAME }}; |
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 lost some time due to this. Nginx is configured in such a way that first server block it encounters is treated as the default one, which responds to requests for domains not matched by any other servers. So this server responds to every host, instead of only: {{ OPENCRAFT_DOMAIN_NAME }}
and www.{{ OPENCRAFT_DOMAIN_NAME }}
.
Solution is to include default server responding with 404 for every request, like on this SO question.
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.
Since we set ALLOWED_HOSTS to only {{ OPENCRAFT_DOMAIN_NAME }}, we already have the same behaviour now, with the 404 served by Django instead of nginx. If you prefer the 404 to come from nginx, I can change it anyway, but I personally don't care enough to change it.
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.
@smarnach Not really. I was just pointing out that despite server_name
being set, this nginx server will respond to every domain.
I have no access to IM production enviorment, so I might be uninformed on deployment methods, but shoudn't we have some code that starts IM here? Like a systemd script, that would launch IM on reboot? |
pip: | ||
requirements="{{ opencraft_root_dir }}/requirements.txt" | ||
virtualenv="{{ opencraft_virtualenv_dir }}" | ||
virtualenv_python=python3.4 |
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.
This PR will need to be reconciled with the upgrade - cf open-craft/opencraft#6
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.
Thanks, I noticed that when I wrote this, but at the time I couldn't predict which PR would get merged first.
@jbzdak Regardin your comment whether we should start the instance manager: We currently don't do this, and instead run it manually inside a screen session. I used the same approach here, and documented it in the README file in open-craft/opencraft#68. |
@smarnach OK. I'm officially happy with the code. Please ping me when the testing instruction will be ready (I'll do my best to fit tests before the meeting). |
c05add4
to
2c123f4
Compare
👍 |
commit 2c123f4 Author: Sven Marnach <sven@marnach.net> Date: Thu May 5 22:17:22 2016 +0200 Add task to install SSH private key for OpenStack access. commit 0b845f4 Author: Sven Marnach <sven@marnach.net> Date: Wed May 4 00:37:08 2016 +0200 Set permissions on /var/www. commit 97c7f7c Author: Sven Marnach <sven@marnach.net> Date: Mon May 2 16:05:25 2016 +0200 Refer to README.md in the opencraft IM repo. commit a93286d Author: Sven Marnach <sven@marnach.net> Date: Mon May 2 15:07:25 2016 +0200 Update Python version to 3.5. commit 88c8d59 Author: Sven Marnach <sven@marnach.net> Date: Sun May 1 23:46:22 2016 +0200 First complete version of the role.
For test instructions, see open-craft/opencraft#68.