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

Add dockerfile #441

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add dockerfile #441

wants to merge 1 commit into from

Conversation

JackPotte
Copy link

@JackPotte JackPotte commented Mar 12, 2023

Per https://phabricator.wikimedia.org/T280734.

To start:

  • docker compose -f docker-compose.yml -f docker-compose.override.yml up -d
  • cp .env.dev .env
  • docker compose exec php bash
    • composer install
    • bin/console doctrine:database:create
    • bin/console doctrine:migrations:migrate
    • ./node_modules/.bin/encore dev --watch

Copy link
Member

@MusikAnimal MusikAnimal left a comment

Choose a reason for hiding this comment

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

Amazing, thank you!! Just a few questions

.env.dev Outdated Show resolved Hide resolved
src/DoctrineMigrations/Version20170623203059.php Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (eb753e0) 65.61% compared to head (8fed805) 65.61%.

❗ Current head 8fed805 differs from pull request most recent head 36c1f13. Consider uploading reports for the commit 36c1f13 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #441   +/-   ##
=========================================
  Coverage     65.61%   65.61%           
  Complexity     1242     1242           
=========================================
  Files            46       46           
  Lines          3705     3705           
=========================================
  Hits           2431     2431           
  Misses         1274     1274           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@MusikAnimal
Copy link
Member

I have now merged #443 which reworks the .env file usage, and moves the Doctrine migration classes to their recommended location. So I think you just need to remove the .env.dev file added with this PR, and the changes to the migration files, and hopefully we're all set. You may want to rebase against main as well.

Thanks again for helping out! We've been wanting a Docker container for a long time now.

@JackPotte
Copy link
Author

The rebase is done, thanks to you.

Copy link
Member

@MusikAnimal MusikAnimal left a comment

Choose a reason for hiding this comment

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

I ran into some issues following the instructions. docker compose -f docker-compose.yml -f docker-compose.override.yml up -d ran without any errors, but entering the container I see this:

I have no name!@php8:/var/www/xtools$

It should have a shell name, right?

Also, the /var/www/xtools directory appears to be empty. Does the repo need to be cloned first? Is it possible to automate that in our Dockerfile?

Finally, it's unclear what port the server is running on.

Apologies if I ask any stupid questions! If it wasn't obvious by now, I'm not incredibly savvy with Docker, hehe :)

Dockerfile Outdated Show resolved Hide resolved
@JackPotte
Copy link
Author

JackPotte commented Mar 16, 2023

"I have no name!" is "Normal" because the I had set the UID 1000 to avoid the container to claim to the host machine root account every file it touches (this UID can be overrided by a Docker .env file).

@JackPotte
Copy link
Author

JackPotte commented Mar 16, 2023

/var/www/xtools is empty because of my mistake in docker-compose.yml:
$HOME/www/xtools:/var/www/xtools
In my host machine, xtools was one of the repos I needed to access to from the same container, so I had put an absolute path instead of a relative one, and then it could only work on my PC. I'm fixing that immediately...

@JackPotte JackPotte force-pushed the add-dockerfile branch 2 times, most recently from 85e148a to 8fed805 Compare March 16, 2023 21:32
@JackPotte
Copy link
Author

JackPotte commented Mar 16, 2023

I think this is now better for CLI.

Concerning the ports, as I said In Phabricator, we would need to add a vhost (ex: Apache or Nginx) to know which container to add to listen to 80 and 443 (and make Xtools work in our navigators).
MySQL is still on 3306.
I had put PHP on 8000 to allow the host machine or other containers (ex: the web server) to contact it through it in the future (but we can also force a fixed IP instead). NB: if these containers are in the same docker-compose, they can call it by its name too.

@MusikAnimal
Copy link
Member

So, I changed my .env.local to have the credentials you used in your .env.dev file (namely, the DATABASE_HOST is mariadb as opposed to localhost). That worked great and I was able to run the migrations.

I did get an error when trying to run ./node_modules/.bin/encore dev however: Node Sass does not yet support your current environment: Linux 64-bit with Unsupported runtime (93). I then tried running npm install and got:

