diff --git a/locales/en.json b/locales/en.json index 7cc1b96b6..2966beb45 100644 --- a/locales/en.json +++ b/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", diff --git a/share/actionsmap.yml b/share/actionsmap.yml index 7f0fdabe9..58787790c 100644 --- a/share/actionsmap.yml +++ b/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: diff --git a/src/app.py b/src/app.py index c2d4d0a89..b1884598f 100644 --- a/src/app.py +++ b/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,20 +821,43 @@ 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: - # display this if there are remaining apps - if apps[number + 1 :]: - not_upgraded_apps = apps[number:] - logger.error( - m18n.n( - "app_not_upgraded", - failed_app=app_instance_name, - apps=", ".join(not_upgraded_apps), - ) + 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", + failed_app=app_instance_name, + apps=", ".join(not_upgraded_apps), + ) + ) + + raise YunohostError( + failure_message_with_debug_instructions, raw_msg=True ) - raise YunohostError( - 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()) @@ -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}} diff --git a/src/tests/test_apps.py b/src/tests/test_apps.py index 965ce5892..830aabf61 100644 --- a/src/tests/test_apps.py +++ b/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)