Skip to content

Commit

Permalink
Merge pull request #1640 from rmartin16/no-docker-hints
Browse files Browse the repository at this point in the history
 Silence hints/recommendations from Docker in the console
  • Loading branch information
freakboy3742 committed Feb 14, 2024
2 parents 0e8aca2 + 4eb8f84 commit 7682008
Show file tree
Hide file tree
Showing 16 changed files with 195 additions and 33 deletions.
1 change: 1 addition & 0 deletions changes/1635.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The hints and recommendations that Docker prints in the console are now silenced.
36 changes: 30 additions & 6 deletions src/briefcase/integrations/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,10 @@ def _version_compat(cls, tools: ToolCache):
# Try to get the version of docker that is installed.
# expected output format: Docker version 25.0.2, build 29cf629\n
docker_version = (
tools.subprocess.check_output(["docker", "--version"])
tools.subprocess.check_output(
["docker", "--version"],
env=cls.subprocess_env(),
)
.split("Docker version ")[1]
.split(",")[0]
)
Expand Down Expand Up @@ -200,7 +203,10 @@ def _user_access(cls, tools: ToolCache):
# Invoke a docker command to check if the daemon is running,
# and the user has sufficient permissions.
# We don't care about the output, just that it succeeds.
tools.subprocess.check_output(["docker", "info"])
tools.subprocess.check_output(
["docker", "info"],
env=cls.subprocess_env(),
)
except subprocess.CalledProcessError as e:
failure_output = e.output
if "permission denied while trying to connect" in failure_output:
Expand All @@ -219,7 +225,10 @@ def _user_access(cls, tools: ToolCache):
def _buildx_installed(cls, tools: ToolCache):
"""Verify the buildx plugin is installed."""
try:
tools.subprocess.check_output(["docker", "buildx", "version"])
tools.subprocess.check_output(
["docker", "buildx", "version"],
env=cls.subprocess_env(),
)
except subprocess.CalledProcessError:
raise BriefcaseCommandError(cls.BUILDX_PLUGIN_MISSING)

