diff --git a/pcs/common/reports/codes.py b/pcs/common/reports/codes.py index c882aa244..f537dc08b 100644 --- a/pcs/common/reports/codes.py +++ b/pcs/common/reports/codes.py @@ -700,6 +700,7 @@ UNABLE_TO_GET_CLUSTER_KNOWN_HOSTS = M("UNABLE_TO_GET_CLUSTER_KNOWN_HOSTS") UNABLE_TO_GET_SBD_CONFIG = M("UNABLE_TO_GET_SBD_CONFIG") UNABLE_TO_GET_SBD_STATUS = M("UNABLE_TO_GET_SBD_STATUS") +UNABLE_TO_SET_SBD_CONFIG = M("UNABLE_TO_SET_SBD_CONFIG") UNABLE_TO_PERFORM_OPERATION_ON_ANY_NODE = M( "UNABLE_TO_PERFORM_OPERATION_ON_ANY_NODE" ) diff --git a/pcs/common/reports/messages.py b/pcs/common/reports/messages.py index 571f71630..78932197b 100644 --- a/pcs/common/reports/messages.py +++ b/pcs/common/reports/messages.py @@ -4318,6 +4318,24 @@ def message(self) -> str: ) +@dataclass(frozen=True) +class UnableToSetSbdConfig(ReportItemMessage): + """ + Unable to set SBD configuration + + reason -- reason of failure + """ + + reason: str + _code = codes.UNABLE_TO_SET_SBD_CONFIG + + @property + def message(self) -> str: + return "Unable to set SBD configuration{reason}".format( + reason=format_optional(self.reason, ": {}"), + ) + + @dataclass(frozen=True) class SbdDeviceInitializationStarted(ReportItemMessage): """ diff --git a/pcs/daemon/app/api_v0.py b/pcs/daemon/app/api_v0.py index 56d9f2ca1..b331d56f1 100644 --- a/pcs/daemon/app/api_v0.py +++ b/pcs/daemon/app/api_v0.py @@ -673,6 +673,28 @@ async def _handle_request(self) -> None: self.write("Succeeded") +class GetSbdConfigHandler(_BaseApiV0Handler): + async def _handle_request(self) -> None: + result = await self._run_library_command( + "sbd.get_node_sbd_config_text", {} + ) + if not result.success: + raise self._error(reports_to_str(result.reports)) + self.write(result.result) + + +class SetSbdConfigHandler(_BaseApiV0Handler): + async def _handle_request(self) -> None: + self._check_required_params({"config"}) + result = await self._run_library_command( + "sbd.set_node_sbd_config_text", + dict(config=self.get_argument("config")), + ) + if not result.success: + raise self._error(reports_to_str(result.reports)) + self.write("SBD configuration saved.") + + def get_routes( api_auth_provider_factory: ApiAuthProviderFactoryInterface, scheduler: Scheduler, @@ -741,4 +763,7 @@ def r(url: str) -> str: (r("check_host"), CheckHostHandler, params), # cluster config (r("set_corosync_conf"), SetCorosyncConf, params), + # sbd + (r("get_sbd_config"), GetSbdConfigHandler, params), + (r("set_sbd_config"), SetSbdConfigHandler, params), ] diff --git a/pcs/daemon/async_tasks/worker/command_mapping.py b/pcs/daemon/async_tasks/worker/command_mapping.py index 9c8f6948a..fcf39a6dc 100644 --- a/pcs/daemon/async_tasks/worker/command_mapping.py +++ b/pcs/daemon/async_tasks/worker/command_mapping.py @@ -491,6 +491,14 @@ class _Cmd: cmd=sbd.enable_sbd, required_permission=p.WRITE, ), + "sbd.get_node_sbd_config_text": _Cmd( + cmd=sbd.get_node_sbd_config_text, + required_permission=p.READ, + ), + "sbd.set_node_sbd_config_text": _Cmd( + cmd=sbd.set_node_sbd_config_text, + required_permission=p.WRITE, + ), "scsi.unfence_node": _Cmd( cmd=scsi.unfence_node, required_permission=p.WRITE, @@ -550,6 +558,10 @@ class _Cmd: "resource_agent.list_agents_for_standard_and_provider", "resource_agent.list_ocf_providers", "resource_agent.list_standards", + # The sbd URLs are ready to be exposed in APIv2, just waiting for all the + # other URLs to get moved to APIv2 + "sbd.get_node_sbd_config_text", + "sbd.set_node_sbd_config_text", # There is a lot of url handlers managing cluster services and they should # be consolidated eventually. These are considered temporary implementation # for the purpose of removing ruby code. Therefore, they are not exposed in diff --git a/pcs/lib/commands/sbd.py b/pcs/lib/commands/sbd.py index 996168085..af66ae40b 100644 --- a/pcs/lib/commands/sbd.py +++ b/pcs/lib/commands/sbd.py @@ -458,6 +458,30 @@ def get_local_sbd_config(lib_env: LibraryEnvironment) -> dict[str, str]: return environment_file_to_dict(sbd.get_local_sbd_config()) +def get_node_sbd_config_text(lib_env: LibraryEnvironment) -> str: + """ + Returns local SBD configuration as a raw string. + """ + try: + return sbd.get_local_sbd_config() + except LibraryError as e: + lib_env.report_processor.report_list(list(e.args)) + raise LibraryError() from e + + +def set_node_sbd_config_text(lib_env: LibraryEnvironment, config: str) -> None: + """ + Set SBD configuration on local node from a config string. + + config -- SBD configuration as string + """ + try: + sbd.set_local_sbd_config(config) + except LibraryError as e: + lib_env.report_processor.report_list(list(e.args)) + raise LibraryError() from e + + def initialize_block_devices( lib_env: LibraryEnvironment, device_list: StringSequence, diff --git a/pcs/lib/sbd.py b/pcs/lib/sbd.py index 950b643b1..15c06d10e 100644 --- a/pcs/lib/sbd.py +++ b/pcs/lib/sbd.py @@ -1,3 +1,4 @@ +import fcntl import re from os import path from typing import ( @@ -252,6 +253,7 @@ def get_local_sbd_config() -> str: """ try: with open(settings.sbd_config, "r") as sbd_cfg: + fcntl.flock(sbd_cfg.fileno(), fcntl.LOCK_SH) return sbd_cfg.read() except EnvironmentError as e: raise LibraryError( @@ -261,6 +263,27 @@ def get_local_sbd_config() -> str: ) from e +def set_local_sbd_config(config: str) -> None: + """ + Write SBD configuration to local SBD config file. Raise LibraryError on + any failure. + + config -- SBD configuration as string + """ + try: + with open(settings.sbd_config, "w") as sbd_cfg: + # the lock is released when the file gets closed on leaving the + # with statement + fcntl.flock(sbd_cfg.fileno(), fcntl.LOCK_EX) + sbd_cfg.write(config) + except EnvironmentError as e: + raise LibraryError( + reports.ReportItem.error( + reports.messages.UnableToSetSbdConfig(str(e)) + ) + ) from e + + def is_sbd_enabled(service_manager: ServiceManagerInterface) -> bool: """ Check if SBD service is enabled in local system. diff --git a/pcs_test/Makefile.am b/pcs_test/Makefile.am index 1679f5678..be94e519f 100644 --- a/pcs_test/Makefile.am +++ b/pcs_test/Makefile.am @@ -557,6 +557,7 @@ EXTRA_DIST = \ tools/command_env/calls.py \ tools/command_env/config_corosync_conf.py \ tools/command_env/config_env.py \ + tools/command_env/config_fcntl.py \ tools/command_env/config_fs.py \ tools/command_env/config_http_booth.py \ tools/command_env/config_http_corosync.py \ @@ -579,6 +580,7 @@ EXTRA_DIST = \ tools/command_env/config_runner_scsi.py \ tools/command_env/config_services.py \ tools/command_env/__init__.py \ + tools/command_env/mock_fcntl.py \ tools/command_env/mock_fs.py \ tools/command_env/mock_get_local_corosync_conf.py \ tools/command_env/mock_node_communicator.py \ diff --git a/pcs_test/tier0/common/reports/test_messages.py b/pcs_test/tier0/common/reports/test_messages.py index ded498fb1..11356acb1 100644 --- a/pcs_test/tier0/common/reports/test_messages.py +++ b/pcs_test/tier0/common/reports/test_messages.py @@ -3104,6 +3104,20 @@ def test_all(self): ) +class UnableToSetSbdConfig(NameBuildTest): + def test_no_reason(self): + self.assert_message_from_report( + "Unable to set SBD configuration", + reports.UnableToSetSbdConfig(""), + ) + + def test_all(self): + self.assert_message_from_report( + "Unable to set SBD configuration: reason", + reports.UnableToSetSbdConfig("reason"), + ) + + class SbdDeviceInitializationStarted(NameBuildTest): def test_more_devices(self): self.assert_message_from_report( diff --git a/pcs_test/tier0/daemon/app/test_api_v0.py b/pcs_test/tier0/daemon/app/test_api_v0.py index 45cf95dc6..9a8d0c130 100644 --- a/pcs_test/tier0/daemon/app/test_api_v0.py +++ b/pcs_test/tier0/daemon/app/test_api_v0.py @@ -1746,3 +1746,52 @@ def test_empty_corosync_conf(self): ) self.assertEqual(response.code, 400) self.mock_run_library_command.assert_not_called() + + +class GetSbdConfigHandler(ApiV0HandlerTest): + url = "/remote/get_sbd_config" + body_data = "sbd config text data\n" + + def test_success(self): + self.mock_run_library_command.return_value = self.result_success( + self.body_data + ) + response = self.fetch(self.url) + self.assert_body(response.body, self.body_data) + self.assertEqual(response.code, 200) + self.mock_run_library_command.assert_called_once_with( + "sbd.get_node_sbd_config_text", {} + ) + + def test_failure(self): + self.assert_error_with_report(self.url) + self.mock_run_library_command.assert_called_once_with( + "sbd.get_node_sbd_config_text", {} + ) + + +class SetSbdConfigHandler(ApiV0HandlerTest): + url = "/remote/set_sbd_config" + body_data = {"config": "sbd config text data\n"} + command_data = {"config": body_data["config"].strip()} + + def test_success(self): + self.mock_run_library_command.return_value = self.result_success() + response = self.fetch(self.url, body=urlencode(self.body_data)) + self.assert_body(response.body, "SBD configuration saved.") + self.assertEqual(response.code, 200) + self.mock_run_library_command.assert_called_once_with( + "sbd.set_node_sbd_config_text", self.command_data + ) + + def test_missing_params(self): + response = self.fetch(self.url) + self.assert_body(response.body, "Required parameters missing: 'config'") + self.assertEqual(response.code, 400) + self.mock_run_library_command.assert_not_called() + + def test_failure(self): + self.assert_error_with_report(self.url, body=urlencode(self.body_data)) + self.mock_run_library_command.assert_called_once_with( + "sbd.set_node_sbd_config_text", self.command_data + ) diff --git a/pcs_test/tier0/lib/commands/cluster/test_add_nodes.py b/pcs_test/tier0/lib/commands/cluster/test_add_nodes.py index 666aa6c3d..a059e5183 100644 --- a/pcs_test/tier0/lib/commands/cluster/test_add_nodes.py +++ b/pcs_test/tier0/lib/commands/cluster/test_add_nodes.py @@ -1,6 +1,7 @@ # pylint: disable=too-many-lines # pylint: disable=no-member import base64 +import fcntl import json import os.path from functools import partial @@ -222,16 +223,23 @@ def atb_needed(self, node_labels): def read_sbd_config(self, config_content="", name_suffix=""): local_prefix = "local.read_sbd_config." + mock_file = fixture.get_mock_file(read_data=config_content) ( self.config.fs.exists( settings.sbd_config, return_value=True, name=f"{local_prefix}fs.exists.sbd_config{name_suffix}", - ).fs.open( + ) + .fs.open( settings.sbd_config, - return_value=mock.mock_open(read_data=config_content)(), + return_value=mock_file, name=f"{local_prefix}fs.open.sbd_config_read{name_suffix}", ) + .fcntl.flock( + mock_file, + fcntl.LOCK_SH, + name=f"{local_prefix}fcntl.flock.sbd_config{name_suffix}", + ) ) def check_sbd(self, node_labels, with_devices=True): @@ -278,12 +286,18 @@ def disable_sbd(self, node_labels): def setup_sbd(self, local_config, config_generator, node_labels): local_prefix = "local.setup_sbd." + mock_file = fixture.get_mock_file(read_data=local_config) ( self.config.fs.open( settings.sbd_config, - return_value=mock.mock_open(read_data=local_config)(), + return_value=mock_file, name=f"{local_prefix}fs.open.sbd_config", ) + .fcntl.flock( + mock_file, + fcntl.LOCK_SH, + name=f"{local_prefix}fcntl.flock.sbd_config", + ) .http.sbd.set_sbd_config( config_generator=config_generator, node_labels=node_labels, @@ -3534,12 +3548,16 @@ def _add_nodes_with_lib_error(self, report_list=None): ) def test_enable_communication_failure(self): + sbd_mock_file = fixture.get_mock_file(read_data=self.sbd_config) ( self.config.fs.open( settings.sbd_config, - return_value=mock.mock_open(read_data=self.sbd_config)(), + return_value=sbd_mock_file, name="fs.open.sbd_config", ) + .fcntl.flock( + sbd_mock_file, fcntl.LOCK_SH, name="fcntl.flock.sbd_config" + ) .http.sbd.set_sbd_config( config_generator=lambda node: sbd_config_generator( node, with_devices=False @@ -3601,12 +3619,17 @@ def test_enable_communication_failure(self): ) def test_send_config_communication_failure(self): + sbd_mock_file = fixture.get_mock_file(read_data=self.sbd_config) ( self.config.fs.open( settings.sbd_config, - return_value=mock.mock_open(read_data=self.sbd_config)(), + return_value=sbd_mock_file, name="fs.open.sbd_config", - ).http.sbd.set_sbd_config( + ) + .fcntl.flock( + sbd_mock_file, fcntl.LOCK_SH, name="fcntl.flock.sbd_config" + ) + .http.sbd.set_sbd_config( communication_list=[ dict( label=node, diff --git a/pcs_test/tier0/lib/commands/cluster/test_remove_nodes.py b/pcs_test/tier0/lib/commands/cluster/test_remove_nodes.py index e53250459..cadd92092 100644 --- a/pcs_test/tier0/lib/commands/cluster/test_remove_nodes.py +++ b/pcs_test/tier0/lib/commands/cluster/test_remove_nodes.py @@ -1,13 +1,11 @@ # pylint: disable=too-many-lines # pylint: disable=no-member +import fcntl import json import re from functools import partial from textwrap import dedent -from unittest import ( - TestCase, - mock, -) +from unittest import TestCase from pcs import settings from pcs.common.reports import codes as report_codes @@ -450,10 +448,9 @@ def set_up(self, staying_num, removing_num): self.config.services.is_installed("sbd", return_value=True) self.config.services.is_enabled("sbd", return_value=True) self.config.fs.exists(settings.sbd_config, return_value=True) - self.config.fs.open( - settings.sbd_config, - mock.mock_open(read_data=sbd_config_data)(), - ) + sbd_mock_file = fixture.get_mock_file(read_data=sbd_config_data) + self.config.fs.open(settings.sbd_config, sbd_mock_file) + self.config.fcntl.flock(sbd_mock_file, fcntl.LOCK_SH) self.config.http.corosync.check_corosync_offline( node_labels=self.nodes_to_stay, ) @@ -512,45 +509,34 @@ def set_up(self, staying_num, removing_num): ] ) - def test_2_staying_1_removed(self): - self.set_up(2, 1) + def _run_and_assert(self, staying_num, removing_num): + self.set_up(staying_num, removing_num) cluster.remove_nodes(self.env_assist.get_env(), self.nodes_to_remove) self.env_assist.assert_reports(self.expected_reports) + def test_2_staying_1_removed(self): + self._run_and_assert(2, 1) + def test_2_staying_2_removed(self): - self.set_up(2, 2) - cluster.remove_nodes(self.env_assist.get_env(), self.nodes_to_remove) - self.env_assist.assert_reports(self.expected_reports) + self._run_and_assert(2, 2) def test_2_staying_3_removed(self): - self.set_up(2, 3) - cluster.remove_nodes(self.env_assist.get_env(), self.nodes_to_remove) - self.env_assist.assert_reports(self.expected_reports) + self._run_and_assert(2, 3) def test_4_staying_1_removed(self): - self.set_up(4, 1) - cluster.remove_nodes(self.env_assist.get_env(), self.nodes_to_remove) - self.env_assist.assert_reports(self.expected_reports) + self._run_and_assert(4, 1) def test_4_staying_2_removed(self): - self.set_up(4, 2) - cluster.remove_nodes(self.env_assist.get_env(), self.nodes_to_remove) - self.env_assist.assert_reports(self.expected_reports) + self._run_and_assert(4, 2) def test_4_staying_3_removed(self): - self.set_up(4, 3) - cluster.remove_nodes(self.env_assist.get_env(), self.nodes_to_remove) - self.env_assist.assert_reports(self.expected_reports) + self._run_and_assert(4, 3) def test_4_staying_4_removed(self): - self.set_up(4, 4) - cluster.remove_nodes(self.env_assist.get_env(), self.nodes_to_remove) - self.env_assist.assert_reports(self.expected_reports) + self._run_and_assert(4, 4) def test_4_staying_5_removed(self): - self.set_up(4, 5) - cluster.remove_nodes(self.env_assist.get_env(), self.nodes_to_remove) - self.env_assist.assert_reports(self.expected_reports) + self._run_and_assert(4, 5) class FailureAtbRequired(TestCase): @@ -592,10 +578,9 @@ def setUp(self): self.config.services.is_installed("sbd", return_value=True) self.config.services.is_enabled("sbd", return_value=True) self.config.fs.exists(settings.sbd_config, return_value=True) - self.config.fs.open( - settings.sbd_config, - mock.mock_open(read_data=sbd_config_data)(), - ) + sbd_mock_file = fixture.get_mock_file(read_data=sbd_config_data) + self.config.fs.open(settings.sbd_config, sbd_mock_file) + self.config.fcntl.flock(sbd_mock_file, fcntl.LOCK_SH) self.expected_reports.extend( [ fixture.info(report_codes.COROSYNC_NOT_RUNNING_CHECK_STARTED), diff --git a/pcs_test/tier0/lib/commands/test_quorum.py b/pcs_test/tier0/lib/commands/test_quorum.py index 4f3aa35da..1fb0246ef 100644 --- a/pcs_test/tier0/lib/commands/test_quorum.py +++ b/pcs_test/tier0/lib/commands/test_quorum.py @@ -1,5 +1,6 @@ # pylint: disable=too-many-lines import base64 +import fcntl import logging import os.path import re @@ -525,14 +526,13 @@ def test_disable_atb_sbd_disabled(self): self.env_assist.assert_reports(self.success_reports) def test_disable_atb_sbd_requires_atb(self): + mock_file = fixture.get_mock_file(read_data=self.fixture_sbd_config()) self.config.corosync_conf.load(auto_tie_breaker=True) self.config.services.is_installed("sbd", return_value=True) self.config.services.is_enabled("sbd", return_value=True) self.config.fs.exists(settings.sbd_config) - self.config.fs.open( - settings.sbd_config, - mock.mock_open(read_data=self.fixture_sbd_config())(), - ) + self.config.fs.open(settings.sbd_config, mock_file) + self.config.fcntl.flock(mock_file, fcntl.LOCK_SH) new_options = {"auto_tie_breaker": "0"} assert_raise_library_error( @@ -548,6 +548,7 @@ def test_disable_atb_sbd_requires_atb(self): ) def test_force_disable_atb_sbd_requires_atb(self): + mock_file = fixture.get_mock_file(read_data=self.fixture_sbd_config()) expected_conf = self.original_corosync_conf.replace( " two_node: 1", " two_node: 1\n auto_tie_breaker: 0" ) @@ -555,10 +556,8 @@ def test_force_disable_atb_sbd_requires_atb(self): self.config.services.is_installed("sbd", return_value=True) self.config.services.is_enabled("sbd", return_value=True) self.config.fs.exists(settings.sbd_config) - self.config.fs.open( - settings.sbd_config, - mock.mock_open(read_data=self.fixture_sbd_config())(), - ) + self.config.fs.open(settings.sbd_config, mock_file) + self.config.fcntl.flock(mock_file, fcntl.LOCK_SH) self.config.http.corosync.check_corosync_offline( node_labels=self.node_labels ) diff --git a/pcs_test/tier0/lib/commands/test_sbd.py b/pcs_test/tier0/lib/commands/test_sbd.py index fb939dd97..00bc08b9a 100644 --- a/pcs_test/tier0/lib/commands/test_sbd.py +++ b/pcs_test/tier0/lib/commands/test_sbd.py @@ -1,7 +1,5 @@ -from unittest import ( - TestCase, - mock, -) +import fcntl +from unittest import TestCase import pcs.lib.commands.sbd as cmd_sbd from pcs import settings @@ -389,10 +387,9 @@ def test_success(self): SBD_WATCHDOG_TIMEOUT=0 """ ) - self.config.fs.open( - settings.sbd_config, - mock.mock_open(read_data=config_content)(), - ) + mock_file = fixture.get_mock_file(read_data=config_content) + self.config.fs.open(settings.sbd_config, mock_file) + self.config.fcntl.flock(mock_file, fcntl.LOCK_SH) self.assertEqual( { @@ -427,6 +424,118 @@ def test_file_error(self): ) +class GetNodeSbdConfigTextTest(TestCase): + def setUp(self): + self.env_assist, self.config = get_env_tools(self) + + def test_success(self): + config_content = outdent( + """\ + # This file has been generated by pcs. + SBD_OPTS="-n node1" + SBD_WATCHDOG_DEV=/dev/watchdog + SBD_WATCHDOG_TIMEOUT=0 + """ + ) + mock_file = fixture.get_mock_file(read_data=config_content) + self.config.fs.open(settings.sbd_config, mock_file) + self.config.fcntl.flock(mock_file, fcntl.LOCK_SH) + self.assertEqual( + config_content, + cmd_sbd.get_node_sbd_config_text(self.env_assist.get_env()), + ) + + def _assert_failure(self, reason): + self.env_assist.assert_raise_library_error( + lambda: cmd_sbd.get_node_sbd_config_text(self.env_assist.get_env()), + ) + self.env_assist.assert_reports( + [ + fixture.error( + reports.codes.UNABLE_TO_GET_SBD_CONFIG, + node="local node", + reason=reason, + ), + ] + ) + + def test_open_failure(self): + reason = "open error" + self.config.fs.open(settings.sbd_config, side_effect=OSError(reason)) + self._assert_failure(reason) + + def test_flock_failure(self): + reason = "flock error" + mock_file = fixture.get_mock_file() + self.config.fs.open(settings.sbd_config, mock_file) + self.config.fcntl.flock( + mock_file, fcntl.LOCK_SH, side_effect=OSError(reason) + ) + self._assert_failure(reason) + + def test_read_failure(self): + reason = "read error" + mock_file = fixture.get_mock_file() + mock_file.read.side_effect = OSError(reason) + self.config.fs.open(settings.sbd_config, mock_file) + self.config.fcntl.flock(mock_file, fcntl.LOCK_SH) + self._assert_failure(reason) + + +class SetNodeSbdConfigTextTest(TestCase): + def setUp(self): + self.env_assist, self.config = get_env_tools(self) + + def test_success(self): + config_data = "SBD_WATCHDOG_TIMEOUT=5\n" + mock_file = fixture.get_mock_file() + self.config.fs.open(settings.sbd_config, mock_file, mode="w") + self.config.fcntl.flock(mock_file, fcntl.LOCK_EX) + cmd_sbd.set_node_sbd_config_text(self.env_assist.get_env(), config_data) + mock_file.write.assert_called_once_with(config_data) + + def _assert_failure(self, reason): + self.env_assist.assert_raise_library_error( + lambda: cmd_sbd.set_node_sbd_config_text( + self.env_assist.get_env(), "config" + ), + ) + self.env_assist.assert_reports( + [ + fixture.error( + reports.codes.UNABLE_TO_SET_SBD_CONFIG, + reason=reason, + ), + ] + ) + + def test_open_failure(self): + reason = "reason" + self.config.fs.open( + settings.sbd_config, side_effect=OSError(reason), mode="w" + ) + self._assert_failure(reason) + + def test_flock_failure(self): + reason = "flock error" + mock_file = fixture.get_mock_file() + self.config.fs.open(settings.sbd_config, mock_file, mode="w") + self.config.fcntl.flock( + mock_file, fcntl.LOCK_EX, side_effect=OSError(reason) + ) + self._assert_failure(reason) + mock_file.write.assert_not_called() + + def test_write_failure(self): + reason = "write error" + mock_file = fixture.get_mock_file() + mock_file.write.side_effect = OSError(reason) + self.config.fs.open(settings.sbd_config, mock_file, mode="w") + self.config.fcntl.flock(mock_file, fcntl.LOCK_EX) + self._assert_failure(reason) + mock_file.write.assert_called_once() + + class InitializeBlockDevicesTest(TestCase): def setUp(self): self.env_assist, self.config = get_env_tools(self) @@ -544,11 +653,11 @@ def setUp(self): def test_success(self): config_data = 'SBD_DEVICE="/dev1;/dev2"\n' + mock_file = fixture.get_mock_file(read_data=config_data) self.config.services.is_enabled("sbd") self.config.fs.exists(settings.sbd_config, return_value=True) - self.config.fs.open( - settings.sbd_config, mock.mock_open(read_data=config_data)() - ) + self.config.fs.open(settings.sbd_config, mock_file) + self.config.fcntl.flock(mock_file, fcntl.LOCK_SH) self.config.runner.sbd.get_device_info("/dev1", stdout="1") self.config.runner.sbd.get_device_info( "/dev2", stdout="2", name="list2" @@ -572,11 +681,11 @@ def test_success(self): def test_with_dump(self): config_data = 'SBD_DEVICE="/dev1;/dev2"\n' + mock_file = fixture.get_mock_file(read_data=config_data) self.config.services.is_enabled("sbd") self.config.fs.exists(settings.sbd_config, return_value=True) - self.config.fs.open( - settings.sbd_config, mock.mock_open(read_data=config_data)() - ) + self.config.fs.open(settings.sbd_config, mock_file) + self.config.fcntl.flock(mock_file, fcntl.LOCK_SH) self.config.runner.sbd.get_device_info("/dev1", stdout="1") self.config.runner.sbd.get_device_dump("/dev1", stdout="2") self.config.runner.sbd.get_device_info( @@ -619,11 +728,11 @@ def test_sbd_disabled(self): def test_with_failures(self): config_data = 'SBD_DEVICE="/dev1;/dev2;/dev3"\n' + mock_file = fixture.get_mock_file(read_data=config_data) self.config.services.is_enabled("sbd") self.config.fs.exists(settings.sbd_config, return_value=True) - self.config.fs.open( - settings.sbd_config, mock.mock_open(read_data=config_data)() - ) + self.config.fs.open(settings.sbd_config, mock_file) + self.config.fcntl.flock(mock_file, fcntl.LOCK_SH) self.config.runner.sbd.get_device_info( "/dev1", stdout="1", return_code=1 ) diff --git a/pcs_test/tools/command_env/assistant.py b/pcs_test/tools/command_env/assistant.py index cbfbe911c..2ac95bc2c 100644 --- a/pcs_test/tools/command_env/assistant.py +++ b/pcs_test/tools/command_env/assistant.py @@ -1,3 +1,4 @@ +import fcntl import inspect import logging import os @@ -18,6 +19,10 @@ from pcs_test.tools.command_env import spy from pcs_test.tools.command_env.calls import Queue as CallQueue from pcs_test.tools.command_env.config import Config +from pcs_test.tools.command_env.mock_fcntl import ( + get_fcntl_mock, + is_fcntl_call_in, +) from pcs_test.tools.command_env.mock_fs import get_fs_mock, is_fs_call_in from pcs_test.tools.command_env.mock_get_local_corosync_conf import ( get_get_local_corosync_conf, @@ -105,6 +110,11 @@ def get_cmd_runner(self, env=None): "_get_service_manager", lambda _: ServiceManagerMock(call_queue) ), ] + if is_fcntl_call_in(call_queue): + fcntl_mock = get_fcntl_mock(call_queue) + patcher_list.append( + mock.patch("fcntl.flock", fcntl_mock("flock", fcntl.flock)), + ) if is_fs_call_in(call_queue): fs_mock = get_fs_mock(call_queue) patcher_list.extend( diff --git a/pcs_test/tools/command_env/config.py b/pcs_test/tools/command_env/config.py index ef75a865d..e0411777f 100644 --- a/pcs_test/tools/command_env/config.py +++ b/pcs_test/tools/command_env/config.py @@ -3,6 +3,7 @@ from pcs_test.tools.command_env.calls import CallListBuilder from pcs_test.tools.command_env.config_corosync_conf import CorosyncConf from pcs_test.tools.command_env.config_env import EnvConfig +from pcs_test.tools.command_env.config_fcntl import FcntlConfig from pcs_test.tools.command_env.config_fs import FsConfig from pcs_test.tools.command_env.config_http import HttpConfig from pcs_test.tools.command_env.config_raw_file import RawFileConfig @@ -27,6 +28,7 @@ def __init__(self): HttpConfig(self.__calls, self.__wrap_helper) ) self.corosync_conf = self.__wrap_helper(CorosyncConf(self.__calls)) + self.fcntl = self.__wrap_helper(FcntlConfig(self.__calls)) # pylint: disable=invalid-name self.fs = self.__wrap_helper(FsConfig(self.__calls)) self.raw_file = self.__wrap_helper(RawFileConfig(self.__calls)) diff --git a/pcs_test/tools/command_env/config_fcntl.py b/pcs_test/tools/command_env/config_fcntl.py new file mode 100644 index 000000000..8c07ea813 --- /dev/null +++ b/pcs_test/tools/command_env/config_fcntl.py @@ -0,0 +1,24 @@ +from pcs_test.tools.command_env.mock_fcntl import Call as FcntlCall + + +class FcntlConfig: + """ + Any new call must be manually added to the patch_env function in the + pcs_test.tools.command_env.assistant module otherwise it will be ignored! + Also its arguments must be described in the _FUNC_ARGS mapping in the + pcs_test.tools.command_env.mock_fcntl module. + """ + + def __init__(self, call_collection): + self.__calls = call_collection + + def flock(self, mock_file, operation, side_effect=None, name="fcntl.flock"): + call = FcntlCall( + "flock", + call_kwargs={ + "fd": mock_file.fileno.return_value, + "operation": operation, + }, + side_effect=side_effect, + ) + self.__calls.place(name, call) diff --git a/pcs_test/tools/command_env/mock_fcntl.py b/pcs_test/tools/command_env/mock_fcntl.py new file mode 100644 index 000000000..6840964cc --- /dev/null +++ b/pcs_test/tools/command_env/mock_fcntl.py @@ -0,0 +1,96 @@ +CALL_TYPE_FCNTL = "CALL_TYPE_FCNTL" + +_FUNC_ARGS = {"flock": ["fd", "operation"]} + + +def _ensure_consistent_args(func_name, call_args, call_kwargs): + if len(call_args) > len(_FUNC_ARGS[func_name]): + raise AssertionError( + "{0}() too many positional arguments ({1} > {2})".format( + func_name, len(call_args), len(_FUNC_ARGS[func_name]) + ) + ) + + param_intersection = set( + _FUNC_ARGS[func_name][: len(call_args)] + ).intersection(call_kwargs.keys()) + if param_intersection: + raise TypeError( + "{0}() got multiple values for keyword argument(s) '{1}'".format( + func_name, "', '".join(param_intersection) + ) + ) + + +def _get_all_args_as_kwargs(func_name, call_args, call_kwargs): + _ensure_consistent_args(func_name, call_args, call_kwargs) + kwargs = call_kwargs.copy() + for i, arg in enumerate(call_args): + kwargs[_FUNC_ARGS[func_name][i]] = arg + return kwargs + + +class Call: + type = CALL_TYPE_FCNTL + + def __init__( + self, func_name, return_value=None, side_effect=None, call_kwargs=None + ): + call_kwargs = call_kwargs if call_kwargs else {} + + self.type = CALL_TYPE_FCNTL + self.func_name = func_name + self.call_kwargs = call_kwargs + self.return_value = return_value + self.side_effect = side_effect + + def finish(self): + if self.side_effect: + if isinstance(self.side_effect, Exception): + raise self.side_effect + raise AssertionError( + "side_effect other than instance of exception not supported yet" + ) + + return self.return_value + + def __repr__(self): + return str("").format( + self.func_name, + self.call_kwargs, + ) + + def __ne__(self, other): + return ( + self.func_name != other.func_name + or self.call_kwargs != other.call_kwargs + ) + + +def get_fcntl_mock(call_queue): + def get_fcntl_call(func_name, _original_call): + def call_fcntl(*args, **kwargs): + real_call = Call( + func_name, + call_kwargs=_get_all_args_as_kwargs(func_name, args, kwargs), + ) + dummy_i, expected_call = call_queue.take( + CALL_TYPE_FCNTL, repr(real_call) + ) + + if expected_call != real_call: + raise call_queue.error_with_context( + "\n expected: '{0}'\n but was: '{1}'".format( + expected_call, real_call + ) + ) + + return expected_call.finish() + + return call_fcntl + + return get_fcntl_call + + +def is_fcntl_call_in(call_queue): + return call_queue.has_type(CALL_TYPE_FCNTL) diff --git a/pcs_test/tools/fixture.py b/pcs_test/tools/fixture.py index df28b7c5f..0d17c1101 100644 --- a/pcs_test/tools/fixture.py +++ b/pcs_test/tools/fixture.py @@ -1,3 +1,4 @@ +import itertools import json from collections import Counter from typing import ( @@ -9,11 +10,19 @@ TypeVar, overload, ) +from unittest import mock from pcs.common import reports from pcs.common.str_tools import format_list ALL_RESOURCE_XML_TAGS = ["bundle", "clone", "group", "master", "primitive"] +_fileno_counter = itertools.count(100) + + +def get_mock_file(*, read_data=None): + mock_file = mock.mock_open(read_data=read_data)() + mock_file.fileno.return_value = next(_fileno_counter) + return mock_file # Previously, a report item fixture was a plain tuple. That was, however, not diff --git a/pcsd/remote.rb b/pcsd/remote.rb index b777ed0d9..a66226bb3 100644 --- a/pcsd/remote.rb +++ b/pcsd/remote.rb @@ -36,8 +36,6 @@ def remote(params, request, auth_user) :get_cluster_known_hosts => method(:get_cluster_known_hosts), :get_cluster_properties_definition => method(:get_cluster_properties_definition), :check_sbd => method(:check_sbd), - :set_sbd_config => method(:set_sbd_config), - :get_sbd_config => method(:get_sbd_config), :sbd_disable => method(:sbd_disable), :sbd_enable => method(:sbd_enable), :remove_stonith_watchdog_timeout=> method(:remove_stonith_watchdog_timeout), @@ -1148,52 +1146,6 @@ def check_sbd(param, request, auth_user) return [200, JSON.generate(out)] end -def set_sbd_config(param, request, auth_user) - unless allowed_for_local_cluster(auth_user, Permissions::WRITE) - return 403, 'Permission denied' - end - config = param[:config] - unless config - return [400, 'Parameter "config" required'] - end - - file = nil - begin - file = File.open(SBD_CONFIG, 'w') - file.flock(File::LOCK_EX) - file.write(config) - rescue => e - return pcsd_error("Unable to save SBD configuration: #{e}") - ensure - if file - file.flock(File::LOCK_UN) - file.close() - end - end - return pcsd_success('SBD configuration saved.') -end - -def get_sbd_config(param, request, auth_user) - unless allowed_for_local_cluster(auth_user, Permissions::READ) - return 403, 'Permission denied' - end - out = [] - file = nil - begin - file = File.open(SBD_CONFIG, 'r') - file.flock(File::LOCK_SH) - out = file.readlines() - rescue => e - return pcsd_error("Unable to get SBD configuration: #{e}") - ensure - if file - file.flock(File::LOCK_UN) - file.close() - end - end - return [200, out.join('')] -end - def sbd_disable(param, request, auth_user) unless allowed_for_local_cluster(auth_user, Permissions::WRITE) return 403, 'Permission denied'