From 1b6ed4d97198e7ca8c1fd5f76bfb8bfc95eeabdc Mon Sep 17 00:00:00 2001 From: Ivan Devat Date: Wed, 14 Sep 2016 09:37:06 +0200 Subject: [PATCH] squash bz1158500 add support for utilization attri 4ab84628f802 fix parsing of utilization attributes 18d526f59679 support utilization on (non-cib) remote node f0b193a681e3 show error when show utilizat. on nonexistent node 9907c123c225 web UI: fix setting utilization attributes --- .pylintrc | 2 +- pcs/node.py | 54 ++++++++++++++++++++++++++++++++++++----- pcs/resource.py | 8 ++++--- pcs/test/test_node.py | 56 +++++++++++++++++++++++++++++++++++++++++++ pcs/test/test_resource.py | 18 ++++++++++++++ pcs/test/test_utils.py | 17 +++++++++---- pcs/utils.py | 12 +++++++++- pcsd/public/js/nodes-ember.js | 4 ++-- pcsd/remote.rb | 2 +- 9 files changed, 155 insertions(+), 18 deletions(-) diff --git a/.pylintrc b/.pylintrc index 1dd6d5d..6101381 100644 --- a/.pylintrc +++ b/.pylintrc @@ -92,7 +92,7 @@ dummy-variables-rgx=_$|dummy [FORMAT] # Maximum number of lines in a module -max-module-lines=4584 +max-module-lines=4616 # Maximum number of characters on a single line. max-line-length=1291 diff --git a/pcs/node.py b/pcs/node.py index ed77d5d..729ea35 100644 --- a/pcs/node.py +++ b/pcs/node.py @@ -56,7 +56,10 @@ def node_cmd(argv): elif len(argv) == 1: print_node_utilization(argv.pop(0), filter_name=filter_name) else: - set_node_utilization(argv.pop(0), argv) + try: + set_node_utilization(argv.pop(0), argv) + except CmdLineInputError as e: + utils.exit_on_cmdline_input_errror(e, "node", "utilization") # pcs-to-pcsd use only elif sub_cmd == "pacemaker-status": node_pacemaker_status() @@ -150,17 +153,56 @@ def set_node_utilization(node, argv): cib = utils.get_cib_dom() node_el = utils.dom_get_node(cib, node) if node_el is None: - utils.err("Unable to find a node: {0}".format(node)) + if utils.usefile: + utils.err("Unable to find a node: {0}".format(node)) - utils.dom_update_utilization( - node_el, utils.convert_args_to_tuples(argv), "nodes-" - ) + for attrs in utils.getNodeAttributesFromPacemaker(): + if attrs.name == node and attrs.type == "remote": + node_attrs = attrs + break + else: + utils.err("Unable to find a node: {0}".format(node)) + + nodes_section_list = cib.getElementsByTagName("nodes") + if len(nodes_section_list) == 0: + utils.err("Unable to get nodes section of cib") + + dom = nodes_section_list[0].ownerDocument + node_el = dom.createElement("node") + node_el.setAttribute("id", node_attrs.id) + node_el.setAttribute("type", node_attrs.type) + node_el.setAttribute("uname", node_attrs.name) + nodes_section_list[0].appendChild(node_el) + + utils.dom_update_utilization(node_el, prepare_options(argv), "nodes-") utils.replace_cib_configuration(cib) def print_node_utilization(filter_node=None, filter_name=None): cib = utils.get_cib_dom() + + node_element_list = cib.getElementsByTagName("node") + + + if( + filter_node + and + filter_node not in [ + node_element.getAttribute("uname") + for node_element in node_element_list + ] + and ( + utils.usefile + or + filter_node not in [ + node_attrs.name for node_attrs + in utils.getNodeAttributesFromPacemaker() + ] + ) + ): + utils.err("Unable to find a node: {0}".format(filter_node)) + utilization = {} - for node_el in cib.getElementsByTagName("node"): + for node_el in node_element_list: node = node_el.getAttribute("uname") if filter_node is not None and node != filter_node: continue diff --git a/pcs/resource.py b/pcs/resource.py index 74adac6..046a826 100644 --- a/pcs/resource.py +++ b/pcs/resource.py @@ -191,7 +191,10 @@ def resource_cmd(argv): elif len(argv) == 1: print_resource_utilization(argv.pop(0)) else: - set_resource_utilization(argv.pop(0), argv) + try: + set_resource_utilization(argv.pop(0), argv) + except CmdLineInputError as e: + utils.exit_on_cmdline_input_errror(e, "resource", "utilization") elif (sub_cmd == "get_resource_agent_info"): get_resource_agent_info(argv) else: @@ -2795,8 +2798,7 @@ def set_resource_utilization(resource_id, argv): resource_el = utils.dom_get_resource(cib, resource_id) if resource_el is None: utils.err("Unable to find a resource: {0}".format(resource_id)) - - utils.dom_update_utilization(resource_el, utils.convert_args_to_tuples(argv)) + utils.dom_update_utilization(resource_el, prepare_options(argv)) utils.replace_cib_configuration(cib) def print_resource_utilization(resource_id): diff --git a/pcs/test/test_node.py b/pcs/test/test_node.py index 9b45e07..137c7c7 100644 --- a/pcs/test/test_node.py +++ b/pcs/test/test_node.py @@ -7,7 +7,9 @@ from __future__ import ( import shutil from pcs.test.tools import pcs_unittest as unittest +from pcs.test.tools.pcs_unittest import mock +from pcs import node from pcs.test.tools.assertions import AssertPcsMixin from pcs.test.tools.misc import ( ac, @@ -268,6 +270,20 @@ Node Utilization: self.assertEqual(0, returnVal) def test_node_utilization_set_invalid(self): + output, returnVal = pcs(temp_cib, "node utilization rh7-1 test") + expected_out = """\ +Error: missing value of 'test' option +""" + ac(expected_out, output) + self.assertEqual(1, returnVal) + + output, returnVal = pcs(temp_cib, "node utilization rh7-1 =10") + expected_out = """\ +Error: missing key in '=10' option +""" + ac(expected_out, output) + self.assertEqual(1, returnVal) + output, returnVal = pcs(temp_cib, "node utilization rh7-0 test=10") expected_out = """\ Error: Unable to find a node: rh7-0 @@ -524,3 +540,43 @@ Node Attributes: "node attribute rh7-1 missing= --force", "" ) + +class SetNodeUtilizationTest(unittest.TestCase, AssertPcsMixin): + def setUp(self): + shutil.copy(empty_cib, temp_cib) + self.pcs_runner = PcsRunner(temp_cib) + + def test_refuse_non_option_attribute_parameter_among_options(self): + self.assert_pcs_fail("node utilization rh7-1 net", [ + "Error: missing value of 'net' option", + ]) + + def test_refuse_option_without_key(self): + self.assert_pcs_fail("node utilization rh7-1 =1", [ + "Error: missing key in '=1' option", + ]) + +class PrintNodeUtilizationTest(unittest.TestCase, AssertPcsMixin): + def setUp(self): + shutil.copy(empty_cib, temp_cib) + self.pcs_runner = PcsRunner(temp_cib) + + @mock.patch("pcs.node.utils") + def test_refuse_when_node_not_in_cib_and_is_not_remote(self, mock_utils): + mock_cib = mock.MagicMock() + mock_cib.getElementsByTagName = mock.Mock(return_value=[]) + + mock_utils.get_cib_dom = mock.Mock(return_value=mock_cib) + mock_utils.usefile = False + mock_utils.getNodeAttributesFromPacemaker = mock.Mock(return_value=[]) + mock_utils.err = mock.Mock(side_effect=SystemExit) + + self.assertRaises( + SystemExit, + lambda: node.print_node_utilization("some") + ) + + def test_refuse_when_node_not_in_mocked_cib(self): + self.assert_pcs_fail("node utilization some_nonexistent_node", [ + "Error: Unable to find a node: some_nonexistent_node", + ]) diff --git a/pcs/test/test_resource.py b/pcs/test/test_resource.py index 87a7fa8..d32cfb4 100644 --- a/pcs/test/test_resource.py +++ b/pcs/test/test_resource.py @@ -4430,6 +4430,24 @@ Resource Utilization: self.assertEqual(0, returnVal) def test_resource_utilization_set_invalid(self): + output, returnVal = pcs( + temp_large_cib, "resource utilization dummy test" + ) + expected_out = """\ +Error: missing value of 'test' option +""" + ac(expected_out, output) + self.assertEqual(1, returnVal) + + output, returnVal = pcs( + temp_large_cib, "resource utilization dummy =10" + ) + expected_out = """\ +Error: missing key in '=10' option +""" + ac(expected_out, output) + self.assertEqual(1, returnVal) + output, returnVal = pcs(temp_large_cib, "resource utilization dummy0") expected_out = """\ Error: Unable to find a resource: dummy0 diff --git a/pcs/test/test_utils.py b/pcs/test/test_utils.py index 252de30..c4c6d87 100644 --- a/pcs/test/test_utils.py +++ b/pcs/test/test_utils.py @@ -1400,12 +1400,12 @@ class UtilsTest(unittest.TestCase): """).documentElement self.assertRaises( SystemExit, - utils.dom_update_utilization, el, [("name", "invalid_val")] + utils.dom_update_utilization, el, {"name": "invalid_val"} ) self.assertRaises( SystemExit, - utils.dom_update_utilization, el, [("name", "0.01")] + utils.dom_update_utilization, el, {"name": "0.01"} ) sys.stderr = tmp_stderr @@ -1415,7 +1415,12 @@ class UtilsTest(unittest.TestCase): """).documentElement utils.dom_update_utilization( - el, [("name", ""), ("key", "-1"), ("keys", "90")] + el, + { + "name": "", + "key": "-1", + "keys": "90", + } ) self.assertEqual(len(dom_get_child_elements(el)), 1) @@ -1459,7 +1464,11 @@ class UtilsTest(unittest.TestCase): """).documentElement utils.dom_update_utilization( - el, [("key", "100"), ("keys", "")] + el, + { + "key": "100", + "keys": "", + } ) u = dom_get_child_elements(el)[0] diff --git a/pcs/utils.py b/pcs/utils.py index a7ff7ca..d5b6dcf 100644 --- a/pcs/utils.py +++ b/pcs/utils.py @@ -472,6 +472,16 @@ def getNodesFromPacemaker(): except LibraryError as e: process_library_reports(e.args) +def getNodeAttributesFromPacemaker(): + try: + return [ + node.attrs + for node in ClusterState(getClusterStateXml()).node_section.nodes + ] + except LibraryError as e: + process_library_reports(e.args) + + def hasCorosyncConf(conf=None): if not conf: if is_rhel6(): @@ -2487,7 +2497,7 @@ def dom_update_utilization(dom_element, attributes, id_prefix=""): id_prefix + dom_element.getAttribute("id") + "-utilization" ) - for name, value in attributes: + for name, value in sorted(attributes.items()): if value != "" and not is_int(value): err( "Value of utilization attribute must be integer: " diff --git a/pcsd/public/js/nodes-ember.js b/pcsd/public/js/nodes-ember.js index c650fe6..19caf14 100644 --- a/pcsd/public/js/nodes-ember.js +++ b/pcsd/public/js/nodes-ember.js @@ -500,9 +500,9 @@ Pcs.UtilizationTableComponent = Ember.Component.extend({ }, add: function(form_id) { var id = "#" + form_id; - var name = $(id + " input[name='new_utilization_name']").val(); + var name = $(id + " input[name='new_utilization_name']").val().trim(); if (name == "") { - return; + alert("Name of utilization attribute should be non-empty string."); } var value = $(id + " input[name='new_utilization_value']").val().trim(); if (!is_integer(value)) { diff --git a/pcsd/remote.rb b/pcsd/remote.rb index e467d0a..7dc7951 100644 --- a/pcsd/remote.rb +++ b/pcsd/remote.rb @@ -2240,7 +2240,7 @@ def set_node_utilization(params, reqest, auth_user) if retval != 0 return [400, "Unable to set utilization '#{name}=#{value}' for node " + - "'#{res_id}': #{stderr.join('')}" + "'#{node}': #{stderr.join('')}" ] end return 200 -- 1.8.3.1