Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Capture exception if image is missing on run #4621

Merged
merged 3 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not do that, but the rest of the PR looks good

# 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
Loading