From fe1ad27f32e69e3e7c046b51e5406a0693ea1c35 Mon Sep 17 00:00:00 2001
From: Ondrej Mular <omular@redhat.com>
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'<node id="{node_id}" uname="{name}"/>'
+
+
+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"""
+ <nodes>
+ {content}
+ </nodes>
+ """
+
+
def _rsc_primitive_fixture(res_id):
return f'<primitive id="{res_id}"/>'
@@ -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 = '<updated_cib with_constraint="True"/>'
+ self.cib_without_constraint = (
+ '<cib with_constraint="False" updated="True"/>'
+ )
self.cib_simulate_constraint = (
'<cib simulate="True" with_constraint="True"/>'
)
@@ -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 with_constraint="True"/>'
+ cib_without_constraint = '<cib with_constraint="False" updated="True"/>'
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):
<rsc_location id="prefer-{resource_id}" rsc="{resource_id}" role="Started" node="{node}" score="INFINITY"/>
</constraints>
""",
+ 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 = '<cib with_constraint="True"/>'
+ self.cib_without_constraint = (
+ '<cib with_constraint="False" updated="True"/>'
+ )
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'<node id="{node_id}" uname="{name}"/>'
+
+
+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"""
+ <nodes>
+ {content}
+ </nodes>
+ """
+
+
+nodes_section = _nodes_section_fixture(
+ _node_list_fixture(["node", "node1", "node2"])
+)
resources_primitive = """
<resources>
<primitive id="A" />
@@ -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