fix(macos-container): preserve working builder dns
This commit is contained in:
@@ -4,6 +4,7 @@ from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import ipaddress
|
||||
import platform
|
||||
import shutil
|
||||
import subprocess
|
||||
@@ -36,7 +37,10 @@ def require_container() -> None:
|
||||
|
||||
|
||||
def dns_server() -> str:
|
||||
return os.environ.get("BOT_BOTTLE_MACOS_CONTAINER_DNS", _DEFAULT_DNS)
|
||||
override = os.environ.get("BOT_BOTTLE_MACOS_CONTAINER_DNS", "").strip()
|
||||
if override:
|
||||
return override
|
||||
return _host_ipv4_dns() or _DEFAULT_DNS
|
||||
|
||||
|
||||
def build_image(ref: str, context: str, *, dockerfile: str = "") -> None:
|
||||
@@ -55,8 +59,16 @@ def build_image(ref: str, context: str, *, dockerfile: str = "") -> None:
|
||||
|
||||
def _ensure_builder_dns() -> None:
|
||||
dns = dns_server()
|
||||
if _builder_has_dns(dns):
|
||||
status = _builder_status()
|
||||
override = os.environ.get("BOT_BOTTLE_MACOS_CONTAINER_DNS", "").strip()
|
||||
if _builder_running(status) and _builder_resolves_build_hosts():
|
||||
if override and not _builder_has_dns(status, dns):
|
||||
_restart_builder_with_dns(dns)
|
||||
return
|
||||
_restart_builder_with_dns(dns)
|
||||
|
||||
|
||||
def _restart_builder_with_dns(dns: str) -> None:
|
||||
subprocess.run(
|
||||
[_CONTAINER, "builder", "stop"],
|
||||
stdout=subprocess.DEVNULL,
|
||||
@@ -69,7 +81,55 @@ def _ensure_builder_dns() -> None:
|
||||
)
|
||||
|
||||
|
||||
def _builder_has_dns(dns: str) -> bool:
|
||||
def _host_ipv4_dns() -> str:
|
||||
if not is_macos():
|
||||
return ""
|
||||
result = subprocess.run(
|
||||
["scutil", "--dns"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
return ""
|
||||
blocks: list[list[str]] = []
|
||||
current: list[str] = []
|
||||
for line in result.stdout.splitlines():
|
||||
if line.startswith("resolver #") and current:
|
||||
blocks.append(current)
|
||||
current = []
|
||||
current.append(line)
|
||||
if current:
|
||||
blocks.append(current)
|
||||
for direct_only in (True, False):
|
||||
for block in blocks:
|
||||
text = "\n".join(block)
|
||||
if direct_only and "Directly Reachable Address" not in text:
|
||||
continue
|
||||
for line in block:
|
||||
if "nameserver[" not in line or ":" not in line:
|
||||
continue
|
||||
candidate = line.split(":", 1)[1].strip()
|
||||
if _usable_ipv4(candidate):
|
||||
return candidate
|
||||
return ""
|
||||
|
||||
|
||||
def _usable_ipv4(value: str) -> bool:
|
||||
try:
|
||||
address = ipaddress.ip_address(value)
|
||||
except ValueError:
|
||||
return False
|
||||
return (
|
||||
address.version == 4
|
||||
and not address.is_loopback
|
||||
and not address.is_link_local
|
||||
and not address.is_multicast
|
||||
and not address.is_unspecified
|
||||
)
|
||||
|
||||
|
||||
def _builder_status() -> list[dict[str, object]]:
|
||||
result = subprocess.run(
|
||||
[_CONTAINER, "builder", "status", "--format", "json"],
|
||||
capture_output=True,
|
||||
@@ -77,18 +137,29 @@ def _builder_has_dns(dns: str) -> bool:
|
||||
check=False,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
return False
|
||||
return []
|
||||
try:
|
||||
data = json.loads(result.stdout or "[]")
|
||||
except json.JSONDecodeError:
|
||||
return []
|
||||
if isinstance(data, list):
|
||||
return [entry for entry in data if isinstance(entry, dict)]
|
||||
if isinstance(data, dict):
|
||||
return [data]
|
||||
return []
|
||||
|
||||
|
||||
def _builder_running(status: list[dict[str, object]]) -> bool:
|
||||
for entry in status:
|
||||
entry_status = entry.get("status")
|
||||
if isinstance(entry_status, dict) and entry_status.get("state") == "running":
|
||||
return True
|
||||
return False
|
||||
entries = data if isinstance(data, list) else [data]
|
||||
for entry in entries:
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
status = entry.get("status")
|
||||
if isinstance(status, dict) and status.get("state") != "running":
|
||||
continue
|
||||
|
||||
|
||||
def _builder_dns_nameservers(status: list[dict[str, object]]) -> list[str]:
|
||||
out: list[str] = []
|
||||
for entry in status:
|
||||
config = entry.get("configuration")
|
||||
config_dns = config.get("dns") if isinstance(config, dict) else None
|
||||
nameservers = (
|
||||
@@ -96,9 +167,24 @@ def _builder_has_dns(dns: str) -> bool:
|
||||
if isinstance(config_dns, dict)
|
||||
else None
|
||||
)
|
||||
if isinstance(nameservers, list) and dns in nameservers:
|
||||
return True
|
||||
return False
|
||||
if not isinstance(nameservers, list):
|
||||
continue
|
||||
out.extend(name for name in nameservers if isinstance(name, str))
|
||||
return out
|
||||
|
||||
|
||||
def _builder_has_dns(status: list[dict[str, object]], dns: str) -> bool:
|
||||
return dns in _builder_dns_nameservers(status)
|
||||
|
||||
|
||||
def _builder_resolves_build_hosts() -> bool:
|
||||
result = subprocess.run(
|
||||
[_CONTAINER, "exec", "buildkit", "getent", "hosts", "deb.debian.org"],
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
check=False,
|
||||
)
|
||||
return result.returncode == 0
|
||||
|
||||
|
||||
def image_exists(ref: str) -> bool:
|
||||
|
||||
@@ -171,9 +171,13 @@ delivery design lands.
|
||||
and verify that egress cannot bypass the sidecar. They also preflight
|
||||
Apple Container BuildKit DNS because image builds must resolve
|
||||
package mirrors before a launch smoke can be meaningful. The backend
|
||||
starts/restarts the Apple Container builder with the configured DNS
|
||||
server before image builds so BuildKit `RUN` steps inherit a working
|
||||
resolver.
|
||||
probes the running builder before image builds and leaves it alone
|
||||
when its current resolver works. If the probe fails, or if the
|
||||
operator explicitly sets `BOT_BOTTLE_MACOS_CONTAINER_DNS`, the backend
|
||||
restarts the Apple Container builder with the configured DNS server.
|
||||
Without an explicit override, that server is discovered from the
|
||||
host's directly reachable IPv4 resolver before falling back to a
|
||||
public resolver.
|
||||
|
||||
## References
|
||||
|
||||
|
||||
@@ -28,6 +28,27 @@ class TestMacosContainerAvailability(unittest.TestCase):
|
||||
|
||||
|
||||
class TestMacosContainerCommands(unittest.TestCase):
|
||||
def test_dns_server_prefers_direct_host_ipv4_resolver(self):
|
||||
scutil = util.subprocess.CompletedProcess(
|
||||
args=[],
|
||||
returncode=0,
|
||||
stdout="""
|
||||
resolver #1
|
||||
nameserver[0] : 100.100.100.100
|
||||
reach : 0x00000003 (Reachable,Transient Connection)
|
||||
|
||||
resolver #2
|
||||
nameserver[0] : 2600:4041:5c43:b900::1
|
||||
nameserver[1] : 192.168.1.1
|
||||
reach : 0x00020002 (Reachable,Directly Reachable Address)
|
||||
""",
|
||||
stderr="",
|
||||
)
|
||||
with patch.object(util.os, "environ", {}), \
|
||||
patch.object(util.platform, "system", return_value="Darwin"), \
|
||||
patch.object(util.subprocess, "run", return_value=scutil):
|
||||
self.assertEqual("192.168.1.1", util.dns_server())
|
||||
|
||||
def test_build_image(self):
|
||||
status = util.subprocess.CompletedProcess(
|
||||
args=[],
|
||||
@@ -81,6 +102,64 @@ class TestMacosContainerCommands(unittest.TestCase):
|
||||
calls[-1],
|
||||
)
|
||||
|
||||
def test_build_image_leaves_working_builder_with_different_dns_alone(self):
|
||||
status = util.subprocess.CompletedProcess(
|
||||
args=[],
|
||||
returncode=0,
|
||||
stdout=(
|
||||
'[{"status":{"state":"running"},'
|
||||
'"configuration":{"dns":{"nameservers":["8.8.8.8"]}}}]'
|
||||
),
|
||||
stderr="",
|
||||
)
|
||||
probe = util.subprocess.CompletedProcess(
|
||||
args=[], returncode=0, stdout="", stderr="",
|
||||
)
|
||||
build = util.subprocess.CompletedProcess(
|
||||
args=[], returncode=0, stdout="", stderr="",
|
||||
)
|
||||
with patch.object(util, "dns_server", return_value="192.168.1.1"), \
|
||||
patch.object(util.os, "environ", {}), \
|
||||
patch.object(util.subprocess, "run", side_effect=[status, probe, build]) as run:
|
||||
util.build_image("bot-bottle-agent:latest", "/repo")
|
||||
calls = [c.args[0] for c in run.call_args_list]
|
||||
self.assertNotIn(["container", "builder", "stop"], calls)
|
||||
self.assertNotIn(
|
||||
["container", "builder", "start", "--dns", "192.168.1.1"],
|
||||
calls,
|
||||
)
|
||||
|
||||
def test_build_image_restarts_builder_when_dns_probe_fails(self):
|
||||
status = util.subprocess.CompletedProcess(
|
||||
args=[],
|
||||
returncode=0,
|
||||
stdout=(
|
||||
'[{"status":{"state":"running"},'
|
||||
'"configuration":{"dns":{"nameservers":["8.8.8.8"]}}}]'
|
||||
),
|
||||
stderr="",
|
||||
)
|
||||
failed_probe = util.subprocess.CompletedProcess(
|
||||
args=[], returncode=2, stdout="", stderr="",
|
||||
)
|
||||
ok = util.subprocess.CompletedProcess(
|
||||
args=[], returncode=0, stdout="", stderr="",
|
||||
)
|
||||
with patch.object(util, "dns_server", return_value="192.168.1.1"), \
|
||||
patch.object(util.os, "environ", {}), \
|
||||
patch.object(
|
||||
util.subprocess,
|
||||
"run",
|
||||
side_effect=[status, failed_probe, ok, ok, ok],
|
||||
) as run:
|
||||
util.build_image("bot-bottle-agent:latest", "/repo")
|
||||
calls = [c.args[0] for c in run.call_args_list]
|
||||
self.assertIn(["container", "builder", "stop"], calls)
|
||||
self.assertIn(
|
||||
["container", "builder", "start", "--dns", "192.168.1.1"],
|
||||
calls,
|
||||
)
|
||||
|
||||
def test_container_exists_parses_quiet_list(self):
|
||||
completed = util.subprocess.CompletedProcess(
|
||||
args=[], returncode=0, stdout="bot-bottle-a\nother\n", stderr="",
|
||||
|
||||
Reference in New Issue
Block a user