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

Support cli var substitution in docker login command env #4871

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

Conversation

andylizf
Copy link
Collaborator

@andylizf andylizf commented Mar 4, 2025

Fix #2707.

This PR supports variable substitution$(...) in docker login envs including SKYPILOT_DOCKER_USERNAME, SKYPILOT_DOCKER_PASSWORD and SKYPILOT_DOCKER_SERVER. This is particularly useful when users need a dynamic pswd, like $(aws ecr get-login-password --region us-west-2).

Tests:

I manually create a private image, and store my docker access token in pswd.txt, then:

andyl@andylizf-dev-server ~/skypilot (fix-docker-password)> docker run andylizf/skypilot-ssh-test                                                                          (sky) 
Unable to find image 'andylizf/skypilot-ssh-test:latest' locally
docker: Error response from daemon: pull access denied for andylizf/skypilot-ssh-test, repository does not exist or may require 'docker login': denied: requested access to the resource is denied.
See 'docker run --help'.

andyl@andylizf-dev-server ~/skypilot (fix-docker-password) > sky api stop;  sky launch _a/helloworld.yaml --cloud gcp                                                     (sky) 
SkyPilot API server stopped.
YAML to run: _a/helloworld.yaml
Successfully executed command for Docker password
Successfully executed command for Docker password
Failed to connect to SkyPilot API server at http://127.0.0.1:46580. Starting a local server.
✓ SkyPilot API server started.
Considered resources (1 node):
----------------------------------------------------------------------------------------------
 CLOUD   INSTANCE        vCPUs   Mem(GB)   ACCELERATORS   REGION/ZONE     COST ($)   CHOSEN   
----------------------------------------------------------------------------------------------
 GCP     n2-standard-2   2       8         -              us-central1-a   0.10          ✔     
----------------------------------------------------------------------------------------------
Launching a new cluster 'sky-f623-andyl'. Proceed? [Y/n]: 
⚙︎ Launching on GCP us-central1 (us-central1-a).
├── Instance is up.
⠼ Launching - Initializing docker container  View logs: sky api logs -l sky-2025-0
└── Docker container is up.
⠴ Preparing SkyPilot runtime (2/3 - dependencies)  View logs: sky api logs -l sky-2025-03-08-04-52-08-695025/provision.log

No idea why Successfully executed command for Docker password appears twice, seems like a known issue, is the DockerLoginConfig inited multiple times causing from_env_vars executed multiple times? Executing user-provided commands not just once may have some issues here. Cc'ing @cblmemo

@andylizf andylizf requested review from cblmemo and Michaelvll March 8, 2025 04:57
@andylizf
Copy link
Collaborator Author

andylizf commented Mar 8, 2025

Also. we only update the sky/provision/docker_utils.py. Do we need to also update the legacy one sky/skylet/providers/command_runner.py?

@cblmemo
Copy link
Collaborator

cblmemo commented Mar 11, 2025

As discussed in our meeting, we might want to move the execution to the skypilot cluster instead of local laptop.

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.

[Provisioner] Support executable command for docker login in docker image
2 participants