From e56f42bf31ae0a52618fe8754fd0b2ae623e6a7a Mon Sep 17 00:00:00 2001
From: Tomas Jelinek <tojeline@redhat.com>
Date: Thu, 12 Dec 2019 14:46:44 +0100
Subject: [PATCH 1/7] squash bz1781303 fix safe-disabling clones, groups,
bundles
fix simulate_cib_error report
Putting only one CIB in the report is not enough info. Both original and
changed CIB as well as crm_simulate output would be needed. All that
info can be seen in debug messages. So there is no need to put it in the
report.
---
pcs/cli/common/console_report.py | 7 +-
pcs/lib/cib/resource/common.py | 21 +-
pcs/lib/commands/resource.py | 27 +-
pcs/lib/pacemaker/live.py | 8 +-
pcs/lib/reports.py | 4 +-
.../tier0/cli/common/test_console_report.py | 10 +-
.../tier0/lib/cib/test_resource_common.py | 60 ++++-
.../resource/test_resource_enable_disable.py | 242 +++++++++++++++++-
pcs_test/tier0/lib/pacemaker/test_live.py | 7 -
9 files changed, 350 insertions(+), 36 deletions(-)
diff --git a/pcs/cli/common/console_report.py b/pcs/cli/common/console_report.py
index d349c823..60dbb2a0 100644
--- a/pcs/cli/common/console_report.py
+++ b/pcs/cli/common/console_report.py
@@ -1269,8 +1269,11 @@ CODE_TO_MESSAGE_BUILDER_MAP = {
,
codes.CIB_SIMULATE_ERROR: lambda info:
- "Unable to simulate changes in CIB: {reason}\n{cib}"
- .format(**info)
+ "Unable to simulate changes in CIB{_reason}"
+ .format(
+ _reason=format_optional(info["reason"], ": {0}"),
+ **info
+ )
,
codes.CIB_PUSH_FORCED_FULL_DUE_TO_CRM_FEATURE_SET: lambda info:
diff --git a/pcs/lib/cib/resource/common.py b/pcs/lib/cib/resource/common.py
index f1891003..e30c5e69 100644
--- a/pcs/lib/cib/resource/common.py
+++ b/pcs/lib/cib/resource/common.py
@@ -1,8 +1,9 @@
from collections import namedtuple
from typing import (
cast,
+ List,
Optional,
- Sequence,
+ Set,
)
from xml.etree.ElementTree import Element
@@ -114,7 +115,23 @@ def find_primitives(resource_el):
return [resource_el]
return []
-def get_inner_resources(resource_el: Element) -> Sequence[Element]:
+def get_all_inner_resources(resource_el: Element) -> Set[Element]:
+ """
+ Return all inner resources (both direct and indirect) of a resource
+ Example: for a clone containing a group, this function will return both
+ the group and the resources inside the group
+
+ resource_el -- resource element to get its inner resources
+ """
+ all_inner: Set[Element] = set()
+ to_process = set([resource_el])
+ while to_process:
+ new_inner = get_inner_resources(to_process.pop())
+ to_process.update(set(new_inner) - all_inner)
+ all_inner.update(new_inner)
+ return all_inner
+
+def get_inner_resources(resource_el: Element) -> List[Element]:
"""
Return list of inner resources (direct descendants) of a resource
specified as resource_el.
diff --git a/pcs/lib/commands/resource.py b/pcs/lib/commands/resource.py
index 1b652ea4..4f975c7f 100644
--- a/pcs/lib/commands/resource.py
+++ b/pcs/lib/commands/resource.py
@@ -802,7 +802,28 @@ def disable_safe(env, resource_ids, strict, wait):
with resource_environment(
env, wait, resource_ids, _ensure_disabled_after_wait(True)
) as resources_section:
- _disable_validate_and_edit_cib(env, resources_section, resource_ids)
+ id_provider = IdProvider(resources_section)
+ resource_el_list = _find_resources_or_raise(
+ resources_section,
+ resource_ids
+ )
+ env.report_processor.process_list(
+ _resource_list_enable_disable(
+ resource_el_list,
+ resource.common.disable,
+ id_provider,
+ env.get_cluster_state()
+ )
+ )
+
+ inner_resources_names_set = set()
+ for resource_el in resource_el_list:
+ inner_resources_names_set.update({
+ inner_resource_el.get("id")
+ for inner_resource_el
+ in resource.common.get_all_inner_resources(resource_el)
+ })
+
plaintext_status, transitions, dummy_cib = simulate_cib(
env.cmd_runner(),
get_root(resources_section)
@@ -830,6 +851,10 @@ def disable_safe(env, resource_ids, strict, wait):
exclude=resource_ids
)
)
+
+ # Stopping a clone stops all its inner resources. That should not block
+ # stopping the clone.
+ other_affected = other_affected - inner_resources_names_set
if other_affected:
raise LibraryError(
reports.resource_disable_affects_other_resources(
diff --git a/pcs/lib/pacemaker/live.py b/pcs/lib/pacemaker/live.py
index 83274af0..233f2e2d 100644
--- a/pcs/lib/pacemaker/live.py
+++ b/pcs/lib/pacemaker/live.py
@@ -271,7 +271,7 @@ def simulate_cib_xml(runner, cib_xml):
transitions_file = write_tmpfile(None)
except OSError as e:
raise LibraryError(
- reports.cib_simulate_error(format_os_error(e), cib_xml)
+ reports.cib_simulate_error(format_os_error(e))
)
cmd = [
@@ -284,7 +284,7 @@ def simulate_cib_xml(runner, cib_xml):
stdout, stderr, retval = runner.run(cmd, stdin_string=cib_xml)
if retval != 0:
raise LibraryError(
- reports.cib_simulate_error(stderr.strip(), cib_xml)
+ reports.cib_simulate_error(stderr.strip())
)
try:
@@ -297,7 +297,7 @@ def simulate_cib_xml(runner, cib_xml):
return stdout, transitions_xml, new_cib_xml
except OSError as e:
raise LibraryError(
- reports.cib_simulate_error(format_os_error(e), cib_xml)
+ reports.cib_simulate_error(format_os_error(e))
)
def simulate_cib(runner, cib):
@@ -319,7 +319,7 @@ def simulate_cib(runner, cib):
)
except (etree.XMLSyntaxError, etree.DocumentInvalid) as e:
raise LibraryError(
- reports.cib_simulate_error(str(e), cib_xml)
+ reports.cib_simulate_error(str(e))
)
### wait for idle
diff --git a/pcs/lib/reports.py b/pcs/lib/reports.py
index 1f081007..c9b4a25d 100644
--- a/pcs/lib/reports.py
+++ b/pcs/lib/reports.py
@@ -1935,18 +1935,16 @@ def cib_diff_error(reason, cib_old, cib_new):
}
)
-def cib_simulate_error(reason, cib):
+def cib_simulate_error(reason):
"""
cannot simulate effects a CIB would have on a live cluster
string reason -- error description
- string cib -- the CIB whose effects were to be simulated
"""
return ReportItem.error(
report_codes.CIB_SIMULATE_ERROR,
info={
"reason": reason,
- "cib": cib,
}
)
diff --git a/pcs_test/tier0/cli/common/test_console_report.py b/pcs_test/tier0/cli/common/test_console_report.py
index 0d0c2457..29e9614d 100644
--- a/pcs_test/tier0/cli/common/test_console_report.py
+++ b/pcs_test/tier0/cli/common/test_console_report.py
@@ -2238,8 +2238,14 @@ class CibDiffError(NameBuildTest):
class CibSimulateError(NameBuildTest):
def test_success(self):
self.assert_message_from_report(
- "Unable to simulate changes in CIB: error message\n<cib />",
- reports.cib_simulate_error("error message", "<cib />")
+ "Unable to simulate changes in CIB: error message",
+ reports.cib_simulate_error("error message")
+ )
+
+ def test_empty_reason(self):
+ self.assert_message_from_report(
+ "Unable to simulate changes in CIB",
+ reports.cib_simulate_error("")
)
diff --git a/pcs_test/tier0/lib/cib/test_resource_common.py b/pcs_test/tier0/lib/cib/test_resource_common.py
index ebba09da..cd716ba2 100644
--- a/pcs_test/tier0/lib/cib/test_resource_common.py
+++ b/pcs_test/tier0/lib/cib/test_resource_common.py
@@ -200,10 +200,12 @@ class FindOneOrMoreResources(TestCase):
class FindResourcesMixin:
+ _iterable_type = list
+
def assert_find_resources(self, input_resource_id, output_resource_ids):
self.assertEqual(
- output_resource_ids,
- [
+ self._iterable_type(output_resource_ids),
+ self._iterable_type([
element.get("id", "")
for element in
self._tested_fn(
@@ -211,7 +213,7 @@ class FindResourcesMixin:
'.//*[@id="{0}"]'.format(input_resource_id)
)
)
- ]
+ ])
)
def test_group(self):
@@ -235,6 +237,27 @@ class FindResourcesMixin:
def test_bundle_with_primitive(self):
self.assert_find_resources("H-bundle", ["H"])
+ def test_primitive(self):
+ raise NotImplementedError()
+
+ def test_primitive_in_clone(self):
+ raise NotImplementedError()
+
+ def test_primitive_in_master(self):
+ raise NotImplementedError()
+
+ def test_primitive_in_group(self):
+ raise NotImplementedError()
+
+ def test_primitive_in_bundle(self):
+ raise NotImplementedError()
+
+ def test_cloned_group(self):
+ raise NotImplementedError()
+
+ def test_mastered_group(self):
+ raise NotImplementedError()
+
class FindPrimitives(TestCase, FindResourcesMixin):
_tested_fn = staticmethod(common.find_primitives)
@@ -266,6 +289,37 @@ class FindPrimitives(TestCase, FindResourcesMixin):
self.assert_find_resources("F-master", ["F1", "F2"])
+class GetAllInnerResources(TestCase, FindResourcesMixin):
+ _iterable_type = set
+ _tested_fn = staticmethod(common.get_all_inner_resources)
+
+ def test_primitive(self):
+ self.assert_find_resources("A", set())
+
+ def test_primitive_in_clone(self):
+ self.assert_find_resources("B", set())
+
+ def test_primitive_in_master(self):
+ self.assert_find_resources("C", set())
+
+ def test_primitive_in_group(self):
+ self.assert_find_resources("D1", set())
+ self.assert_find_resources("D2", set())
+ self.assert_find_resources("E1", set())
+ self.assert_find_resources("E2", set())
+ self.assert_find_resources("F1", set())
+ self.assert_find_resources("F2", set())
+
+ def test_primitive_in_bundle(self):
+ self.assert_find_resources("H", set())
+
+ def test_cloned_group(self):
+ self.assert_find_resources("E-clone", {"E", "E1", "E2"})
+
+ def test_mastered_group(self):
+ self.assert_find_resources("F-master", {"F", "F1", "F2"})
+
+
class GetInnerResources(TestCase, FindResourcesMixin):
_tested_fn = staticmethod(common.get_inner_resources)
diff --git a/pcs_test/tier0/lib/commands/resource/test_resource_enable_disable.py b/pcs_test/tier0/lib/commands/resource/test_resource_enable_disable.py
index 634f0f33..62899940 100644
--- a/pcs_test/tier0/lib/commands/resource/test_resource_enable_disable.py
+++ b/pcs_test/tier0/lib/commands/resource/test_resource_enable_disable.py
@@ -1729,12 +1729,6 @@ class DisableSimulate(TestCase):
fixture.error(
report_codes.CIB_SIMULATE_ERROR,
reason="some stderr",
- # curently, there is no way to normalize xml with our lxml
- # version 4.2.3, so this never passes equality tests
- # cib=self.config.calls.get(
- # "runner.pcmk.simulate_cib"
- # ).check_stdin.expected_stdin
- # ,
),
],
expected_in_processor=False
@@ -1988,12 +1982,6 @@ class DisableSafeMixin():
fixture.error(
report_codes.CIB_SIMULATE_ERROR,
reason="some stderr",
- # curently, there is no way to normalize xml with our lxml
- # version 4.2.3, so this never passes equality tests
- # cib=self.config.calls.get(
- # "runner.pcmk.simulate_cib"
- # ).check_stdin.expected_stdin
- # ,
),
],
expected_in_processor=False
@@ -2118,6 +2106,236 @@ class DisableSafeMixin():
fixture.report_resource_not_running("B"),
])
+ def test_inner_resources(self, mock_write_tmpfile):
+ cib_xml = """
+ <resources>
+ <primitive id="A" />
+ <clone id="B-clone">
+ <primitive id="B" />
+ </clone>
+ <master id="C-master">
+ <primitive id="C" />
+ </master>
+ <group id="D">
+ <primitive id="D1" />
+ <primitive id="D2" />
+ </group>
+ <clone id="E-clone">
+ <group id="E">
+ <primitive id="E1" />
+ <primitive id="E2" />
+ </group>
+ </clone>
+ <master id="F-master">
+ <group id="F">
+ <primitive id="F1" />
+ <primitive id="F2" />
+ </group>
+ </master>
+ <bundle id="G-bundle" />
+ <bundle id="H-bundle">
+ <primitive id="H" />
+ </bundle>
+ </resources>
+ """
+ status_xml = """
+ <resources>
+ <resource id="A" managed="true" />
+ <clone id="B-clone" managed="true" multi_state="false"
+ unique="false"
+ >
+ <resource id="B" managed="true" />
+ <resource id="B" managed="true" />
+ </clone>
+ <clone id="C-master" managed="true" multi_state="true"
+ unique="false"
+ >
+ <resource id="C" managed="true" />
+ <resource id="C" managed="true" />
+ </clone>
+ <group id="D" number_resources="2">
+ <resource id="D1" managed="true" />
+ <resource id="D2" managed="true" />
+ </group>
+ <clone id="E-clone" managed="true" multi_state="false"
+ unique="false"
+ >
+ <group id="E:0" number_resources="2">
+ <resource id="E1" managed="true" />
+ <resource id="E2" managed="true" />
+ </group>
+ <group id="E:1" number_resources="2">
+ <resource id="E1" managed="true" />
+ <resource id="E2" managed="true" />
+ </group>
+ </clone>
+ <clone id="F-master" managed="true" multi_state="true"
+ unique="false"
+ >
+ <group id="F:0" number_resources="2">
+ <resource id="F1" managed="true" />
+ <resource id="F2" managed="true" />
+ </group>
+ <group id="F:1" number_resources="2">
+ <resource id="F1" managed="true" />
+ <resource id="F2" managed="true" />
+ </group>
+ </clone>
+ <bundle id="H-bundle" type="docker" image="pcmktest:http"
+ unique="false" managed="true" failed="false"
+ >
+ <replica id="0">
+ <resource id="H" />
+ </replica>
+ <replica id="1">
+ <resource id="H" />
+ </replica>
+ </bundle>
+ </resources>
+ """
+ synapses = []
+ index = 0
+ for res_name, is_clone in [
+ ("A", False),
+ ("B", True),
+ ("C", True),
+ ("D1", False),
+ ("D2", False),
+ ("E1", True),
+ ("E2", True),
+ ("F1", True),
+ ("F2", True),
+ ("H", False),
+ ]:
+ if is_clone:
+ synapses.append(f"""
+ <synapse>
+ <action_set>
+ <rsc_op id="{index}" operation="stop" on_node="node1">
+ <primitive id="{res_name}" long_id="{res_name}:0" />
+ </rsc_op>
+ </action_set>
+ </synapse>
+ <synapse>
+ <action_set>
+ <rsc_op id="{index + 1}" operation="stop" on_node="node2">
+ <primitive id="{res_name}" long_id="{res_name}:1" />
+ </rsc_op>
+ </action_set>
+ </synapse>
+ """)
+ index += 2
+ else:
+ synapses.append(f"""
+ <synapse>
+ <action_set>
+ <rsc_op id="{index}" operation="stop" on_node="node1">
+ <primitive id="{res_name}" />
+ </rsc_op>
+ </action_set>
+ </synapse>
+ """)
+ index += 1
+ transitions_xml = (
+ "<transition_graph>" + "\n".join(synapses) + "</transition_graph>"
+ )
+
+ self.tmpfile_transitions.read.return_value = transitions_xml
+ mock_write_tmpfile.side_effect = [
+ self.tmpfile_new_cib, self.tmpfile_transitions,
+ AssertionError("No other write_tmpfile call expected")
+ ]
+ (self.config
+ .runner.cib.load(resources=cib_xml)
+ .runner.pcmk.load_state(resources=status_xml)
+ )
+ self.config.runner.pcmk.simulate_cib(
+ self.tmpfile_new_cib.name,
+ self.tmpfile_transitions.name,
+ stdout="simulate output",
+ resources="""
+ <resources>
+ <primitive id="A" />
+ <clone id="B-clone">
+ <meta_attributes id="B-clone-meta_attributes">
+ <nvpair name="target-role" value="Stopped"
+ id="B-clone-meta_attributes-target-role"
+ />
+ </meta_attributes>
+ <primitive id="B" />
+ </clone>
+ <master id="C-master">
+ <meta_attributes id="C-master-meta_attributes">
+ <nvpair name="target-role" value="Stopped"
+ id="C-master-meta_attributes-target-role"
+ />
+ </meta_attributes>
+ <primitive id="C" />
+ </master>
+ <group id="D">
+ <meta_attributes id="D-meta_attributes">
+ <nvpair name="target-role" value="Stopped"
+ id="D-meta_attributes-target-role"
+ />
+ </meta_attributes>
+ <primitive id="D1" />
+ <primitive id="D2" />
+ </group>
+ <clone id="E-clone">
+ <meta_attributes id="E-clone-meta_attributes">
+ <nvpair name="target-role" value="Stopped"
+ id="E-clone-meta_attributes-target-role"
+ />
+ </meta_attributes>
+ <group id="E">
+ <primitive id="E1" />
+ <primitive id="E2" />
+ </group>
+ </clone>
+ <master id="F-master">
+ <meta_attributes id="F-master-meta_attributes">
+ <nvpair name="target-role" value="Stopped"
+ id="F-master-meta_attributes-target-role"
+ />
+ </meta_attributes>
+ <group id="F">
+ <primitive id="F1" />
+ <primitive id="F2" />
+ </group>
+ </master>
+ <bundle id="G-bundle" />
+ <bundle id="H-bundle">
+ <meta_attributes id="H-bundle-meta_attributes">
+ <nvpair name="target-role" value="Stopped"
+ id="H-bundle-meta_attributes-target-role"
+ />
+ </meta_attributes>
+ <primitive id="H" />
+ </bundle>
+ </resources>
+ """
+ )
+ self.env_assist.assert_raise_library_error(
+ lambda: resource.disable_safe(
+ self.env_assist.get_env(),
+ ["B-clone", "C-master", "D", "E-clone", "F-master", "H-bundle"],
+ self.strict,
+ False,
+ ),
+ [
+ fixture.error(
+ report_codes.RESOURCE_DISABLE_AFFECTS_OTHER_RESOURCES,
+ disabled_resource_list=[
+ "B-clone", "C-master", "D", "E-clone", "F-master",
+ "H-bundle"
+ ],
+ affected_resource_list=["A"],
+ crm_simulate_plaintext_output="simulate output",
+ ),
+ ],
+ expected_in_processor=False
+ )
+
@mock.patch("pcs.lib.pacemaker.live.write_tmpfile")
class DisableSafe(DisableSafeMixin, TestCase):
strict = False
diff --git a/pcs_test/tier0/lib/pacemaker/test_live.py b/pcs_test/tier0/lib/pacemaker/test_live.py
index dfebcb17..1ea5454e 100644
--- a/pcs_test/tier0/lib/pacemaker/test_live.py
+++ b/pcs_test/tier0/lib/pacemaker/test_live.py
@@ -686,7 +686,6 @@ class SimulateCibXml(LibraryPacemakerTest):
fixture.error(
report_codes.CIB_SIMULATE_ERROR,
reason="some error",
- cib="<cib />",
),
)
mock_runner.run.assert_not_called()
@@ -703,7 +702,6 @@ class SimulateCibXml(LibraryPacemakerTest):
fixture.error(
report_codes.CIB_SIMULATE_ERROR,
reason="some error",
- cib="<cib />",
),
)
mock_runner.run.assert_not_called()
@@ -729,7 +727,6 @@ class SimulateCibXml(LibraryPacemakerTest):
fixture.error(
report_codes.CIB_SIMULATE_ERROR,
reason="some error",
- cib="<cib />",
),
)
@@ -755,7 +752,6 @@ class SimulateCibXml(LibraryPacemakerTest):
fixture.error(
report_codes.CIB_SIMULATE_ERROR,
reason="some error",
- cib="<cib />",
),
)
@@ -782,7 +778,6 @@ class SimulateCibXml(LibraryPacemakerTest):
fixture.error(
report_codes.CIB_SIMULATE_ERROR,
reason="some error",
- cib="<cib />",
),
)
@@ -819,7 +814,6 @@ class SimulateCib(TestCase):
"Start tag expected, '<' not found, line 1, column 1 "
"(<string>, line 1)"
),
- cib=self.cib_xml,
),
)
@@ -835,7 +829,6 @@ class SimulateCib(TestCase):
"Start tag expected, '<' not found, line 1, column 1 "
"(<string>, line 1)"
),
- cib=self.cib_xml,
),
)
--
2.21.1