Skip to content

Commit

Permalink
Cache image prior to checking docker user mapping
Browse files Browse the repository at this point in the history
  • Loading branch information
rmartin16 committed Jul 28, 2023
1 parent 5483fd9 commit c7c8457
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 3 deletions.
1 change: 1 addition & 0 deletions changes/1368.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
When the target Docker image name is not valid when building Linux System packages, the error message now mentions the unknown image name instead of an error for Docker user mapping.
7 changes: 6 additions & 1 deletion src/briefcase/integrations/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,13 +271,18 @@ def _is_user_mapping_enabled(self, image_tag: str | None = None) -> bool:
"/host_write_test", host_write_test_path.name
)

image_tag = "alpine" if image_tag is None else image_tag
# Cache the image first so the attempts below to run the image don't
# log irrelevant errors when the image may just have a simple typo
self.cache_image(image_tag)

docker_run_cmd = [
"docker",
"run",
"--rm",
"--volume",
f"{host_write_test_path.parent}:{container_write_test_path.parent}:z",
"alpine" if image_tag is None else image_tag,
image_tag,
]

host_write_test_path.parent.mkdir(exist_ok=True)
Expand Down
2 changes: 2 additions & 0 deletions tests/integrations/docker/test_DockerAppContext__verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ def test_success(mock_tools, first_app_config, verify_kwargs):
"Docker version 19.03.8, build afacb8b\n",
"docker info return value",
"github.com/docker/buildx v0.10.2 00ed17d\n",
"1ed313b0551f", # cached image check
]

DockerAppContext.verify(mock_tools, first_app_config, **verify_kwargs)
Expand Down Expand Up @@ -97,6 +98,7 @@ def test_docker_image_build_fail(mock_tools, first_app_config, verify_kwargs):
"Docker version 19.03.8, build afacb8b\n",
"docker info return value",
"github.com/docker/buildx v0.10.2 00ed17d\n",
"1ed313b0551f", # cached image check
]

mock_tools.subprocess.run.side_effect = [
Expand Down
4 changes: 2 additions & 2 deletions tests/integrations/docker/test_Docker__cache_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def test_cache_image(mock_tools, user_mapping_run_calls):
mock_tools.docker.cache_image("ubuntu:jammy")

# Confirms that image is not available
mock_tools.subprocess.check_output.assert_called_once_with(
mock_tools.subprocess.check_output.assert_called_with(
["docker", "images", "-q", "ubuntu:jammy"]
)

Expand All @@ -44,7 +44,7 @@ def test_cache_image_already_cached(mock_tools, user_mapping_run_calls):
mock_tools.docker.cache_image("ubuntu:jammy")

# Confirms that image is not available
mock_tools.subprocess.check_output.assert_called_once_with(
mock_tools.subprocess.check_output.assert_called_with(
["docker", "images", "-q", "ubuntu:jammy"]
)

Expand Down
8 changes: 8 additions & 0 deletions tests/integrations/docker/test_Docker__verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
VALID_DOCKER_VERSION = "Docker version 19.03.8, build afacb8b\n"
VALID_DOCKER_INFO = "docker info printout"
VALID_BUILDX_VERSION = "github.com/docker/buildx v0.10.2 00ed17d\n"
VALID_USER_MAPPING_IMAGE_CACHE = "1ed313b0551f"
DOCKER_VERIFICATION_CALLS = [
call(["docker", "--version"]),
call(["docker", "info"]),
Expand Down Expand Up @@ -70,6 +71,7 @@ def test_docker_exists(mock_tools, user_mapping_run_calls, capsys, tmp_path):
VALID_DOCKER_VERSION,
VALID_DOCKER_INFO,
VALID_BUILDX_VERSION,
VALID_USER_MAPPING_IMAGE_CACHE,
]

# Invoke docker verify
Expand Down Expand Up @@ -114,6 +116,7 @@ def test_docker_failure(mock_tools, user_mapping_run_calls, capsys):
),
"Success!",
VALID_BUILDX_VERSION,
VALID_USER_MAPPING_IMAGE_CACHE,
]

# Invoke Docker verify
Expand Down Expand Up @@ -280,6 +283,7 @@ def test_docker_image_hint(mock_tools):
VALID_DOCKER_VERSION,
VALID_DOCKER_INFO,
VALID_BUILDX_VERSION,
VALID_USER_MAPPING_IMAGE_CACHE,
]

Docker.verify(mock_tools, image_tag="myimage:tagtorulethemall")
Expand Down Expand Up @@ -333,6 +337,7 @@ def test_user_mapping_write_file_exists(mock_tools, mock_write_test_path):
VALID_DOCKER_VERSION,
VALID_DOCKER_INFO,
VALID_BUILDX_VERSION,
VALID_USER_MAPPING_IMAGE_CACHE,
]

# Mock failure for deleting an existing write test file
Expand All @@ -353,6 +358,7 @@ def test_user_mapping_write_test_file_creation_fails(mock_tools, mock_write_test
VALID_DOCKER_VERSION,
VALID_DOCKER_INFO,
VALID_BUILDX_VERSION,
VALID_USER_MAPPING_IMAGE_CACHE,
]

# Mock failure for deleting an existing write test file
Expand All @@ -376,6 +382,7 @@ def test_user_mapping_write_test_file_cleanup_fails(mock_tools, mock_write_test_
VALID_DOCKER_VERSION,
VALID_DOCKER_INFO,
VALID_BUILDX_VERSION,
VALID_USER_MAPPING_IMAGE_CACHE,
]

# Mock failure for deleting an existing write test file
Expand Down Expand Up @@ -406,6 +413,7 @@ def test_user_mapping_setting(
VALID_DOCKER_VERSION,
VALID_DOCKER_INFO,
VALID_BUILDX_VERSION,
VALID_USER_MAPPING_IMAGE_CACHE,
]

stat_result = namedtuple("stat_result", "st_uid")
Expand Down

0 comments on commit c7c8457

Please sign in to comment.