Skip to content

Commit

Permalink
Capture exception if image is missing on run (#4621)
Browse files Browse the repository at this point in the history
* Retry run if image missing and handle fixup

* Fix lint and run error test

* Remove retry and just capture exception
  • Loading branch information
mdegat01 authored Oct 17, 2023
1 parent ab6745b commit 77fd1b4
Show file tree
Hide file tree
Showing 12 changed files with 189 additions and 100 deletions.
11 changes: 1 addition & 10 deletions supervisor/docker/addon.py
Original file line number Diff line number Diff line change
Expand Up @@ -501,24 +501,16 @@ def mounts(self) -> list[Mount]:
)
async def run(self) -> None:
"""Run Docker image."""
if await self.is_running():
return

# Security check
if not self.addon.protected:
_LOGGER.warning("%s running with disabled protected mode!", self.addon.name)

# Cleanup
await self.stop()

# Don't set a hostname if no separate UTS namespace is used
hostname = None if self.uts_mode else self.addon.hostname

# Create & Run container
try:
docker_container = await self.sys_run_in_executor(
self.sys_docker.run,
self.image,
await self._run(
tag=str(self.addon.version),
name=self.name,
hostname=hostname,
Expand Down Expand Up @@ -549,7 +541,6 @@ async def run(self) -> None:
)
raise

self._meta = docker_container.attrs
_LOGGER.info(
"Starting Docker add-on %s with version %s", self.image, self.version
)
Expand Down
13 changes: 1 addition & 12 deletions supervisor/docker/audio.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,7 @@ def cpu_rt_runtime(self) -> int | None:
)
async def run(self) -> None:
"""Run Docker image."""
if await self.is_running():
return

# Cleanup
await self.stop()

# Create & Run container
docker_container = await self.sys_run_in_executor(
self.sys_docker.run,
self.image,
await self._run(
tag=str(self.sys_plugins.audio.version),
init=False,
ipv4=self.sys_docker.network.audio,
Expand All @@ -118,8 +109,6 @@ async def run(self) -> None:
},
mounts=self.mounts,
)

self._meta = docker_container.attrs
_LOGGER.info(
"Starting Audio %s with version %s - %s",
self.image,
Expand Down
13 changes: 1 addition & 12 deletions supervisor/docker/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,7 @@ def name(self) -> str:
)
async def run(self) -> None:
"""Run Docker image."""
if await self.is_running():
return

# Cleanup
await self.stop()

# Create & Run container
docker_container = await self.sys_run_in_executor(
self.sys_docker.run,
self.image,
await self._run(
entrypoint=["/init"],
tag=str(self.sys_plugins.cli.version),
init=False,
Expand All @@ -60,8 +51,6 @@ async def run(self) -> None:
ENV_TOKEN: self.sys_plugins.cli.supervisor_token,
},
)

self._meta = docker_container.attrs
_LOGGER.info(
"Starting CLI %s with version %s - %s",
self.image,
Expand Down
13 changes: 1 addition & 12 deletions supervisor/docker/dns.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,7 @@ def name(self) -> str:
)
async def run(self) -> None:
"""Run Docker image."""
if await self.is_running():
return

# Cleanup
await self.stop()

# Create & Run container
docker_container = await self.sys_run_in_executor(
self.sys_docker.run,
self.image,
await self._run(
tag=str(self.sys_plugins.dns.version),
init=False,
dns=False,
Expand All @@ -65,8 +56,6 @@ async def run(self) -> None:
],
oom_score_adj=-300,
)

self._meta = docker_container.attrs
_LOGGER.info(
"Starting DNS %s with version %s - %s",
self.image,
Expand Down
13 changes: 1 addition & 12 deletions supervisor/docker/homeassistant.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,7 @@ def mounts(self) -> list[Mount]:
)
async def run(self) -> None:
"""Run Docker image."""
if await self.is_running():
return

# Cleanup
await self.stop()

