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

[Docs] Update docker container doc #2573

Merged
merged 2 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions docs/source/examples/docker-containers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,25 @@ For example, to use the :code:`ubuntu:20.04` image from Docker Hub:
run: |
# Will run inside container

For Docker images hosted on private registries, you can provide the registry authentication details using the :code:`envs` section:
For Docker images hosted on private registries, you can provide the registry authentication details using :ref:`task environment variables <env-vars>`:

.. code-block:: yaml

# private_docker.yaml
# ecr_private_docker.yaml
resources:
image_id: docker:<your-private-image>:<tag>
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you're giving a concrete example, might as well also update this to something concrete:

Suggested change
image_id: docker:<your-private-image>:<tag>
image_id: docker:<your-user-id>.dkr.ecr.us-east-1.amazonaws.com/your-private-image:latest

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh actually we only require users to specify your-private-image in the image_id section... See here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see the confusion. Maybe we could support both? Let me raise an PR to fix it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in #2574


envs:
SKYPILOT_DOCKER_USERNAME: <username>
SKYPILOT_DOCKER_PASSWORD: <password>
SKYPILOT_DOCKER_SERVER: <server>
SKYPILOT_DOCKER_USERNAME: AWS
# SKYPILOT_DOCKER_PASSWORD: <password>
SKYPILOT_DOCKER_SERVER: <your-user-id>.dkr.ecr.us-east-1.amazonaws.com

Alternatively, you can set these environment variables directly through the CLI:
We suggest to setting the :code:`SKYPILOT_DOCKER_PASSWORD` environment variable through the CLI (see :ref:`passing secrets <passing-secrets>`):

.. code-block:: bash

$ sky launch \
--env SKYPILOT_DOCKER_PASSWORD=$(aws ecr get-login-password --region us-east-1) \
--env SKYPILOT_DOCKER_USERNAME=AWS \
private_docker.yaml
$ export SKYPILOT_DOCKER_PASSWORD=$(aws ecr get-login-password --region us-east-1)
$ sky launch ecr_private_docker.yaml --env SKYPILOT_DOCKER_PASSWORD

Running Docker Containers as Tasks
----------------------------------
Expand Down
3 changes: 3 additions & 0 deletions docs/source/running-jobs/environment-variables.rst
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ For example, this is useful for passing secrets (see below) or passing configura

See complete examples at `llm/vllm/serve.yaml <https://github.com/skypilot-org/skypilot/blob/596c1415b5039adec042594f45b342374e5e6a00/llm/vllm/serve.yaml#L4-L5>`_ and `llm/vicuna/train.yaml <https://github.com/skypilot-org/skypilot/blob/596c1415b5039adec042594f45b342374e5e6a00/llm/vicuna/train.yaml#L111-L116>`_.


.. _passing-secrets:

Passing secrets
~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down