diff --git a/.werks/17220.md b/.werks/17220.md new file mode 100644 index 00000000000..9958c4b23ad --- /dev/null +++ b/.werks/17220.md @@ -0,0 +1,20 @@ +[//]: # (werk v2) +# cmk-update-config: No Default in Conflict Mode 'ask' + +key | value +---------- | --- +date | 2025-01-08T10:23:54+00:00 +version | 2.3.0p25 +class | fix +edition | cre +component | omd +level | 1 +compatible | yes + +This change affects the update process (either using `cmk-update-config` or `omd update`). +Sites may have configuration, MKPs and other local files, which are incompatible with the version targeted by omd update. +If such a problem occurs, then the user may be prompted for input. +In the past, if the user typed any input other than `c`, `continue`, `d`, `disable`, then the update would be aborted. +This would lead to an aborted update in case of simple typos such as `contnue`. +With this Werk, the update only accepts the following for input strings: `a`, `abort`, `c`, `continue`, `d`, `disable`. +If anything else is typed, then the user will be prompted repeatedly until some valid input is provided. diff --git a/cmk/update_config/plugins/pre_actions/agent_based_plugins.py b/cmk/update_config/plugins/pre_actions/agent_based_plugins.py index f1c18684b48..49833e8b086 100644 --- a/cmk/update_config/plugins/pre_actions/agent_based_plugins.py +++ b/cmk/update_config/plugins/pre_actions/agent_based_plugins.py @@ -15,13 +15,14 @@ from cmk.update_config.plugins.pre_actions.utils import ( AGENT_BASED_PLUGINS_PREACTION_SORT_INDEX, ConflictMode, - continue_on_incomp_local_file, + continue_per_users_choice, disable_incomp_mkp, error_message_incomp_local_file, get_installer_and_package_map, get_path_config, is_applicable_mkp, PACKAGE_STORE, + Resume, ) from cmk.update_config.registry import pre_update_action_registry, PreUpdateAction @@ -52,7 +53,7 @@ def _disable_failure_and_reload_plugins( # unpackaged files if manifest is None: logger.error(error_message_incomp_local_file(path, error)) - if continue_on_incomp_local_file(conflict_mode): + if _continue_on_incomp_local_file(conflict_mode).is_not_abort(): continue raise MKUserError(None, "incompatible local file") @@ -87,6 +88,20 @@ def _disable_failure_and_reload_plugins( return False +def _continue_on_incomp_local_file(conflict_mode: ConflictMode) -> Resume: + match conflict_mode: + case ConflictMode.ABORT: + return Resume.ABORT + case ConflictMode.INSTALL | ConflictMode.KEEP_OLD: + return Resume.ABORT + case ConflictMode.ASK: + return continue_per_users_choice( + "You can abort the update process (A) and try to fix " + "the incompatibilities or continue the update (c).\n\n" + "Abort the update process? [A/c] \n", + ) + + pre_update_action_registry.register( PreUpdateAgentBasedPlugins( name="agent_based_plugins", diff --git a/cmk/update_config/plugins/pre_actions/autochecks.py b/cmk/update_config/plugins/pre_actions/autochecks.py index f9a8cd1a96a..7d721c1aa04 100644 --- a/cmk/update_config/plugins/pre_actions/autochecks.py +++ b/cmk/update_config/plugins/pre_actions/autochecks.py @@ -18,6 +18,7 @@ AUTOCHECK_REWRITE_PREACTION_SORT_INDEX, ConflictMode, continue_per_users_choice, + Resume, ) from cmk.update_config.registry import pre_update_action_registry, PreUpdateAction @@ -33,12 +34,12 @@ def __call__(self, logger: Logger, conflict_mode: ConflictMode) -> None: for error in rewrite_yielding_errors(write=False): if error.plugin is None: logger.error(f"{error.host_name}: {error.message}.") - if continue_per_users_choice( - conflict_mode, + if _continue_on_failed_to_migrate( " You can abort and fix this manually." " If you continue, the affected service(s) will be lost, but can be rediscovered." " Abort the update process? [A/c] \n", - ): + conflict_mode, + ).is_not_abort(): continue raise MKUserError(None, "Failed to migrate autochecks") @@ -55,14 +56,23 @@ def __call__(self, logger: Logger, conflict_mode: ConflictMode) -> None: all_messages = list(itertools.chain(*hosts.values())) logger.error(f"{plugin}: {all_messages[0]}. ") - if continue_per_users_choice( - conflict_mode, + if _continue_on_failed_to_migrate( "You can abort and fix this manually. " f"If you continue, {len(all_messages)} service(s) on {len(hosts)} host(s) will be lost, but can be rediscovered." " Abort the update process? [A/c] \n", - ): - continue - raise MKUserError(None, "Failed to migrate autochecks") + conflict_mode, + ).is_abort(): + raise MKUserError(None, "Failed to migrate autochecks") + + +def _continue_on_failed_to_migrate(prompt: str, conflict_mode: ConflictMode) -> Resume: + match conflict_mode: + case ConflictMode.ABORT: + return Resume.ABORT + case ConflictMode.INSTALL | ConflictMode.KEEP_OLD: + return Resume.ABORT + case ConflictMode.ASK: + return continue_per_users_choice(prompt) pre_update_action_registry.register( diff --git a/cmk/update_config/plugins/pre_actions/legacy_check_plugins.py b/cmk/update_config/plugins/pre_actions/legacy_check_plugins.py index d4bf5512de7..9073c62a168 100644 --- a/cmk/update_config/plugins/pre_actions/legacy_check_plugins.py +++ b/cmk/update_config/plugins/pre_actions/legacy_check_plugins.py @@ -15,6 +15,7 @@ from cmk.update_config.plugins.pre_actions.utils import ( ConflictMode, continue_per_users_choice, + Resume, ) from cmk.update_config.registry import pre_update_action_registry, PreUpdateAction @@ -31,13 +32,21 @@ def __call__(self, logger: Logger, conflict_mode: ConflictMode) -> None: ) if errors: logger.error(errors) - if continue_per_users_choice( - conflict_mode, + if _continue_on_incomp_legacy_check(conflict_mode).is_abort(): + raise MKUserError(None, "incompatible local legacy check file(s)") + + +def _continue_on_incomp_legacy_check(conflict_mode: ConflictMode) -> Resume: + match conflict_mode: + case ConflictMode.ABORT: + return Resume.ABORT + case ConflictMode.INSTALL | ConflictMode.KEEP_OLD: + return Resume.ABORT + case ConflictMode.ASK: + return continue_per_users_choice( "You can abort the update process (A) and try to fix the incompatibilities or " "continue the update (c).\n\nAbort the update process? [A/c] \n", - ): - return - raise MKUserError(None, "incompatible local legacy check file(s)") + ) pre_update_action_registry.register( diff --git a/cmk/update_config/plugins/pre_actions/rulesets.py b/cmk/update_config/plugins/pre_actions/rulesets.py index a7e154af31c..69755f035fe 100644 --- a/cmk/update_config/plugins/pre_actions/rulesets.py +++ b/cmk/update_config/plugins/pre_actions/rulesets.py @@ -6,7 +6,6 @@ from collections.abc import Sequence from logging import Logger -from typing import assert_never from cmk.utils import version from cmk.utils.log import VERBOSE @@ -24,8 +23,8 @@ from cmk.update_config.plugins.actions.rulesets import REPLACED_RULESETS from cmk.update_config.plugins.pre_actions.utils import ( ConflictMode, - prompt, - USER_INPUT_CONTINUE, + continue_per_users_choice, + Resume, ) from cmk.update_config.registry import pre_update_action_registry, PreUpdateAction @@ -40,10 +39,7 @@ def __call__(self, logger: Logger, conflict_mode: ConflictMode) -> None: rulesets = AllRulesets.load_all_rulesets() except Exception as exc: logger.error(f"Exception while trying to load rulesets: {exc}\n\n") - if ( - conflict_mode is ConflictMode.ASK - and _request_user_input_on_ruleset_exception().lower() in USER_INPUT_CONTINUE - ): + if _continue_on_ruleset_exception(conflict_mode).is_not_abort(): return None raise MKUserError(None, "an incompatible ruleset") from exc @@ -65,25 +61,50 @@ def __call__(self, logger: Logger, conflict_mode: ConflictMode) -> None: ruleset.name, ) logger.exception("This is the exception: ") - if conflict_mode is ConflictMode.ASK: - user_input = prompt( - "You can abort the update process (A) or continue (c) the update. Abort update? [A/c]\n" - ) - if not user_input.lower() in USER_INPUT_CONTINUE: - raise MKUserError(None, "broken ruleset") + if _continue_on_broken_ruleset(conflict_mode).is_abort(): + raise MKUserError(None, "broken ruleset") if not result: raise MKUserError(None, "failed ruleset validation") - return None -def _request_user_input_on_ruleset_exception() -> str: - return prompt( - "You can abort the update process (A) and try to fix " - "the incompatibilities or try to continue the update (c).\n" - "Abort update? [A/c]\n" - ) +def _continue_on_broken_ruleset(conflict_mode: ConflictMode) -> Resume: + match conflict_mode: + case ConflictMode.ABORT: + return Resume.UPDATE + case ConflictMode.INSTALL | ConflictMode.KEEP_OLD: + return Resume.UPDATE + case ConflictMode.ASK: + return continue_per_users_choice( + "You can abort the update process (A) or continue (c) the update. Abort update? [A/c]\n" + ) + + +def _continue_on_invalid_rule(conflict_mode: ConflictMode) -> Resume: + match conflict_mode: + case ConflictMode.ABORT: + return Resume.ABORT + case ConflictMode.INSTALL | ConflictMode.KEEP_OLD: + return Resume.UPDATE + case ConflictMode.ASK: + return continue_per_users_choice( + "You can abort the update process (A) or continue (c) the update. Abort update? [A/c]\n" + ) + + +def _continue_on_ruleset_exception(conflict_mode: ConflictMode) -> Resume: + match conflict_mode: + case ConflictMode.ABORT: + return Resume.ABORT + case ConflictMode.INSTALL | ConflictMode.KEEP_OLD: + return Resume.ABORT + case ConflictMode.ASK: + return continue_per_users_choice( + "You can abort the update process (A) and try to fix " + "the incompatibilities or try to continue the update (c).\n" + "Abort update? [A/c]\n" + ) def _validate_rule_values( @@ -147,21 +168,8 @@ def _validate_rule_values( addition_info = [] error_message = _error_message(ruleset, folder, index, e, addition_info) logger.error(error_message) - match conflict_mode: - case ConflictMode.ABORT: - return False - case ConflictMode.ASK: - if ( - prompt( - "You can abort the update process (A) or continue (c) the update. Abort update? [A/c]\n" - ).lower() - not in USER_INPUT_CONTINUE - ): - return False - case ConflictMode.KEEP_OLD | ConflictMode.INSTALL: - continue - case _: - assert_never(conflict_mode) + if _continue_on_invalid_rule(conflict_mode).is_abort(): + return False return True diff --git a/cmk/update_config/plugins/pre_actions/ui_extensions.py b/cmk/update_config/plugins/pre_actions/ui_extensions.py index 002ba31a019..a0c2c05b707 100644 --- a/cmk/update_config/plugins/pre_actions/ui_extensions.py +++ b/cmk/update_config/plugins/pre_actions/ui_extensions.py @@ -17,7 +17,7 @@ from cmk.mkp_tool import Manifest, PackageID from cmk.update_config.plugins.pre_actions.utils import ( ConflictMode, - continue_on_incomp_local_file, + continue_per_users_choice, disable_incomp_mkp, error_message_incomp_local_file, get_installer_and_package_map, @@ -25,6 +25,7 @@ GUI_PLUGINS_PREACTION_SORT_INDEX, is_applicable_mkp, PACKAGE_STORE, + Resume, ) from cmk.update_config.registry import pre_update_action_registry, PreUpdateAction @@ -55,7 +56,7 @@ def __call__(self, logger: Logger, conflict_mode: ConflictMode) -> None: # unpackaged files if manifest is None: logger.error(error_message_incomp_local_file(path, error)) - if continue_on_incomp_local_file(conflict_mode): + if _continue_on_incomp_local_file(conflict_mode).is_not_abort(): continue raise MKUserError(None, "incompatible local file") @@ -95,6 +96,20 @@ def __call__(self, logger: Logger, conflict_mode: ConflictMode) -> None: print(e) +def _continue_on_incomp_local_file(conflict_mode: ConflictMode) -> Resume: + match conflict_mode: + case ConflictMode.ABORT: + return Resume.ABORT + case ConflictMode.INSTALL | ConflictMode.KEEP_OLD: + return Resume.ABORT + case ConflictMode.ASK: + return continue_per_users_choice( + "You can abort the update process (A) and try to fix " + "the incompatibilities or continue the update (c).\n\n" + "Abort the update process? [A/c] \n", + ) + + pre_update_action_registry.register( PreUpdateUIExtensions( name="ui_extensions", diff --git a/cmk/update_config/plugins/pre_actions/utils.py b/cmk/update_config/plugins/pre_actions/utils.py index f5692a77059..fbe133cf15e 100644 --- a/cmk/update_config/plugins/pre_actions/utils.py +++ b/cmk/update_config/plugins/pre_actions/utils.py @@ -40,7 +40,7 @@ ) -def prompt(message: str) -> str: +def _prompt(message: str) -> str: tcflush(sys.stdin, TCIFLUSH) return input(message) @@ -100,8 +100,20 @@ class ConflictMode(enum.StrEnum): ABORT = "abort" -USER_INPUT_CONTINUE: Final[Sequence] = ["c", "continue"] -USER_INPUT_DISABLE: Final[Sequence] = ["d", "disable"] +_USER_INPUT_ABORT: Final = ("a", "abort") +_USER_INPUT_CONTINUE: Final = ("c", "continue") +_USER_INPUT_DISABLE: Final = ("d", "disable") + + +class Resume(enum.Enum): + UPDATE = enum.auto() + ABORT = enum.auto() + + def is_abort(self) -> bool: + return self is Resume.ABORT + + def is_not_abort(self) -> bool: + return self is not Resume.ABORT def disable_incomp_mkp( @@ -116,10 +128,7 @@ def disable_incomp_mkp( path: Path, ) -> bool: logger.error(error_message_incomp_package(path, package_id, error)) - if conflict_mode in (ConflictMode.INSTALL, ConflictMode.KEEP_OLD) or ( - conflict_mode is ConflictMode.ASK - and _request_user_input_on_incompatible_file().lower() in USER_INPUT_DISABLE - ): + if _request_user_input_on_incompatible_file(conflict_mode).is_not_abort(): if ( disabled := disable( installer, @@ -136,12 +145,18 @@ def disable_incomp_mkp( return False -def _request_user_input_on_incompatible_file() -> str: - return prompt( - "You can abort the update process (A) or disable the " - "extension package (d) and continue the update process.\n" - "Abort the update process? [A/d] \n" - ) +def _request_user_input_on_incompatible_file(conflict_mode: ConflictMode) -> Resume: + match conflict_mode: + case ConflictMode.ABORT: + return Resume.ABORT + case ConflictMode.INSTALL | ConflictMode.KEEP_OLD: + return Resume.UPDATE + case ConflictMode.ASK: + return continue_per_users_choice( + "You can abort the update process (A) or disable the " + "extension package (d) and continue the update process.\n" + "Abort the update process? [A/d] \n" + ) def error_message_incomp_package(path: Path, package_id: PackageID, error: BaseException) -> str: @@ -157,23 +172,30 @@ def _make_post_change_actions() -> Callable[[Sequence[Manifest]], None]: ) -def continue_on_incomp_local_file(conflict_mode: ConflictMode) -> bool: - return continue_per_users_choice( - conflict_mode, - "You can abort the update process (A) and try to fix " - "the incompatibilities or continue the update (c).\n\n" - "Abort the update process? [A/c] \n", - ) - - def error_message_incomp_local_file(path: Path, error: BaseException) -> str: return f"Incompatible local file '{path}'.\nError: {error}\n\n" -def continue_per_users_choice(conflict_mode: ConflictMode, propt_text: str) -> bool: - if conflict_mode is ConflictMode.ASK: - return prompt(propt_text).lower() in USER_INPUT_CONTINUE - return False +def continue_per_users_choice(prompt_text: str) -> Resume: + while (response := _prompt(prompt_text).lower()) not in [ + *_USER_INPUT_CONTINUE, + *_USER_INPUT_ABORT, + ]: + sys.stdout.write(f"Invalid input '{response}'.\n") + if response in _USER_INPUT_CONTINUE: + return Resume.UPDATE + return Resume.ABORT + + +def _disable_per_users_choice(prompt_text: str) -> Resume: + while (response := _prompt(prompt_text).lower()) not in [ + *_USER_INPUT_DISABLE, + *_USER_INPUT_ABORT, + ]: + sys.stdout.write(f"Invalid input '{response}'.\n") + if response in _USER_INPUT_DISABLE: + return Resume.UPDATE + return Resume.ABORT def get_installer_and_package_map(