bentoml / BentoML

The easiest way to serve AI apps and models - Build Model Inference APIs, Job queues, LLM apps, Multi-model pipelines, and more!
https://bentoml.com
Apache License 2.0
7.14k stars 791 forks source link

Env var BENTOML_PORT should set API server port #1588

Closed parano closed 2 years ago

parano commented 3 years ago

Describe the bug

The environment variable BENTOML_PORT is expected to be used as the port number for API server, and this is currently implemented using click's env_var option: https://github.com/bentoml/BentoML/blob/v0.12.1/bentoml/cli/bento_service.py#L254

This works when launching API server from CLI, e.g.:

BENTOML_PORT=3030 bentoml serve-gunicorn IrisClassifier:latest

But failed when using docker run to pass the env var, see steps to reproduce below:

To Reproduce

cd $(bentoml get IrisClassifier:latest --print-location --quiet)
docker build . -t iris_classifier:latest
docker run -p 5001:5001 -e BENTOML_PORT='5001' iris_classifier

See user report: https://github.com/bentoml/BentoML/discussions/1585#discussioncomment-632910

Expected behavior

The server is expected to run on port 5001, but instead it still runs on default 5000 port.

Screenshots/Logs

Environment:

Additional context

ssheng commented 3 years ago

The issue is only limited to BENTOML_PORT environment variable. Other click parameters work just fine. The problem is in how we generate Dockerfile. In the Dockerfile template, we set a environment variable PORT to 5000, seemingly for Heroku.

https://github.com/bentoml/BentoML/blob/238bc9757033e5b3ffad6810c5dba95ac596c70e/bentoml/saved_bundle/templates.py#L113

In the ENTRYPOINT script, we override BENTOML_PORT to with the value of PORT, which explained why --port parameter works but not BENTOML_PORT.

https://github.com/bentoml/BentoML/blob/238bc9757033e5b3ffad6810c5dba95ac596c70e/bentoml/saved_bundle/docker-entrypoint.sh#L19

Anyone knows the contexts of Heroku port 5000? The solution maybe to set PORT with the value of BENTOML_PORT instead of vice versa.

jjmachan commented 3 years ago

The web process must listen for HTTP traffic on $PORT, which is set by Heroku. EXPOSE in Dockerfile is not respected, but can be used for local testing. Only HTTP requests are supported. Unsupported Dockerfile commands - EXPOSE - While EXPOSE can be used for local testing, it is not supported in Heroku’s container runtime. Instead, your web process/code should get the $PORT environment variable. Heroku docs

so the the bento service should listen on $PORT env var which will be set by Heroku

I was playing around with this and like what @ssheng had suggested.

My idea

assign the default port to the BENTOML_PORT directly so we can overide this via the -e flag in docker also.

#templates.py
...
ENV BENTOML_PORT 5000
EXPOSE $BENTOML_PORT
...

if a $PORT variable is present in the env, then overide the BENOTML_PORT to use that

#entrypoint 
if [[ -v PORT ]]; then
export BENTOML_PORT=$PORT
fi

what do you guys think of this approach

jjmachan commented 3 years ago

I've tested this locally and fixes this issue, I'll test it on heroku to verify nothing breaks there

harshitsinghai77 commented 3 years ago

should we close this issue as it's already merged?

parano commented 2 years ago

Yes, thanks @harshitsinghai77