Remove I/O in event loop for add-on backup and restore (#5649)

* Remove I/O in event loop for add-on backup and restore

Remove I/O in event loop for add-on backup and restore operations. On
backup, this moves the add-on shutdown before metadata is stored in the
backup, which slightly lenghens the time the add-on is actually stopped.

However, the biggest contributor here is likely adding the image
itself if it is a local backup. However, since that is the minority of
cases, I've opted for simplicity over optimizing for this case.

* Use partial to explicitly bind arguments
This commit is contained in:
Stefan Agner 2025-02-21 00:24:36 +01:00 committed by GitHub
parent cda6325be4
commit 997a51fc42
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 102 additions and 82 deletions

View File

@ -1238,46 +1238,45 @@ class Addon(AddonModel):
Returns a Task that completes when addon has state 'started' (see start) Returns a Task that completes when addon has state 'started' (see start)
for cold backup. Else nothing is returned. for cold backup. Else nothing is returned.
""" """
wait_for_start: Awaitable[None] | None = None
def _addon_backup(
store_image: bool,
metadata: dict[str, Any],
apparmor_profile: str | None,
addon_config_used: bool,
):
"""Start the backup process."""
with TemporaryDirectory(dir=self.sys_config.path_tmp) as temp: with TemporaryDirectory(dir=self.sys_config.path_tmp) as temp:
temp_path = Path(temp) temp_path = Path(temp)
# store local image # store local image
if self.need_build: if store_image:
try: try:
await self.instance.export_image(temp_path.joinpath("image.tar")) self.instance.export_image(temp_path.joinpath("image.tar"))
except DockerError as err: except DockerError as err:
raise AddonsError() from err raise AddonsError() from err
data = {
ATTR_USER: self.persist,
ATTR_SYSTEM: self.data,
ATTR_VERSION: self.version,
ATTR_STATE: _MAP_ADDON_STATE.get(self.state, self.state),
}
# Store local configs/state # Store local configs/state
try: try:
write_json_file(temp_path.joinpath("addon.json"), data) write_json_file(temp_path.joinpath("addon.json"), metadata)
except ConfigurationFileError as err: except ConfigurationFileError as err:
raise AddonsError( raise AddonsError(
f"Can't save meta for {self.slug}", _LOGGER.error f"Can't save meta for {self.slug}", _LOGGER.error
) from err ) from err
# Store AppArmor Profile # Store AppArmor Profile
if self.sys_host.apparmor.exists(self.slug): if apparmor_profile:
profile = temp_path.joinpath("apparmor.txt") profile_backup_file = temp_path.joinpath("apparmor.txt")
try: try:
await self.sys_host.apparmor.backup_profile(self.slug, profile) self.sys_host.apparmor.backup_profile(
apparmor_profile, profile_backup_file
)
except HostAppArmorError as err: except HostAppArmorError as err:
raise AddonsError( raise AddonsError(
"Can't backup AppArmor profile", _LOGGER.error "Can't backup AppArmor profile", _LOGGER.error
) from err ) from err
# write into tarfile # Write tarfile
def _write_tarfile():
"""Write tar inside loop."""
with tar_file as backup: with tar_file as backup:
# Backup metadata # Backup metadata
backup.add(temp, arcname=".") backup.add(temp, arcname=".")
@ -1293,7 +1292,7 @@ class Addon(AddonModel):
) )
# Backup config # Backup config
if self.addon_config_used: if addon_config_used:
atomic_contents_add( atomic_contents_add(
backup, backup,
self.path_config, self.path_config,
@ -1303,19 +1302,39 @@ class Addon(AddonModel):
arcname="config", arcname="config",
) )
is_running = await self.begin_backup() wait_for_start: Awaitable[None] | None = None
data = {
ATTR_USER: self.persist,
ATTR_SYSTEM: self.data,
ATTR_VERSION: self.version,
ATTR_STATE: _MAP_ADDON_STATE.get(self.state, self.state),
}
apparmor_profile = (
self.slug if self.sys_host.apparmor.exists(self.slug) else None
)
was_running = await self.begin_backup()
try: try:
_LOGGER.info("Building backup for add-on %s", self.slug) _LOGGER.info("Building backup for add-on %s", self.slug)
await self.sys_run_in_executor(_write_tarfile) await self.sys_run_in_executor(
partial(
_addon_backup,
store_image=self.need_build,
metadata=data,
apparmor_profile=apparmor_profile,
addon_config_used=self.addon_config_used,
)
)
_LOGGER.info("Finish backup for addon %s", self.slug)
except (tarfile.TarError, OSError) as err: except (tarfile.TarError, OSError) as err:
raise AddonsError( raise AddonsError(
f"Can't write tarfile {tar_file}: {err}", _LOGGER.error f"Can't write tarfile {tar_file}: {err}", _LOGGER.error
) from err ) from err
finally: finally:
if is_running: if was_running:
wait_for_start = await self.end_backup() wait_for_start = await self.end_backup()
_LOGGER.info("Finish backup for addon %s", self.slug)
return wait_for_start return wait_for_start
@Job( @Job(
@ -1330,30 +1349,36 @@ class Addon(AddonModel):
if addon is started after restore. Else nothing is returned. if addon is started after restore. Else nothing is returned.
""" """
wait_for_start: Awaitable[None] | None = None wait_for_start: Awaitable[None] | None = None
with TemporaryDirectory(dir=self.sys_config.path_tmp) as temp:
# extract backup # Extract backup
def _extract_tarfile(): def _extract_tarfile() -> tuple[TemporaryDirectory, dict[str, Any]]:
"""Extract tar backup.""" """Extract tar backup."""
tmp = TemporaryDirectory(dir=self.sys_config.path_tmp)
try:
with tar_file as backup: with tar_file as backup:
backup.extractall( backup.extractall(
path=Path(temp), path=tmp.name,
members=secure_path(backup), members=secure_path(backup),
filter="fully_trusted", filter="fully_trusted",
) )
data = read_json_file(Path(tmp.name, "addon.json"))
except:
tmp.cleanup()
raise
return tmp, data
try: try:
await self.sys_run_in_executor(_extract_tarfile) tmp, data = await self.sys_run_in_executor(_extract_tarfile)
except tarfile.TarError as err: except tarfile.TarError as err:
raise AddonsError( raise AddonsError(
f"Can't read tarfile {tar_file}: {err}", _LOGGER.error f"Can't read tarfile {tar_file}: {err}", _LOGGER.error
) from err ) from err
# Read backup data
try:
data = read_json_file(Path(temp, "addon.json"))
except ConfigurationFileError as err: except ConfigurationFileError as err:
raise AddonsError() from err raise AddonsError() from err
try:
# Validate # Validate
try: try:
data = SCHEMA_ADDON_BACKUP(data) data = SCHEMA_ADDON_BACKUP(data)
@ -1387,7 +1412,7 @@ class Addon(AddonModel):
if not await self.instance.exists(): if not await self.instance.exists():
_LOGGER.info("Restore/Install of image for addon %s", self.slug) _LOGGER.info("Restore/Install of image for addon %s", self.slug)
image_file = Path(temp, "image.tar") image_file = Path(tmp.name, "image.tar")
if image_file.is_file(): if image_file.is_file():
with suppress(DockerError): with suppress(DockerError):
await self.instance.import_image(image_file) await self.instance.import_image(image_file)
@ -1406,13 +1431,13 @@ class Addon(AddonModel):
# Restore data and config # Restore data and config
def _restore_data(): def _restore_data():
"""Restore data and config.""" """Restore data and config."""
temp_data = Path(temp, "data") temp_data = Path(tmp.name, "data")
if temp_data.is_dir(): if temp_data.is_dir():
shutil.copytree(temp_data, self.path_data, symlinks=True) shutil.copytree(temp_data, self.path_data, symlinks=True)
else: else:
self.path_data.mkdir() self.path_data.mkdir()
temp_config = Path(temp, "config") temp_config = Path(tmp.name, "config")
if temp_config.is_dir(): if temp_config.is_dir():
shutil.copytree(temp_config, self.path_config, symlinks=True) shutil.copytree(temp_config, self.path_config, symlinks=True)
elif self.addon_config_used: elif self.addon_config_used:
@ -1432,7 +1457,7 @@ class Addon(AddonModel):
) from err ) from err
# Restore AppArmor # Restore AppArmor
profile_file = Path(temp, "apparmor.txt") profile_file = Path(tmp.name, "apparmor.txt")
if profile_file.exists(): if profile_file.exists():
try: try:
await self.sys_host.apparmor.load_profile( await self.sys_host.apparmor.load_profile(
@ -1440,7 +1465,8 @@ class Addon(AddonModel):
) )
except HostAppArmorError as err: except HostAppArmorError as err:
_LOGGER.error( _LOGGER.error(
"Can't restore AppArmor profile for add-on %s", self.slug "Can't restore AppArmor profile for add-on %s",
self.slug,
) )
raise AddonsError() from err raise AddonsError() from err
@ -1452,7 +1478,8 @@ class Addon(AddonModel):
# Run add-on # Run add-on
if data[ATTR_STATE] == AddonState.STARTED: if data[ATTR_STATE] == AddonState.STARTED:
wait_for_start = await self.start() wait_for_start = await self.start()
finally:
tmp.cleanup()
_LOGGER.info("Finished restore for add-on %s", self.slug) _LOGGER.info("Finished restore for add-on %s", self.slug)
return wait_for_start return wait_for_start

