Skip to content

Commit

Permalink
add warning to pcs utilization commands for the case utilization
Browse files Browse the repository at this point in the history
configuration is not in effect
  • Loading branch information
mirecheck committed May 30, 2023
1 parent d5642c2 commit 0aa8a28
Show file tree
Hide file tree
Showing 10 changed files with 198 additions and 38 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
8 changes: 7 additions & 1 deletion pcs/node.py
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions pcs/pcs.8.in
Expand Up @@ -409,7 +409,7 @@ relocate clear
Remove all constraints created by the 'relocate run' command.
.TP
utilization [<resource id> [<name>=<value> ...]]
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 <resource id> [\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.
Expand Down Expand Up @@ -1224,7 +1224,7 @@ unstandby [\fB\-\-all\fR | <node>...] [\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 [[<node>] [\fB\-\-name\fR <name>] | <node> <name>=<value> ...]
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]
Expand Down
8 changes: 7 additions & 1 deletion pcs/resource.py
Expand Up @@ -21,6 +21,7 @@
constraint,
utils,
)
from pcs.cli.cluster_property.output import PropertyConfigurationFacade
from pcs.cli.common.errors import (
CmdLineInputError,
raise_command_replaced,
Expand Down Expand Up @@ -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:
Expand Down
17 changes: 16 additions & 1 deletion pcs/usage.py
Expand Up @@ -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):
Expand Down Expand Up @@ -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 <resource id> [--full]
Display relations of a resource specified by its id with other resources
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions pcs/utils.py
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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)}"
)
1 change: 1 addition & 0 deletions pcs_test/Makefile.am
Expand Up @@ -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 \
Expand Down
5 changes: 5 additions & 0 deletions 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"
)
88 changes: 69 additions & 19 deletions pcs_test/tier1/legacy/test_node.py
Expand Up @@ -10,6 +10,7 @@
utils,
)

from pcs_test.tier1.legacy.common import FIXTURE_UTILIZATION_WARNING
from pcs_test.tools.assertions import (
AssertPcsMixin,
ac,
Expand Down Expand Up @@ -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)
Expand All @@ -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
"""
Expand All @@ -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
"""
Expand All @@ -132,20 +136,22 @@ 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
"""
ac(expected_out, output)
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
Expand All @@ -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
Expand All @@ -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
"""
Expand All @@ -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(
"""
<crm_config>
<cluster_property_set id="cib-bootstrap-options">
<nvpair id="cib-bootstrap-options-placement-strategy"
name="placement-strategy" value="minimal"
/>
</cluster_property_set>
</crm_config>
""",
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",
)


Expand Down Expand Up @@ -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"
),
)


Expand Down

0 comments on commit 0aa8a28

Please sign in to comment.