feat: add '--continue-on-failure' to 'yunohost app upgrade

locales/en.json

@@ -27,6 +27,7 @@
     "app_config_unable_to_apply": "Failed to apply config panel values.",
     "app_config_unable_to_read": "Failed to read config panel values.",
     "app_extraction_failed": "Could not extract the installation files",
+    "app_failed_to_upgrade_but_continue": "App {failed_app} failed to upgrade, continue to next upgrades as requested. Run 'yunohost log show {operation_logger_name}' to see failure log",
     "app_full_domain_unavailable": "Sorry, this app must be installed on a domain of its own, but other apps are already installed on the domain '{domain}'. You could use a subdomain dedicated to this app instead.",
     "app_id_invalid": "Invalid app ID",
     "app_install_failed": "Unable to install {app}: {error}",
@@ -48,6 +49,8 @@
     "app_not_installed": "Could not find {app} in the list of installed apps: {all_apps}",
     "app_not_properly_removed": "{app} has not been properly removed",
     "app_not_upgraded": "The app '{failed_app}' failed to upgrade, and as a consequence the following apps' upgrades have been cancelled: {apps}",
+    "app_not_upgraded_broken_system": "The app '{failed_app}' failed to upgrade and put the system in a broken state, and as a consequence the following apps' upgrades have been cancelled: {apps}",
+    "app_not_upgraded_broken_system_continue": "The app '{failed_app}' failed to upgrade and put the system in a broken state (so --continue-on-failure is ignored), and as a consequence the following apps' upgrades have been cancelled: {apps}",
     "app_packaging_format_not_supported": "This app cannot be installed because its packaging format is not supported by your YunoHost version. You should probably consider upgrading your system.",
     "app_remove_after_failed_install": "Removing the app after installation failure...",
     "app_removed": "{app} uninstalled",
@@ -75,6 +78,8 @@
     "apps_catalog_obsolete_cache": "The app catalog cache is empty or obsolete.",
     "apps_catalog_update_success": "The application catalog has been updated!",
     "apps_catalog_updating": "Updating application catalog...",
+    "apps_failed_to_upgrade": "Those applications failed to upgrade:{apps}",
+    "apps_failed_to_upgrade_line": "\n * {app_id} (to see corresponding log do a 'yunohost log show {operation_logger_name}')",
     "ask_admin_fullname": "Admin full name",
     "ask_admin_username": "Admin username",
     "ask_fullname": "Full name",

share/actionsmap.yml

@@ -911,6 +911,10 @@ app:
                     full: --no-safety-backup
                     help: Disable the safety backup during upgrade
                     action: store_true
+                -c:
+                    full: --continue-on-failure
+                    help: Continue to upgrade apps event if one or more upgrade failed
+                    action: store_true
 
         ### app_change_url()
         change-url:

src/app.py

@@ -533,7 +533,7 @@ def app_change_url(operation_logger, app, domain, path):
             hook_callback("post_app_change_url", env=env_dict)
 
 