npm ERR! code EACCES
npm ERR! syscall mkdir
npm ERR! path /.npm
npm ERR! errno -13
npm ERR! 
npm ERR! Your cache folder contains root-owned files, due to a bug in
npm ERR! previous versions of npm which has since been addressed.
npm ERR! 
npm ERR! To permanently fix this problem, please run:
npm ERR!   sudo chown -R 1000:1000 "/.npm"

I tried running chown as suggested and two problems: there's no sudo on the VM (I think that's expected?), and the /.npm directory doesn't exist. Any suggestions on how to fix that?

I'm still confused about the server. I see something is running on port 8000. Browsing to that I get a browser error "The connection was reset". I know you said you need to add Apache or Nginx (I think we'd prefer the former since that's what we use in production). Is that still needed for the server to work? When I work off of my local without Docker, I use the built-in Symfony server, which I believe is just a wrapper around the PHP default server. So I think it's OK if it's all single-threaded or whatever, if that means anything.

Thanks for your patience and working on this for us!

@JackPotte
Copy link
Author

JackPotte commented Apr 16, 2023

Hello and sorry for the late response.

I've just fixed the npm install problem.

Concerning the HTTP server, one purpose of Docker is to provide the exact same configuration for one app in all of its environments. So, I think that using the Symfony server only locally would just add some complexity and potential undetected bugs in the environments which would not use it. For example, a typical one I've seen in my carrer was a difference in these variables, causing some crashes in prod:

client_max_body_size 32m;
fastcgi_read_timeout 600;
proxy_read_timeout 600;

So I've begun to look in SSH on the wmflabs.org server, in /data/project/xtools, and I didn't find any Apache or Nginx vhost but the config for https://wikitech.wikimedia.org/wiki/Help:Toolforge/Web/Lighttpd instead.
Do you think this is the target for our configurations, especially since the Kubernetes server has become the norm: https://wikitech.wikimedia.org/wiki/Help:Toolforge/Kubernetes?
FYI, Lighttpd doesn't provide an official Docker image, so we should add in the docker-compose.yml the most used among https://hub.docker.com/search?q=lighthttpd

@MusikAnimal
Copy link
Member

Hello and sorry for the late response.

No worries :) I've been very busy as well!

Concerning the HTTP server, one purpose of Docker is to provide the exact same configuration for one app in all of its environments. So, I think that using the Symfony server only locally would just add some complexity and potential undetected bugs in the environments which would not use it.

If we get it running with Apache that'd be great, just so long as the profiler is still there: https://symfony.com/doc/current/profiler.html That is a very important development tool.

So I've begun to look in SSH on the wmflabs.org server, in /data/project/xtools, and I didn't find any Apache or Nginx vhost but the config for https://wikitech.wikimedia.org/wiki/Help:Toolforge/Web/Lighttpd instead. Do you think this is the target for our configurations, especially since the Kubernetes server has become the norm: https://wikitech.wikimedia.org/wiki/Help:Toolforge/Kubernetes? FYI, Lighttpd doesn't provide an official Docker image, so we should add in the docker-compose.yml the most used among https://hub.docker.com/search?q=lighthttpd

We're not on Toolforge anymore but on Cloud VPS. The instructions for setting up the server can be found at https://wikitech.wikimedia.org/wiki/Tool:XTools. Sorry I didn't mention that doc sooner! Most everything in that doc can probably be followed for the Docker container, but you can ignore the API server part, as well awstats, and the Apache config doesn't need to be quite as complicated. If it's too much work, I think it's totally fine to use the Symfony web server. You're right it's better to be close to prod as possible, but in our case there isn't much variance between dev and prod when it comes to writing code.

I wanted to also say that since you started this PR, I've reworked a lot of the code. That's all done now, so what's in main is stable.

@Justman100
Copy link

@MusikAnimal

What here now?

@MusikAnimal
Copy link
Member

All contributions are welcome! Feel free to fork the repo and borrow from this PR, since you can't push to it. I would prefer to ping @JackPotte once more to see if they're interested in finishing it up first, though.

The truth for me is that I'm a complete dummy when it comes to Docker :/ I'm happy to help test, but won't be able to offer much help beyond that unfortunately.

@JackPotte
Copy link
Author

Hello, it's in my TODO list for one year (sorry about that)... So maybe next month.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants