Development Docker Setup #1206

chrisrosset · 2023-06-17T04:15:12Z

This tries to automate most of the setup instructions present in the INSTALL file by using Docker. I don't know if this is a good way of getting this up and running locally but it worked for me so I wanted to share it.

It's a bit janky in places - I know. I'd be happy to improve it if there's any feedback.

If it's not useful, no problem.

README.md

reedy · 2023-06-19T14:02:03Z

Thanks!

I know @jpatokal was looking to do something like this...

I'm not sure offhand how many things are hardcoded https in the code, so whether this may result in broken links if nginx is only listening on port 80 (I know there's fun and games with self signed certs too...)

chrisrosset · 2023-06-19T14:37:57Z

Thanks!

I know @jpatokal was looking to do something like this...

I'm not sure offhand how many things are hardcoded https in the code, so whether this may result in broken links if nginx is only listening on port 80 (I know there's fun and games with self signed certs too...)

Having spent a bit more time with this setup, I've already found a couple of https only links (e.g. export buttons). I'll have to do some more work to enable https.

Still, I think even in it's current form it's already useful as most of the functionality is available. As a side note, I've been using another container with PHP 8 (because the INSTALL notes mention that it's not compatible) installed to manually test side-by-side. I actually haven't found any breakage on that front (though there are quite a few warnings).

reedy · 2023-06-19T14:45:38Z

I've been fixing code up that I "knew" should be problematic on PHP 8...

If you've got warnings, feel free to dump them on another issue and I'll take a look. A lot of them should relatively easily resolved.

reedy · 2023-06-19T15:29:05Z

Still, I think even in it's current form it's already useful as most of the functionality is available. As a side note, I've been using another container with PHP 8 (because the INSTALL notes mention that it's not compatible) installed to manually test side-by-side. I actually haven't found any breakage on that front (though there are quite a few warnings).

FWIW, I would merge this without it being "perfect"; I would want to at least quickly test it first though ;)

