Skip to content

Commit

Permalink
Support DOCKER_HOST variable passing to Breeze commands (apache#36011)
Browse files Browse the repository at this point in the history
In some cases you might want to override the default way how
Docker determines where to connect by seting DOCKER_HOST variable.

This is generally working out of the box by having the variable
set, but then it does not work well with `--builder` settings
in `builder` plugin. This PR adds support for DOCKER_HOST to be added
to all shell and build commands by disabling the builder when the
variable is set and explicit forwarding of the DOCKER_HOST
variable when running docker commands. You can also override the
DOCKER_HOST variable via `--docker-host` command line switch.

Build arguments in Breeze commands have been sorted as well as their
click options as the command option list grew large enough to be
messy if they are not alphabetically sorted.

We also add automated support for docker-in-docker case which we use in
some tests - the Docker-In-Docker should now work in most cases even
if you set DOCKER_HOST:

* if the ~/.docker/run/docker.sock is used - we change the mapping
  of socket to /var/run/docker.sock
* if DOCKER_HOST is not a "unix://" socket, we pass it through
  (hoping it will **just** work as tcp:// or ssh:// - named pipes
  are not forwardeable anyway)
* if Rootless Docker is used, we follow the rootless docker standard
  for XDG_RUNTIME_DIR/docker.sock (fallback to /run/(UID)/docker.sock

Without DOCKER_HOST we just forward /var/run/docker.sock which
should work also on MacOS - with, or without root-owned socket..

This gets a litte tricky because the way it works on MacOS /
DockerDesktop is a little magicalx. If you set DOCKER_HOST manually to
point to your HOME_DIR socket, you are actually supposed to forward
`/var/run/docker.sock` not the original socket because mappings you
define on MacOS are actually the VM mappings (and your home dir is
mounted to VM - EXCEPT the docker socket which is available on the VM as
/var/run/docker.sock)

More about it in this issue: docker/for-mac#6529
Also docs about MacOS Docker: https://docs.docker.com/desktop/mac/permission-requirements/
  • Loading branch information
potiuk authored Dec 2, 2023
1 parent 41f4766 commit e331559
Show file tree
Hide file tree
Showing 24 changed files with 452 additions and 258 deletions.
9 changes: 7 additions & 2 deletions dev/breeze/src/airflow_breeze/commands/ci_image_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
option_dev_apt_command,
option_dev_apt_deps,
option_docker_cache,
option_docker_host,
option_dry_run,
option_eager_upgrade_additional_requirements,
option_github_repository,
Expand Down Expand Up @@ -85,6 +86,7 @@
from airflow_breeze.utils.docker_command_utils import (
build_cache,
check_remote_ghcr_io_commands,
get_docker_build_env,
make_sure_builder_configured,
perform_environment_checks,
prepare_docker_build_command,
Expand Down Expand Up @@ -253,6 +255,7 @@ def get_exitcode(status: int) -> int:
@option_dev_apt_command
@option_dev_apt_deps
@option_docker_cache
@option_docker_host
@option_dry_run
@option_eager_upgrade_additional_requirements
@option_github_repository
Expand Down Expand Up @@ -293,6 +296,7 @@ def build(
dev_apt_command: str | None,
dev_apt_deps: str | None,
docker_cache: str,
docker_host: str | None,
eager_upgrade_additional_requirements: str | None,
github_repository: str,
github_token: str | None,
Expand Down Expand Up @@ -363,6 +367,7 @@ def run_build(ci_image_params: BuildCiParams) -> None:
dev_apt_command=dev_apt_command,
dev_apt_deps=dev_apt_deps,
docker_cache=docker_cache,
docker_host=docker_host,
eager_upgrade_additional_requirements=eager_upgrade_additional_requirements,
force_build=True,
github_repository=github_repository,
Expand Down Expand Up @@ -724,8 +729,7 @@ def run_build_ci_image(
output=output,
)
else:
env = os.environ.copy()
env["DOCKER_BUILDKIT"] = "1"
env = get_docker_build_env(ci_image_params)
subprocess.run(
[
sys.executable,
Expand Down Expand Up @@ -797,6 +801,7 @@ def rebuild_or_pull_ci_image_if_needed(command_params: ShellParams | BuildCiPara
)
ci_image_params = BuildCiParams(
builder=command_params.builder,
docker_host=command_params.docker_host,
force_build=command_params.force_build,
github_repository=command_params.github_repository,
image_tag=command_params.image_tag,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"--docker-cache",
"--version-suffix-for-pypi",
"--build-progress",
"--docker-host",
],
},
{
Expand Down
7 changes: 7 additions & 0 deletions dev/breeze/src/airflow_breeze/commands/developer_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
option_celery_flower,
option_database_isolation,
option_db_reset,
option_docker_host,
option_downgrade_sqlalchemy,
option_dry_run,
option_executor_shell,
Expand Down Expand Up @@ -180,6 +181,7 @@ def run(self):
@option_celery_flower
@option_database_isolation
@option_db_reset
@option_docker_host
@option_downgrade_sqlalchemy
@option_dry_run
@option_executor_shell
Expand Down Expand Up @@ -220,6 +222,7 @@ def shell(
database_isolation: bool,
db_reset: bool,
downgrade_sqlalchemy: bool,
docker_host: str | None,
executor: str,
extra_args: tuple,
force_build: bool,
Expand Down Expand Up @@ -272,6 +275,7 @@ def shell(
database_isolation=database_isolation,
db_reset=db_reset,
downgrade_sqlalchemy=downgrade_sqlalchemy,
docker_host=docker_host,
executor=executor,
extra_args=extra_args if not max_time else ["exit"],
force_build=force_build,
Expand Down Expand Up @@ -330,6 +334,7 @@ def shell(
@option_celery_flower
@option_database_isolation
@option_db_reset
@option_docker_host
@option_dry_run
@option_executor_start_airflow
@option_force_build
Expand Down Expand Up @@ -362,6 +367,7 @@ def start_airflow(
database_isolation: bool,
db_reset: bool,
dev_mode: bool,
docker_host: str | None,
executor: str,
extra_args: tuple,
force_build: bool,
Expand Down Expand Up @@ -410,6 +416,7 @@ def start_airflow(
database_isolation=database_isolation,
db_reset=db_reset,
dev_mode=dev_mode,
docker_host=docker_host,
executor=executor,
extra_args=extra_args,
force_build=force_build,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"options": [
"--project-name",
"--restart",
"--docker-host",
],
},
{
Expand Down Expand Up @@ -87,6 +88,7 @@
"options": [
"--project-name",
"--restart",
"--docker-host",
],
},
{
Expand Down Expand Up @@ -195,6 +197,7 @@
"options": [
"--project-name",
"--restart",
"--docker-host",
],
},
{
Expand Down
2 changes: 2 additions & 0 deletions dev/breeze/src/airflow_breeze/commands/main_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
option_builder,
option_database_isolation,
option_db_reset,
option_docker_host,
option_dry_run,
option_forward_credentials,
option_github_repository,
Expand Down Expand Up @@ -108,6 +109,7 @@ def get_command(self, ctx: Context, cmd_name: str):
@option_builder
@option_database_isolation
@option_db_reset
@option_docker_host
@option_dry_run
@option_forward_credentials
@option_github_repository
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
# under the License.
from __future__ import annotations

import os
import sys
from copy import deepcopy

Expand Down Expand Up @@ -48,6 +47,7 @@
option_dev_apt_command,
option_dev_apt_deps,
option_docker_cache,
option_docker_host,
option_dry_run,
option_github_repository,
option_github_token,
Expand Down Expand Up @@ -80,6 +80,7 @@
from airflow_breeze.utils.docker_command_utils import (
build_cache,
check_remote_ghcr_io_commands,
get_docker_build_env,
make_sure_builder_configured,
perform_environment_checks,
prepare_docker_build_command,
Expand Down Expand Up @@ -220,6 +221,7 @@ def prod_image():
@option_dev_apt_command
@option_dev_apt_deps
@option_docker_cache
@option_docker_host
@option_dry_run
@option_github_repository
@option_github_token
Expand Down Expand Up @@ -267,6 +269,7 @@ def build(
disable_mysql_client_installation: bool,
disable_postgres_client_installation: bool,
docker_cache: str,
docker_host: str | None,
github_repository: str,
github_token: str | None,
image_tag: str,
Expand Down Expand Up @@ -325,6 +328,7 @@ def run_build(prod_image_params: BuildProdParams) -> None:
debian_version=debian_version,
dev_apt_command=dev_apt_command,
dev_apt_deps=dev_apt_deps,
docker_host=docker_host,
disable_airflow_repo_cache=disable_airflow_repo_cache,
disable_mssql_client_installation=disable_mssql_client_installation,
disable_mysql_client_installation=disable_mysql_client_installation,
Expand Down Expand Up @@ -673,8 +677,7 @@ def run_build_production_image(
if prod_image_params.prepare_buildx_cache:
build_command_result = build_cache(image_params=prod_image_params, output=output)
else:
env = os.environ.copy()
env["DOCKER_BUILDKIT"] = "1"
env = get_docker_build_env(prod_image_params)
build_command_result = run_command(
prepare_docker_build_command(
image_params=prod_image_params,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"--docker-cache",
"--version-suffix-for-pypi",
"--build-progress",
"--docker-host",
],
},
{
Expand Down
1 change: 1 addition & 0 deletions dev/breeze/src/airflow_breeze/params/build_ci_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ def prepare_arguments_for_docker_build_command(self) -> list[str]:
self._opt_arg("ADDITIONAL_PYTHON_DEPS", self.additional_python_deps)
self._opt_arg("DEV_APT_COMMAND", self.dev_apt_command)
self._opt_arg("DEV_APT_DEPS", self.dev_apt_deps)
self._opt_arg("DOCKER_HOST", self.docker_host)
self._opt_arg("ADDITIONAL_DEV_APT_COMMAND", self.additional_dev_apt_command)
self._opt_arg("ADDITIONAL_DEV_APT_DEPS", self.additional_dev_apt_deps)
self._opt_arg("ADDITIONAL_DEV_APT_ENV", self.additional_dev_apt_env)
Expand Down
1 change: 1 addition & 0 deletions dev/breeze/src/airflow_breeze/params/build_prod_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ def prepare_arguments_for_docker_build_command(self) -> list[str]:
self._opt_arg("ADDITIONAL_RUNTIME_APT_ENV", self.additional_runtime_apt_env)
self._opt_arg("DEV_APT_COMMAND", self.dev_apt_command)
self._opt_arg("DEV_APT_DEPS", self.dev_apt_deps)
self._opt_arg("DOCKER_HOST", self.docker_host)
self._opt_arg("RUNTIME_APT_COMMAND", self.runtime_apt_command)
self._opt_arg("RUNTIME_APT_DEPS", self.runtime_apt_deps)
self._opt_arg("VERSION_SUFFIX_FOR_PYPI", self.version_suffix_for_pypi)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class CommonBuildParams:
dev_apt_command: str | None = None
dev_apt_deps: str | None = None
docker_cache: str = "registry"
docker_host: str | None = os.environ.get("DOCKER_HOST")
github_actions: str = os.environ.get("GITHUB_ACTIONS", "false")
github_repository: str = APACHE_AIRFLOW_GITHUB_REPOSITORY
github_token: str = os.environ.get("GITHUB_TOKEN", "")
Expand Down
84 changes: 77 additions & 7 deletions dev/breeze/src/airflow_breeze/params/shell_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
from copy import deepcopy
from dataclasses import dataclass, field
from functools import cached_property
from os import _Environ
from pathlib import Path

from airflow_breeze.branch_defaults import AIRFLOW_BRANCH, DEFAULT_AIRFLOW_CONSTRAINTS_BRANCH
Expand Down Expand Up @@ -54,7 +53,7 @@
get_airflow_version,
)
from airflow_breeze.utils.console import get_console
from airflow_breeze.utils.host_info_utils import get_host_group_id, get_host_os
from airflow_breeze.utils.host_info_utils import get_host_group_id, get_host_os, get_host_user_id
from airflow_breeze.utils.packages import get_suspended_provider_folders
from airflow_breeze.utils.path_utils import (
AIRFLOW_SOURCES_ROOT,
Expand All @@ -80,6 +79,26 @@ def add_mssql_compose_file(compose_file_list: list[Path]):
compose_file_list.append(DOCKER_COMPOSE_DIR / "backend-mssql-docker-volume.yml")


def generated_socket_compose_file(local_socket_path: str) -> str:
return f"""
---
services:
airflow:
volumes:
- {local_socket_path}:/var/run/docker.sock
"""


def generated_docker_host_environment() -> str:
return """
---
services:
airflow:
environment:
- DOCKER_HOST=${DOCKER_HOST}
"""


def _set_var(env: dict[str, str], variable: str, attribute: str | bool | None, default: str | None = None):
"""Set variable in env dict.
Expand Down Expand Up @@ -126,6 +145,7 @@ class ShellParams:
"DEFAULT_CONSTRAINTS_BRANCH", DEFAULT_AIRFLOW_CONSTRAINTS_BRANCH
)
dev_mode: bool = False
docker_host: str | None = os.environ.get("DOCKER_HOST")
downgrade_sqlalchemy: bool = False
dry_run: bool = False
enable_coverage: bool = False
Expand Down Expand Up @@ -300,6 +320,7 @@ def compose_file(self) -> str:
compose_file_list.append(DOCKER_COMPOSE_DIR / "integration-celery.yml")

compose_file_list.append(DOCKER_COMPOSE_DIR / "base.yml")
self.add_docker_in_docker(compose_file_list)
compose_file_list.extend(backend_files)
compose_file_list.append(DOCKER_COMPOSE_DIR / "files.yml")

Expand Down Expand Up @@ -382,6 +403,56 @@ def mssql_data_volume(self) -> str:
# mssql_data_volume variable is only used in case of tmpfs
return ""

def add_docker_in_docker(self, compose_file_list: list[Path]):
generated_compose_file = DOCKER_COMPOSE_DIR / "_generated_docker_in_docker.yml"
unix_prefix = "unix://"
if self.docker_host:
if self.docker_host.startswith(unix_prefix):
# Socket is locally available
socket_path = Path(self.docker_host[len(unix_prefix) :])
if (
get_host_os() == "darwin"
and socket_path.resolve() == (Path.home() / ".docker" / "run" / "docker.sock").resolve()
):
# We are running on MacOS and the socket is the default "user" bound one
# We need to pretend that we are running on Linux and use the default socket
# in the VM instead - see https://github.com/docker/for-mac/issues/6545
compose_file_list.append(DOCKER_COMPOSE_DIR / "docker-socket.yml")
return
if socket_path.is_socket():
generated_compose_file.write_text(generated_socket_compose_file(socket_path.as_posix()))
compose_file_list.append(generated_compose_file)
else:
get_console().print(
f"[warning]The socket {socket_path} pointed at by DOCKER_HOST does not exist or is "
"not a socket. Cannot use it for docker-compose for docker-in-docker forwarding[/]\n"
"[info]If you know where your socket is, you can set DOCKER_HOST "
"environment variable to unix://path_to_docker.sock[/]\n"
)
else:
# Socket is something different (TCP?) just pass it through as DOCKER_HOST variable
generated_compose_file.write_text(generated_docker_host_environment())
compose_file_list.append(generated_compose_file)
elif self.rootless_docker:
xdg_runtime_dir = Path(os.environ.get("XDG_RUNTIME_DIR", f"/run/user/{get_host_user_id()}"))
socket_path = xdg_runtime_dir / "docker.sock"
if socket_path.is_socket():
generated_compose_file.write_text(generated_socket_compose_file(socket_path.as_posix()))
compose_file_list.append(generated_compose_file)
else:
get_console().print(
f"[warning]The socket {socket_path} does not exist or is not a socket. "
"Cannot use it for docker-compose for docker-in-docker forwarding[/]\n"
"[info]If you know where your socket is, you can set DOCKER_HOST environment variable "
"to unix://path_to_docker.sock[/]\n"
)
else:
# We fall back to default docker socket when no host is defined including MacOS
# NOTE! Even if we are using "desktop-linux" context where "/var/run/docker.sock" is not used,
# Docker engine works fine because "/var/run/docker.sock" is mounted at the VM and there
# the /var/run/docker.sock is available. See https://github.com/docker/for-mac/issues/6545
compose_file_list.append(DOCKER_COMPOSE_DIR / "docker-socket.yml")

@cached_property
def rootless_docker(self) -> bool:
try:
Expand All @@ -400,7 +471,7 @@ def rootless_docker(self) -> bool:
return False

@cached_property
def env_variables_for_docker_commands(self) -> _Environ:
def env_variables_for_docker_commands(self) -> dict[str, str]:
"""
Constructs environment variables needed by the docker-compose command, based on Shell parameters
passed to it.
Expand Down Expand Up @@ -505,12 +576,11 @@ def env_variables_for_docker_commands(self) -> _Environ:
_set_var(_env, "WEBSERVER_HOST_PORT", None, WEBSERVER_HOST_PORT)
_set_var(_env, "_AIRFLOW_RUN_DB_TESTS_ONLY", self.run_db_tests_only)
_set_var(_env, "_AIRFLOW_SKIP_DB_TESTS", self.skip_db_tests)

self._generate_env_for_docker_compose_file_if_needed(_env)

target_environment = deepcopy(os.environ)
target_environment.update(_env)
return target_environment
_target_env: dict[str, str] = os.environ.copy()
_target_env.update(_env)
return _target_env

@staticmethod
def _generate_env_for_docker_compose_file_if_needed(env: dict[str, str]):
Expand Down
Loading

0 comments on commit e331559

Please sign in to comment.