From fe1ad27f32e69e3e7c046b51e5406a0693ea1c35 Mon Sep 17 00:00:00 2001 From: Ondrej Mular Date: Tue, 11 Jan 2022 08:01:10 +0100 Subject: [PATCH 3/5] Multiple fixes of `pcs resource move --autodelete` command --- pcs/common/reports/codes.py | 1 + pcs/common/reports/messages.py | 21 ++ pcs/lib/cib/node.py | 14 +- pcs/lib/commands/resource.py | 105 ++++++- pcs/lib/node.py | 7 +- .../tier0/common/reports/test_messages.py | 12 + .../resource/test_resource_move_autoclean.py | 280 +++++++++++++++++- .../resource/test_resource_move_ban.py | 45 ++- .../tools/command_env/config_runner_pcmk.py | 2 + pcs_test/tools/command_env/mock_runner.py | 2 +- pcs_test/tools/fixture_cib.py | 1 + 11 files changed, 456 insertions(+), 34 deletions(-) diff --git a/pcs/common/reports/codes.py b/pcs/common/reports/codes.py index 5bae7170..3e0512d9 100644 --- a/pcs/common/reports/codes.py +++ b/pcs/common/reports/codes.py @@ -418,6 +418,7 @@ RESOURCE_UNMOVE_UNBAN_PCMK_EXPIRED_NOT_SUPPORTED = M( ) RESOURCE_MOVE_CONSTRAINT_CREATED = M("RESOURCE_MOVE_CONSTRAINT_CREATED") RESOURCE_MOVE_CONSTRAINT_REMOVED = M("RESOURCE_MOVE_CONSTRAINT_REMOVED") +RESOURCE_MOVE_NOT_AFFECTING_RESOURCE = M("RESOURCE_MOVE_NOT_AFFECTING_RESOURCE") RESOURCE_MOVE_AFFECTS_OTRHER_RESOURCES = M( "RESOURCE_MOVE_AFFECTS_OTRHER_RESOURCES" ) diff --git a/pcs/common/reports/messages.py b/pcs/common/reports/messages.py index 43ce38e1..9d665e73 100644 --- a/pcs/common/reports/messages.py +++ b/pcs/common/reports/messages.py @@ -6110,6 +6110,27 @@ class ResourceMoveConstraintRemoved(ReportItemMessage): ) +@dataclass(frozen=True) +class ResourceMoveNotAffectingResource(ReportItemMessage): + """ + Creating a location constraint to move a resource has no effect on the + resource. + + resource_id -- id of the resource to be moved + """ + + resource_id: str + _code = codes.RESOURCE_MOVE_NOT_AFFECTING_RESOURCE + + @property + def message(self) -> str: + return ( + f"Unable to move resource '{self.resource_id}' using a location " + "constraint. Current location of the resource may be affected by " + "some other constraint." + ) + + @dataclass(frozen=True) class ResourceMoveAffectsOtherResources(ReportItemMessage): """ diff --git a/pcs/lib/cib/node.py b/pcs/lib/cib/node.py index 20a41ca0..df2ffbaa 100644 --- a/pcs/lib/cib/node.py +++ b/pcs/lib/cib/node.py @@ -1,12 +1,17 @@ from collections import namedtuple +from typing import Set from lxml import etree +from lxml.etree import _Element from pcs.common import reports from pcs.common.reports.item import ReportItem from pcs.lib.cib.nvpair import update_nvset from pcs.lib.cib.tools import get_nodes from pcs.lib.errors import LibraryError -from pcs.lib.xml_tools import append_when_useful +from pcs.lib.xml_tools import ( + append_when_useful, + get_root, +) class PacemakerNode(namedtuple("PacemakerNode", "name addr")): @@ -58,6 +63,13 @@ def update_node_instance_attrs( append_when_useful(cib_nodes, node_el) +def get_node_names(cib: _Element) -> Set[str]: + return { + str(node.attrib["uname"]) + for node in get_nodes(get_root(cib)).iterfind("./node") + } + + def _ensure_node_exists(tree, node_name, state_nodes=None): """ Make sure node with specified name exists diff --git a/pcs/lib/commands/resource.py b/pcs/lib/commands/resource.py index d0e8f4db..82ce73e0 100644 --- a/pcs/lib/commands/resource.py +++ b/pcs/lib/commands/resource.py @@ -50,12 +50,16 @@ from pcs.lib.cib.tools import ( from pcs.lib.env import LibraryEnvironment, WaitType from pcs.lib.errors import LibraryError from pcs.lib.external import CommandRunner -from pcs.lib.node import get_existing_nodes_names_addrs +from pcs.lib.node import ( + get_existing_nodes_names_addrs, + get_pacemaker_node_names, +) from pcs.lib.pacemaker import simulate as simulate_tools from pcs.lib.pacemaker.live import ( diff_cibs_xml, get_cib, get_cib_xml, + get_cluster_status_dom, has_resource_unmove_unban_expired_support, push_cib_diff_xml, resource_ban, @@ -1589,6 +1593,16 @@ def move( ) +def _nodes_exist_reports( + cib: _Element, node_names: Iterable[str] +) -> ReportItemList: + existing_node_names = get_pacemaker_node_names(cib) + return [ + reports.ReportItem.error(reports.messages.NodeNotFound(node_name)) + for node_name in (set(node_names) - existing_node_names) + ] + + def move_autoclean( env: LibraryEnvironment, resource_id: str, @@ -1626,6 +1640,9 @@ def move_autoclean( if resource_el is not None: report_list.extend(resource.common.validate_move(resource_el, master)) + if node: + report_list.extend(_nodes_exist_reports(cib, [node])) + if env.report_processor.report_list(report_list).has_errors: raise LibraryError() @@ -1659,8 +1676,32 @@ def move_autoclean( add_constraint_cib_diff = diff_cibs_xml( env.cmd_runner(), env.report_processor, cib_xml, rsc_moved_cib_xml ) + with get_tmp_cib( + env.report_processor, rsc_moved_cib_xml + ) as rsc_moved_constraint_cleared_cib_file: + stdout, stderr, retval = resource_unmove_unban( + env.cmd_runner( + dict(CIB_file=rsc_moved_constraint_cleared_cib_file.name) + ), + resource_id, + node, + master, + ) + if retval != 0: + raise LibraryError( + ReportItem.error( + reports.messages.ResourceUnmoveUnbanPcmkError( + resource_id, stdout, stderr + ) + ) + ) + rsc_moved_constraint_cleared_cib_file.seek(0) + constraint_removed_cib = rsc_moved_constraint_cleared_cib_file.read() remove_constraint_cib_diff = diff_cibs_xml( - env.cmd_runner(), env.report_processor, rsc_moved_cib_xml, cib_xml + env.cmd_runner(), + env.report_processor, + rsc_moved_cib_xml, + constraint_removed_cib, ) if not (add_constraint_cib_diff and remove_constraint_cib_diff): @@ -1689,13 +1730,15 @@ def move_autoclean( ) ) ) - _ensure_resource_is_not_moved( + _ensure_resource_moved_and_not_moved_back( env.cmd_runner, env.report_processor, etree_to_str(after_move_simulated_cib), remove_constraint_cib_diff, resource_id, strict, + resource_state_before, + node, ) push_cib_diff_xml(env.cmd_runner(), add_constraint_cib_diff) env.report_processor.report( @@ -1704,13 +1747,15 @@ def move_autoclean( ) ) env.wait_for_idle(wait_timeout) - _ensure_resource_is_not_moved( + _ensure_resource_moved_and_not_moved_back( env.cmd_runner, env.report_processor, get_cib_xml(env.cmd_runner()), remove_constraint_cib_diff, resource_id, strict, + resource_state_before, + node, ) push_cib_diff_xml(env.cmd_runner(), remove_constraint_cib_diff) env.report_processor.report( @@ -1730,16 +1775,35 @@ def move_autoclean( raise LibraryError() -def _ensure_resource_is_not_moved( +def _ensure_resource_moved_and_not_moved_back( runner_factory: Callable[[Optional[Mapping[str, str]]], CommandRunner], report_processor: reports.ReportProcessor, cib_xml: str, remove_constraint_cib_diff: str, resource_id: str, strict: bool, + resource_state_before: Dict[str, List[str]], + node: Optional[str], ) -> None: # pylint: disable=too-many-locals with get_tmp_cib(report_processor, cib_xml) as rsc_unmove_cib_file: + if not _was_resource_moved( + node, + resource_state_before, + get_resource_state( + get_cluster_status_dom( + runner_factory(dict(CIB_file=rsc_unmove_cib_file.name)) + ), + resource_id, + ), + ): + raise LibraryError( + reports.ReportItem.error( + reports.messages.ResourceMoveNotAffectingResource( + resource_id + ) + ) + ) push_cib_diff_xml( runner_factory(dict(CIB_file=rsc_unmove_cib_file.name)), remove_constraint_cib_diff, @@ -1809,20 +1873,31 @@ def _resource_running_on_nodes( return frozenset() +def _was_resource_moved( + node: Optional[str], + resource_state_before: Dict[str, List[str]], + resource_state_after: Dict[str, List[str]], +) -> bool: + running_on_nodes = _resource_running_on_nodes(resource_state_after) + return not bool( + resource_state_before + and ( # running resource moved + not running_on_nodes + or (node and node not in running_on_nodes) + or (resource_state_before == resource_state_after) + ) + ) + + def _move_wait_report( resource_id: str, node: Optional[str], resource_state_before: Dict[str, List[str]], resource_state_after: Dict[str, List[str]], ) -> ReportItem: - allowed_nodes = frozenset([node] if node else []) - running_on_nodes = _resource_running_on_nodes(resource_state_after) - severity = reports.item.ReportItemSeverity.info() - if resource_state_before and ( # running resource moved - not running_on_nodes - or (allowed_nodes and allowed_nodes.isdisjoint(running_on_nodes)) - or (resource_state_before == resource_state_after) + if not _was_resource_moved( + node, resource_state_before, resource_state_after ): severity = reports.item.ReportItemSeverity.error() if not resource_state_after: @@ -1873,14 +1948,18 @@ class _MoveBanTemplate: lifetime=None, wait: WaitType = False, ): + # pylint: disable=too-many-locals # validate wait_timeout = env.ensure_wait_satisfiable(wait) # raises on error + cib = env.get_cib() resource_el, report_list = resource.common.find_one_resource( - get_resources(env.get_cib()), resource_id + get_resources(cib), resource_id ) if resource_el is not None: report_list.extend(self._validate(resource_el, master)) + if node: + report_list.extend(_nodes_exist_reports(cib, [node])) if env.report_processor.report_list(report_list).has_errors: raise LibraryError() diff --git a/pcs/lib/node.py b/pcs/lib/node.py index ff08f747..3a7f236e 100644 --- a/pcs/lib/node.py +++ b/pcs/lib/node.py @@ -3,6 +3,7 @@ from typing import ( List, Optional, Tuple, + Set, ) from lxml.etree import _Element @@ -11,7 +12,7 @@ from pcs.common import reports from pcs.common.reports import ReportItemList from pcs.common.reports import ReportItemSeverity from pcs.common.reports.item import ReportItem -from pcs.lib.cib.node import PacemakerNode +from pcs.lib.cib.node import PacemakerNode, get_node_names from pcs.lib.cib.resource import remote_node, guest_node from pcs.lib.corosync.config_facade import ConfigFacade as CorosyncConfigFacade from pcs.lib.corosync.node import CorosyncNode @@ -28,6 +29,10 @@ def get_existing_nodes_names( ) +def get_pacemaker_node_names(cib: _Element) -> Set[str]: + return get_node_names(cib) | set(get_existing_nodes_names(None, cib)[0]) + + def get_existing_nodes_names_addrs( corosync_conf=None, cib=None, error_on_missing_name=False ): diff --git a/pcs_test/tier0/common/reports/test_messages.py b/pcs_test/tier0/common/reports/test_messages.py index c85aaa9c..4a7b4945 100644 --- a/pcs_test/tier0/common/reports/test_messages.py +++ b/pcs_test/tier0/common/reports/test_messages.py @@ -4515,6 +4515,18 @@ class ResourceMoveConstraintRemoved(NameBuildTest): ) +class ResourceMoveNotAffectingResource(NameBuildTest): + def test_success(self): + self.assert_message_from_report( + ( + "Unable to move resource 'R1' using a location constraint. " + "Current location of the resource may be affected by some " + "other constraint." + ), + reports.ResourceMoveNotAffectingResource("R1"), + ) + + class ResourceMoveAffectsOtherResources(NameBuildTest): def test_multiple(self): self.assert_message_from_report( diff --git a/pcs_test/tier0/lib/commands/resource/test_resource_move_autoclean.py b/pcs_test/tier0/lib/commands/resource/test_resource_move_autoclean.py index 32d758de..1bd4ee82 100644 --- a/pcs_test/tier0/lib/commands/resource/test_resource_move_autoclean.py +++ b/pcs_test/tier0/lib/commands/resource/test_resource_move_autoclean.py @@ -20,6 +20,25 @@ from pcs_test.tools.command_env import get_env_tools from pcs_test.tools.misc import get_test_resource as rc +def _node_fixture(name, node_id): + return f'' + + +def _node_list_fixture(nodes): + return "\n".join( + _node_fixture(node_name, node_id) + for node_id, node_name in enumerate(nodes) + ) + + +def _nodes_section_fixture(content): + return f""" + + {content} + + """ + + def _rsc_primitive_fixture(res_id): return f'' @@ -145,11 +164,17 @@ class MoveAutocleanSuccess(MoveAutocleanCommonSetup): resources=_resources_tag( _resource_primitive + _resource_promotable_clone ), + nodes=_nodes_section_fixture( + _node_list_fixture([self.orig_node, self.new_node]) + ), ) self.orig_cib = etree_to_str( xml_fromstring(self.config.calls.get(config_load_cib_name).stdout) ) self.cib_with_constraint = '' + self.cib_without_constraint = ( + '' + ) self.cib_simulate_constraint = ( '' ) @@ -160,6 +185,9 @@ class MoveAutocleanSuccess(MoveAutocleanCommonSetup): self.cib_diff_add_constraint_updated_tmp_file_name = ( "cib_diff_add_constraint_updated" ) + self.cib_constraint_removed_by_unmove_file_name = ( + "cib_constraint_removed_by_unmove" + ) self.cib_diff_remove_constraint_orig_tmp_file_name = ( "cib_diff_remove_constraint_orig" ) @@ -220,13 +248,18 @@ class MoveAutocleanSuccess(MoveAutocleanCommonSetup): self.cib_diff_add_constraint_updated_tmp_file_name, orig_content=self.cib_with_constraint, ), + TmpFileCall( + self.cib_constraint_removed_by_unmove_file_name, + orig_content=self.cib_with_constraint, + new_content=self.cib_without_constraint, + ), TmpFileCall( self.cib_diff_remove_constraint_orig_tmp_file_name, orig_content=self.cib_with_constraint, ), TmpFileCall( self.cib_diff_remove_constraint_updated_tmp_file_name, - orig_content=self.orig_cib, + orig_content=self.cib_without_constraint, ), TmpFileCall( self.simulated_cib_add_constraint_tmp_file_name, @@ -296,6 +329,12 @@ class MoveAutocleanSuccess(MoveAutocleanCommonSetup): stdout=self.cib_diff_add_constraint, name="runner.cib.diff.add_constraint", ) + self.config.runner.pcmk.resource_clear( + resource=resource_id, + master=is_promotable, + node=self.new_node if with_node else None, + env=dict(CIB_file=self.cib_constraint_removed_by_unmove_file_name), + ) self.config.runner.cib.diff( self.cib_diff_remove_constraint_orig_tmp_file_name, self.cib_diff_remove_constraint_updated_tmp_file_name, @@ -308,6 +347,13 @@ class MoveAutocleanSuccess(MoveAutocleanCommonSetup): cib_xml=self.cib_with_constraint, name="pcmk.simulate.rsc.move", ) + self.config.runner.pcmk.load_state( + resources=status_after, + name="runner.pcmk.load_state.mid_simulation", + env=dict( + CIB_file=self.cib_apply_diff_remove_constraint_from_simulated_cib_tmp_file_name + ), + ) self.config.runner.cib.push_diff( cib_diff=self.cib_diff_remove_constraint, name="pcmk.push_cib_diff.simulation.remove_constraint", @@ -335,6 +381,13 @@ class MoveAutocleanSuccess(MoveAutocleanCommonSetup): self.cib_with_constraint, name="load_cib_after_move", ) + self.config.runner.pcmk.load_state( + resources=status_after, + name="runner.pcmk.load_state.after_push", + env=dict( + CIB_file=self.cib_apply_diff_remove_constraint_after_push_tmp_file_name + ), + ) self.config.runner.cib.push_diff( cib_diff=self.cib_diff_remove_constraint, name="pcmk.push_cib_diff.simulation.remove_constraint_after_move", @@ -380,6 +433,11 @@ class MoveAutocleanSuccess(MoveAutocleanCommonSetup): file_path=self.cib_diff_add_constraint_updated_tmp_file_name, content=self.cib_with_constraint, ), + fixture.debug( + reports.codes.TMP_FILE_WRITE, + file_path=self.cib_constraint_removed_by_unmove_file_name, + content=self.cib_with_constraint, + ), fixture.debug( reports.codes.TMP_FILE_WRITE, file_path=self.cib_diff_remove_constraint_orig_tmp_file_name, @@ -388,7 +446,7 @@ class MoveAutocleanSuccess(MoveAutocleanCommonSetup): fixture.debug( reports.codes.TMP_FILE_WRITE, file_path=self.cib_diff_remove_constraint_updated_tmp_file_name, - content=self.orig_cib, + content=self.cib_without_constraint, ), fixture.debug( reports.codes.TMP_FILE_WRITE, @@ -758,9 +816,7 @@ class MoveAutocleanValidations(MoveAutocleanCommonSetup): resources=_state_resource_fixture(resource_id, "Stopped"), ) self.env_assist.assert_raise_library_error( - lambda: move_autoclean( - self.env_assist.get_env(), resource_id, node="node" - ), + lambda: move_autoclean(self.env_assist.get_env(), resource_id), [ fixture.error( reports.codes.CANNOT_MOVE_RESOURCE_NOT_RUNNING, @@ -770,11 +826,33 @@ class MoveAutocleanValidations(MoveAutocleanCommonSetup): expected_in_processor=False, ) + def test_node_not_found(self): + resource_id = "A" + node = "non_existing_node" + self.config.runner.cib.load( + resources=_resources_tag(_rsc_primitive_fixture(resource_id)), + ) + self.env_assist.assert_raise_library_error( + lambda: move_autoclean( + self.env_assist.get_env(), resource_id, node + ), + ) + self.env_assist.assert_reports( + [ + fixture.error( + reports.codes.NODE_NOT_FOUND, + node=node, + searched_types=[], + ) + ], + ) + def test_constraint_already_exist(self): resource_id = "A" config_load_cib_name = "load_cib" node = "node1" cib_with_constraint = '' + cib_without_constraint = '' cib_rsc_move_tmp_file_name = "cib_rsc_move_tmp_file" cib_diff_add_constraint_orig_tmp_file_name = ( "cib_diff_add_constraint_orig" @@ -788,6 +866,9 @@ class MoveAutocleanValidations(MoveAutocleanCommonSetup): cib_diff_remove_constraint_updated_tmp_file_name = ( "cib_diff_remove_constraint_updated" ) + cib_constraint_removed_by_unmove_file_name = ( + "cib_constraint_removed_by_unmove" + ) self.config.runner.cib.load( resources=_resources_tag(_rsc_primitive_fixture(resource_id)), constraints=f""" @@ -795,6 +876,7 @@ class MoveAutocleanValidations(MoveAutocleanCommonSetup): """, + nodes=_nodes_section_fixture(_node_list_fixture([node])), name=config_load_cib_name, ) orig_cib = etree_to_str( @@ -815,13 +897,18 @@ class MoveAutocleanValidations(MoveAutocleanCommonSetup): cib_diff_add_constraint_updated_tmp_file_name, orig_content=cib_with_constraint, ), + TmpFileCall( + cib_constraint_removed_by_unmove_file_name, + orig_content=cib_with_constraint, + new_content=cib_without_constraint, + ), TmpFileCall( cib_diff_remove_constraint_orig_tmp_file_name, orig_content=cib_with_constraint, ), TmpFileCall( cib_diff_remove_constraint_updated_tmp_file_name, - orig_content=orig_cib, + orig_content=cib_without_constraint, ), ] ) @@ -839,6 +926,11 @@ class MoveAutocleanValidations(MoveAutocleanCommonSetup): stdout="", name="runner.cib.diff.add_constraint", ) + self.config.runner.pcmk.resource_clear( + resource=resource_id, + node=node, + env=dict(CIB_file=cib_constraint_removed_by_unmove_file_name), + ) self.config.runner.cib.diff( cib_diff_remove_constraint_orig_tmp_file_name, cib_diff_remove_constraint_updated_tmp_file_name, @@ -863,6 +955,11 @@ class MoveAutocleanValidations(MoveAutocleanCommonSetup): file_path=cib_diff_add_constraint_updated_tmp_file_name, content=cib_with_constraint, ), + fixture.debug( + reports.codes.TMP_FILE_WRITE, + file_path=cib_constraint_removed_by_unmove_file_name, + content=cib_with_constraint, + ), fixture.debug( reports.codes.TMP_FILE_WRITE, file_path=cib_diff_remove_constraint_orig_tmp_file_name, @@ -871,7 +968,7 @@ class MoveAutocleanValidations(MoveAutocleanCommonSetup): fixture.debug( reports.codes.TMP_FILE_WRITE, file_path=cib_diff_remove_constraint_updated_tmp_file_name, - content=orig_cib, + content=cib_without_constraint, ), fixture.info( reports.codes.NO_ACTION_NECESSARY, @@ -896,6 +993,9 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): self.cib_diff_add_constraint = "diff_add_constraint" self.cib_diff_remove_constraint = "diff_remove_constraint" self.cib_with_constraint = '' + self.cib_without_constraint = ( + '' + ) self.cib_rsc_move_tmp_file_name = "cib_rsc_move_tmp_file" self.cib_diff_add_constraint_orig_tmp_file_name = ( "cib_diff_add_constraint_orig" @@ -903,6 +1003,9 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): self.cib_diff_add_constraint_updated_tmp_file_name = ( "cib_diff_add_constraint_updated" ) + self.cib_constraint_removed_by_unmove_file_name = ( + "cib_constraint_removed_by_unmove" + ) self.cib_diff_remove_constraint_orig_tmp_file_name = ( "cib_diff_remove_constraint_orig" ) @@ -951,6 +1054,9 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): self.config.runner.cib.load( resources=_resources_tag(_rsc_primitive_fixture(self.resource_id)), + nodes=_nodes_section_fixture( + _node_list_fixture(["node1", "node2"]) + ), name=self.config_load_cib_name, ) self.orig_cib = etree_to_str( @@ -979,13 +1085,18 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): self.cib_diff_add_constraint_updated_tmp_file_name, orig_content=self.cib_with_constraint, ), + TmpFileCall( + self.cib_constraint_removed_by_unmove_file_name, + orig_content=self.cib_with_constraint, + new_content=self.cib_without_constraint, + ), TmpFileCall( self.cib_diff_remove_constraint_orig_tmp_file_name, orig_content=self.cib_with_constraint, ), TmpFileCall( self.cib_diff_remove_constraint_updated_tmp_file_name, - orig_content=self.orig_cib, + orig_content=self.cib_without_constraint, ), TmpFileCall( self.simulated_cib_add_constraint_tmp_file_name, @@ -1067,6 +1178,11 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): stdout=self.cib_diff_add_constraint, name="runner.cib.diff.add_constraint", ) + self.config.runner.pcmk.resource_clear( + resource=self.resource_id, + node=node, + env=dict(CIB_file=self.cib_constraint_removed_by_unmove_file_name), + ) self.config.runner.cib.diff( self.cib_diff_remove_constraint_orig_tmp_file_name, self.cib_diff_remove_constraint_updated_tmp_file_name, @@ -1081,6 +1197,15 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): ) if stage <= 1: return + self.config.runner.pcmk.load_state( + resources=_state_resource_fixture( + self.resource_id, "Started", node if node else "node2" + ), + name="runner.pcmk.load_state.mid_simulation", + env=dict( + CIB_file=self.cib_apply_diff_remove_constraint_from_simulated_cib_tmp_file_name + ), + ) self.config.runner.cib.push_diff( cib_diff=self.cib_diff_remove_constraint, name="pcmk.push_cib_diff.simulation.remove_constraint", @@ -1110,6 +1235,17 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): self.cib_with_constraint, name="load_cib_after_move", ) + if stage <= 3: + return + self.config.runner.pcmk.load_state( + resources=_state_resource_fixture( + self.resource_id, "Started", node if node else "node2" + ), + name="runner.pcmk.load_state.after_push", + env=dict( + CIB_file=self.cib_apply_diff_remove_constraint_after_push_tmp_file_name + ), + ) self.config.runner.cib.push_diff( cib_diff=self.cib_diff_remove_constraint, name="pcmk.push_cib_diff.simulation.remove_constraint_after_move", @@ -1126,7 +1262,7 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): ), name="pcmk.simulate.rsc.unmove.after_push", ) - if stage <= 3: + if stage <= 4: return self.config.runner.cib.push_diff( cib_diff=self.cib_diff_remove_constraint, @@ -1153,6 +1289,11 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): file_path=self.cib_diff_add_constraint_updated_tmp_file_name, content=self.cib_with_constraint, ), + fixture.debug( + reports.codes.TMP_FILE_WRITE, + file_path=self.cib_constraint_removed_by_unmove_file_name, + content=self.cib_with_constraint, + ), fixture.debug( reports.codes.TMP_FILE_WRITE, file_path=self.cib_diff_remove_constraint_orig_tmp_file_name, @@ -1161,7 +1302,7 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): fixture.debug( reports.codes.TMP_FILE_WRITE, file_path=self.cib_diff_remove_constraint_updated_tmp_file_name, - content=self.orig_cib, + content=self.cib_without_constraint, ), fixture.debug( reports.codes.TMP_FILE_WRITE, @@ -1199,7 +1340,7 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): reports.codes.WAIT_FOR_IDLE_STARTED, timeout=0, ), - ][: {None: None, 3: -2, 2: 7, 1: 5}[stage]] + ][: {None: None, 4: -2, 3: 10, 2: 8, 1: 6}[stage]] def test_move_affects_other_resources_strict(self): self.tmp_file_mock_obj.set_calls( @@ -1304,7 +1445,8 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): ), ) ) - self.set_up_testing_env(stage=3) + setup_stage = 4 + self.set_up_testing_env(stage=setup_stage) self.env_assist.assert_raise_library_error( lambda: move_autoclean(self.env_assist.get_env(), self.resource_id), [ @@ -1316,7 +1458,7 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): ], expected_in_processor=False, ) - self.env_assist.assert_reports(self.get_reports(stage=3)) + self.env_assist.assert_reports(self.get_reports(stage=setup_stage)) def test_unmove_after_push_affects_other_resources_strict(self): self.tmp_file_mock_obj.set_calls( @@ -1330,7 +1472,8 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): ), ) ) - self.set_up_testing_env(stage=3) + setup_stage = 4 + self.set_up_testing_env(stage=setup_stage) self.env_assist.assert_raise_library_error( lambda: move_autoclean( self.env_assist.get_env(), @@ -1346,7 +1489,7 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): ], expected_in_processor=False, ) - self.env_assist.assert_reports(self.get_reports(stage=3)) + self.env_assist.assert_reports(self.get_reports(stage=setup_stage)) def test_resource_not_runnig_after_move(self): self.tmp_file_mock_obj.set_calls( @@ -1381,8 +1524,113 @@ class MoveAutocleanFailures(MoveAutocleanCommonSetup): ] ) + def test_simulation_resource_not_moved(self): + node = "node2" + different_node = f"different-{node}" + setup_stage = 1 + self.tmp_file_mock_obj.set_calls( + self.get_tmp_files_mocks( + _simulation_transition_fixture( + _simulation_synapses_fixture(self.resource_id) + ), + ) + + [ + TmpFileCall( + self.cib_apply_diff_remove_constraint_from_simulated_cib_tmp_file_name, + orig_content=self.cib_simulate_constraint, + ), + ] + ) + self.set_up_testing_env(node=node, stage=setup_stage) + self.config.runner.pcmk.load_state( + resources=_state_resource_fixture( + self.resource_id, "Started", different_node + ), + name="runner.pcmk.load_state.final", + env=dict( + CIB_file=self.cib_apply_diff_remove_constraint_from_simulated_cib_tmp_file_name + ), + ) + self.env_assist.assert_raise_library_error( + lambda: move_autoclean( + self.env_assist.get_env(), + self.resource_id, + node=node, + ), + [ + fixture.error( + reports.codes.RESOURCE_MOVE_NOT_AFFECTING_RESOURCE, + resource_id=self.resource_id, + ) + ], + expected_in_processor=False, + ) + self.env_assist.assert_reports( + self.get_reports(stage=setup_stage) + + [ + fixture.debug( + reports.codes.TMP_FILE_WRITE, + file_path=self.cib_apply_diff_remove_constraint_from_simulated_cib_tmp_file_name, + content=self.cib_simulate_constraint, + ), + ] + ) + + def test_after_push_resource_not_moved(self): + node = "node2" + different_node = f"different-{node}" + setup_stage = 3 + self.tmp_file_mock_obj.set_calls( + self.get_tmp_files_mocks( + _simulation_transition_fixture( + _simulation_synapses_fixture(self.resource_id) + ), + _simulation_transition_fixture(), + ) + + [ + TmpFileCall( + self.cib_apply_diff_remove_constraint_after_push_tmp_file_name, + orig_content=self.cib_with_constraint, + ), + ] + ) + self.set_up_testing_env(node=node, stage=setup_stage) + self.config.runner.pcmk.load_state( + resources=_state_resource_fixture( + self.resource_id, "Started", different_node + ), + name="runner.pcmk.load_state.final", + env=dict( + CIB_file=self.cib_apply_diff_remove_constraint_after_push_tmp_file_name, + ), + ) + self.env_assist.assert_raise_library_error( + lambda: move_autoclean( + self.env_assist.get_env(), + self.resource_id, + node=node, + ), + [ + fixture.error( + reports.codes.RESOURCE_MOVE_NOT_AFFECTING_RESOURCE, + resource_id=self.resource_id, + ) + ], + expected_in_processor=False, + ) + self.env_assist.assert_reports( + self.get_reports(stage=setup_stage) + + [ + fixture.debug( + reports.codes.TMP_FILE_WRITE, + file_path=self.cib_apply_diff_remove_constraint_after_push_tmp_file_name, + content=self.cib_with_constraint, + ), + ] + ) + def test_resource_running_on_a_different_node(self): - node = "node1" + node = "node2" different_node = f"different-{node}" self.tmp_file_mock_obj.set_calls( self.get_tmp_files_mocks( diff --git a/pcs_test/tier0/lib/commands/resource/test_resource_move_ban.py b/pcs_test/tier0/lib/commands/resource/test_resource_move_ban.py index 5d57fa06..28dd1cd1 100644 --- a/pcs_test/tier0/lib/commands/resource/test_resource_move_ban.py +++ b/pcs_test/tier0/lib/commands/resource/test_resource_move_ban.py @@ -10,6 +10,29 @@ from pcs.common.reports import ReportItemSeverity as severities from pcs.common.reports import codes as report_codes from pcs.lib.commands import resource + +def _node_fixture(name, node_id): + return f'' + + +def _node_list_fixture(nodes): + return "\n".join( + _node_fixture(node_name, node_id) + for node_id, node_name in enumerate(nodes) + ) + + +def _nodes_section_fixture(content): + return f""" + + {content} + + """ + + +nodes_section = _nodes_section_fixture( + _node_list_fixture(["node", "node1", "node2"]) +) resources_primitive = """ @@ -128,8 +151,24 @@ class MoveBanBaseMixin(MoveBanClearBaseMixin): expected_in_processor=False, ) + def test_node_not_found(self): + self.config.runner.cib.load(resources=resources_primitive) + node = "node" + self.env_assist.assert_raise_library_error( + lambda: self.lib_action(self.env_assist.get_env(), "A", node) + ) + self.env_assist.assert_reports( + [ + fixture.error( + report_codes.NODE_NOT_FOUND, node=node, searched_types=[] + ) + ] + ) + def test_all_options(self): - self.config.runner.cib.load(resources=resources_promotable) + self.config.runner.cib.load( + resources=resources_promotable, nodes=nodes_section + ) self.config_pcmk_action( resource="A-clone", master=True, @@ -274,7 +313,9 @@ class MoveBanWaitMixin: def setUp(self): self.timeout = 10 self.env_assist, self.config = get_env_tools(self) - self.config.runner.cib.load(resources=resources_primitive) + self.config.runner.cib.load( + resources=resources_primitive, nodes=nodes_section + ) @mock.patch.object( settings, diff --git a/pcs_test/tools/command_env/config_runner_pcmk.py b/pcs_test/tools/command_env/config_runner_pcmk.py index e276e03b..213941b8 100644 --- a/pcs_test/tools/command_env/config_runner_pcmk.py +++ b/pcs_test/tools/command_env/config_runner_pcmk.py @@ -706,6 +706,7 @@ class PcmkShortcuts: stdout="", stderr="", returncode=0, + env=None, ): """ Create a call for crm_resource --clear @@ -722,6 +723,7 @@ class PcmkShortcuts: string stdout -- crm_resource's stdout string stderr -- crm_resource's stderr int returncode -- crm_resource's returncode + dict env -- CommandRunner environment variables """ # arguments are used via locals() # pylint: disable=unused-argument diff --git a/pcs_test/tools/command_env/mock_runner.py b/pcs_test/tools/command_env/mock_runner.py index f7871fc2..8520ce02 100644 --- a/pcs_test/tools/command_env/mock_runner.py +++ b/pcs_test/tools/command_env/mock_runner.py @@ -143,6 +143,6 @@ class Runner: env.update(env_extend) if env != call.env: raise self.__call_queue.error_with_context( - f"ENV doesn't match. Expected: {call.env}; Real: {env}" + f"Command #{i}: ENV doesn't match. Expected: {call.env}; Real: {env}" ) return call.stdout, call.stderr, call.returncode diff --git a/pcs_test/tools/fixture_cib.py b/pcs_test/tools/fixture_cib.py index 602491c8..bf02bacc 100644 --- a/pcs_test/tools/fixture_cib.py +++ b/pcs_test/tools/fixture_cib.py @@ -310,6 +310,7 @@ MODIFIER_GENERATORS = { "replace": replace_all, "append": append_all, "resources": lambda xml: replace_all({"./configuration/resources": xml}), + "nodes": lambda xml: replace_all({"./configuration/nodes": xml}), "constraints": lambda xml: replace_all( {"./configuration/constraints": xml} ), -- 2.31.1