From 0aa8a28822d4667658576b37be541432991f2b9c Mon Sep 17 00:00:00 2001 From: Miroslav Lisik Date: Fri, 26 May 2023 13:37:07 +0200 Subject: [PATCH] add warning to pcs utilization commands for the case utilization configuration is not in effect --- CHANGELOG.md | 4 ++ pcs/node.py | 8 ++- pcs/pcs.8.in | 4 +- pcs/resource.py | 8 ++- pcs/usage.py | 17 ++++- pcs/utils.py | 22 +++++++ pcs_test/Makefile.am | 1 + pcs_test/tier1/legacy/common.py | 5 ++ pcs_test/tier1/legacy/test_node.py | 88 ++++++++++++++++++++------ pcs_test/tier1/legacy/test_resource.py | 79 +++++++++++++++++++---- 10 files changed, 198 insertions(+), 38 deletions(-) create mode 100644 pcs_test/tier1/legacy/common.py diff --git a/CHANGELOG.md b/CHANGELOG.md index fe22ece0d..98fbdc88c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,11 @@ ([rhbz#2166289]) - Commands `pcs resource describe` and `pcs stonith describe` print detailed info about resource options (data type or allowed values, default value) +- Add warning to `pcs resource utilization` and `pcs node utilization` for the + case configuration is not in effect (cluster property `placement-strategy` is + not set appropriately) ([rhbz#2112259]) +[rhbz#2112259]: https://bugzilla.redhat.com/show_bug.cgi?id=2112259 [rhbz#2166289]: https://bugzilla.redhat.com/show_bug.cgi?id=2166289 [rhbz#2189958]: https://bugzilla.redhat.com/show_bug.cgi?id=2189958 diff --git a/pcs/node.py b/pcs/node.py index 9e7394c5b..43c59642a 100644 --- a/pcs/node.py +++ b/pcs/node.py @@ -2,6 +2,7 @@ import pcs.lib.pacemaker.live as lib_pacemaker from pcs import utils +from pcs.cli.cluster_property.output import PropertyConfigurationFacade from pcs.cli.common.errors import ( ERR_NODE_LIST_AND_ALL_MUTUALLY_EXCLUSIVE, CmdLineInputError, @@ -35,10 +36,15 @@ def node_utilization_cmd(lib, argv, modifiers): * -f - CIB file (in lib wrapper) * --name - specify attribute name to filter out """ - del lib modifiers.ensure_only_supported("-f", "--name") if modifiers.get("--name") and len(argv) > 1: raise CmdLineInputError() + utils.print_warning_if_utilization_attrs_has_no_effect( + PropertyConfigurationFacade.from_properties_dtos( + lib.cluster_property.get_properties(), + lib.cluster_property.get_properties_metadata(), + ) + ) if not argv: print_node_utilization(filter_name=modifiers.get("--name")) elif len(argv) == 1: diff --git a/pcs/pcs.8.in b/pcs/pcs.8.in index a89f453ca..4d8f1baec 100644 --- a/pcs/pcs.8.in +++ b/pcs/pcs.8.in @@ -409,7 +409,7 @@ relocate clear Remove all constraints created by the 'relocate run' command. .TP utilization [ [= ...]] -Add specified utilization options to specified resource. If resource is not specified, shows utilization of all resources. If utilization options are not specified, shows utilization of specified resource. Utilization option should be in format name=value, value has to be integer. Options may be removed by setting an option without a value. Example: pcs resource utilization TestResource cpu= ram=20 +Add specified utilization options to specified resource. If resource is not specified, shows utilization of all resources. If utilization options are not specified, shows utilization of specified resource. Utilization option should be in format name=value, value has to be integer. Options may be removed by setting an option without a value. Example: pcs resource utilization TestResource cpu= ram=20 For the utilization configuration to be in effect, cluster property 'placement-strategy' must be configured accordingly. .TP relations [\fB\-\-full\fR] Display relations of a resource specified by its id with other resources in a tree structure. Supported types of resource relations are: ordering constraints, ordering set constraints, relations defined by resource hierarchy (clones, groups, bundles). If \fB\-\-full\fR is used, more verbose output will be printed. @@ -1224,7 +1224,7 @@ unstandby [\fB\-\-all\fR | ...] [\fB\-\-wait\fR[=n]] Remove node(s) from standby mode (the node specified will now be able to host resources), if no nodes or options are specified the current node will be removed from standby mode, if \fB\-\-all\fR is specified all nodes will be removed from standby mode. If \fB\-\-wait\fR is specified, pcs will wait up to 'n' seconds for the node(s) to be removed from standby mode and then return 0 on success or 1 if the operation not succeeded yet. If 'n' is not specified it defaults to 60 minutes. .TP utilization [[] [\fB\-\-name\fR ] | = ...] -Add specified utilization options to specified node. If node is not specified, shows utilization of all nodes. If \fB\-\-name\fR is specified, shows specified utilization value from all nodes. If utilization options are not specified, shows utilization of specified node. Utilization option should be in format name=value, value has to be integer. Options may be removed by setting an option without a value. Example: pcs node utilization node1 cpu=4 ram= +Add specified utilization options to specified node. If node is not specified, shows utilization of all nodes. If \fB\-\-name\fR is specified, shows specified utilization value from all nodes. If utilization options are not specified, shows utilization of specified node. Utilization option should be in format name=value, value has to be integer. Options may be removed by setting an option without a value. Example: pcs node utilization node1 cpu=4 ram= For the utilization configuration to be in effect, cluster property 'placement-strategy' must be configured accordingly. .SS "alert" .TP [config|show] diff --git a/pcs/resource.py b/pcs/resource.py index 7500b37e3..be7fc2efa 100644 --- a/pcs/resource.py +++ b/pcs/resource.py @@ -21,6 +21,7 @@ constraint, utils, ) +from pcs.cli.cluster_property.output import PropertyConfigurationFacade from pcs.cli.common.errors import ( CmdLineInputError, raise_command_replaced, @@ -137,8 +138,13 @@ def resource_utilization_cmd(lib, argv, modifiers): Options: * -f - CIB file """ - del lib modifiers.ensure_only_supported("-f") + utils.print_warning_if_utilization_attrs_has_no_effect( + PropertyConfigurationFacade.from_properties_dtos( + lib.cluster_property.get_properties(), + lib.cluster_property.get_properties_metadata(), + ) + ) if not argv: print_resources_utilization() elif len(argv) == 1: diff --git a/pcs/usage.py b/pcs/usage.py index 78c4adb0f..dee8639fa 100644 --- a/pcs/usage.py +++ b/pcs/usage.py @@ -340,6 +340,12 @@ def _resource_config_desc(obj: str) -> Iterable[str]: "", _output_format_desc(), ) +_UTILIZATION_PLACEMENT_STRATEGY_DESC = ( + """ + For the utilization configuration to be in effect, cluster property + 'placement-strategy' must be configured accordingly. + """, +) def resource(args=(), pout=True): @@ -931,6 +937,7 @@ def resource(args=(), pout=True): Utilization option should be in format name=value, value has to be integer. Options may be removed by setting an option without a value. Example: pcs resource utilization TestResource cpu= ram=20 +{utilization_placement_strategy_desc} relations [--full] Display relations of a resource specified by its id with other resources @@ -974,6 +981,9 @@ def resource(args=(), pout=True): """.format( config_syntax=_format_syntax(_resource_config_syntax("resource")), config_desc=_format_desc(_resource_config_desc("resource")), + utilization_placement_strategy_desc=_format_desc( + _UTILIZATION_PLACEMENT_STRATEGY_DESC + ), ) if pout: print(sub_usage(args, output)) @@ -2241,7 +2251,12 @@ def node(args=(), pout=True): option should be in format name=value, value has to be integer. Options may be removed by setting an option without a value. Example: pcs node utilization node1 cpu=4 ram= -""" +{utilization_placement_strategy_desc} +""".format( + utilization_placement_strategy_desc=_format_desc( + _UTILIZATION_PLACEMENT_STRATEGY_DESC + ) + ) if pout: print(sub_usage(args, output)) return None diff --git a/pcs/utils.py b/pcs/utils.py index 3cdb7ce30..6ccdfaa0e 100644 --- a/pcs/utils.py +++ b/pcs/utils.py @@ -32,6 +32,7 @@ settings, usage, ) +from pcs.cli.cluster_property.output import PropertyConfigurationFacade from pcs.cli.common import middleware from pcs.cli.common.env_cli import Env from pcs.cli.common.errors import CmdLineInputError @@ -56,6 +57,7 @@ from pcs.common.reports.messages import CibUpgradeFailedToMinimalRequiredVersion from pcs.common.services.errors import ManageServiceError from pcs.common.services.interfaces import ServiceManagerInterface +from pcs.common.str_tools import format_list from pcs.common.tools import ( Version, timeout_to_seconds, @@ -2672,3 +2674,23 @@ def get_token_from_file(file_name: str) -> str: except OSError as e: err(f"Unable to read file '{file_name}': {e}", exit_after_error=False) raise SystemExit(1) from e + + +def print_warning_if_utilization_attrs_has_no_effect( + properties_facade: PropertyConfigurationFacade, +): + PLACEMENT_STRATEGIES_USING_UTILIZATION_ATTRS = [ + "balanced", + "minimal", + "utilization", + ] + value = properties_facade.get_property_value_or_default( + "placement-strategy" + ) + if value not in PLACEMENT_STRATEGIES_USING_UTILIZATION_ATTRS: + reports_output.warn( + "Utilization attributes configuration has no effect until cluster " + "property option 'placement-strategy' is set to one of the " + "values: " + f"{format_list(PLACEMENT_STRATEGIES_USING_UTILIZATION_ATTRS)}" + ) diff --git a/pcs_test/Makefile.am b/pcs_test/Makefile.am index 3de984bbe..27b7445c9 100644 --- a/pcs_test/Makefile.am +++ b/pcs_test/Makefile.am @@ -330,6 +330,7 @@ EXTRA_DIST = \ tier1/cluster/test_setup_local.py \ tier1/__init__.py \ tier1/legacy/__init__.py \ + tier1/legacy/common.py \ tier1/legacy/test_acl.py \ tier1/legacy/test_alert.py \ tier1/legacy/test_cluster.py \ diff --git a/pcs_test/tier1/legacy/common.py b/pcs_test/tier1/legacy/common.py new file mode 100644 index 000000000..c6b727f91 --- /dev/null +++ b/pcs_test/tier1/legacy/common.py @@ -0,0 +1,5 @@ +FIXTURE_UTILIZATION_WARNING = ( + "Warning: Utilization attributes configuration has no effect until cluster " + "property option 'placement-strategy' is set to one of the values: " + "'balanced', 'minimal', 'utilization'\n" +) diff --git a/pcs_test/tier1/legacy/test_node.py b/pcs_test/tier1/legacy/test_node.py index a0974adc8..09f68e840 100644 --- a/pcs_test/tier1/legacy/test_node.py +++ b/pcs_test/tier1/legacy/test_node.py @@ -10,6 +10,7 @@ utils, ) +from pcs_test.tier1.legacy.common import FIXTURE_UTILIZATION_WARNING from pcs_test.tools.assertions import ( AssertPcsMixin, ac, @@ -90,13 +91,14 @@ def test_node_utilization_set(self): output, returnVal = pcs( self.temp_cib.name, "node utilization rh7-1 test1=10".split() ) - ac("", output) + ac(FIXTURE_UTILIZATION_WARNING, output) self.assertEqual(0, returnVal) output, returnVal = pcs( self.temp_cib.name, "node utilization rh7-2".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Node Utilization: """ ac(expected_out, output) @@ -105,7 +107,8 @@ def test_node_utilization_set(self): output, returnVal = pcs( self.temp_cib.name, "node utilization rh7-1".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Node Utilization: rh7-1: test1=10 """ @@ -116,12 +119,13 @@ def test_node_utilization_set(self): self.temp_cib.name, "node utilization rh7-1 test1=-10 test4=1234".split(), ) - ac("", output) + ac(FIXTURE_UTILIZATION_WARNING, output) self.assertEqual(0, returnVal) output, returnVal = pcs( self.temp_cib.name, "node utilization rh7-1".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Node Utilization: rh7-1: test1=-10 test4=1234 """ @@ -132,12 +136,13 @@ def test_node_utilization_set(self): self.temp_cib.name, "node utilization rh7-2 test2=321 empty=".split(), ) - ac("", output) + ac(FIXTURE_UTILIZATION_WARNING, output) self.assertEqual(0, returnVal) output, returnVal = pcs( self.temp_cib.name, "node utilization rh7-2".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Node Utilization: rh7-2: test2=321 """ @@ -145,7 +150,8 @@ def test_node_utilization_set(self): self.assertEqual(0, returnVal) output, returnVal = pcs(self.temp_cib.name, "node utilization".split()) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Node Utilization: rh7-1: test1=-10 test4=1234 rh7-2: test2=321 @@ -156,13 +162,14 @@ def test_node_utilization_set(self): output, returnVal = pcs( self.temp_cib.name, "node utilization rh7-2 test1=-20".split() ) - ac("", output) + ac(FIXTURE_UTILIZATION_WARNING, output) self.assertEqual(0, returnVal) output, returnVal = pcs( self.temp_cib.name, "node utilization --name test1".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Node Utilization: rh7-1: test1=-10 rh7-2: test1=-20 @@ -174,7 +181,8 @@ def test_node_utilization_set(self): self.temp_cib.name, "node utilization --name test1 rh7-2".split(), ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Node Utilization: rh7-2: test1=-20 """ @@ -184,42 +192,83 @@ def test_node_utilization_set(self): def test_refuse_non_option_attribute_parameter_among_options(self): self.assert_pcs_fail( "node utilization rh7-1 net".split(), - "Error: missing value of 'net' option\n", + ( + f"{FIXTURE_UTILIZATION_WARNING}" + "Error: missing value of 'net' option\n" + ), ) def test_refuse_option_without_key(self): self.assert_pcs_fail( "node utilization rh7-1 =1".split(), - "Error: missing key in '=1' option\n", + f"{FIXTURE_UTILIZATION_WARNING}Error: missing key in '=1' option\n", ) def test_refuse_unknown_node(self): self.assert_pcs_fail( "node utilization rh7-0 test=10".split(), - "Error: Unable to find a node: rh7-0\n", + ( + f"{FIXTURE_UTILIZATION_WARNING}" + "Error: Unable to find a node: rh7-0\n" + ), ) def test_refuse_value_not_int(self): self.assert_pcs_fail( "node utilization rh7-1 test1=10 test=int".split(), - "Error: Value of utilization attribute must be integer: " - "'test=int'\n", + ( + f"{FIXTURE_UTILIZATION_WARNING}" + "Error: Value of utilization attribute must be integer: " + "'test=int'\n" + ), ) def test_keep_empty_nvset(self): self.assert_effect( "node utilization rh7-1 test=100".split(), self.fixture_xml_with_utilization(), + FIXTURE_UTILIZATION_WARNING, ) self.assert_effect( "node utilization rh7-1 test=".split(), self.fixture_xml_empty_utilization(), + FIXTURE_UTILIZATION_WARNING, ) def test_dont_create_nvset_on_removal(self): self.assert_effect( "node utilization rh7-1 test=".split(), self.fixture_xml_no_utilization(), + FIXTURE_UTILIZATION_WARNING, + ) + + def test_no_warning_printed_placement_strategy_is_set(self): + self.assert_effect( + "property set placement-strategy=minimal".split(), + self.fixture_xml_no_utilization(), + ) + self.assert_resources_xml_in_cib( + """ + + + + + + """, + get_cib_part_func=lambda cib: etree.tostring( + etree.parse(cib).findall(".//crm_config")[0], + ), + ) + self.assert_effect( + "node utilization rh7-1 test=".split(), + self.fixture_xml_no_utilization(), + ) + self.assert_effect( + "node utilization".split(), + self.fixture_xml_no_utilization(), + "Node Utilization:\n", ) @@ -249,9 +298,10 @@ def test_refuse_when_node_not_in_cib_and_is_not_remote(self, mock_utils): def test_refuse_when_node_not_in_mocked_cib(self): self.assert_pcs_fail( "node utilization some_nonexistent_node".split(), - [ - "Error: Unable to find a node: some_nonexistent_node", - ], + ( + f"{FIXTURE_UTILIZATION_WARNING}" + "Error: Unable to find a node: some_nonexistent_node\n" + ), ) diff --git a/pcs_test/tier1/legacy/test_resource.py b/pcs_test/tier1/legacy/test_resource.py index 61177da83..f1141a222 100644 --- a/pcs_test/tier1/legacy/test_resource.py +++ b/pcs_test/tier1/legacy/test_resource.py @@ -17,6 +17,7 @@ from pcs.constraint import LOCATION_NODE_VALIDATION_SKIP_MSG from pcs_test.tier1.cib_resource.common import ResourceTest +from pcs_test.tier1.legacy.common import FIXTURE_UTILIZATION_WARNING from pcs_test.tools.assertions import ( AssertPcsMixin, ac, @@ -5280,6 +5281,7 @@ def fixture_resource_utilization(self): self.assert_effect( "resource utilization R test=100".split(), self.fixture_xml_resource_with_utilization(), + FIXTURE_UTILIZATION_WARNING, ) def testResourceUtilizationSet(self): @@ -5288,13 +5290,14 @@ def testResourceUtilizationSet(self): self.temp_large_cib.name, "resource utilization dummy test1=10".split(), ) - ac("", output) + ac(FIXTURE_UTILIZATION_WARNING, output) self.assertEqual(0, returnVal) output, returnVal = pcs( self.temp_large_cib.name, "resource utilization dummy1".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Resource Utilization: dummy1: \n""" ac(expected_out, output) @@ -5303,7 +5306,8 @@ def testResourceUtilizationSet(self): output, returnVal = pcs( self.temp_large_cib.name, "resource utilization dummy".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Resource Utilization: dummy: test1=10 """ @@ -5314,12 +5318,13 @@ def testResourceUtilizationSet(self): self.temp_large_cib.name, "resource utilization dummy test1=-10 test4=1234".split(), ) - ac("", output) + ac(FIXTURE_UTILIZATION_WARNING, output) self.assertEqual(0, returnVal) output, returnVal = pcs( self.temp_large_cib.name, "resource utilization dummy".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Resource Utilization: dummy: test1=-10 test4=1234 """ @@ -5330,12 +5335,13 @@ def testResourceUtilizationSet(self): self.temp_large_cib.name, "resource utilization dummy1 test2=321 empty=".split(), ) - ac("", output) + ac(FIXTURE_UTILIZATION_WARNING, output) self.assertEqual(0, returnVal) output, returnVal = pcs( self.temp_large_cib.name, "resource utilization dummy1".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Resource Utilization: dummy1: test2=321 """ @@ -5345,7 +5351,8 @@ def testResourceUtilizationSet(self): output, returnVal = pcs( self.temp_large_cib.name, "resource utilization".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Resource Utilization: dummy: test1=-10 test4=1234 dummy1: test2=321 @@ -5353,11 +5360,46 @@ def testResourceUtilizationSet(self): ac(expected_out, output) self.assertEqual(0, returnVal) + def test_no_warning_printed_placement_strategy_is_set(self): + self.fixture_resource() + self.assert_effect( + "property set placement-strategy=minimal".split(), + self.fixture_xml_resource_no_utilization(), + ) + self.assert_resources_xml_in_cib( + """ + + + + + + """, + get_cib_part_func=lambda cib: etree.tostring( + etree.parse(cib).findall(".//crm_config")[0], + ), + ) + self.assert_effect( + "resource utilization R test=100".split(), + self.fixture_xml_resource_with_utilization(), + ) + self.assert_pcs_success( + "resource utilization".split(), + dedent( + """\ + Resource Utilization: + R: test=100 + """ + ), + ) + def test_resource_utilization_set_invalid(self): output, returnVal = pcs( self.temp_large_cib.name, "resource utilization dummy test".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Error: missing value of 'test' option """ ac(expected_out, output) @@ -5366,7 +5408,8 @@ def test_resource_utilization_set_invalid(self): output, returnVal = pcs( self.temp_large_cib.name, "resource utilization dummy =10".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Error: missing key in '=10' option """ ac(expected_out, output) @@ -5375,7 +5418,8 @@ def test_resource_utilization_set_invalid(self): output, returnVal = pcs( self.temp_large_cib.name, "resource utilization dummy0".split() ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Error: Unable to find a resource: dummy0 """ ac(expected_out, output) @@ -5385,7 +5429,8 @@ def test_resource_utilization_set_invalid(self): self.temp_large_cib.name, "resource utilization dummy0 test=10".split(), ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Error: Unable to find a resource: dummy0 """ ac(expected_out, output) @@ -5395,7 +5440,8 @@ def test_resource_utilization_set_invalid(self): self.temp_large_cib.name, "resource utilization dummy1 test1=10 test=int".split(), ) - expected_out = """\ + expected_out = f"""\ +{FIXTURE_UTILIZATION_WARNING}\ Error: Value of utilization attribute must be integer: 'test=int' """ ac(expected_out, output) @@ -5406,6 +5452,7 @@ def test_keep_empty_nvset(self): self.assert_effect( "resource utilization R test=".split(), self.fixture_xml_resource_empty_utilization(), + FIXTURE_UTILIZATION_WARNING, ) def test_dont_create_nvset_on_removal(self): @@ -5413,6 +5460,7 @@ def test_dont_create_nvset_on_removal(self): self.assert_effect( "resource utilization R test=".split(), self.fixture_xml_resource_no_utilization(), + FIXTURE_UTILIZATION_WARNING, ) @@ -6438,7 +6486,10 @@ def test_utilization(self): self.fixture_bundle("B") self.assert_pcs_fail( "resource utilization B aaa=10".split(), - "Error: Unable to find a resource: B\n", + ( + f"{FIXTURE_UTILIZATION_WARNING}" + "Error: Unable to find a resource: B\n" + ), ) @skip(