# Create & Run container
docker_container = await self.sys_run_in_executor(
self.sys_docker.run,
self.image,
await self._run(
tag=(self.sys_homeassistant.version),
name=self.name,
hostname=self.name,
Expand All @@ -186,8 +177,6 @@ async def run(self) -> None:
tmpfs={"/tmp": ""},
oom_score_adj=-300,
)

self._meta = docker_container.attrs
_LOGGER.info(
"Starting Home Assistant %s with version %s", self.image, self.version
)
Expand Down
21 changes: 21 additions & 0 deletions supervisor/docker/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,27 @@ async def run(self) -> None:
"""Run Docker image."""
raise NotImplementedError()

async def _run(self, **kwargs) -> None:
"""Run Docker image with retry inf necessary."""
if await self.is_running():
return

# Cleanup
await self.stop()

# Create & Run container
try:
docker_container = await self.sys_run_in_executor(
self.sys_docker.run, self.image, **kwargs
)
except DockerNotFound as err:
# If image is missing, capture the exception as this shouldn't happen
capture_exception(err)
raise

# Store metadata
self._meta = docker_container.attrs

@Job(
name="docker_interface_stop",
limit=JobExecutionLimit.GROUP_ONCE,
Expand Down
13 changes: 1 addition & 12 deletions supervisor/docker/multicast.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,7 @@ def capabilities(self) -> list[Capabilities]:
)
async def run(self) -> None:
"""Run Docker image."""
if await self.is_running():
return

# Cleanup
await self.stop()

# Create & Run container
docker_container = await self.sys_run_in_executor(
self.sys_docker.run,
self.image,
await self._run(
tag=str(self.sys_plugins.multicast.version),
init=False,
name=self.name,
Expand All @@ -59,8 +50,6 @@ async def run(self) -> None:
extra_hosts={"supervisor": self.sys_docker.network.supervisor},
environment={ENV_TIME: self.sys_timezone},
)

self._meta = docker_container.attrs
_LOGGER.info(
"Starting Multicast %s with version %s - Host", self.image, self.version
)
13 changes: 1 addition & 12 deletions supervisor/docker/observer.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,7 @@ def name(self) -> str:
)
async def run(self) -> None:
"""Run Docker image."""
if await self.is_running():
return

# Cleanup
await self.stop()

# Create & Run container
docker_container = await self.sys_run_in_executor(
self.sys_docker.run,
self.image,
await self._run(
tag=str(self.sys_plugins.observer.version),
init=False,
ipv4=self.sys_docker.network.observer,
Expand All @@ -63,8 +54,6 @@ async def run(self) -> None:
ports={"80/tcp": 4357},
oom_score_adj=-300,
)

self._meta = docker_container.attrs
_LOGGER.info(
"Starting Observer %s with version %s - %s",
self.image,
Expand Down
57 changes: 57 additions & 0 deletions supervisor/resolution/fixups/addon_execute_repair.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
"""Helper to fix missing image for addon."""

import logging

from ...coresys import CoreSys
from ..const import ContextType, IssueType, SuggestionType
from .base import FixupBase

_LOGGER: logging.Logger = logging.getLogger(__name__)


def setup(coresys: CoreSys) -> FixupBase:
"""Check setup function."""
return FixupAddonExecuteRepair(coresys)


class FixupAddonExecuteRepair(FixupBase):
"""Storage class for fixup."""

async def process_fixup(self, reference: str | None = None) -> None:
"""Pull the addons image."""
addon = self.sys_addons.get(reference, local_only=True)
if not addon:
_LOGGER.info(
"Cannot repair addon %s as it is not installed, dismissing suggestion",
reference,
)
return

if await addon.instance.exists():
_LOGGER.info(
"Addon %s does not need repair, dismissing suggestion", reference
)
return

_LOGGER.info("Installing image for addon %s")
await addon.instance.install(addon.version)

@property
def suggestion(self) -> SuggestionType:
"""Return a SuggestionType enum."""
return SuggestionType.EXECUTE_REPAIR

@property
def context(self) -> ContextType:
"""Return a ContextType enum."""
return ContextType.ADDON

@property
def issues(self) -> list[IssueType]:
"""Return a IssueType enum list."""
return [IssueType.MISSING_IMAGE]

@property
def auto(self) -> bool:
"""Return if a fixup can be apply as auto fix."""
return True
23 changes: 6 additions & 17 deletions tests/docker/test_addon.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,6 @@ def fixture_addonsdata_user() -> dict[str, Data]:
yield mock


@pytest.fixture(name="os_environ")
def fixture_os_environ():
"""Mock os.environ."""
with patch("supervisor.config.os.environ") as mock:
yield mock


def get_docker_addon(
coresys: CoreSys, addonsdata_system: dict[str, Data], config_file: str
):
Expand All @@ -60,7 +53,7 @@ def get_docker_addon(


def test_base_volumes_included(
coresys: CoreSys, addonsdata_system: dict[str, Data], os_environ
coresys: CoreSys, addonsdata_system: dict[str, Data], path_extern
):
"""Dev and data volumes always included."""
docker_addon = get_docker_addon(
Expand All @@ -86,7 +79,7 @@ def test_base_volumes_included(


def test_addon_map_folder_defaults(
coresys: CoreSys, addonsdata_system: dict[str, Data], os_environ
coresys: CoreSys, addonsdata_system: dict[str, Data], path_extern
):
"""Validate defaults for mapped folders in addons."""
docker_addon = get_docker_addon(
Expand Down Expand Up @@ -143,7 +136,7 @@ def test_addon_map_folder_defaults(


def test_journald_addon(
coresys: CoreSys, addonsdata_system: dict[str, Data], os_environ
coresys: CoreSys, addonsdata_system: dict[str, Data], path_extern
):
"""Validate volume for journald option."""
docker_addon = get_docker_addon(
Expand Down Expand Up @@ -171,7 +164,7 @@ def test_journald_addon(


def test_not_journald_addon(
coresys: CoreSys, addonsdata_system: dict[str, Data], os_environ
coresys: CoreSys, addonsdata_system: dict[str, Data], path_extern
):
"""Validate journald option defaults off."""
docker_addon = get_docker_addon(
Expand All @@ -182,10 +175,7 @@ def test_not_journald_addon(


async def test_addon_run_docker_error(
coresys: CoreSys,
addonsdata_system: dict[str, Data],
capture_exception: Mock,
os_environ,
coresys: CoreSys, addonsdata_system: dict[str, Data], path_extern
):
"""Test docker error when addon is run."""
await coresys.dbus.timedate.connect(coresys.dbus.bus)
Expand All @@ -203,14 +193,13 @@ async def test_addon_run_docker_error(
Issue(IssueType.MISSING_IMAGE, ContextType.ADDON, reference="test_addon")
in coresys.resolution.issues
)
capture_exception.assert_not_called()


async def test_addon_run_add_host_error(
coresys: CoreSys,
addonsdata_system: dict[str, Data],
capture_exception: Mock,
os_environ,
path_extern,
):
"""Test error adding host when addon is run."""
await coresys.dbus.timedate.connect(coresys.dbus.bus)
Expand Down
Loading

0 comments on commit 77fd1b4

Please sign in to comment.