sustainable-computing-io / kepler-model-server

Model Server for Kepler
Apache License 2.0
23 stars 25 forks source link

Include Production WSGI (Gunicorn) to replace Flask Default Server #263

Open KaiyiLiu1234 opened 2 months ago

KaiyiLiu1234 commented 2 months ago

To Address: https://github.com/sustainable-computing-io/kepler-model-server/issues/259

A side effect of using Gunicorn is that it will replace the address with default address and default port (8000). This can be resolved by binding (ex. binding model_server to 0.0.0.0:8100). Binding with 0.0.0.0 is acceptable but according to Flask, it is better to also introduce nginx server to act as a reverse proxy. Will this be necessary to include?

Also, since model_server AND offline_trainer (and in future online_trainer) expects to use their own Flask Apps, supervisord is needed to run multiple CMD for model_server and offline_trainer in Dockerfile.

Currently, this changes here works for metal docker compose so long as python3.8 -u src/server/model_server.py is replaced by gunicorn -b -0.0.0.0.8100 -src.server.model_server:app. However, there is more functionality that can be introduced depending on what is acceptable or not.

sthaha commented 2 months ago

@KaiyiLiu1234 https://developers.redhat.com/articles/2023/08/17/how-deploy-flask-application-python-gunicorn#containerization is good reference to follow

Binding with 0.0.0.0 is acceptable but according to Flask, it is better to also introduce nginx server to act as a reverse proxy. Will this be necessary to include?

I am not sure if the recommendation applies to containers / k8s.

Gunicorn should not be run as root because it would cause your application code to run as root, which is not secure. However, this means it will not be possible to bind to port 80 or 443. Instead, a reverse proxy such as nginx or Apache httpd should be used in front of Gunicorn.

The part applies to containers is don't run as root. However, binding to port 80/443 isn't needed for containers.

You can bind to all external IPs on a non-privileged port using the -b 0.0.0.0 option. Don’t do this when using a reverse proxy setup, otherwise it will be possible to bypass the proxy.

Again, this is fine for a containerized env. I feel following the article linked should be good enough to fix our issue.

KaiyiLiu1234 commented 2 months ago

@sthaha @sunya-ch I incorporated the Gunicorn changes into a test dockerfile for now. How we interact with the image will change now in docker composes and kubernetes resources (for instance command should not be python model_server.py - it should instead use gunicorn or no command as the CMD for the dockerfile can handle launching the gunicorn servers for model server, offline trainer and online trainer). I can also include in the makefile commands to run the new gunicorn test dockerfile in this PR or in a future PR?