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

[Serve] Enable multiple ports in SkyServe replicas #4356

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Conless
Copy link
Contributor

@Conless Conless commented Nov 14, 2024

Current implementation of SkyServe only allows the replicas to expose one port. In some cases, services may need to expose multiple ports for custom controller, GUI interface, etc. This PR adds support for it by allowing multiple ports, but requiring user to specify one as the main port in the service section, if multiple ports are provided.

service:
  readiness_probe: /
  replicas: 2
  port: 8000

resources:
  ports:
  - 8000-8001
  - 3000
  cpus: 2

The output of sky serve status will look like:

Services
NAME              VERSION  UPTIME  STATUS  REPLICAS  ENDPOINT            
sky-service-xxxx  1        28s     READY   2/2       xxxx:30001  

Service Replicas
SERVICE_NAME      ID  VERSION  ENDPOINT             LAUNCHED     RESOURCES       STATUS  REGION     
sky-service-xxxx  1   1        http://xxxx:8000     50 secs ago  1x AWS(vCPU=2)  READY   us-east-1  
sky-service-xxxx  2   1        http://xxxx:8000     1 min ago    1x AWS(vCPU=2)  READY   us-east-1 

while the other ports (8001, 3000) are still accessible.

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below)
    • sky serve up, sky serve status and accessing the service with config:
    service:
      readiness_probe: /
      replicas: 2
      port: 8000
    
    resources:
      ports:
      - 8000-8001
      - 3000
      cpus: 2
    
    run: |
      python -m http.server 8000 &
      python -m http.server 8001 &
      python -m http.server 3000 &
  • All smoke tests: pytest tests/test_smoke.py
  • Relevant individual smoke tests: pytest tests/test_smoke.py::test_fill_in_the_name
  • Backward compatibility tests: conda deactivate; bash -i tests/backward_compatibility_tests.sh

@Conless
Copy link
Contributor Author

Conless commented Nov 14, 2024

Hi @cblmemo ! Would you like to have a look at this?

Copy link
Collaborator

@cblmemo cblmemo left a comment

Choose a reason for hiding this comment

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

Thanks for adding this feature @Conless ! Mostly LGTM. Left a discussion.

Also we might want to update related documents for this :))

sky/cli.py Outdated Show resolved Hide resolved
@cblmemo
Copy link
Collaborator

cblmemo commented Nov 18, 2024

Can we also update the PR description?

sky/cli.py Outdated Show resolved Hide resolved
@cblmemo
Copy link
Collaborator

cblmemo commented Nov 18, 2024

Also, it will be great if we can test on some real world usage (e.g. deploy an LLM service and expose metric port in vLLM or dashboard in ray

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.

2 participants