-def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False):
+def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False, continue_on_failure=False):
     """
     Upgrade app
 
@@ -585,6 +585,7 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False
         logger.info(m18n.n("app_upgrade_several_apps", apps=", ".join(apps)))
 
     notifications = {}
+    failed_to_upgrade_apps = []
 
     for number, app_instance_name in enumerate(apps):
         logger.info(m18n.n("app_upgrade_app_name", app=app_instance_name))
@@ -820,9 +821,27 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False
             # If upgrade failed or broke the system,
             # raise an error and interrupt all other pending upgrades
             if upgrade_failed or broke_the_system:
+                if not continue_on_failure or broke_the_system:
                     # display this if there are remaining apps
                     if apps[number + 1 :]:
                         not_upgraded_apps = apps[number:]
+                        if broke_the_system and not continue_on_failure:
+                            logger.error(
+                                m18n.n(
+                                    "app_not_upgraded_broken_system",
+                                    failed_app=app_instance_name,
+                                    apps=", ".join(not_upgraded_apps),
+                                )
+                            )
+                        elif broke_the_system and continue_on_failure:
+                            logger.error(
+                                m18n.n(
+                                    "app_not_upgraded_broken_system_continue",
+                                    failed_app=app_instance_name,
+                                    apps=", ".join(not_upgraded_apps),
+                                )
+                            )
+                        else:
                             logger.error(
                                 m18n.n(
                                     "app_not_upgraded",
@@ -835,6 +854,11 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False
                         failure_message_with_debug_instructions, raw_msg=True
                     )
 
+                else:
+                    operation_logger.close()
+                    logger.error(m18n.n("app_failed_to_upgrade_but_continue", failed_app=app_instance_name, operation_logger_name=operation_logger.name))
+                    failed_to_upgrade_apps.append((app_instance_name, operation_logger.name))
+
             # Otherwise we're good and keep going !
             now = int(time.time())
             app_setting(app_instance_name, "update_time", now)
@@ -895,6 +919,13 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False
 
     logger.success(m18n.n("upgrade_complete"))
 
+    if failed_to_upgrade_apps:
+        apps = ""
+        for app_id, operation_logger_name in failed_to_upgrade_apps:
+            apps += m18n.n("apps_failed_to_upgrade_line", app_id=app_id, operation_logger_name=operation_logger_name)
+
+        logger.warning(m18n.n("apps_failed_to_upgrade", apps=apps))
+
     if Moulinette.interface.type == "api":
         return {"notifications": {"POST_UPGRADE": notifications}}
 

src/tests/test_apps.py

@@ -19,7 +19,7 @@ from yunohost.app import (
     app_info,
 )
 from yunohost.domain import _get_maindomain, domain_add, domain_remove, domain_list
-from yunohost.utils.error import YunohostError
+from yunohost.utils.error import YunohostError, YunohostValidationError
 from yunohost.tests.test_permission import (
     check_LDAP_db_integrity,
     check_permission_for_apps,
@@ -541,3 +541,151 @@ def test_failed_multiple_app_upgrade(mocker, secondary_domain):
                     "legacy": os.path.join(get_test_apps_dir(), "legacy_app_ynh"),
                 },
             )
+
+
+class TestMockedAppUpgrade:
+    """
+    This class is here to test the logical workflow of app_upgrade and thus
+    mock nearly all side effects
+    """
+    def setup_method(self, method):
+        self.apps_list = []
+        self.upgradable_apps_list = []
+
+    def _mock_app_upgrade(self, mocker):
+        # app list
+        self._installed_apps = mocker.patch("yunohost.app._installed_apps", side_effect=lambda: self.apps_list)
+
+        # just check if an app is really installed
+        mocker.patch("yunohost.app._is_installed", side_effect=lambda app: app in self.apps_list)
+
+        # app_dict =
+        mocker.patch("yunohost.app.app_info", side_effect=lambda app, full: {
+            "upgradable": "yes" if app in self.upgradable_apps_list else "no",
+            "manifest": {"id": app},
+            "version": "?",
+        })
+
+        def custom_extract_app(app):
+            return ({
+                "version": "?",
+                "packaging_format": 1,
+                "id": app,
+                "notifications": {"PRE_UPGRADE": None, "POST_UPGRADE": None},
+            }, "MOCKED_BY_TEST")
+
+        # return (manifest, extracted_app_folder)
+        mocker.patch("yunohost.app._extract_app", side_effect=custom_extract_app)
+
+        # for [(name, passed, values, err), ...] in
+        mocker.patch("yunohost.app._check_manifest_requirements", return_value=[(None, True, None, None)])
+
+        # raise on failure
+        mocker.patch("yunohost.app._assert_system_is_sane_for_app", return_value=True)
+
+        from os.path import exists  # import the unmocked function
+
+        def custom_os_path_exists(path):
+            if path.endswith("manifest.toml"):
+                return True
+            return exists(path)
+
+        mocker.patch("os.path.exists", side_effect=custom_os_path_exists)
+
+        # manifest =
+        mocker.patch("yunohost.app.read_toml", return_value={
+            "arguments": {"install": []}
+        })
+
+        # install_failed, failure_message_with_debug_instructions =
+        self.hook_exec_with_script_debug_if_failure = mocker.patch("yunohost.hook.hook_exec_with_script_debug_if_failure", return_value=(False, ""))
+        # settings =
+        mocker.patch("yunohost.app._get_app_settings", return_value={})
+        # return nothing
+        mocker.patch("yunohost.app._set_app_settings")
+
+        from os import listdir  # import the unmocked function
+
+        def custom_os_listdir(path):
+            if path.endswith("MOCKED_BY_TEST"):
+                return []
+            return listdir(path)
+
+        mocker.patch("os.listdir", side_effect=custom_os_listdir)
+        mocker.patch("yunohost.app.rm")
+        mocker.patch("yunohost.app.cp")
+        mocker.patch("shutil.rmtree")
+        mocker.patch("yunohost.app.chmod")
+        mocker.patch("yunohost.app.chown")
+        mocker.patch("yunohost.app.app_ssowatconf")
+
+    def test_app_upgrade_no_apps(self, mocker):
+        self._mock_app_upgrade(mocker)
+
+        with pytest.raises(YunohostValidationError):
+            app_upgrade()
+
+    def test_app_upgrade_app_not_install(self, mocker):
+        self._mock_app_upgrade(mocker)
+
+        with pytest.raises(YunohostValidationError):
+            app_upgrade("some_app")
+
+    def test_app_upgrade_one_app(self, mocker):
+        self._mock_app_upgrade(mocker)
+        self.apps_list = ["some_app"]
+
+        # yunohost is happy, not apps to upgrade
+        app_upgrade()
+
+        self.hook_exec_with_script_debug_if_failure.assert_not_called()
+
+        self.upgradable_apps_list.append("some_app")
+        app_upgrade()
+
+        self.hook_exec_with_script_debug_if_failure.assert_called_once()
+        assert self.hook_exec_with_script_debug_if_failure.call_args.kwargs["env"]["YNH_APP_ID"] == "some_app"
+
+    def test_app_upgrade_continue_on_failure(self, mocker):
+        self._mock_app_upgrade(mocker)
+        self.apps_list = ["a", "b", "c"]
+        self.upgradable_apps_list = self.apps_list
+
+        def fails_on_b(self, *args, env, **kwargs):
+            if env["YNH_APP_ID"] == "b":
+                return True, "failed"
+            return False, "ok"
+
+        self.hook_exec_with_script_debug_if_failure.side_effect = fails_on_b
+
+        with pytest.raises(YunohostError):
+            app_upgrade()
+
+        app_upgrade(continue_on_failure=True)
+
+    def test_app_upgrade_continue_on_failure_broken_system(self, mocker):
+        """--continue-on-failure should stop on a broken system"""
+
+        self._mock_app_upgrade(mocker)
+        self.apps_list = ["a", "broke_the_system", "c"]
+        self.upgradable_apps_list = self.apps_list
+
+        def fails_on_b(self, *args, env, **kwargs):
+            if env["YNH_APP_ID"] == "broke_the_system":
+                return True, "failed"
+            return False, "ok"
+
+        self.hook_exec_with_script_debug_if_failure.side_effect = fails_on_b
+
+        def _assert_system_is_sane_for_app(manifest, state):
+            if state == "post" and manifest["id"] == "broke_the_system":
+                raise Exception()
+            return True
+
+        mocker.patch("yunohost.app._assert_system_is_sane_for_app", side_effect=_assert_system_is_sane_for_app)
+
+        with pytest.raises(YunohostError):
+            app_upgrade()
+
+        with pytest.raises(YunohostError):
+            app_upgrade(continue_on_failure=True)