This came up as part of my [Docker setup testing](jpatokal#1206 (comment)) where I don't have `https` set up. I think it makes sense to follow whichever origin (protocol, host, and port) was used to reach the page in the first place. The code already uses `location.host` and gets the port instead of hardcoding 443. The real `nginx` file redirects all requests to HTTPS so this change does not affect any real traffic. When testing with my `http` setup this fixes: - flight list export buttons (CSV and KML) - settings page backup button - sign-up page new profile URL (display only, it's not a link since the profile doesn't exist yet) I haven't been able to reproduce the `case 2:` behavior from the [login function](https://github.com/jpatokal/openflights/blob/master/openflights.js#L2962). I believe this is meant to reset the language from whatever was set prior to login to the user's settings-defined language. Still, it does not work the way I described it above on `openflights.org` either.

This came up as part of my [Docker setup testing](jpatokal#1206 (comment)) where I don't have `https` set up. I think it makes sense to follow whichever origin (protocol, host, and port) was used to reach the page in the first place. The code already uses `location.host` and gets the port instead of hardcoding 443. MDN shows this feature is widely supported by browsers: https://developer.mozilla.org/en-US/docs/Web/API/Location/origin The real `nginx` file redirects all requests to HTTPS so this change does not affect any real traffic. When testing with my `http` setup this fixes: - flight list export buttons (CSV and KML) - settings page backup button - sign-up page new profile URL (display only, it's not a link since the profile doesn't exist yet) I haven't been able to reproduce the `case 2:` behavior from the [login function](https://github.com/jpatokal/openflights/blob/master/openflights.js#L2962). I believe this is meant to reset the language from whatever was set prior to login to the user's settings-defined language. Still, it does not work the way I described it above on `openflights.org` either.

This came up as part of my [Docker setup testing](#1206 (comment)) where I don't have `https` set up. I think it makes sense to follow whichever origin (protocol, host, and port) was used to reach the page in the first place. The code already uses `location.host` and gets the port instead of hardcoding 443. MDN shows this feature is widely supported by browsers: https://developer.mozilla.org/en-US/docs/Web/API/Location/origin The real `nginx` file redirects all requests to HTTPS so this change does not affect any real traffic. When testing with my `http` setup this fixes: - flight list export buttons (CSV and KML) - settings page backup button - sign-up page new profile URL (display only, it's not a link since the profile doesn't exist yet) I haven't been able to reproduce the `case 2:` behavior from the [login function](https://github.com/jpatokal/openflights/blob/master/openflights.js#L2962). I believe this is meant to reset the language from whatever was set prior to login to the user's settings-defined language. Still, it does not work the way I described it above on `openflights.org` either.

reedy

I'm not going to claim to be any sort of Docker expert (to any extent), but it should(TM) be possible to make a common "base" image, and then have the differences between the images (pretty much the PHP install stuff at this point)

https://docs.docker.com/build/building/multi-stage/ as at least one way

I don't know if this is any harder/easier with docker compose too...

It probably doesn't make too much difference as things are relatively simple, and probably aren't be going to get muc more complex in future, but it would reduce some duplication.

reedy · 2023-07-09T18:06:30Z

README.md

+1. `cp php/config.sample.php php/config.php` and set `$host = "db";` so the host name matches the
+   the database container host name.
+2. Run `docker-compose up` to create the containers.
+3. Install local PHP dependencies inside the container.


Any reason we can't run composer automatically?

We only mount the source code when the container is created. It might be possible as part of CMD but this wasn't a priority for me. If you have PHP installed locally, you can run it outside of Docker too.

reedy · 2023-07-09T18:12:52Z

Dockerfile

+# note that these symlinks require the repo be mounted @ /var/www/openflights
+RUN rm /etc/nginx/sites-enabled/default
+RUN ln -s /var/www/openflights/nginx/openflights-dev /etc/nginx/sites-available/
+RUN ln -s /var/www/openflights/nginx/openflights-dev /etc/nginx/sites-enabled/


Isn't it more usual to chain the sites-enabled/foo to symlink to sites-available/foo, even if that is a symlink itself?

/shrug

We don't even need the sites-available one.

chrisrosset · 2023-07-09T18:31:37Z

It probably doesn't make too much difference as things are relatively simple, and probably aren't be going to get muc more complex in future, but it would reduce some duplication.

Wrapping up the PHP8 migration would achieve the same de-duplication. :)

reedy · 2023-07-09T18:38:34Z

Dockerfile

+RUN add-apt-repository ppa:ondrej/php -y
+RUN apt-get install -y \
+    php7.4 php7.4-curl php7.4-fpm php7.4-gd php7.4-mbstring php7.4-mysql php7.4-xml
+RUN mkdir /run/php # For the php-fpm socket file


/var/run/php/ is used in the nginx config below...

And does this actually need doing manually?

reedy · 2023-07-09T18:39:05Z

Dockerfile-php8

+# System-level PHP setup
+RUN apt-get install -y \
+    php8.1 php8.1-curl php8.1-fpm php8.1-gd php8.1-mbstring php8.1-mysql php8.1-xml
+RUN mkdir /run/php # For the php-fpm socket file


/var/run/php/ is used in the nginx config below...

And does this actually need doing manually?

I don't remember the details - I did need it at some point.

It appears that /var/run is a symlink to /run on Ubuntu.

root@af5f1b02b4f1:/# ls -lah /var/run lrwxrwxrwx 1 root root 4 Nov 30 2022 /var/run -> /run

Still need to double-check if this needs to be done manually.

/var/run/php/ is used in the nginx config below...

And does this actually need doing manually?

OK, it's because we run php-fpm directly instead of going through systemd...

reedy · 2023-07-09T18:39:31Z

nginx/openflights-dev

+
+    location ~ \.php$ {
+        include snippets/fastcgi-php.conf;
+        fastcgi_pass unix:/var/run/php/php7.4-fpm.sock;


This isn't going to work for PHP 8.1 of course

Right, forgot I have a local change for this in the PHP8 container.

I could symlink these to an unversioned name or update /etc/php/fpm/pool.d/www.conf to remove the version from the name.

reedy · 2023-07-09T18:41:17Z

It probably doesn't make too much difference as things are relatively simple, and probably aren't be going to get muc more complex in future, but it would reduce some duplication.

Wrapping up the PHP8 migration would achieve the same de-duplication. :)

Also requires Jani having the time ;). Not as time intensive as rebuilding on a new machine, though. Mostly just installing the newer packages, minor change to the default nginx config, update and reboot...

But then there'll be later PHP upgrades to do. PHP 8.2 and PHP 8.3 will bring their own pain, almost certainly.

chrisrosset · 2023-07-09T19:05:01Z

How about I just remove the PHP8 one for now. It's a later addition which I was using for one of the PRs and definitely not needed in the MVP.

jpatokal · 2023-07-11T12:03:28Z

PHP8 broke a lot of stuff, which is why I fell back on 7, and even that caused significant pain. I'm hesitant to upgrade unless there's a pressing reason to do so.

Re: hardcoded https, these are messy leftovers from the days when OpenFlights was upgrading to be 100% HTTPS, but it's been HTTPS-only for years now so explicitly specifying the protocol should be unnecessary.

reedy · 2023-07-11T12:05:51Z

FWIW, I've changed my dev install to using PHP 8.2, and while there's a bit of logspam (filing issues as I've seen them), it's mostly working just fine. I think we've fixed up most of the big issues.

PHP 7.4 has been EOL since late 2022 as per https://www.php.net/supported-versions.php

jpatokal · 2023-07-11T12:10:53Z

OK, that's good to hear. I do recall 8 upgrading a lot of warnings to errors, so I'm hoping these have been actually fixed at source as opposed to us just having different php.ini settings or something!

reedy · 2023-07-11T12:32:51Z

I'm not quite confident to say it's time to upgrade now, but I think it should be in a near term plan

This automates most of the setup instructions present in the `INSTALL` file by using Docker.

chrisrosset · 2023-07-12T00:40:11Z

@reedy A couple of small updates if you want to test. Otherwise, I'll merge and we can improve upon this later.

reedy reviewed Jun 19, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

chrisrosset mentioned this pull request Jun 21, 2023

Links to follow host protocol #1208

Merged

reedy mentioned this pull request Jul 8, 2023

"Recently added flights" aren't recent #1225

Open

chrisrosset mentioned this pull request Jul 8, 2023

SQL updates for the planes table #1211

Merged

reedy added the enhancement New feature requests or improvements to existing functionality. label Jul 8, 2023

chrisrosset force-pushed the docker branch from 5afdf3f to 9269565 Compare July 9, 2023 17:34

reedy reviewed Jul 9, 2023

View reviewed changes

chrisrosset force-pushed the docker branch from 9269565 to 59e882f Compare July 9, 2023 19:50

chrisrosset force-pushed the docker branch from 59e882f to 4ba13bb Compare July 12, 2023 00:34

Development Docker Setup

e8a7576

This automates most of the setup instructions present in the `INSTALL` file by using Docker.

chrisrosset force-pushed the docker branch from 4ba13bb to e8a7576 Compare July 12, 2023 00:36

chrisrosset mentioned this pull request Jul 12, 2023

Simpletest #821

Closed

This was referenced Jul 13, 2023

KML export can result in OOM #1326

Closed

Return JSON-encoded results from stats.php #1345

Merged

chrisrosset merged commit e4f7614 into jpatokal:master Jul 16, 2023
2 checks passed

chrisrosset deleted the docker branch July 16, 2023 00:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Development Docker Setup #1206

Development Docker Setup #1206

chrisrosset commented Jun 17, 2023

reedy commented Jun 19, 2023 •

edited

Loading

chrisrosset commented Jun 19, 2023

reedy commented Jun 19, 2023

reedy commented Jun 19, 2023

reedy left a comment

reedy Jul 9, 2023

chrisrosset Jul 9, 2023

reedy Jul 9, 2023

chrisrosset Jul 9, 2023

chrisrosset commented Jul 9, 2023

reedy Jul 9, 2023

reedy Jul 9, 2023

chrisrosset Jul 9, 2023

chrisrosset Jul 9, 2023

chrisrosset Jul 9, 2023

reedy Jul 9, 2023

chrisrosset Jul 9, 2023

reedy commented Jul 9, 2023

chrisrosset commented Jul 9, 2023

jpatokal commented Jul 11, 2023

reedy commented Jul 11, 2023

jpatokal commented Jul 11, 2023

reedy commented Jul 11, 2023

chrisrosset commented Jul 12, 2023

Development Docker Setup #1206

Development Docker Setup #1206

Conversation

chrisrosset commented Jun 17, 2023

reedy commented Jun 19, 2023 • edited Loading

chrisrosset commented Jun 19, 2023

reedy commented Jun 19, 2023

reedy commented Jun 19, 2023

reedy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisrosset commented Jul 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reedy commented Jul 9, 2023

chrisrosset commented Jul 9, 2023

jpatokal commented Jul 11, 2023

reedy commented Jul 11, 2023

jpatokal commented Jul 11, 2023

reedy commented Jul 11, 2023

chrisrosset commented Jul 12, 2023

reedy commented Jun 19, 2023 •

edited

Loading