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

fix: Improve docker context support #1724

Merged
merged 13 commits into from
Nov 22, 2023

Conversation

achimnol
Copy link
Member

@achimnol achimnol commented Nov 19, 2023

Also a follow-up to #1694.
Overall, this PR improves the search process of docker socket locations and make it consistent across other Backend.AI codebase and the installer.

  • Share the implementation of common.docker.get_docker_connector() with the installer and other Backend.AI codebase.
  • Update get_docker_connector() to support docker contexts (~/.docker/contexts/meta/...) to detect the currently used docker host settings.
    • According to docker context ls output, I assumed that meta.json always has .Endpoints.docker.Host field.
  • Clarify the priority of docker host configurations as shown in the precedence order:
    • DOCKER_HOST environment variable
    • The current client-side docker context
    • The platform-specific list of potential docker socket locations
  • Fix the installer:
    • Correct the usage of determine_docker_sudo()
    • Let it skip sudo chmod 666 <docker-socket> if the docker socket file resides inside the user's home directory (e.g., OrbStack, Docker Desktop) instead of the system-global /run or /var/run.

Example on macOS with OrbStack

$ docker context ls
NAME         DESCRIPTION                               DOCKER ENDPOINT                                  ERROR
default      Current DOCKER_HOST based configuration   unix:///var/run/docker.sock
orbstack *   OrbStack                                  unix:///Users/joongi/.orbstack/run/docker.sock
>>> from ai.backend.common.docker import get_docker_context_host, get_docker_connector
>>> get_docker_context_host()
'unix:///Users/joongi/.orbstack/run/docker.sock'
>>> get_docker_connector()
(PosixPath('/Users/joongi/.orbstack/run/docker.sock'), URL('http://localhost'), <aiohttp.connector.UnixConnector object at 0x104663ed0>)

Example on Linux with native Docker installation

>>> from ai.backend.common.docker import get_docker_context_host, get_docker_connector
>>> get_docker_context_host()
>>> get_docker_connector()
(PosixPath('/run/docker.sock'), URL('http://localhost'), <aiohttp.connector.UnixConnector object at 0xffffab584950>)

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • Test case(s) to demonstrate the difference of before/after

@achimnol achimnol added this to the 24.03 milestone Nov 19, 2023
@achimnol achimnol added type:bug Reports about that are not working comp:common Related to Common component comp:installer Related to Installer type:refactor Refactor codes or add tests. labels Nov 19, 2023
@achimnol achimnol self-assigned this Nov 19, 2023
@github-actions github-actions bot added the size:L 100~500 LoC label Nov 19, 2023
@achimnol achimnol added the urgency:3 Must be finished within a certain time frame. label Nov 19, 2023
- Fix a bug to miss awaiting on the `determine_docker_sudo()` function.
- For a docker socket residing inside the user's home directory,
  we can assume that it is accessible without sudo.
@github-actions github-actions bot added the comp:manager Related to Manager component label Nov 19, 2023
@achimnol achimnol requested a review from kyujin-cho November 20, 2023 16:50
@achimnol achimnol modified the milestones: 24.03, 23.09 Nov 22, 2023
@kyujin-cho kyujin-cho added this pull request to the merge queue Nov 22, 2023
Merged via the queue into main with commit 717079a Nov 22, 2023
22 checks passed
@kyujin-cho kyujin-cho deleted the fix/installer-better-docker-context-support branch November 22, 2023 05:17
github-merge-queue bot pushed a commit that referenced this pull request Nov 22, 2023
achimnol added a commit that referenced this pull request Nov 22, 2023
Backported-from: main
Backported-to: 23.09
achimnol added a commit that referenced this pull request Nov 22, 2023
…richer debug info (#1732)

Backported-from: main
Backported-to: 23.09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:common Related to Common component comp:installer Related to Installer comp:manager Related to Manager component size:L 100~500 LoC type:bug Reports about that are not working type:refactor Refactor codes or add tests. urgency:3 Must be finished within a certain time frame.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants