From 663143e3abbf2798a3c780c691242511b64046a2 Mon Sep 17 00:00:00 2001 From: Tomas Jelinek Date: Wed, 6 Dec 2017 17:38:15 +0100 Subject: [PATCH] fix a crash when --wait is used in stonith create --- pcs/lib/commands/stonith.py | 19 ++- pcs/lib/commands/test/test_stonith.py | 188 ++++++++++++++++++++++ pcs/lib/resource_agent.py | 4 + pcs/test/resources/stonith_agent_fence_simple.xml | 33 ++++ pcs/test/resources/stonithd_metadata.xml | 156 ++++++++++++++++++ pcs/test/tools/command_env/config_runner_pcmk.py | 36 +++++ 6 files changed, 430 insertions(+), 6 deletions(-) create mode 100644 pcs/lib/commands/test/test_stonith.py create mode 100644 pcs/test/resources/stonith_agent_fence_simple.xml create mode 100644 pcs/test/resources/stonithd_metadata.xml diff --git a/pcs/lib/commands/stonith.py b/pcs/lib/commands/stonith.py index bb9fb98..584e1b2 100644 --- a/pcs/lib/commands/stonith.py +++ b/pcs/lib/commands/stonith.py @@ -4,11 +4,14 @@ from __future__ import ( print_function, ) -from pcs.lib.resource_agent import find_valid_stonith_agent_by_name as get_agent from pcs.lib.cib import resource from pcs.lib.cib.resource.common import are_meta_disabled +from pcs.lib.commands.resource import ( + _ensure_disabled_after_wait, + resource_environment +) from pcs.lib.pacemaker.values import validate_id -from pcs.lib.commands.resource import resource_environment +from pcs.lib.resource_agent import find_valid_stonith_agent_by_name as get_agent def create( env, stonith_id, stonith_agent_name, @@ -55,8 +58,10 @@ def create( with resource_environment( env, wait, - stonith_id, - ensure_disabled or are_meta_disabled(meta_attributes), + [stonith_id], + _ensure_disabled_after_wait( + ensure_disabled or are_meta_disabled(meta_attributes), + ) ) as resources_section: stonith_element = resource.primitive.create( env.report_processor, @@ -125,8 +130,10 @@ def create_in_group( with resource_environment( env, wait, - stonith_id, - ensure_disabled or are_meta_disabled(meta_attributes), + [stonith_id], + _ensure_disabled_after_wait( + ensure_disabled or are_meta_disabled(meta_attributes), + ) ) as resources_section: stonith_element = resource.primitive.create( env.report_processor, resources_section, diff --git a/pcs/lib/commands/test/test_stonith.py b/pcs/lib/commands/test/test_stonith.py new file mode 100644 index 0000000..912742f --- /dev/null +++ b/pcs/lib/commands/test/test_stonith.py @@ -0,0 +1,188 @@ +from __future__ import ( + absolute_import, + division, + print_function, +) + +from pcs.common import report_codes +from pcs.lib.commands import stonith +from pcs.lib.resource_agent import StonithAgent +from pcs.test.tools import fixture +from pcs.test.tools.command_env import get_env_tools +from pcs.test.tools.pcs_unittest import TestCase + + +class Create(TestCase): + def setUp(self): + self.env_assist, self.config = get_env_tools(test_case=self) + self.agent_name = "test_simple" + self.instance_name = "stonith-test" + self.timeout = 10 + self.expected_cib = """ + + + + + + + + + + + """ + self.expected_status = """ + + + + + + """.format(id=self.instance_name, agent=self.agent_name) + (self.config + .runner.pcmk.load_agent( + agent_name="stonith:{0}".format(self.agent_name), + agent_filename="stonith_agent_fence_simple.xml" + ) + .runner.cib.load() + .runner.pcmk.load_stonithd_metadata() + ) + + def tearDown(self): + StonithAgent.clear_stonithd_metadata_cache() + + def test_minimal_success(self): + self.config.env.push_cib(resources=self.expected_cib) + stonith.create( + self.env_assist.get_env(), + self.instance_name, + self.agent_name, + operations=[], + meta_attributes={}, + instance_attributes={"must-set": "value"} + ) + + def test_minimal_wait_ok_run_ok(self): + (self.config + .runner.pcmk.can_wait(before="runner.cib.load") + .env.push_cib( + resources=self.expected_cib, + wait=self.timeout + ) + .runner.pcmk.load_state(resources=self.expected_status) + ) + stonith.create( + self.env_assist.get_env(), + self.instance_name, + self.agent_name, + operations=[], + meta_attributes={}, + instance_attributes={"must-set": "value"}, + wait=self.timeout + ) + self.env_assist.assert_reports([ + fixture.info( + report_codes.RESOURCE_RUNNING_ON_NODES, + roles_with_nodes={"Started": ["node1"]}, + resource_id=self.instance_name, + ), + ]) + + +class CreateInGroup(TestCase): + def setUp(self): + self.env_assist, self.config = get_env_tools(test_case=self) + self.agent_name = "test_simple" + self.instance_name = "stonith-test" + self.timeout = 10 + self.expected_cib = """ + + + + + + + + + + + + + """ + self.expected_status = """ + + + + + + """.format(id=self.instance_name, agent=self.agent_name) + (self.config + .runner.pcmk.load_agent( + agent_name="stonith:{0}".format(self.agent_name), + agent_filename="stonith_agent_fence_simple.xml" + ) + .runner.cib.load() + .runner.pcmk.load_stonithd_metadata() + ) + + def tearDown(self): + StonithAgent.clear_stonithd_metadata_cache() + + def test_minimal_success(self): + self.config.env.push_cib(resources=self.expected_cib) + stonith.create_in_group( + self.env_assist.get_env(), + self.instance_name, + self.agent_name, + "my-group", + operations=[], + meta_attributes={}, + instance_attributes={"must-set": "value"} + ) + + def test_minimal_wait_ok_run_ok(self): + (self.config + .runner.pcmk.can_wait(before="runner.cib.load") + .env.push_cib( + resources=self.expected_cib, + wait=self.timeout + ) + .runner.pcmk.load_state(resources=self.expected_status) + ) + stonith.create_in_group( + self.env_assist.get_env(), + self.instance_name, + self.agent_name, + "my-group", + operations=[], + meta_attributes={}, + instance_attributes={"must-set": "value"}, + wait=self.timeout + ) + self.env_assist.assert_reports([ + fixture.info( + report_codes.RESOURCE_RUNNING_ON_NODES, + roles_with_nodes={"Started": ["node1"]}, + resource_id=self.instance_name, + ), + ]) diff --git a/pcs/lib/resource_agent.py b/pcs/lib/resource_agent.py index 4639477..2f2686d 100644 --- a/pcs/lib/resource_agent.py +++ b/pcs/lib/resource_agent.py @@ -836,6 +836,10 @@ class StonithAgent(CrmAgent): """ _stonithd_metadata = None + @classmethod + def clear_stonithd_metadata_cache(cls): + cls._stonithd_metadata = None + def _prepare_name_parts(self, name): # pacemaker doesn't support stonith (nor resource) agents with : in type if ":" in name: diff --git a/pcs/test/resources/stonith_agent_fence_simple.xml b/pcs/test/resources/stonith_agent_fence_simple.xml new file mode 100644 index 0000000..bb86af2 --- /dev/null +++ b/pcs/test/resources/stonith_agent_fence_simple.xml @@ -0,0 +1,33 @@ + + + + This is a testing fence agent. Its purpose is to provide a mock of a fence + agent which is always available no matter what is the configuration of a + system pcs test suite runs on. + + https://github.com/ClusterLabs/pcs + + + + An example of a required attribute + + + + An example of an optional attribute + + + + + + + + + + + + + + diff --git a/pcs/test/resources/stonithd_metadata.xml b/pcs/test/resources/stonithd_metadata.xml new file mode 100644 index 0000000..fc638a2 --- /dev/null +++ b/pcs/test/resources/stonithd_metadata.xml @@ -0,0 +1,156 @@ + + + 1.0 + This is a fake resource that details the instance attributes handled by stonithd. + Options available for all stonith resources + + + The priority of the stonith resource. Devices are tried in order of highest priority to lowest. + + + + Advanced use only: An alternate parameter to supply instead of 'port' + Some devices do not support the standard 'port' parameter or may provide additional ones. +Use this to specify an alternate, device-specific, parameter that should indicate the machine to be fenced. +A value of 'none' can be used to tell the cluster not to supply any additional parameters. + + + + + A mapping of host names to ports numbers for devices that do not support host names. + Eg. node1:1;node2:2,3 would tell the cluster to use port 1 for node1 and ports 2 and 3 for node2 + + + + A list of machines controlled by this device (Optional unless pcmk_host_check=static-list). + + + + How to determine which machines are controlled by the device. + Allowed values: dynamic-list (query the device), static-list (check the pcmk_host_list attribute), none (assume every device can fence every machine) + + + + Enable a random delay for stonith actions and specify the maximum of random delay. + This prevents double fencing when using slow devices such as sbd. +Use this to enable a random delay for stonith actions. +The overall delay is derived from this random delay value adding a static delay so that the sum is kept below the maximum delay. + + + + Enable a base delay for stonith actions and specify base delay value. + This prevents double fencing when different delays are configured on the nodes. +Use this to enable a static delay for stonith actions. +The overall delay is derived from a random delay value adding this static delay so that the sum is kept below the maximum delay. + + + + The maximum number of actions can be performed in parallel on this device + Pengine property concurrent-fencing=true needs to be configured first. +Then use this to specify the maximum number of actions can be performed in parallel on this device. -1 is unlimited. + + + + Advanced use only: An alternate command to run instead of 'reboot' + Some devices do not support the standard commands or may provide additional ones. +Use this to specify an alternate, device-specific, command that implements the 'reboot' action. + + + + Advanced use only: Specify an alternate timeout to use for reboot actions instead of stonith-timeout + Some devices need much more/less time to complete than normal. +Use this to specify an alternate, device-specific, timeout for 'reboot' actions. + + + + Advanced use only: The maximum number of times to retry the 'reboot' command within the timeout period + Some devices do not support multiple connections. Operations may 'fail' if the device is busy with another task so Pacemaker will automatically retry the operation, if there is time remaining. Use this option to alter the number of times Pacemaker retries 'reboot' actions before giving up. + + + + Advanced use only: An alternate command to run instead of 'off' + Some devices do not support the standard commands or may provide additional ones. +Use this to specify an alternate, device-specific, command that implements the 'off' action. + + + + Advanced use only: Specify an alternate timeout to use for off actions instead of stonith-timeout + Some devices need much more/less time to complete than normal. +Use this to specify an alternate, device-specific, timeout for 'off' actions. + + + + Advanced use only: The maximum number of times to retry the 'off' command within the timeout period + Some devices do not support multiple connections. Operations may 'fail' if the device is busy with another task so Pacemaker will automatically retry the operation, if there is time remaining. Use this option to alter the number of times Pacemaker retries 'off' actions before giving up. + + + + Advanced use only: An alternate command to run instead of 'on' + Some devices do not support the standard commands or may provide additional ones. +Use this to specify an alternate, device-specific, command that implements the 'on' action. + + + + Advanced use only: Specify an alternate timeout to use for on actions instead of stonith-timeout + Some devices need much more/less time to complete than normal. +Use this to specify an alternate, device-specific, timeout for 'on' actions. + + + + Advanced use only: The maximum number of times to retry the 'on' command within the timeout period + Some devices do not support multiple connections. Operations may 'fail' if the device is busy with another task so Pacemaker will automatically retry the operation, if there is time remaining. Use this option to alter the number of times Pacemaker retries 'on' actions before giving up. + + + + Advanced use only: An alternate command to run instead of 'list' + Some devices do not support the standard commands or may provide additional ones. +Use this to specify an alternate, device-specific, command that implements the 'list' action. + + + + Advanced use only: Specify an alternate timeout to use for list actions instead of stonith-timeout + Some devices need much more/less time to complete than normal. +Use this to specify an alternate, device-specific, timeout for 'list' actions. + + + + Advanced use only: The maximum number of times to retry the 'list' command within the timeout period + Some devices do not support multiple connections. Operations may 'fail' if the device is busy with another task so Pacemaker will automatically retry the operation, if there is time remaining. Use this option to alter the number of times Pacemaker retries 'list' actions before giving up. + + + + Advanced use only: An alternate command to run instead of 'monitor' + Some devices do not support the standard commands or may provide additional ones. +Use this to specify an alternate, device-specific, command that implements the 'monitor' action. + + + + Advanced use only: Specify an alternate timeout to use for monitor actions instead of stonith-timeout + Some devices need much more/less time to complete than normal. +Use this to specify an alternate, device-specific, timeout for 'monitor' actions. + + + + Advanced use only: The maximum number of times to retry the 'monitor' command within the timeout period + Some devices do not support multiple connections. Operations may 'fail' if the device is busy with another task so Pacemaker will automatically retry the operation, if there is time remaining. Use this option to alter the number of times Pacemaker retries 'monitor' actions before giving up. + + + + Advanced use only: An alternate command to run instead of 'status' + Some devices do not support the standard commands or may provide additional ones. +Use this to specify an alternate, device-specific, command that implements the 'status' action. + + + + Advanced use only: Specify an alternate timeout to use for status actions instead of stonith-timeout + Some devices need much more/less time to complete than normal. +Use this to specify an alternate, device-specific, timeout for 'status' actions. + + + + Advanced use only: The maximum number of times to retry the 'status' command within the timeout period + Some devices do not support multiple connections. Operations may 'fail' if the device is busy with another task so Pacemaker will automatically retry the operation, if there is time remaining. Use this option to alter the number of times Pacemaker retries 'status' actions before giving up. + + + + diff --git a/pcs/test/tools/command_env/config_runner_pcmk.py b/pcs/test/tools/command_env/config_runner_pcmk.py index 6499ef8..059f3eb 100644 --- a/pcs/test/tools/command_env/config_runner_pcmk.py +++ b/pcs/test/tools/command_env/config_runner_pcmk.py @@ -70,6 +70,42 @@ class PcmkShortcuts(object): instead=instead, ) + def load_stonithd_metadata( + self, + name="runner.pcmk.load_stonithd_metadata", + stdout=None, + stderr="", + returncode=0, + instead=None, + before=None, + ): + """ + Create a call for loading stonithd metadata - additional fence options + + string name -- the key of this call + string stdout -- stonithd stdout, default metadata if None + string stderr -- stonithd stderr + int returncode -- stonithd returncode + string instead -- the key of a call instead of which this new call is to + be placed + string before -- the key of a call before which this new call is to be + placed + """ + self.__calls.place( + name, + RunnerCall( + "/usr/libexec/pacemaker/stonithd metadata", + stdout=( + stdout if stdout is not None + else open(rc("stonithd_metadata.xml")).read() + ), + stderr=stderr, + returncode=returncode + ), + before=before, + instead=instead, + ) + def resource_cleanup( self, name="runner.pcmk.cleanup", -- 1.8.3.1