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

Bind torchserve container ports to localhost ports #2646

Merged
merged 3 commits into from
Oct 3, 2023
Merged

Conversation

namannandan
Copy link
Collaborator

@namannandan namannandan commented Oct 3, 2023

Description

When starting torchserve inside a container, although the address configurations are set to 0.0.0.0 which is required to access torchserve from outside the container, access to torchserve in the container can be limited to localhost by binding the container ports to localhost ports as follows

config.properties

inference_address=http://0.0.0.0:8080
management_address=http://0.0.0.0:8081
metrics_address=http://0.0.0.0:8082
number_of_netty_threads=32
job_queue_size=1000
model_store=/home/model-server/model-store
workflow_store=/home/model-server/wf-store

Container ports not bound to localhost ports

$ docker run --rm -it -p 8080:8080 -p 8081:8081 -p 8082:8082 pytorch/torchserve:latest-cpu
$ curl http://localhost:8080/ping
{
  "status": "Healthy"
}
$ curl http://10.31.59.11:8080/ping
{
  "status": "Healthy"
}

Container ports bound to localhost ports

$ docker run --rm -it -p 127.0.0.1:8080:8080 -p 127.0.0.1:8081:8081 -p 127.0.0.1:8082:8082 pytorch/torchserve:latest-cpu
$ curl http://localhost:8080/ping  
{
  "status": "Healthy"
}
$ curl http://10.31.59.11:8080/ping
curl: (7) Failed to connect to 10.31.59.11 port 8080 after 2 ms: Couldn't connect to server

Fixes #2610

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Feature/Issue validation/testing

  • Manual test shown in the description above
  • CI

@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Merging #2646 (dee0484) into master (1f1ab2b) will not change coverage.
The diff coverage is n/a.

❗ Current head dee0484 differs from pull request most recent head 043f824. Consider uploading reports for the commit 043f824 to get more accurate results

@@           Coverage Diff           @@
##           master    #2646   +/-   ##
=======================================
  Coverage   71.34%   71.34%           
=======================================
  Files          85       85           
  Lines        3905     3905           
  Branches       58       58           
=======================================
  Hits         2786     2786           
  Misses       1115     1115           
  Partials        4        4           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@msaroufim
Copy link
Member

LGTM feel free to add a workflow trigger to benchmark CI job to trigger it from your branch

@lxning lxning marked this pull request as ready for review October 3, 2023 16:34
@lxning lxning added this pull request to the merge queue Oct 3, 2023
Merged via the queue into master with commit 4051ae3 Oct 3, 2023
12 checks passed
@namannandan namannandan deleted the issue_2610_docker branch November 9, 2023 18:09
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.

default IP address setting
3 participants