Expand Down Expand Up @@ -347,20 +356,22 @@ def cache_image(self, image_tag: str):
:param image_tag: Image name/tag to pull if not locally cached
"""
image_id = self.tools.subprocess.check_output(
["docker", "images", "-q", image_tag]
["docker", "images", "-q", image_tag],
env=self.subprocess_env(),
).strip()

if not image_id:
self.tools.logger.info(
f"Downloading Docker base image for {image_tag}...",
prefix="Docker",
prefix=self.full_name,
)
try:
# disable streaming so image download progress bar is shown
self.tools.subprocess.run(
["docker", "pull", image_tag],
check=True,
stream_output=False,
env=self.subprocess_env(),
)
except subprocess.CalledProcessError as e:
raise BriefcaseCommandError(
Expand Down Expand Up @@ -388,6 +399,17 @@ def check_output(self, args: SubprocessArgsT, image_tag: str, **kwargs) -> str:
**self.dockerize_args(args, image_tag=image_tag, **kwargs)
)

@classmethod
def subprocess_env(cls, env: dict[str, str] | None = None) -> dict[str, str]:
"""Adds environment variables to the context that Docker runs in."""
final_env = {
# Disable the hints/recommendations that Docker prints in the console
"DOCKER_CLI_HINTS": "false",
}
if env is not None:
final_env.update(env)
return final_env

def dockerize_args(
self,
args: SubprocessArgsT,
Expand Down Expand Up @@ -460,7 +482,9 @@ def dockerize_args(
# Finally, add the command (and its arguments) to run in the container
docker_cmdline.extend(self.dockerize_path(arg, path_map) for arg in args)

# Augment configuration to drive the subprocess call
subprocess_kwargs["args"] = docker_cmdline
subprocess_kwargs["env"] = self.subprocess_env()

return subprocess_kwargs

Expand Down Expand Up @@ -504,7 +528,7 @@ def x11_passthrough(
through a Docker-provided mapping of the host's network interface to the spoofed
display that finally proxies those commands to the actual display.
Full docs: https://briefcase.readthedocs.io/en/stable/how-to/internal/x11passthrough.html
Full docs: https://briefcase.readthedocs.io/en/latest/how-to/internal/x11passthrough.html
:param subprocess_kwargs: Existing keywords args from the caller
:returns: augmented keyword args for the call to subprocess
Expand Down
9 changes: 8 additions & 1 deletion tests/integrations/docker/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ def mock_docker(mock_tools, monkeypatch) -> Docker:

mock_tools.docker = Docker(mock_tools)

mock_tools.os.environ = {"PROCESS_ENV_VAR": "VALUE"}

# Reset the mock so that the user mapping calls don't appear in test results
mock_tools.subprocess._subprocess.check_output.reset_mock()

Expand Down Expand Up @@ -111,7 +113,10 @@ def user_mapping_run_calls(tmp_path, monkeypatch) -> list[call]:
MagicMock(return_value=tmp_path / "build/mock_write_test"),
)
return [
call(["docker", "images", "-q", "alpine"]),
call(
["docker", "images", "-q", "alpine"],
env={"DOCKER_CLI_HINTS": "false"},
),
call(
args=[
"docker",
Expand All @@ -123,6 +128,7 @@ def user_mapping_run_calls(tmp_path, monkeypatch) -> list[call]:
"touch",
"/host_write_test/mock_write_test",
],
env={"DOCKER_CLI_HINTS": "false"},
),
call(
args=[
Expand All @@ -136,5 +142,6 @@ def user_mapping_run_calls(tmp_path, monkeypatch) -> list[call]:
"-f",
"/host_write_test/mock_write_test",
],
env={"DOCKER_CLI_HINTS": "false"},
),
]
13 changes: 11 additions & 2 deletions tests/integrations/docker/test_DockerAppContext__Popen.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ def test_simple_call(mock_tools, my_app, tmp_path, sub_kw, capsys):
mock_tools[my_app].app_context._dockerize_args.assert_called_once_with(
["hello", "world"]
)
mock_tools.subprocess._subprocess.Popen.assert_called_once_with(ANY, **sub_kw)
mock_tools.subprocess._subprocess.Popen.assert_called_once_with(
ANY,
env={"DOCKER_CLI_HINTS": "false", "PROCESS_ENV_VAR": "VALUE"},
**sub_kw,
)
assert capsys.readouterr().out == ""


Expand All @@ -45,6 +49,7 @@ def test_call_with_extra_kwargs(mock_tools, my_app, tmp_path, capsys):
encoding="ISO-42",
text=True,
errors="backslashreplace",
env={"DOCKER_CLI_HINTS": "false", "PROCESS_ENV_VAR": "VALUE"},
)
assert capsys.readouterr().out == ""

Expand All @@ -61,5 +66,9 @@ def test_simple_verbose_call(mock_tools, my_app, tmp_path, sub_kw, capsys):
mock_tools[my_app].app_context._dockerize_args.assert_called_once_with(
["hello", "world"]
)
mock_tools.subprocess._subprocess.Popen.assert_called_once_with(ANY, **sub_kw)
mock_tools.subprocess._subprocess.Popen.assert_called_once_with(
ANY,
env={"DOCKER_CLI_HINTS": "false", "PROCESS_ENV_VAR": "VALUE"},
**sub_kw,
)
assert ">>> Running Command:\n" in capsys.readouterr().out
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ def test_simple_call(mock_tools, my_app, tmp_path, sub_check_output_kw, capsys):
["hello", "world"]
)
mock_tools.subprocess._subprocess.check_output.assert_called_with(
ANY, **sub_check_output_kw
ANY,
env={"DOCKER_CLI_HINTS": "false", "PROCESS_ENV_VAR": "VALUE"},
**sub_check_output_kw,
)
assert capsys.readouterr().out == ""

Expand Down Expand Up @@ -48,6 +50,7 @@ def test_call_with_extra_kwargs(mock_tools, my_app, tmp_path, capsys):
stderr=subprocess.STDOUT,
text=True,
errors="backslashreplace",
env={"DOCKER_CLI_HINTS": "false", "PROCESS_ENV_VAR": "VALUE"},
)
assert capsys.readouterr().out == ""

Expand All @@ -71,6 +74,8 @@ def test_simple_verbose_call(
["hello", "world"]
)
mock_tools.subprocess._subprocess.check_output.assert_called_once_with(
ANY, **sub_check_output_kw
ANY,
env={"DOCKER_CLI_HINTS": "false", "PROCESS_ENV_VAR": "VALUE"},
**sub_check_output_kw,
)
assert ">>> Running Command:\n" in capsys.readouterr().out
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def test_dockerize_args(mock_tools, my_app, tmp_path):
"hello",
"world",
],
env={"DOCKER_CLI_HINTS": "false"},
)


Expand All @@ -48,6 +49,7 @@ def test_dockerize_args_sys_executable(mock_tools, my_app, tmp_path):
"-m",
"pip",
],
env={"DOCKER_CLI_HINTS": "false"},
)


Expand Down Expand Up @@ -80,6 +82,7 @@ def test_dockerize_args_mounts(mock_tools, my_app, tmp_path):
"hello",
"world",
],
env={"DOCKER_CLI_HINTS": "false"},
)


Expand Down Expand Up @@ -114,6 +117,7 @@ def test_dockerize_args_mounts_path(mock_tools, my_app, tmp_path):
"world",
"/container/second/bin",
],
env={"DOCKER_CLI_HINTS": "false"},
)


Expand Down Expand Up @@ -144,6 +148,7 @@ def test_dockerize_args_cwd(mock_tools, my_app, tmp_path):
"hello",
"world",
],
env={"DOCKER_CLI_HINTS": "false"},
)


Expand Down Expand Up @@ -180,6 +185,7 @@ def test_dockerize_args_arg_and_env(mock_tools, my_app, tmp_path):
],
check=True,
steam_output=True,
env={"DOCKER_CLI_HINTS": "false"},
)


Expand Down Expand Up @@ -223,4 +229,5 @@ def test_dockerize_args_path_arg_and_env(mock_tools, my_app, tmp_path):
"hello",
os.fsdecode(tmp_path / "location"),
],
env={"DOCKER_CLI_HINTS": "false"},
)
15 changes: 12 additions & 3 deletions tests/integrations/docker/test_DockerAppContext__run.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ def test_simple_call(mock_tools, my_app, tmp_path, sub_stream_kw, capsys):
)
# calls to run() default to using Popen() due to output streaming
mock_tools.subprocess._subprocess.Popen.assert_called_once_with(
ANY, **sub_stream_kw
ANY,
env={"DOCKER_CLI_HINTS": "false", "PROCESS_ENV_VAR": "VALUE"},
**sub_stream_kw,
)
assert capsys.readouterr().out == (
"\n"
Expand All @@ -46,7 +48,11 @@ def test_interactive(mock_tools, my_app, tmp_path, sub_kw, capsys):
stream_output=False,
)
# Interactive means the call to run is direct, rather than going through Popen
mock_tools.subprocess._subprocess.run.assert_called_once_with(ANY, **sub_kw)
mock_tools.subprocess._subprocess.run.assert_called_once_with(
ANY,
env={"DOCKER_CLI_HINTS": "false", "PROCESS_ENV_VAR": "VALUE"},
**sub_kw,
)
assert capsys.readouterr().out == (
"\n"
"Entering Docker context...\n"
Expand Down Expand Up @@ -89,6 +95,7 @@ def test_call_with_extra_kwargs(
bufsize=1,
text=True,
errors="backslashreplace",
env={"DOCKER_CLI_HINTS": "false", "PROCESS_ENV_VAR": "VALUE"},
)
assert capsys.readouterr().out == (
"\n"
Expand All @@ -112,7 +119,9 @@ def test_simple_verbose_call(mock_tools, my_app, tmp_path, sub_stream_kw, capsys
["hello", "world"],
)
mock_tools.subprocess._subprocess.Popen.assert_called_once_with(
ANY, **sub_stream_kw
ANY,
env={"DOCKER_CLI_HINTS": "false", "PROCESS_ENV_VAR": "VALUE"},
**sub_stream_kw,
)
console_output = capsys.readouterr().out
assert "Entering Docker context...\n" in console_output
Expand Down
17 changes: 14 additions & 3 deletions tests/integrations/docker/test_Docker__cache_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,21 @@ def test_cache_image(mock_tools, sub_kw, sub_check_output_kw):

# Confirms that image is not available
mock_tools.subprocess._subprocess.check_output.assert_called_with(
["docker", "images", "-q", "ubuntu:jammy"], **sub_check_output_kw
["docker", "images", "-q", "ubuntu:jammy"],
env={"PROCESS_ENV_VAR": "VALUE", "DOCKER_CLI_HINTS": "false"},
**sub_check_output_kw,
)

# Image is pulled and cached
mock_tools.subprocess._subprocess.run.assert_has_calls(
[call(["docker", "pull", "ubuntu:jammy"], check=True, **sub_kw)]
[
call(
["docker", "pull", "ubuntu:jammy"],
check=True,
env={"PROCESS_ENV_VAR": "VALUE", "DOCKER_CLI_HINTS": "false"},
**sub_kw,
),
]
)


Expand All @@ -37,7 +46,9 @@ def test_cache_image_already_cached(mock_tools, sub_check_output_kw):

# Confirms that image is not available
mock_tools.subprocess._subprocess.check_output.assert_called_with(
["docker", "images", "-q", "ubuntu:jammy"], **sub_check_output_kw
["docker", "images", "-q", "ubuntu:jammy"],
env={"PROCESS_ENV_VAR": "VALUE", "DOCKER_CLI_HINTS": "false"},
**sub_check_output_kw,
)

# Image is not pulled and cached
Expand Down
14 changes: 12 additions & 2 deletions tests/integrations/docker/test_Docker__check_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@ def test_check_output(mock_tools, sub_check_output_kw):
mock_tools.subprocess._subprocess.check_output.assert_has_calls(
[
# Verify image is cached in Docker
call(["docker", "images", "-q", "ubuntu:jammy"], **sub_check_output_kw),
call(
["docker", "images", "-q", "ubuntu:jammy"],
env={"PROCESS_ENV_VAR": "VALUE", "DOCKER_CLI_HINTS": "false"},
**sub_check_output_kw,
),
# Run command in Docker using image
call(
["docker", "run", "--rm", "ubuntu:jammy", "cmd", "arg1", "arg2"],
env={"PROCESS_ENV_VAR": "VALUE", "DOCKER_CLI_HINTS": "false"},
**sub_check_output_kw,
),
]
Expand All @@ -47,10 +52,15 @@ def test_check_output_fail(mock_tools, sub_check_output_kw):
mock_tools.subprocess._subprocess.check_output.assert_has_calls(
[
# Verify image is cached in Docker
call(["docker", "images", "-q", "ubuntu:jammy"], **sub_check_output_kw),
call(
["docker", "images", "-q", "ubuntu:jammy"],
env={"PROCESS_ENV_VAR": "VALUE", "DOCKER_CLI_HINTS": "false"},
**sub_check_output_kw,
),
# Command errors in Docker using image
call(
["docker", "run", "--rm", "ubuntu:jammy", "cmd", "arg1", "arg2"],
env={"PROCESS_ENV_VAR": "VALUE", "DOCKER_CLI_HINTS": "false"},
**sub_check_output_kw,
),
]
Expand Down

0 comments on commit 7682008

Please sign in to comment.