From 245ccebba6337d1ec31a0bf3fcf79149d758173a Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Wed, 23 Apr 2025 15:49:03 +0000 Subject: [PATCH] More review fixes --- cmapi/cmapi_server/managers/process.py | 24 ++++++++++--------- cmapi/cmapi_server/node_manipulation.py | 2 +- .../test/test_mcs_process_operations.py | 4 ++-- cmapi/cmapi_server/test/test_node_manip.py | 13 +++++----- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/cmapi/cmapi_server/managers/process.py b/cmapi/cmapi_server/managers/process.py index 9b0d2264d..98802931a 100644 --- a/cmapi/cmapi_server/managers/process.py +++ b/cmapi/cmapi_server/managers/process.py @@ -36,7 +36,7 @@ class MCSProcessManager: mcs_progs = {} mcs_version_info = None dispatcher_name = None - process_dispatcher = None + process_dispatcher: BaseDispatcher = None @classmethod def _get_prog_name(cls, name: str) -> str: @@ -48,12 +48,13 @@ class MCSProcessManager: :rtype: str """ if cls.dispatcher_name == 'systemd': - return ALL_MCS_PROGS[name].service_name + prog = MCSProgs[name] + return ALL_MCS_PROGS[prog].service_name return name @classmethod def _get_sorted_progs( - cls, is_primary: bool, reverse: bool = False + cls, is_primary: bool, reverse: bool = False, is_read_only: bool = False ) -> dict: """Get sorted services dict. @@ -73,6 +74,13 @@ class MCSProcessManager: for prog_name, prog_info in cls.mcs_progs.items() if prog_name not in PRIMARY_PROGS } + + if is_read_only: + logging.debug('Node is in read-only mode, skipping WriteEngine') + unsorted_progs.pop( + MCSProgs.WRITE_ENGINE_SERVER.value, None + ) + if reverse: # stop sequence builds using stop_priority property return dict( @@ -421,7 +429,7 @@ class MCSProcessManager: :type is_read_only: bool, optional :raises CMAPIBasicError: immediately if one mcs process not started """ - for prog_name in cls._get_sorted_progs(is_primary): + for prog_name in cls._get_sorted_progs(is_primary=is_primary, is_read_only=is_read_only): if ( cls.dispatcher_name == 'systemd' and prog_name == MCSProgs.STORAGE_MANAGER.value @@ -436,12 +444,6 @@ class MCSProcessManager: cls._wait_for_workernodes() if prog_name in (MCSProgs.DML_PROC.value, MCSProgs.DDL_PROC.value): cls._wait_for_controllernode() - if ( - is_read_only and - prog_name == MCSProgs.WRITE_ENGINE_SERVER.value - ): - logging.debug('Node is in read-only mode, not starting WriteEngine') - continue if not cls.start(prog_name, is_primary, use_sudo): logging.error(f'Process "{prog_name}" not started properly.') raise CMAPIBasicError(f'Error while starting "{prog_name}".') @@ -470,7 +472,7 @@ class MCSProcessManager: # so use full available list of processes. Otherwise, it could cause # undefined behaviour when primary gone and then recovers (failover # triggered 2 times). - for prog_name in cls._get_sorted_progs(True, reverse=True): + for prog_name in cls._get_sorted_progs(is_primary=True, reverse=True): if not cls.stop(prog_name, is_primary, use_sudo): logging.error(f'Process "{prog_name}" not stopped properly.') raise CMAPIBasicError(f'Error while stopping "{prog_name}"') diff --git a/cmapi/cmapi_server/node_manipulation.py b/cmapi/cmapi_server/node_manipulation.py index 4c2e2ff1d..8d8ed1413 100644 --- a/cmapi/cmapi_server/node_manipulation.py +++ b/cmapi/cmapi_server/node_manipulation.py @@ -167,7 +167,7 @@ def remove_node( if len(active_nodes) > 1: pm_num = _remove_node_from_PMS(c_root, node) - is_read_only = node in helpers.get_read_only_nodes(c_root) + is_read_only = node in node_config.get_read_only_nodes(c_root) if not is_read_only: _remove_WES(c_root, pm_num) diff --git a/cmapi/cmapi_server/test/test_mcs_process_operations.py b/cmapi/cmapi_server/test/test_mcs_process_operations.py index aa2efe800..9c171b709 100644 --- a/cmapi/cmapi_server/test/test_mcs_process_operations.py +++ b/cmapi/cmapi_server/test/test_mcs_process_operations.py @@ -65,7 +65,7 @@ class MCSProcessManagerTest(BaseProcessDispatcherCase): def test_mcs_process_manager(self): MCSProcessManager.detect('systemd', '') - for prog in MCSProcessManager._get_sorted_progs(True, True).values(): + for prog in MCSProcessManager._get_sorted_progs(is_primary=True, reverse=True).values(): serv_name = self.get_systemd_serv_name(prog.service_name) os.system(f'{SYSTEMCTL} stop {serv_name}') self.assertIsNone(MCSProcessManager.start_node(True)) @@ -95,7 +95,7 @@ class MCSProcessManagerTest(BaseProcessDispatcherCase): ) ) - for prog in MCSProcessManager._get_sorted_progs(True).values(): + for prog in MCSProcessManager._get_sorted_progs(is_primary=True).values(): serv_name = self.get_systemd_serv_name(prog.service_name) os.system(f'{SYSTEMCTL} start {serv_name}') diff --git a/cmapi/cmapi_server/test/test_node_manip.py b/cmapi/cmapi_server/test/test_node_manip.py index 195c28649..8dd0bac2b 100644 --- a/cmapi/cmapi_server/test/test_node_manip.py +++ b/cmapi/cmapi_server/test/test_node_manip.py @@ -1,19 +1,17 @@ import logging import socket import unittest -from unittest.mock import patch, ANY +from unittest.mock import ANY, patch + from lxml import etree from cmapi_server import node_manipulation from cmapi_server.constants import MCS_DATA_PATH from cmapi_server.helpers import get_read_only_nodes from cmapi_server.node_manipulation import add_dbroots_of_other_nodes, remove_dbroots_of_node -from cmapi_server.test.unittest_global import ( - tmp_mcs_config_filename, BaseNodeManipTestCase -) +from cmapi_server.test.unittest_global import BaseNodeManipTestCase, tmp_mcs_config_filename from mcs_node_control.models.node_config import NodeConfig - logging.basicConfig(level='DEBUG') SINGLE_NODE_XML = "./cmapi_server/SingleNode.xml" @@ -83,7 +81,7 @@ class NodeManipTester(BaseNodeManipTestCase): root = nc.get_current_config_root(self.tmp_files[1]) # Check if read-only nodes section exists and is filled - read_only_nodes = get_read_only_nodes(root) + read_only_nodes = nc.get_read_only_nodes(root) self.assertEqual(len(read_only_nodes), 1) self.assertEqual(read_only_nodes[0], self.NEW_NODE_NAME) @@ -102,11 +100,12 @@ class NodeManipTester(BaseNodeManipTestCase): # Test read-only node removal node_manipulation.remove_node( self.NEW_NODE_NAME, self.tmp_files[1], self.tmp_files[2], + deactivate_only=False, ) nc = NodeConfig() root = nc.get_current_config_root(self.tmp_files[2]) - read_only_nodes = get_read_only_nodes(root) + read_only_nodes = nc.get_read_only_nodes(root) self.assertEqual(len(read_only_nodes), 0) mock_rebalance_dbroots.assert_not_called()