flightplan-tool / flightplan

Search for award inventory using Node.js and Headless Chrome!
Apache License 2.0
142 stars 41 forks source link

Dockerize #21

Closed Fffrank closed 5 years ago

Fffrank commented 5 years ago

Whoops -- I don't totally know how this works. I think I need to merge this into my own fork first. Sorry!

jd20 commented 5 years ago

Thanks for contributing this, I will take a look through. As I'm not super familiar with running Docker containers, would it be possible to get a brief idea how to test these changes, and perhaps we could get a section added to the README for others who would prefer to run it as a container.

Fffrank commented 5 years ago

Yes, I'll put something together. But for now if you'd like to try it, it's very easy:

Install Docker Install Docker-Compose

curl -O https://github.com/Fffrank/flightplan/master/docker-compose.yml
sudo docker-compose up

You'll need to edit the docker-compose file to point your config directory to where you'd like it to be on your host file system. You'll also need to create the db/, data/, and config/ directories (these should be created by flightplan if they do not exist, more on that below....)

That brings the server up and allows the client to connect to it.

Here's where it breaks at this point:

(I'm not a developer so I'm hacking at this trying to make it work) is that the client and server need to be run in two different containers (each docker should only run one process.) The server container does have it's port exposed and you can connect using the IP, but when the client container tries to reach it using localhost it fails (because each docker container gets it's own internal IP that resolves to localhost.)

I've modified the server to listen on the network name that docker-compose enables. There must be a way to make this change without breaking the server for everyone else. I'll keep researching that -- and obviously until I get that figured out, I don't want to push to have my merges made.

Lastly -- I have a problem where it appears that the server is not initializing tables in the db on first launch. It also doesn't create the required db/data/config directories. Is this something I should open a ticket for?

jd20 commented 5 years ago

The client is using CRA (create-react-app), which routes API requests to the server by using the "proxy" setting in package.json:

"proxy": "http://localhost:5000/"

You might find this article useful, it gives an example of running a CRA front-end that talks to a Node backend (which is exactly what Flightplan is doing):

https://codeburst.io/develop-create-react-app-with-docker-fb7fa3869ba

Regarding the database / directories getting created, all Flightplan is doing is a fs.existsSync() call on the path (and all the paths are listed in paths.js). If the files are getting created, either the fs.existsSync() call is returning true or the fs calls to create the files / database are failing. If you can reproduce that outside of Docket, please file an issue, and I'll take a look. If it only happens within the Docker containers, I won't really have a way to test it, and it's going to be a Docker-specific issue anyway.

Fffrank commented 5 years ago

@jd20 Yes, I understand how the CRA works. The problem is when you run from docker it's the same as running the client on two different machines -- hence localhost does not work. I'm working on a fix (possibly it could try localhost first and if that fails, roll over to a secondary proxy.)

It could be a permissions issue that is preventing the directories from being made. I'm doing some testing to see what I can figure out. It's good to know that the app should create it's own paths -- it initially appeared to me like they needed to be there before the app was run.

Fffrank commented 5 years ago

@jd20 Yeah, I figured that part. Does the server have the ability to create the db and account files or are they only created when you launch a "search" via the CLI tool? I wasn't even messing with the CLI but now it appears that it's crucial. I'm also running into serious issues getting chromium to run in a docker container.

jd20 commented 5 years ago

@Fffrank If a process tries to access some piece of data (database, accounts, etc...) it's also responsible for check if it exists, and creating it if necessary. The cli-server.js process can create the database (since it reads from it), but it would be empty. What's your use case, how are you planning for the database to get populated?

RE: Chromium in a Docker container, I'm afraid I don't have any experience with that, can't think of any reason it wouldn't be possible though.

yitzc commented 5 years ago

I'm also running into serious issues getting chromium to run in a docker container.

I also ran into serious difficulties, but managed it in the end. The laptop I did it on is stuck in a box somewhere (I moved this week and never uploaded it to github) so it'll take me some time to get to it, but it was based off of this: https://github.com/skalfyfan/dockerized-puppeteer (See conversation here https://github.com/GoogleChrome/puppeteer/issues/1645)

Fffrank commented 5 years ago

I was not able to get the CLI to work without modifying the underlying flightplan code. In order for this to run under docker without using horribly out-of-date versions of puppeteer/chromium, it needs to launch the browser using the --no-sandbox and --headless flags. This seems to make it play nice. As long as we're doing that, it also makes sense to add --disable-dev-shm-usage (docker limits processes to 64mb of RAM, this pushes the chromium ram cache to a disk cache instead.)

I'm wondering if it would be acceptable for @jd20 to add a --docker option to the flightplan cli? This could enable the required flags for pupeteer without breaking anything else for other people. I also found out that we can pass an environment variable via docker (or docker-compose) that will solve the proxy issue. That I have not tested, yet -- but I will continue to work on it.

jd20 commented 5 years ago

I think a --docker option to cli-search makes sense. The "--no-sandbox" and "--disable-dev-shm-usage" options should be passable to the engine's initialize() call using the args option, so the change can be localized to cli-search.js, seems like a pretty clean solution to me.

jd20 commented 5 years ago

Also, I'm getting ready to push a large internal API refactoring soon, along with much-needed documentation for those who are looking to tinker more with Flightplan (or modify it directly). But once your changes are ready, I can help with reapply-ing it to the new code base. The files in /bin are the least changed, it's mostly just everything inside /src. I may commit it to a branch first if it needs more bake time.

Fffrank commented 5 years ago

I think if you add the --docker option it should be the only change required to the code. I originally thought I was going to need to modify more of the code to create the dockers but with that change, I can create a Dockerfile that starts with a git pull (and shouldn't break when you release new versions.) For the cli option it will be clunky, but for the server and client it should really, really make deployment easy across any kind of architecture. You'll be impressed with how easy docker deployments are

edit: In other words: go ahead and push your changes. This is going to be cool!

jd20 commented 5 years ago

Cool 👍 If you have a chance, you may want to post a short guide on how to do a docker deployment to the Flightplan thread on FlyerTalk, that might get more people interested to try it out, myself included.

Fffrank commented 5 years ago

Will do. I'll wait for you to push your changes and add the --docker run option so I can prove it all works. then I'll post up on flyertalk and that should give you the necessary info to update the Read.me

jd20 commented 5 years ago

The refactoring changes have been pushed, btw. cli-search.js did not change much, as I predicted... if you need any help though on where to put things let me know, thanks!