Skip to content

Commit 18563e5

Browse files
authored
merge duplicate extra_hosts kwarg (#18192)
1 parent 49301a2 commit 18563e5

File tree

2 files changed

+52
-3
lines changed

2 files changed

+52
-3
lines changed

src/integrations/prefect-docker/prefect_docker/worker.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ def get_network_mode(self) -> Optional[str]:
320320
# Default to unset
321321
return None
322322

323-
def get_extra_hosts(self, docker_client) -> Optional[dict[str, str]]:
323+
def get_extra_hosts(self, docker_client: DockerClient) -> Optional[dict[str, str]]:
324324
"""
325325
A host.docker.internal -> host-gateway mapping is necessary for communicating
326326
with the API on Linux machines. Docker Desktop on macOS will automatically
@@ -665,9 +665,29 @@ def _build_container_settings(
665665
container_create_kwargs = {
666666
k: v
667667
for k, v in container_create_kwargs.items()
668-
if k not in configuration.model_fields.keys()
668+
if k not in configuration.__class__.model_fields.keys()
669669
}
670670

671+
# Get extra_hosts from configuration
672+
extra_hosts = configuration.get_extra_hosts(docker_client)
673+
674+
# If user provided extra_hosts in container_create_kwargs, merge them
675+
if "extra_hosts" in container_create_kwargs:
676+
user_extra_hosts = container_create_kwargs.pop("extra_hosts")
677+
if extra_hosts:
678+
# Merge user's extra_hosts with the auto-generated ones
679+
# Convert list format to dict if necessary
680+
if isinstance(user_extra_hosts, list):
681+
for host_entry in user_extra_hosts:
682+
if ":" in host_entry:
683+
host, ip = host_entry.split(":", 1)
684+
extra_hosts[host] = ip
685+
elif isinstance(user_extra_hosts, dict):
686+
extra_hosts.update(user_extra_hosts)
687+
else:
688+
# No auto-generated extra_hosts, use user's directly
689+
extra_hosts = user_extra_hosts
690+
671691
return dict(
672692
image=configuration.image,
673693
network=configuration.networks[0] if configuration.networks else None,
@@ -676,7 +696,7 @@ def _build_container_settings(
676696
environment=configuration.env,
677697
auto_remove=configuration.auto_remove,
678698
labels=configuration.labels,
679-
extra_hosts=configuration.get_extra_hosts(docker_client),
699+
extra_hosts=extra_hosts,
680700
name=configuration.name,
681701
volumes=configuration.volumes,
682702
mem_limit=configuration.mem_limit,

src/integrations/prefect-docker/tests/test_worker.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,35 @@ async def test_adds_docker_host_gateway_on_linux(
655655
assert call_extra_hosts == {"host.docker.internal": "host-gateway"}
656656

657657

658+
async def test_user_provided_extra_hosts_merge_with_auto_generated(
659+
mock_docker_client: MagicMock,
660+
monkeypatch: pytest.MonkeyPatch,
661+
flow_run: FlowRun,
662+
default_docker_worker_job_configuration: DockerWorkerJobConfiguration,
663+
):
664+
"""Test that user-provided extra_hosts are merged with auto-generated ones without error.
665+
666+
this is a regression test for https://github.com/PrefectHQ/prefect/issues/18187
667+
"""
668+
monkeypatch.setattr("sys.platform", "linux")
669+
670+
default_docker_worker_job_configuration.container_create_kwargs = {
671+
"extra_hosts": ["host.docker.internal:host-gateway"]
672+
}
673+
default_docker_worker_job_configuration.prepare_for_flow_run(flow_run=flow_run)
674+
675+
async with DockerWorker(work_pool_name="test") as worker:
676+
await worker.run(
677+
flow_run=flow_run, configuration=default_docker_worker_job_configuration
678+
)
679+
680+
mock_docker_client.containers.create.assert_called_once()
681+
call_extra_hosts = mock_docker_client.containers.create.call_args[1].get(
682+
"extra_hosts"
683+
)
684+
assert call_extra_hosts == {"host.docker.internal": "host-gateway"}
685+
686+
658687
async def test_default_image_pull_policy_pulls_image_with_latest_tag(
659688
mock_docker_client, flow_run, default_docker_worker_job_configuration
660689
):

0 commit comments

Comments
 (0)