View File

@ -697,16 +697,9 @@ class DockerAddon(DockerInterface):
_LOGGER.info("Build %s:%s done", self.image, version) _LOGGER.info("Build %s:%s done", self.image, version)
@Job(
name="docker_addon_export_image",
limit=JobExecutionLimit.GROUP_ONCE,
on_condition=DockerJobError,
)
def export_image(self, tar_file: Path) -> Awaitable[None]: def export_image(self, tar_file: Path) -> Awaitable[None]:
"""Export current images into a tar file.""" """Export current images into a tar file."""
return self.sys_run_in_executor( return self.sys_docker.export_image(self.image, self.version, tar_file)
self.sys_docker.export_image, self.image, self.version, tar_file
)
@Job( @Job(
name="docker_addon_import_image", name="docker_addon_import_image",

View File

@ -115,12 +115,12 @@ class AppArmorControl(CoreSysAttributes):
_LOGGER.info("Removing AppArmor profile: %s", profile_name) _LOGGER.info("Removing AppArmor profile: %s", profile_name)
self._profiles.remove(profile_name) self._profiles.remove(profile_name)
async def backup_profile(self, profile_name: str, backup_file: Path) -> None: def backup_profile(self, profile_name: str, backup_file: Path) -> None:
"""Backup A profile into a new file.""" """Backup A profile into a new file."""
profile_file: Path = self._get_profile(profile_name) profile_file: Path = self._get_profile(profile_name)
try: try:
await self.sys_run_in_executor(shutil.copy, profile_file, backup_file) shutil.copy(profile_file, backup_file)
except OSError as err: except OSError as err:
if err.errno == errno.EBADMSG: if err.errno == errno.EBADMSG:
self.sys_resolution.unhealthy = UnhealthyReason.OSERROR_BAD_MESSAGE self.sys_resolution.unhealthy = UnhealthyReason.OSERROR_BAD_MESSAGE

View File

@ -45,7 +45,7 @@ async def test_remove_profile_error(coresys: CoreSys, path_extern):
assert coresys.core.healthy is False assert coresys.core.healthy is False
async def test_backup_profile_error(coresys: CoreSys, path_extern): def test_backup_profile_error(coresys: CoreSys, path_extern):
"""Test error while backing up apparmor profile.""" """Test error while backing up apparmor profile."""
test_path = Path("test") test_path = Path("test")
coresys.host.apparmor._profiles.add("test") # pylint: disable=protected-access coresys.host.apparmor._profiles.add("test") # pylint: disable=protected-access
@ -54,10 +54,10 @@ async def test_backup_profile_error(coresys: CoreSys, path_extern):
): ):
err.errno = errno.EBUSY err.errno = errno.EBUSY
with raises(HostAppArmorError): with raises(HostAppArmorError):
await coresys.host.apparmor.backup_profile("test", test_path) coresys.host.apparmor.backup_profile("test", test_path)
assert coresys.core.healthy is True assert coresys.core.healthy is True
err.errno = errno.EBADMSG err.errno = errno.EBADMSG
with raises(HostAppArmorError): with raises(HostAppArmorError):
await coresys.host.apparmor.backup_profile("test", test_path) coresys.host.apparmor.backup_profile("test", test_path)
assert coresys.core.healthy is False assert coresys.core.healthy is False