-
Notifications
You must be signed in to change notification settings - Fork 20
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
Put peqp in a separate volume called "static" #27
Put peqp in a separate volume called "static" #27
Conversation
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.
Looks good, almost ready to merge - thanks!
nginx/Dockerfile
Outdated
@@ -1,4 +1,5 @@ | |||
FROM nginx:mainline | |||
COPY metakgp.org /etc/nginx/sites-enabled/ |
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.
Can you rename these to wiki.metakgp.org and static.metakgp.org?
@@ -27,7 +27,7 @@ server { | |||
autoindex off; | |||
|
|||
# Make site accessible from everywhere | |||
server_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.
Unfortunately I think this causes things like locahost to stop working, which we want for development. Can you verify whether this is the case, and see if there's a way around 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.
Hey, I don't have docker installed on my computer. I did wget localhost
on the server and that just gave me a 301 to the server URL. I will see if I have the space on my computer to set the whole thing up and check this.
location /peqp { | ||
try_files $uri $uri/ =404; | ||
} | ||
|
||
location ~ ^/google557cb96b33ddc6b5\.html$ {} | ||
|
||
location /images { |
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.
For the mediawiki volume to be truly ephemeral, we also want to put image uploads into the static volume. You could take a look at whether it's possible to configure media uploads to a separate subdomain, but I think the simplest solution might be to symlink the images dir to a dir in the static volume. Feel free to defer this to a later patch though.
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.
Yeah, I got it. Will defer to a later patch.
nginx/static
Outdated
|
||
root /srv/static; | ||
# index index.php index.html; | ||
# autoindex off; |
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.
Does this turn off showing an index of all the files? I think we want that to be off, but what do you think?
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.
Yeah, I think we should turn it off too. The default is off
. I will keep this in the config to keep it explicit.
nginx/static
Outdated
@@ -0,0 +1,60 @@ | |||
# You may add here your |
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.
We can get rid of this boilerplate, I think.
docker-compose.override.yml
Outdated
@@ -7,6 +7,7 @@ services: | |||
nginx: | |||
volumes: | |||
- mediawiki-volume:/srv/mediawiki | |||
- static-volume:/srv/static |
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 this belongs to the prod configuration - default and override should only contain config for development.
|
||
source .env | ||
NGINX_CONTAINER=$(docker ps --format '{{ .Names }}' | grep nginx) | ||
docker cp $1/. $NGINX_CONTAINER:/srv/static |
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 have a slight preference against having scripts to do very simple things, like run docker cp
. Perhaps this belongs to the runbook instead?
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.
Hmm, We could put this in the runbook, this script just felt natural beside the mysql backup restore script. I think we should keep it.
96d1275
to
efb30e7
Compare
Rebased to latest master. Remaining:
|
efb30e7
to
392baec
Compare
@amrav Localhost still works. I just setup a new server and tried it and it works. I have rebased to latest master and used this particular build to set this up. Please review and approve! 🙂 |
Hey @amrav, I have solved the issues now. Can you please review and merge? I think all the requirements have been met. |
Lgtm if it works, but do you understand why nginx serves the correct domain for localhost? Why isn't it defaulting to static.metakgp.org, for instance? |
@amrav Okay, the mechanism was easy to understand. It's well documented here In particular: A verbose log of the request: So, the I hope that clears it up. This was a good thing to find out! 😄 Okay to merge this patch now? |
Sounds good, thanks for clarifying!
…On Tue, 14 Nov 2017 at 08:08, Siddharth Kannan ***@***.***> wrote:
@amrav <https://github.com/amrav> Okay, the mechanism was easy to
understand. It's well documented here
<https://nginx.org/en/docs/http/request_processing.html>
In particular:
[image: image]
<https://user-images.githubusercontent.com/3668034/32769172-c9f135d0-c940-11e7-9f53-b2544764cf93.png>
A verbose log of the request:
[image: image]
<https://user-images.githubusercontent.com/3668034/32769184-d8cd06f6-c940-11e7-9e15-9acb358769c8.png>
So, the Host: localhost doesn't match anything configured and it just
routes the request to the default_server which we have configured to be
the wiki server.
I hope that clears it up. This was a good thing to find out! 😄 Okay to
merge this patch now?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#27 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABIZ_7ts5HjS8vnZfVr311KSVFobCV7Gks5s2UqMgaJpZM4QJk4b>
.
|
I hope this is what you meant in your comment in #6 . The changes were pretty easy to make once I got the hang of it. 😄
This patch makes the
mediawiki
volume completely ephemeralAll static assets can be served from this subdomain:
static.metakgp.org
Serving static assets from the same domain as the wiki turns out to be tricky, so I avoided it. It's just a DNS record and we can easily keep track of it.
The script at the end of this patch does the same thing as the restore mysql DB does. It is convenient to use:
wget -r 10.17.32.9
tar czf peqp.tar.gz 10.17.32.9/
rsync -avzhe "ssh" peqp.tar.gz root@IP:~
tar zxf peqp.tar.gz
metakgp-wiki
and then call the script:./script/restore-peqp-to-static.sh 10.17.32.9
(The first argument is the directory that consists the peqp folder alongwith all the PDFs)I will add this to the runbook as soon as these two patches are merged in after testing it one more time.