From 0225f574bee6a0d06de572af10bb0c93397a60f7 Mon Sep 17 00:00:00 2001 From: Mike Degatano Date: Wed, 13 Sep 2023 02:50:32 -0400 Subject: [PATCH] Only tell HA to refresh ingress on restore on change (#4552) * Only tell HA to refresh ingress on restore on change * Fix test expecting ingress change * Assume ingress_panel is false for new addons --- supervisor/addons/__init__.py | 4 +- tests/addons/test_addon.py | 4 +- tests/backups/test_manager.py | 86 +++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/supervisor/addons/__init__.py b/supervisor/addons/__init__.py index b22243b0a..71b822789 100644 --- a/supervisor/addons/__init__.py +++ b/supervisor/addons/__init__.py @@ -389,9 +389,11 @@ class AddonManager(CoreSysAttributes): if slug not in self.local: _LOGGER.debug("Add-on %s is not local available for restore", slug) addon = Addon(self.coresys, slug) + had_ingress = False else: _LOGGER.debug("Add-on %s is local available for restore", slug) addon = self.local[slug] + had_ingress = addon.ingress_panel wait_for_start = await addon.restore(tar_file) @@ -401,7 +403,7 @@ class AddonManager(CoreSysAttributes): self.local[slug] = addon # Update ingress - if addon.with_ingress: + if had_ingress != addon.ingress_panel: await self.sys_ingress.reload() with suppress(HomeAssistantAPIError): await self.sys_ingress.update_hass_panel(addon) diff --git a/tests/addons/test_addon.py b/tests/addons/test_addon.py index c895ffa8d..0b9a7ea4f 100644 --- a/tests/addons/test_addon.py +++ b/tests/addons/test_addon.py @@ -532,11 +532,9 @@ async def test_restore( tarfile = SecureTarFile(get_fixture_path(f"backup_local_ssh_{status}.tar.gz"), "r") with patch.object(DockerAddon, "is_running", return_value=False), patch.object( CpuArch, "supported", new=PropertyMock(return_value=["aarch64"]) - ), patch.object(Ingress, "update_hass_panel") as update_hass_panel: + ): start_task = await coresys.addons.restore(TEST_ADDON_SLUG, tarfile) - update_hass_panel.assert_called_once() - assert bool(start_task) is (status == "running") diff --git a/tests/backups/test_manager.py b/tests/backups/test_manager.py index 698f35574..a4b52d981 100644 --- a/tests/backups/test_manager.py +++ b/tests/backups/test_manager.py @@ -21,6 +21,7 @@ from supervisor.docker.const import ContainerState from supervisor.docker.homeassistant import DockerHomeAssistant from supervisor.docker.monitor import DockerContainerStateEvent from supervisor.exceptions import AddonsError, BackupError, DockerError +from supervisor.homeassistant.api import HomeAssistantAPI from supervisor.homeassistant.core import HomeAssistantCore from supervisor.homeassistant.module import HomeAssistant from supervisor.mounts.mount import Mount @@ -1307,3 +1308,88 @@ async def test_cannot_manually_thaw_normal_freeze(coresys: CoreSys): coresys.core.state = CoreState.FREEZE with pytest.raises(BackupError): await coresys.backups.thaw_all() + + +async def test_restore_only_reloads_ingress_on_change( + coresys: CoreSys, + install_addon_ssh: Addon, + tmp_supervisor_data, + path_extern, +): + """Test restore only tells core to reload ingress when something has changed.""" + install_addon_ssh.path_data.mkdir() + coresys.core.state = CoreState.RUNNING + coresys.hardware.disk.get_disk_free_space = lambda x: 5000 + + backup_no_ingress: Backup = await coresys.backups.do_backup_partial( + addons=["local_ssh"] + ) + + install_addon_ssh.ingress_panel = True + install_addon_ssh.save_persist() + backup_with_ingress: Backup = await coresys.backups.do_backup_partial( + addons=["local_ssh"] + ) + + async def mock_is_running(*_) -> bool: + return True + + with patch.object( + HomeAssistantCore, "is_running", new=mock_is_running + ), patch.object(AddonModel, "_validate_availability"), patch.object( + DockerAddon, "attach" + ), patch.object( + HomeAssistantAPI, "make_request" + ) as make_request: + make_request.return_value.__aenter__.return_value.status = 200 + + # Has ingress before and after - not called + await coresys.backups.do_restore_partial( + backup_with_ingress, addons=["local_ssh"] + ) + make_request.assert_not_called() + + # Restore removes ingress - tell Home Assistant + await coresys.backups.do_restore_partial( + backup_no_ingress, addons=["local_ssh"] + ) + make_request.assert_called_once_with( + "delete", "api/hassio_push/panel/local_ssh" + ) + + # No ingress before or after - not called + make_request.reset_mock() + await coresys.backups.do_restore_partial( + backup_no_ingress, addons=["local_ssh"] + ) + make_request.assert_not_called() + + # Restore adds ingress - tell Home Assistant + await coresys.backups.do_restore_partial( + backup_with_ingress, addons=["local_ssh"] + ) + make_request.assert_called_once_with("post", "api/hassio_push/panel/local_ssh") + + +async def test_restore_new_addon( + coresys: CoreSys, + install_addon_ssh: Addon, + container: MagicMock, + tmp_supervisor_data, + path_extern, +): + """Test restore installing new addon.""" + install_addon_ssh.path_data.mkdir() + coresys.core.state = CoreState.RUNNING + coresys.hardware.disk.get_disk_free_space = lambda x: 5000 + + backup: Backup = await coresys.backups.do_backup_partial(addons=["local_ssh"]) + await coresys.addons.uninstall("local_ssh") + assert "local_ssh" not in coresys.addons.local + + with patch.object(AddonModel, "_validate_availability"), patch.object( + DockerAddon, "attach" + ): + assert await coresys.backups.do_restore_partial(backup, addons=["local_ssh"]) + + assert "local_ssh" in coresys.addons.local