From 37453ad47ec1c07c118f4032a777f46f03285883 Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Wed, 12 Mar 2025 13:21:37 +0000 Subject: [PATCH 01/13] MCOL-5806: added ability to start node in read-only mode --- cmapi/cmapi_server/constants.py | 29 +++++++--- cmapi/cmapi_server/controllers/endpoints.py | 10 +++- cmapi/cmapi_server/failover_agent.py | 2 +- cmapi/cmapi_server/handlers/cluster.py | 24 +++++--- cmapi/cmapi_server/helpers.py | 11 ++++ cmapi/cmapi_server/managers/process.py | 35 ++++++++---- cmapi/cmapi_server/node_manipulation.py | 47 +++++++++++++--- cmapi/cmapi_server/test/test_cluster.py | 7 ++- cmapi/cmapi_server/test/test_node_manip.py | 59 ++++++++++++++++++++ cmapi/mcs_cluster_tool/cluster_app.py | 12 +++- cmapi/mcs_node_control/models/node_config.py | 19 ++++++- 11 files changed, 213 insertions(+), 42 deletions(-) diff --git a/cmapi/cmapi_server/constants.py b/cmapi/cmapi_server/constants.py index 464b61d99a..37790b7654 100644 --- a/cmapi/cmapi_server/constants.py +++ b/cmapi/cmapi_server/constants.py @@ -4,6 +4,7 @@ """ import os from typing import NamedTuple +from enum import Enum # default MARIADB ColumnStore config path @@ -53,6 +54,16 @@ CMAPI_INSTALL_PATH, 'cmapi_server/SingleNode.xml' ) +class MCSProgs(Enum): + STORAGE_MANAGER = 'StorageManager' + WORKER_NODE = 'workernode' + CONTROLLER_NODE = 'controllernode' + PRIM_PROC = 'PrimProc' + EXE_MGR = 'ExeMgr' + WRITE_ENGINE_SERVER = 'WriteEngineServer' + DML_PROC = 'DMLProc' + DDL_PROC = 'DDLProc' + # constants for dispatchers class ProgInfo(NamedTuple): """NamedTuple for some additional info about handling mcs processes.""" @@ -66,17 +77,17 @@ class ProgInfo(NamedTuple): # on top level of process handling # mcs-storagemanager starts conditionally inside mcs-loadbrm, but should be # stopped using cmapi -ALL_MCS_PROGS = { +ALL_MCS_PROGS: dict[str, ProgInfo] = { # workernode starts on primary and non primary node with 1 or 2 added # to subcommand (DBRM_Worker1 - on primary, DBRM_Worker2 - non primary) - 'StorageManager': ProgInfo(15, 'mcs-storagemanager', '', False, 1), - 'workernode': ProgInfo(13, 'mcs-workernode', 'DBRM_Worker{}', False, 1), - 'controllernode': ProgInfo(11, 'mcs-controllernode', 'fg', True), - 'PrimProc': ProgInfo(5, 'mcs-primproc', '', False, 1), - 'ExeMgr': ProgInfo(9, 'mcs-exemgr', '', False, 1), - 'WriteEngineServer': ProgInfo(7, 'mcs-writeengineserver', '', False, 3), - 'DMLProc': ProgInfo(3, 'mcs-dmlproc', '', False), - 'DDLProc': ProgInfo(1, 'mcs-ddlproc', '', False), + MCSProgs.STORAGE_MANAGER.value: ProgInfo(15, 'mcs-storagemanager', '', False, 1), + MCSProgs.WORKER_NODE.value: ProgInfo(13, 'mcs-workernode', 'DBRM_Worker{}', False, 1), + MCSProgs.CONTROLLER_NODE.value: ProgInfo(11, 'mcs-controllernode', 'fg', True), + MCSProgs.PRIM_PROC.value: ProgInfo(5, 'mcs-primproc', '', False, 1), + MCSProgs.EXE_MGR.value: ProgInfo(9, 'mcs-exemgr', '', False, 1), + MCSProgs.WRITE_ENGINE_SERVER.value: ProgInfo(7, 'mcs-writeengineserver', '', False, 3), + MCSProgs.DML_PROC.value: ProgInfo(3, 'mcs-dmlproc', '', False), + MCSProgs.DDL_PROC.value: ProgInfo(1, 'mcs-ddlproc', '', False), } # constants for docker container dispatcher diff --git a/cmapi/cmapi_server/controllers/endpoints.py b/cmapi/cmapi_server/controllers/endpoints.py index 870e07b350..d4059512ff 100644 --- a/cmapi/cmapi_server/controllers/endpoints.py +++ b/cmapi/cmapi_server/controllers/endpoints.py @@ -434,7 +434,8 @@ def put_config(self): MCSProcessManager.stop_node( is_primary=node_config.is_primary_node(), use_sudo=use_sudo, - timeout=request_timeout + timeout=request_timeout, + is_read_only=node_config.is_read_only(), ) except CMAPIBasicError as err: raise_422_error( @@ -463,6 +464,7 @@ def put_config(self): MCSProcessManager.start_node( is_primary=node_config.is_primary_node(), use_sudo=use_sudo, + is_read_only=node_config.is_read_only(), ) except CMAPIBasicError as err: raise_422_error( @@ -666,7 +668,8 @@ def put_start(self): try: MCSProcessManager.start_node( is_primary=node_config.is_primary_node(), - use_sudo=use_sudo + use_sudo=use_sudo, + is_read_only=node_config.is_read_only(), ) except CMAPIBasicError as err: raise_422_error( @@ -701,7 +704,8 @@ def put_shutdown(self): MCSProcessManager.stop_node( is_primary=node_config.is_primary_node(), use_sudo=use_sudo, - timeout=timeout + timeout=timeout, + is_read_only=node_config.is_read_only(), ) except CMAPIBasicError as err: raise_422_error( diff --git a/cmapi/cmapi_server/failover_agent.py b/cmapi/cmapi_server/failover_agent.py index 864715e090..9502d958af 100644 --- a/cmapi/cmapi_server/failover_agent.py +++ b/cmapi/cmapi_server/failover_agent.py @@ -95,7 +95,7 @@ def enterStandbyMode(self, test_mode = False): try: # TODO: remove test_mode condition and add mock for testing if not test_mode: - MCSProcessManager.stop_node(is_primary=nc.is_primary_node()) + MCSProcessManager.stop_node(is_primary=nc.is_primary_node(), is_read_only=nc.is_read_only()) logger.info( 'FA.enterStandbyMode(): successfully stopped node.' ) diff --git a/cmapi/cmapi_server/handlers/cluster.py b/cmapi/cmapi_server/handlers/cluster.py index 10a213fb70..245c7e2734 100644 --- a/cmapi/cmapi_server/handlers/cluster.py +++ b/cmapi/cmapi_server/handlers/cluster.py @@ -139,7 +139,10 @@ def shutdown( return {'timestamp': operation_start_time} @staticmethod - def add_node(node: str, config: str = DEFAULT_MCS_CONF_PATH) -> dict: + def add_node( + node: str, config: str = DEFAULT_MCS_CONF_PATH, + read_only: bool = False, + ) -> dict: """Method to add node to MCS CLuster. :param node: node IP or name or FQDN @@ -147,6 +150,8 @@ def add_node(node: str, config: str = DEFAULT_MCS_CONF_PATH) -> dict: :param config: columnstore xml config file path, defaults to DEFAULT_MCS_CONF_PATH :type config: str, optional + :param read_only: add node in read-only mode, defaults to False + :type read_only: bool, optional :raises CMAPIBasicError: on exception while starting transaction :raises CMAPIBasicError: if transaction start isn't successful :raises CMAPIBasicError: on exception while adding node @@ -157,20 +162,25 @@ def add_node(node: str, config: str = DEFAULT_MCS_CONF_PATH) -> dict: :rtype: dict """ logger: logging.Logger = logging.getLogger('cmapi_server') - logger.debug(f'Cluster add node command called. Adding node {node}.') + logger.debug('Cluster add node command called. Adding node %s in %s mode.', node, 'read-only' if read_only else 'read-write') response = {'timestamp': str(datetime.now())} try: add_node( node, input_config_filename=config, - output_config_filename=config + output_config_filename=config, + read_only=read_only, ) if not get_dbroots(node, config): - add_dbroot( - host=node, input_config_filename=config, - output_config_filename=config - ) + if not read_only: # Read-only nodes don't own dbroots + add_dbroot( + host=node, input_config_filename=config, + output_config_filename=config + ) + else: + logger.debug("Node %s is read-only, skipping dbroot addition", node) + except Exception as err: raise CMAPIBasicError('Error while adding node.') from err diff --git a/cmapi/cmapi_server/helpers.py b/cmapi/cmapi_server/helpers.py index 53fb003e78..9ef711088e 100644 --- a/cmapi/cmapi_server/helpers.py +++ b/cmapi/cmapi_server/helpers.py @@ -541,6 +541,10 @@ def get_desired_nodes(config=DEFAULT_MCS_CONF_PATH): return [ node.text for node in nodes ] +def get_read_only_nodes(root) -> list[str]: + return [node.text for node in root.findall("./ReadOnlyNodes/Node")] + + def in_maintenance_state(config=DEFAULT_MCS_CONF_PATH): nc = NodeConfig() root = nc.get_current_config_root(config, upgrade=False) @@ -577,6 +581,7 @@ def get_dbroots(node, config=DEFAULT_MCS_CONF_PATH): dbroots = [] smc_node = root.find('./SystemModuleConfig') mod_count = int(smc_node.find('./ModuleCount3').text) + for i in range(1, mod_count+1): ip_addr = smc_node.find(f'./ModuleIPAddr{i}-1-3').text hostname = smc_node.find(f'./ModuleHostName{i}-1-3').text @@ -596,6 +601,12 @@ def get_dbroots(node, config=DEFAULT_MCS_CONF_PATH): dbroots.append( smc_node.find(f"./ModuleDBRootID{i}-{j}-3").text ) + + if dbroots and nc.is_read_only(): + logger = logging.getLogger("dbroots") + logger.warning("Config contains dbroots %s for this read-only node, ignoring", dbroots) + return [] + return dbroots diff --git a/cmapi/cmapi_server/managers/process.py b/cmapi/cmapi_server/managers/process.py index 1adb62e8d6..8ddca4522d 100644 --- a/cmapi/cmapi_server/managers/process.py +++ b/cmapi/cmapi_server/managers/process.py @@ -7,7 +7,8 @@ import psutil from cmapi_server.exceptions import CMAPIBasicError -from cmapi_server.constants import MCS_INSTALL_BIN, ALL_MCS_PROGS +from cmapi_server.constants import MCS_INSTALL_BIN, ALL_MCS_PROGS, MCSProgs +from cmapi_server.process_dispatchers.base import BaseDispatcher from cmapi_server.process_dispatchers.systemd import SystemdDispatcher from cmapi_server.process_dispatchers.container import ( ContainerDispatcher @@ -18,7 +19,7 @@ from mcs_node_control.models.process import Process -PROCESS_DISPATCHERS = { +PROCESS_DISPATCHERS: dict[str, type[BaseDispatcher]] = { 'systemd': SystemdDispatcher, # could be used in docker containers and OSes w/o systemd 'container': ContainerDispatcher, @@ -404,19 +405,21 @@ def is_node_processes_ok( return set(node_progs) == set(p['name'] for p in running_procs) @classmethod - def start_node(cls, is_primary: bool, use_sudo: bool = True): + def start_node(cls, is_primary: bool, use_sudo: bool = True, is_read_only: bool = False) -> None: """Start mcs node processes. :param is_primary: is node primary or not, defaults to True :type is_primary: bool :param use_sudo: use sudo or not, defaults to True :type use_sudo: bool, optional + :param is_read_only: if true, doesn't start WriteEngine + :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): if ( cls.dispatcher_name == 'systemd' - and prog_name == 'StorageManager' + and prog_name == MCSProgs.STORAGE_MANAGER.value ): # TODO: MCOL-5458 logging.info( @@ -424,17 +427,24 @@ def start_node(cls, is_primary: bool, use_sudo: bool = True): ) continue # TODO: additional error handling - if prog_name == 'controllernode': + if prog_name == MCSProgs.CONTROLLER_NODE.value: cls._wait_for_workernodes() - if prog_name in ('DMLProc', 'DDLProc'): + 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}".') @classmethod def stop_node( - cls, is_primary: bool, use_sudo: bool = True, timeout: int = 10 + cls, + is_primary: bool, + use_sudo: bool = True, + timeout: int = 10, + is_read_only: bool = False, ): """Stop mcs node processes. @@ -444,6 +454,8 @@ def stop_node( :type use_sudo: bool, optional :param timeout: timeout for DMLProc gracefully stop using DBRM, seconds :type timeout: int + :param is_read_only: if true, doesn't stop WriteEngine + :type is_read_only: bool, optional :raises CMAPIBasicError: immediately if one mcs process not stopped """ # Every time try to stop all processes no matter primary it or slave, @@ -451,13 +463,16 @@ def stop_node( # undefined behaviour when primary gone and then recovers (failover # triggered 2 times). for prog_name in cls._get_sorted_progs(True, reverse=True): + if is_read_only and prog_name == MCSProgs.WRITE_ENGINE_SERVER.value: + logging.debug('Node is in read-only mode, not stopping WriteEngine') + continue 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}"') @classmethod - def restart_node(cls, is_primary: bool, use_sudo: bool): + def restart_node(cls, is_primary: bool, use_sudo: bool, is_read_only: bool = False): """TODO: For next releases.""" if cls.get_running_mcs_procs(): - cls.stop_node(is_primary, use_sudo) - cls.start_node(is_primary, use_sudo) + cls.stop_node(is_primary, use_sudo, is_read_only) + cls.start_node(is_primary, use_sudo, is_read_only) diff --git a/cmapi/cmapi_server/node_manipulation.py b/cmapi/cmapi_server/node_manipulation.py index 7eee0ad18e..8a96d26535 100644 --- a/cmapi/cmapi_server/node_manipulation.py +++ b/cmapi/cmapi_server/node_manipulation.py @@ -61,7 +61,8 @@ def switch_node_maintenance( def add_node( node: str, input_config_filename: str = DEFAULT_MCS_CONF_PATH, output_config_filename: Optional[str] = None, - use_rebalance_dbroots: bool = True + use_rebalance_dbroots: bool = True, + read_only: bool = False, ): """Add node to a cluster. @@ -95,14 +96,23 @@ def add_node( try: if not _replace_localhost(c_root, node): pm_num = _add_node_to_PMS(c_root, node) - _add_WES(c_root, pm_num, node) + + if not read_only: + _add_WES(c_root, pm_num, node) + else: + logging.info("Node is read-only, skipping WES addition") + _add_read_only_node(c_root, node) + _add_DBRM_Worker(c_root, node) _add_Module_entries(c_root, node) _add_active_node(c_root, node) _add_node_to_ExeMgrs(c_root, node) if use_rebalance_dbroots: - _rebalance_dbroots(c_root) - _move_primary_node(c_root) + if not read_only: + _rebalance_dbroots(c_root) + _move_primary_node(c_root) + else: + logging.debug("Node is read-only, skipping dbroots rebalancing") except Exception: logging.error( 'Caught exception while adding node, config file is unchanged', @@ -156,7 +166,11 @@ def remove_node( if len(active_nodes) > 1: pm_num = _remove_node_from_PMS(c_root, node) - _remove_WES(c_root, pm_num) + + is_read_only = node in helpers.get_read_only_nodes(c_root) + if not is_read_only: + _remove_WES(c_root, pm_num) + _remove_DBRM_Worker(c_root, node) _remove_Module_entries(c_root, node) _remove_from_ExeMgrs(c_root, node) @@ -167,7 +181,7 @@ def remove_node( # TODO: unspecific name, need to think of a better one _remove_node(c_root, node) - if use_rebalance_dbroots: + if use_rebalance_dbroots and not is_read_only: _rebalance_dbroots(c_root) _move_primary_node(c_root) else: @@ -375,12 +389,16 @@ def __remove_helper(parent_node, node): def _remove_node(root, node): ''' - remove node from DesiredNodes, InactiveNodes, and ActiveNodes + remove node from DesiredNodes, InactiveNodes, ActiveNodes and (if present) ReadOnlyNodes ''' for n in (root.find("./DesiredNodes"), root.find("./InactiveNodes"), root.find("./ActiveNodes")): __remove_helper(n, node) + read_only_nodes = root.find("./ReadOnlyNodes") + if read_only_nodes is not None: + __remove_helper(read_only_nodes, node) + # This moves a node from ActiveNodes to InactiveNodes def _deactivate_node(root, node): @@ -988,6 +1006,19 @@ def _add_WES(root, pm_num, node): etree.SubElement(wes_node, "Port").text = "8630" +def _add_read_only_node(root, node) -> None: + """Add node name to ReadOnlyNodes if it's not already there""" + read_only_nodes = root.find("./ReadOnlyNodes") + if read_only_nodes is None: + read_only_nodes = etree.SubElement(root, "ReadOnlyNodes") + else: + for n in read_only_nodes.findall("./Node"): + if n.text == node: + return + + etree.SubElement(read_only_nodes, "Node").text = node + + def _add_DBRM_Worker(root, node): ''' find the highest numbered DBRM_Worker entry, or one that isn't used atm @@ -1090,7 +1121,7 @@ def _add_node_to_PMS(root, node): return new_pm_num -def _replace_localhost(root, node): +def _replace_localhost(root: etree.Element, node: str) -> bool: # if DBRM_Controller/IPAddr is 127.0.0.1 or localhost, # then replace all instances, else do nothing. controller_host = root.find('./DBRM_Controller/IPAddr') diff --git a/cmapi/cmapi_server/test/test_cluster.py b/cmapi/cmapi_server/test/test_cluster.py index 7af9226c8d..903ee4e1e4 100644 --- a/cmapi/cmapi_server/test/test_cluster.py +++ b/cmapi/cmapi_server/test/test_cluster.py @@ -6,6 +6,7 @@ import requests +from cmapi_server.constants import MCSProgs from cmapi_server.controllers.dispatcher import _version from cmapi_server.managers.process import MCSProcessManager from cmapi_server.test.unittest_global import ( @@ -199,9 +200,13 @@ def test_endpoint(self): # Check Columntore started controllernode = subprocess.check_output( - ['pgrep', 'controllernode']) + ['pgrep', MCSProgs.CONTROLLER_NODE.value]) self.assertIsNotNone(controllernode) + # Check that WriteEngineServer was started + wes = subprocess.check_output(['pgrep', MCSProgs.WRITE_ENGINE_SERVER.value]) + self.assertIsNotNone(wes) + class ClusterRemoveNodeTestCase(BaseClusterTestCase): URL = ClusterAddNodeTestCase.URL diff --git a/cmapi/cmapi_server/test/test_node_manip.py b/cmapi/cmapi_server/test/test_node_manip.py index 22a35c64a7..bfdf122622 100644 --- a/cmapi/cmapi_server/test/test_node_manip.py +++ b/cmapi/cmapi_server/test/test_node_manip.py @@ -1,10 +1,12 @@ import logging import socket +from unittest.mock import 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.test.unittest_global import ( tmp_mcs_config_filename, BaseNodeManipTestCase ) @@ -13,6 +15,8 @@ logging.basicConfig(level='DEBUG') +SINGLE_NODE_XML = "./cmapi_server/SingleNode.xml" + class NodeManipTester(BaseNodeManipTestCase): @@ -52,6 +56,61 @@ def test_add_remove_node(self): # node = root.find('./PMS2/IPAddr') # self.assertEqual(node, None) + def test_add_remove_read_only_node(self): + """add_node(read_only=True) should add a read-only node into the config, it does not add a WriteEngineServer (WES) and does not own dbroots""" + self.tmp_files = ('./config_output_rw.xml', './config_output_ro.xml', './config_output_ro_removed.xml') + + # Add this host as a read-write node + local_host_addr = socket.gethostbyname(socket.gethostname()) + node_manipulation.add_node( + local_host_addr, SINGLE_NODE_XML, self.tmp_files[0] + ) + + # Mock _rebalance_dbroots and _move_primary_node (only after the first node is added) + with patch('cmapi_server.node_manipulation._rebalance_dbroots') as mock_rebalance_dbroots, \ + patch('cmapi_server.node_manipulation._move_primary_node') as mock_move_primary_node: + + # Add a read-only node + node_manipulation.add_node( + self.NEW_NODE_NAME, self.tmp_files[0], self.tmp_files[1], + read_only=True, + ) + + nc = NodeConfig() + 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) + self.assertEqual(len(read_only_nodes), 1) + self.assertEqual(read_only_nodes[0], self.NEW_NODE_NAME) + + # Check if PMS was added + pms_node_ipaddr = root.find('./PMS2/IPAddr') + self.assertEqual(pms_node_ipaddr.text, self.NEW_NODE_NAME) + + # Check that WriteEngineServer was not added + wes_node = root.find('./pm2_WriteEngineServer') + self.assertIsNone(wes_node) + + # Check that the dbroot related methods were not called + mock_rebalance_dbroots.assert_not_called() + mock_move_primary_node.assert_not_called() + + # Test read-only node removal + node_manipulation.remove_node( + self.NEW_NODE_NAME, self.tmp_files[1], self.tmp_files[2], + ) + + nc = NodeConfig() + root = nc.get_current_config_root(self.tmp_files[2]) + read_only_nodes = get_read_only_nodes(root) + self.assertEqual(len(read_only_nodes), 0) + + # Check that dbroot related methods were not called + mock_rebalance_dbroots.assert_not_called() + mock_move_primary_node.assert_not_called() + + def test_add_dbroots_nodes_rebalance(self): self.tmp_files = ( './extra-dbroots-0.xml', './extra-dbroots-1.xml', diff --git a/cmapi/mcs_cluster_tool/cluster_app.py b/cmapi/mcs_cluster_tool/cluster_app.py index d93c8cc2c9..9b0f37a6f1 100644 --- a/cmapi/mcs_cluster_tool/cluster_app.py +++ b/cmapi/mcs_cluster_tool/cluster_app.py @@ -198,6 +198,14 @@ def add( 'node IP, name or FQDN. ' 'Can be used multiple times to add several nodes at a time.' ) + ), + read_only: bool = typer.Option( + False, + '--read-only', + help=( + 'Add node (or nodes, if more than one is passed) in read-only ' + 'mode.' + ) ) ): """Add nodes to the Columnstore cluster.""" @@ -207,7 +215,9 @@ def add( extra_nodes=nodes ): for node in nodes: - result.append(client.add_node({'node': node})) + result.append( + client.add_node({'node': node, 'read_only': read_only}) + ) return result diff --git a/cmapi/mcs_node_control/models/node_config.py b/cmapi/mcs_node_control/models/node_config.py index 7dac18bce4..2943d4c98a 100644 --- a/cmapi/mcs_node_control/models/node_config.py +++ b/cmapi/mcs_node_control/models/node_config.py @@ -36,7 +36,7 @@ class NodeConfig: """ def get_current_config_root( self, config_filename: str = DEFAULT_MCS_CONF_PATH, upgrade=True - ): + ) -> etree.Element: """Retrieves current configuration. Read the config and returns Element. @@ -49,7 +49,7 @@ def get_current_config_root( self.upgrade_config(tree=tree, upgrade=upgrade) return tree.getroot() - def get_root_from_string(self, config_string: str): + def get_root_from_string(self, config_string: str) -> etree.Element: root = etree.fromstring(config_string) self.upgrade_config(root=root) return root @@ -566,4 +566,19 @@ def get_all_dbroots(self, root): for i in range(1, mod_count+1): for j in range(1, int(smc_node.find(f"./ModuleDBRootCount{i}-3").text) + 1): dbroots.append(smc_node.find(f"./ModuleDBRootID{i}-{j}-3").text) + + # TODO not sure about it + if dbroots and self.is_read_only(root): + module_logger.warning("Config contains dbroots %s for this read-only node, ignoring", dbroots) + return [] + return dbroots + + def is_read_only(self, root=None) -> bool: + """Checks if this node is in read-only mode""" + from cmapi_server.helpers import get_read_only_nodes # Avoid circular import + + root = root or self.get_current_config_root() + read_only_nodes = set(get_read_only_nodes(root)) + my_names = set(self.get_network_addresses_and_names()) + return bool(read_only_nodes.intersection(my_names)) \ No newline at end of file From 9a1ac0eb74796584783af3d454de03fb32b0284d Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Tue, 1 Apr 2025 14:51:52 +0000 Subject: [PATCH 02/13] Part of review changes to retrigger package build --- cmapi/cmapi_server/failover_agent.py | 5 ++++- cmapi/cmapi_server/handlers/cluster.py | 7 +++++-- cmapi/cmapi_server/helpers.py | 5 +++-- cmapi/cmapi_server/managers/process.py | 12 ++++++++++-- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/cmapi/cmapi_server/failover_agent.py b/cmapi/cmapi_server/failover_agent.py index 9502d958af..1e4f5fdac7 100644 --- a/cmapi/cmapi_server/failover_agent.py +++ b/cmapi/cmapi_server/failover_agent.py @@ -95,7 +95,10 @@ def enterStandbyMode(self, test_mode = False): try: # TODO: remove test_mode condition and add mock for testing if not test_mode: - MCSProcessManager.stop_node(is_primary=nc.is_primary_node(), is_read_only=nc.is_read_only()) + MCSProcessManager.stop_node( + is_primary=nc.is_primary_node(), + is_read_only=nc.is_read_only(), + ) logger.info( 'FA.enterStandbyMode(): successfully stopped node.' ) diff --git a/cmapi/cmapi_server/handlers/cluster.py b/cmapi/cmapi_server/handlers/cluster.py index 245c7e2734..f672d1762d 100644 --- a/cmapi/cmapi_server/handlers/cluster.py +++ b/cmapi/cmapi_server/handlers/cluster.py @@ -162,7 +162,10 @@ def add_node( :rtype: dict """ logger: logging.Logger = logging.getLogger('cmapi_server') - logger.debug('Cluster add node command called. Adding node %s in %s mode.', node, 'read-only' if read_only else 'read-write') + logger.debug( + 'Cluster add node command called. ' + f'Adding node {node} in {"read-only" if read_only else "read-write"} mode.' + ) response = {'timestamp': str(datetime.now())} @@ -179,7 +182,7 @@ def add_node( output_config_filename=config ) else: - logger.debug("Node %s is read-only, skipping dbroot addition", node) + logger.debug(f"Node {node} is read-only, skipping dbroot addition") except Exception as err: raise CMAPIBasicError('Error while adding node.') from err diff --git a/cmapi/cmapi_server/helpers.py b/cmapi/cmapi_server/helpers.py index 9ef711088e..3efc134296 100644 --- a/cmapi/cmapi_server/helpers.py +++ b/cmapi/cmapi_server/helpers.py @@ -542,6 +542,7 @@ def get_desired_nodes(config=DEFAULT_MCS_CONF_PATH): def get_read_only_nodes(root) -> list[str]: + """Get names of read-only nodes from config""" return [node.text for node in root.findall("./ReadOnlyNodes/Node")] @@ -603,8 +604,8 @@ def get_dbroots(node, config=DEFAULT_MCS_CONF_PATH): ) if dbroots and nc.is_read_only(): - logger = logging.getLogger("dbroots") - logger.warning("Config contains dbroots %s for this read-only node, ignoring", dbroots) + logger = logging.getLogger('dbroots') + logger.warning(f'Config contains dbroots {dbroots} for this read-only node, ignoring') return [] return dbroots diff --git a/cmapi/cmapi_server/managers/process.py b/cmapi/cmapi_server/managers/process.py index 8ddca4522d..2feaec2673 100644 --- a/cmapi/cmapi_server/managers/process.py +++ b/cmapi/cmapi_server/managers/process.py @@ -405,7 +405,12 @@ def is_node_processes_ok( return set(node_progs) == set(p['name'] for p in running_procs) @classmethod - def start_node(cls, is_primary: bool, use_sudo: bool = True, is_read_only: bool = False) -> None: + def start_node( + cls, + is_primary: bool, + use_sudo: bool = True, + is_read_only: bool = False, + ) -> None: """Start mcs node processes. :param is_primary: is node primary or not, defaults to True @@ -431,7 +436,10 @@ def start_node(cls, is_primary: bool, use_sudo: bool = True, is_read_only: bool 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: + 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): From 93cd9354a906da5ec560fb985f83f6817679406b Mon Sep 17 00:00:00 2001 From: mariadb-AlanMologorsky Date: Wed, 2 Apr 2025 21:44:54 +0300 Subject: [PATCH 03/13] feat(cmapi): MCOL-5806 Review and rebase interface fixes. * feat(cmapi): add read_only param for API add node endpoint * style(cmapi): fixes for string length and quotes --- cmapi/cmapi_server/controllers/api_clients.py | 1 + cmapi/cmapi_server/controllers/endpoints.py | 5 +++-- cmapi/cmapi_server/handlers/cluster.py | 8 +++++--- cmapi/cmapi_server/helpers.py | 2 +- cmapi/cmapi_server/node_manipulation.py | 6 ++++-- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/cmapi/cmapi_server/controllers/api_clients.py b/cmapi/cmapi_server/controllers/api_clients.py index 7b7e696220..d7ea2cf80a 100644 --- a/cmapi/cmapi_server/controllers/api_clients.py +++ b/cmapi/cmapi_server/controllers/api_clients.py @@ -68,6 +68,7 @@ def add_node( :param node_info: Information about the node to add. :return: The response from the API. """ + #TODO: fix interface as in remove_node used or think about universal return self._request('PUT', 'node', {**node_info, **extra}) def remove_node( diff --git a/cmapi/cmapi_server/controllers/endpoints.py b/cmapi/cmapi_server/controllers/endpoints.py index d4059512ff..360c1fad8c 100644 --- a/cmapi/cmapi_server/controllers/endpoints.py +++ b/cmapi/cmapi_server/controllers/endpoints.py @@ -914,6 +914,7 @@ def put_add_node(self): node = request_body.get('node', None) config = request_body.get('config', DEFAULT_MCS_CONF_PATH) in_transaction = request_body.get('in_transaction', False) + read_only = request_body.get('read_only', False) if node is None: raise_422_error(module_logger, func_name, 'missing node argument') @@ -921,9 +922,9 @@ def put_add_node(self): try: if not in_transaction: with TransactionManager(extra_nodes=[node]): - response = ClusterHandler.add_node(node, config) + response = ClusterHandler.add_node(node, config, read_only) else: - response = ClusterHandler.add_node(node, config) + response = ClusterHandler.add_node(node, config, read_only) except CMAPIBasicError as err: raise_422_error(module_logger, func_name, err.message) diff --git a/cmapi/cmapi_server/handlers/cluster.py b/cmapi/cmapi_server/handlers/cluster.py index f672d1762d..1cc9a1d6ff 100644 --- a/cmapi/cmapi_server/handlers/cluster.py +++ b/cmapi/cmapi_server/handlers/cluster.py @@ -163,8 +163,8 @@ def add_node( """ logger: logging.Logger = logging.getLogger('cmapi_server') logger.debug( - 'Cluster add node command called. ' - f'Adding node {node} in {"read-only" if read_only else "read-write"} mode.' + f'Cluster add node command called. Adding node {node} in ' + f'{"read-only" if read_only else "read-write"} mode.' ) response = {'timestamp': str(datetime.now())} @@ -182,7 +182,9 @@ def add_node( output_config_filename=config ) else: - logger.debug(f"Node {node} is read-only, skipping dbroot addition") + logger.debug( + f'Node {node} is read-only, skipping dbroot addition' + ) except Exception as err: raise CMAPIBasicError('Error while adding node.') from err diff --git a/cmapi/cmapi_server/helpers.py b/cmapi/cmapi_server/helpers.py index 3efc134296..7f4349956a 100644 --- a/cmapi/cmapi_server/helpers.py +++ b/cmapi/cmapi_server/helpers.py @@ -543,7 +543,7 @@ def get_desired_nodes(config=DEFAULT_MCS_CONF_PATH): def get_read_only_nodes(root) -> list[str]: """Get names of read-only nodes from config""" - return [node.text for node in root.findall("./ReadOnlyNodes/Node")] + return [node.text for node in root.findall('./ReadOnlyNodes/Node')] def in_maintenance_state(config=DEFAULT_MCS_CONF_PATH): diff --git a/cmapi/cmapi_server/node_manipulation.py b/cmapi/cmapi_server/node_manipulation.py index 8a96d26535..5e19c14889 100644 --- a/cmapi/cmapi_server/node_manipulation.py +++ b/cmapi/cmapi_server/node_manipulation.py @@ -100,7 +100,7 @@ def add_node( if not read_only: _add_WES(c_root, pm_num, node) else: - logging.info("Node is read-only, skipping WES addition") + logging.info('Node is read-only, skipping WES addition.') _add_read_only_node(c_root, node) _add_DBRM_Worker(c_root, node) @@ -112,7 +112,9 @@ def add_node( _rebalance_dbroots(c_root) _move_primary_node(c_root) else: - logging.debug("Node is read-only, skipping dbroots rebalancing") + logging.debug( + 'Node is read-only, skipping dbroots rebalancing.' + ) except Exception: logging.error( 'Caught exception while adding node, config file is unchanged', From fbf4844e7155b8324f34c0e5fa3547eda36dab2c Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Wed, 2 Apr 2025 19:51:14 +0000 Subject: [PATCH 04/13] Zero-diff to trigger package rebuild --- cmapi/cmapi_server/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmapi/cmapi_server/helpers.py b/cmapi/cmapi_server/helpers.py index 7f4349956a..a899e3ee63 100644 --- a/cmapi/cmapi_server/helpers.py +++ b/cmapi/cmapi_server/helpers.py @@ -542,7 +542,7 @@ def get_desired_nodes(config=DEFAULT_MCS_CONF_PATH): def get_read_only_nodes(root) -> list[str]: - """Get names of read-only nodes from config""" + """Get names of read only nodes from config""" return [node.text for node in root.findall('./ReadOnlyNodes/Node')] From 3671d4953f8cd40c8a449b69c7337ec213a5baeb Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Thu, 3 Apr 2025 18:22:29 +0000 Subject: [PATCH 05/13] Remove WES stop exclusion logic --- cmapi/cmapi_server/managers/process.py | 3 --- cmapi/cmapi_server/node_manipulation.py | 3 +++ cmapi/mcs_cluster_tool/__main__.py | 5 ++++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/cmapi/cmapi_server/managers/process.py b/cmapi/cmapi_server/managers/process.py index 2feaec2673..9b0d2264d0 100644 --- a/cmapi/cmapi_server/managers/process.py +++ b/cmapi/cmapi_server/managers/process.py @@ -471,9 +471,6 @@ def stop_node( # undefined behaviour when primary gone and then recovers (failover # triggered 2 times). for prog_name in cls._get_sorted_progs(True, reverse=True): - if is_read_only and prog_name == MCSProgs.WRITE_ENGINE_SERVER.value: - logging.debug('Node is in read-only mode, not stopping WriteEngine') - continue 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 5e19c14889..9751978910 100644 --- a/cmapi/cmapi_server/node_manipulation.py +++ b/cmapi/cmapi_server/node_manipulation.py @@ -1016,6 +1016,9 @@ def _add_read_only_node(root, node) -> None: else: for n in read_only_nodes.findall("./Node"): if n.text == node: + logging.warning( + f"_add_read_only_node(): node {node} already exists in ReadOnlyNodes" + ) return etree.SubElement(read_only_nodes, "Node").text = node diff --git a/cmapi/mcs_cluster_tool/__main__.py b/cmapi/mcs_cluster_tool/__main__.py index 4c15284b6c..3b0d9d0bf2 100644 --- a/cmapi/mcs_cluster_tool/__main__.py +++ b/cmapi/mcs_cluster_tool/__main__.py @@ -70,7 +70,10 @@ def setup_logging(verbose: bool = False) -> None: add_logging_level('TRACE', 5) dict_config(MCS_CLI_LOG_CONF_PATH) if verbose: - enable_console_logging(logging.getLogger()) + for logger_name in ("", "mcs_cli"): + logger = logging.getLogger(logger_name) + logger.setLevel(logging.DEBUG) + enable_console_logging(logger) if __name__ == '__main__': From 1714befa642713148e0f056e9ac39a443cab5a2d Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Thu, 3 Apr 2025 18:47:26 +0000 Subject: [PATCH 06/13] Zero-diff to retrigger build --- cmapi/mcs_cluster_tool/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmapi/mcs_cluster_tool/__main__.py b/cmapi/mcs_cluster_tool/__main__.py index 3b0d9d0bf2..02e73d3b68 100644 --- a/cmapi/mcs_cluster_tool/__main__.py +++ b/cmapi/mcs_cluster_tool/__main__.py @@ -70,7 +70,7 @@ def setup_logging(verbose: bool = False) -> None: add_logging_level('TRACE', 5) dict_config(MCS_CLI_LOG_CONF_PATH) if verbose: - for logger_name in ("", "mcs_cli"): + for logger_name in ('', 'mcs_cli'): logger = logging.getLogger(logger_name) logger.setLevel(logging.DEBUG) enable_console_logging(logger) From 7255340ae5b343fc7a53150d3fdbb30d97d1ed54 Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Fri, 18 Apr 2025 02:07:15 +0000 Subject: [PATCH 07/13] Add dbroots of other nodes to the read-only node --- cmapi/cmapi_server/managers/transaction.py | 4 +- cmapi/cmapi_server/node_manipulation.py | 81 ++++++++++++++++++---- cmapi/cmapi_server/test/test_cluster.py | 4 -- cmapi/cmapi_server/test/test_node_manip.py | 67 ++++++++++++++++-- 4 files changed, 133 insertions(+), 23 deletions(-) diff --git a/cmapi/cmapi_server/managers/transaction.py b/cmapi/cmapi_server/managers/transaction.py index 68bb7bc77c..98de505241 100644 --- a/cmapi/cmapi_server/managers/transaction.py +++ b/cmapi/cmapi_server/managers/transaction.py @@ -106,10 +106,10 @@ def rollback_transaction(self, nodes: Optional[list] = None) -> None: try: rollback_transaction(self.txn_id, nodes=nodes) self.active_transaction = False - logging.debug(f'Success rollback of transaction "{self.txn_id}".') + logging.debug(f'Successfull rollback of transaction "{self.txn_id}".') except Exception: logging.error( - f'Error while rollback transaction "{self.txn_id}"', + f'Error while rolling back transaction "{self.txn_id}"', exc_info=True ) diff --git a/cmapi/cmapi_server/node_manipulation.py b/cmapi/cmapi_server/node_manipulation.py index 9751978910..0a3f4c4c72 100644 --- a/cmapi/cmapi_server/node_manipulation.py +++ b/cmapi/cmapi_server/node_manipulation.py @@ -112,9 +112,7 @@ def add_node( _rebalance_dbroots(c_root) _move_primary_node(c_root) else: - logging.debug( - 'Node is read-only, skipping dbroots rebalancing.' - ) + add_dbroots_of_other_nodes(c_root, pm_num) except Exception: logging.error( 'Caught exception while adding node, config file is unchanged', @@ -186,6 +184,9 @@ def remove_node( if use_rebalance_dbroots and not is_read_only: _rebalance_dbroots(c_root) _move_primary_node(c_root) + + if is_read_only: + remove_dbroots_of_node(c_root, pm_num) else: # TODO: # - IMO undefined behaviour here. Removing one single node @@ -548,6 +549,19 @@ def unassign_dbroot1(root): i += 1 +def _get_existing_db_roots(root: etree.Element) -> list[int]: + '''Get all the existing dbroot IDs from the config file''' + # There can be holes in the dbroot numbering, so can't just scan from [1-dbroot_count] + # Going to scan from 1-99 instead + sysconf_node = root.find("./SystemConfig") + existing_dbroots = [] + for num in range(1, 100): + node = sysconf_node.find(f"./DBRoot{num}") + if node is not None: + existing_dbroots.append(num) + return existing_dbroots + + def _rebalance_dbroots(root, test_mode=False): # TODO: add code to detect whether we are using shared storage or not. If not, exit # without doing anything. @@ -591,14 +605,7 @@ def _rebalance_dbroots(root, test_mode=False): current_mapping = get_current_dbroot_mapping(root) sysconf_node = root.find("./SystemConfig") - - # There can be holes in the dbroot numbering, so can't just scan from [1-dbroot_count] - # Going to scan from 1-99 instead. - existing_dbroots = [] - for num in range(1, 100): - node = sysconf_node.find(f"./DBRoot{num}") - if node is not None: - existing_dbroots.append(num) + existing_dbroots = _get_existing_db_roots(root) # assign the unassigned dbroots unassigned_dbroots = set(existing_dbroots) - set(current_mapping[0]) @@ -650,7 +657,7 @@ def _rebalance_dbroots(root, test_mode=False): # timed out # possible node is not ready, leave retry as-is pass - except Exception as e: + except Exception: retry = False if not found_master: @@ -986,7 +993,7 @@ def _add_Module_entries(root, node): logging.info(f"_add_Module_entries(): hostname doesn't match, updating address to {new_ip_addr}") smc_node.find(f"ModuleHostName{i}-1-3").text = new_ip_addr else: - logging.info(f"_add_Module_entries(): no update is necessary") + logging.info("_add_Module_entries(): no update is necessary") return # if we find a matching hostname, update the ip addr @@ -1174,3 +1181,51 @@ def _replace_localhost(root: etree.Element, node: str) -> bool: # New Exception types class NodeNotFoundException(Exception): pass + + +def add_dbroots_of_other_nodes(root: etree.Element, module_num: int) -> None: + """Adds all the dbroots listed in the config to this (read-only) node""" + existing_dbroots = _get_existing_db_roots(root) + sysconf_node = root.find("./SystemModuleConfig") + + # Write node's dbroot count + dbroot_count_node = sysconf_node.find(f"./ModuleDBRootCount{module_num}-3") + if dbroot_count_node is not None: + sysconf_node.remove(dbroot_count_node) + dbroot_count_node = etree.SubElement( + sysconf_node, f"ModuleDBRootCount{module_num}-3" + ) + dbroot_count_node.text = str(len(existing_dbroots)) + + # Remove existing dbroot IDs of this module if present + for i in range(1, 100): + dbroot_id_node = sysconf_node.find(f"./ModuleDBRootID{module_num}-{i}-3") + if dbroot_id_node is not None: + sysconf_node.remove(dbroot_id_node) + + # Write new dbroot IDs to the module mapping + for i, dbroot_id in enumerate(existing_dbroots, start=1): + dbroot_id_node = etree.SubElement( + sysconf_node, f"ModuleDBRootID{module_num}-{i}-3" + ) + dbroot_id_node.text = str(dbroot_id) + + logging.info("Added %d dbroots to read-only node %d: %s", len(existing_dbroots), module_num, sorted(existing_dbroots)) + + +def remove_dbroots_of_node(root: etree.Element, module_num: int) -> None: + """Removes all the dbroots listed in the config from this (read-only) node""" + sysconf_node = root.find("./SystemModuleConfig") + dbroot_count_node = sysconf_node.find(f"./ModuleDBRootCount{module_num}-3") + if dbroot_count_node is not None: + sysconf_node.remove(dbroot_count_node) + else: + logging.error( + f"ModuleDBRootCount{module_num}-3 not found in SystemModuleConfig" + ) + + # Remove existing dbroot IDs + for i in range(1, 100): + dbroot_id_node = sysconf_node.find(f"./ModuleDBRootID{module_num}-{i}-3") + if dbroot_id_node is not None: + sysconf_node.remove(dbroot_id_node) \ No newline at end of file diff --git a/cmapi/cmapi_server/test/test_cluster.py b/cmapi/cmapi_server/test/test_cluster.py index 903ee4e1e4..a3f1980fb6 100644 --- a/cmapi/cmapi_server/test/test_cluster.py +++ b/cmapi/cmapi_server/test/test_cluster.py @@ -203,10 +203,6 @@ def test_endpoint(self): ['pgrep', MCSProgs.CONTROLLER_NODE.value]) self.assertIsNotNone(controllernode) - # Check that WriteEngineServer was started - wes = subprocess.check_output(['pgrep', MCSProgs.WRITE_ENGINE_SERVER.value]) - self.assertIsNotNone(wes) - class ClusterRemoveNodeTestCase(BaseClusterTestCase): URL = ClusterAddNodeTestCase.URL diff --git a/cmapi/cmapi_server/test/test_node_manip.py b/cmapi/cmapi_server/test/test_node_manip.py index bfdf122622..d52953ed5c 100644 --- a/cmapi/cmapi_server/test/test_node_manip.py +++ b/cmapi/cmapi_server/test/test_node_manip.py @@ -1,12 +1,13 @@ import logging import socket - +import unittest from unittest.mock import 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 ) @@ -68,7 +69,9 @@ def test_add_remove_read_only_node(self): # Mock _rebalance_dbroots and _move_primary_node (only after the first node is added) with patch('cmapi_server.node_manipulation._rebalance_dbroots') as mock_rebalance_dbroots, \ - patch('cmapi_server.node_manipulation._move_primary_node') as mock_move_primary_node: + patch('cmapi_server.node_manipulation._move_primary_node') as mock_move_primary_node, \ + patch('cmapi_server.node_manipulation.add_dbroots_of_other_nodes') as mock_add_dbroots_of_other_nodes, \ + patch('cmapi_server.node_manipulation.remove_dbroots_of_node') as mock_remove_dbroots_of_node: # Add a read-only node node_manipulation.add_node( @@ -92,9 +95,9 @@ def test_add_remove_read_only_node(self): wes_node = root.find('./pm2_WriteEngineServer') self.assertIsNone(wes_node) - # Check that the dbroot related methods were not called mock_rebalance_dbroots.assert_not_called() mock_move_primary_node.assert_not_called() + mock_add_dbroots_of_other_nodes.assert_called_once_with(root, 2) # Test read-only node removal node_manipulation.remove_node( @@ -106,9 +109,9 @@ def test_add_remove_read_only_node(self): read_only_nodes = get_read_only_nodes(root) self.assertEqual(len(read_only_nodes), 0) - # Check that dbroot related methods were not called mock_rebalance_dbroots.assert_not_called() mock_move_primary_node.assert_not_called() + mock_remove_dbroots_of_node.assert_called_once_with(root, 2) def test_add_dbroots_nodes_rebalance(self): @@ -268,3 +271,59 @@ def test_unassign_dbroot1(self): caught_it = True self.assertTrue(caught_it) + + +class TestReadOnlyNodeDBRootsManip(unittest.TestCase): + our_module_idx = 2 + + def setUp(self): + # Mock initial XML structure (add two dbroots) + self.root = etree.Element('Columnstore') + etree.SubElement(self.root, 'SystemModuleConfig') + system_config = etree.SubElement(self.root, 'SystemConfig') + dbroot_count = etree.SubElement(system_config, 'DBRootCount') + dbroot_count.text = '2' + dbroot1 = etree.SubElement(system_config, 'DBRoot1') + dbroot1.text = '/data/dbroot1' + dbroot2 = etree.SubElement(system_config, 'DBRoot2') + dbroot2.text = '/data/dbroot2' + + def test_add_dbroots_of_other_nodes(self): + '''add_dbroots_of_other_nodes must add dbroots of other nodes into mapping of the node.''' + add_dbroots_of_other_nodes(self.root, self.our_module_idx) + + # Check that ModuleDBRootCount of the module was updated + module_count = self.root.find(f'./SystemModuleConfig/ModuleDBRootCount{self.our_module_idx}-3') + self.assertIsNotNone(module_count) + self.assertEqual(module_count.text, '2') + + # Check that dbroots were added to ModuleDBRootID{module_num}-{i}-3 + dbroot1 = self.root.find(f'./SystemModuleConfig/ModuleDBRootID{self.our_module_idx}-1-3') + dbroot2 = self.root.find(f'./SystemModuleConfig/ModuleDBRootID{self.our_module_idx}-2-3') + self.assertIsNotNone(dbroot1) + self.assertIsNotNone(dbroot2) + self.assertEqual(dbroot1.text, '1') + self.assertEqual(dbroot2.text, '2') + + def test_remove_dbroots_of_node(self): + '''Test that remove_dbroots_of_node correctly removes dbroots from the node's mapping''' + # Add dbroot association to the node + smc = self.root.find('./SystemModuleConfig') + dbroot1 = etree.SubElement(smc, f'ModuleDBRootID{self.our_module_idx}-1-3') + dbroot1.text = '1' + dbroot2 = etree.SubElement(smc, f'ModuleDBRootID{self.our_module_idx}-2-3') + dbroot2.text = '2' + # Add ModuleDBRootCount to the node + module_count = etree.SubElement(smc, f'ModuleDBRootCount{self.our_module_idx}-3') + module_count.text = '2' + + remove_dbroots_of_node(self.root, self.our_module_idx) + + # Check that ModuleDBRootCount was removed + module_count = self.root.find(f'./SystemModuleConfig/ModuleDBRootCount{self.our_module_idx}-3') + self.assertIsNone(module_count) + # Check that dbroot mappings of the module were removed + dbroot1 = self.root.find(f'./SystemModuleConfig/ModuleDBRootID{self.our_module_idx}-1-3') + dbroot2 = self.root.find(f'./SystemModuleConfig/ModuleDBRootID{self.our_module_idx}-2-3') + self.assertIsNone(dbroot1) + self.assertIsNone(dbroot2) From 04015ae41415f30bff538197f7244a8adc2ca0d8 Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Fri, 18 Apr 2025 03:59:40 +0000 Subject: [PATCH 08/13] Review fixes and ruff settings --- cmapi/cmapi_server/helpers.py | 6 ------ cmapi/cmapi_server/node_manipulation.py | 10 ++++----- cmapi/cmapi_server/test/test_node_manip.py | 6 +++--- cmapi/mcs_node_control/models/node_config.py | 5 ----- cmapi/pyproject.toml | 22 ++++++++++++++++++++ 5 files changed, 30 insertions(+), 19 deletions(-) create mode 100644 cmapi/pyproject.toml diff --git a/cmapi/cmapi_server/helpers.py b/cmapi/cmapi_server/helpers.py index a899e3ee63..e022d35033 100644 --- a/cmapi/cmapi_server/helpers.py +++ b/cmapi/cmapi_server/helpers.py @@ -11,7 +11,6 @@ import socket import time from collections import namedtuple -from functools import partial from random import random from shutil import copyfile from typing import Tuple, Optional @@ -603,11 +602,6 @@ def get_dbroots(node, config=DEFAULT_MCS_CONF_PATH): smc_node.find(f"./ModuleDBRootID{i}-{j}-3").text ) - if dbroots and nc.is_read_only(): - logger = logging.getLogger('dbroots') - logger.warning(f'Config contains dbroots {dbroots} for this read-only node, ignoring') - return [] - return dbroots diff --git a/cmapi/cmapi_server/node_manipulation.py b/cmapi/cmapi_server/node_manipulation.py index 0a3f4c4c72..4c2e2ff1db 100644 --- a/cmapi/cmapi_server/node_manipulation.py +++ b/cmapi/cmapi_server/node_manipulation.py @@ -398,7 +398,7 @@ def _remove_node(root, node): for n in (root.find("./DesiredNodes"), root.find("./InactiveNodes"), root.find("./ActiveNodes")): __remove_helper(n, node) - read_only_nodes = root.find("./ReadOnlyNodes") + read_only_nodes = root.find('./ReadOnlyNodes') if read_only_nodes is not None: __remove_helper(read_only_nodes, node) @@ -1015,11 +1015,11 @@ def _add_WES(root, pm_num, node): etree.SubElement(wes_node, "Port").text = "8630" -def _add_read_only_node(root, node) -> None: - """Add node name to ReadOnlyNodes if it's not already there""" - read_only_nodes = root.find("./ReadOnlyNodes") +def _add_read_only_node(root: etree.Element, node: str) -> None: + '''Add node name to ReadOnlyNodes if it is not already there''' + read_only_nodes = root.find('./ReadOnlyNodes') if read_only_nodes is None: - read_only_nodes = etree.SubElement(root, "ReadOnlyNodes") + read_only_nodes = etree.SubElement(root, 'ReadOnlyNodes') else: for n in read_only_nodes.findall("./Node"): if n.text == node: diff --git a/cmapi/cmapi_server/test/test_node_manip.py b/cmapi/cmapi_server/test/test_node_manip.py index d52953ed5c..195c286499 100644 --- a/cmapi/cmapi_server/test/test_node_manip.py +++ b/cmapi/cmapi_server/test/test_node_manip.py @@ -1,7 +1,7 @@ import logging import socket import unittest -from unittest.mock import patch +from unittest.mock import patch, ANY from lxml import etree from cmapi_server import node_manipulation @@ -97,7 +97,7 @@ def test_add_remove_read_only_node(self): mock_rebalance_dbroots.assert_not_called() mock_move_primary_node.assert_not_called() - mock_add_dbroots_of_other_nodes.assert_called_once_with(root, 2) + mock_add_dbroots_of_other_nodes.assert_called_once_with(ANY, 2) # Test read-only node removal node_manipulation.remove_node( @@ -111,7 +111,7 @@ def test_add_remove_read_only_node(self): mock_rebalance_dbroots.assert_not_called() mock_move_primary_node.assert_not_called() - mock_remove_dbroots_of_node.assert_called_once_with(root, 2) + mock_remove_dbroots_of_node.assert_called_once_with(ANY, 2) def test_add_dbroots_nodes_rebalance(self): diff --git a/cmapi/mcs_node_control/models/node_config.py b/cmapi/mcs_node_control/models/node_config.py index 2943d4c98a..c71e397032 100644 --- a/cmapi/mcs_node_control/models/node_config.py +++ b/cmapi/mcs_node_control/models/node_config.py @@ -567,11 +567,6 @@ def get_all_dbroots(self, root): for j in range(1, int(smc_node.find(f"./ModuleDBRootCount{i}-3").text) + 1): dbroots.append(smc_node.find(f"./ModuleDBRootID{i}-{j}-3").text) - # TODO not sure about it - if dbroots and self.is_read_only(root): - module_logger.warning("Config contains dbroots %s for this read-only node, ignoring", dbroots) - return [] - return dbroots def is_read_only(self, root=None) -> bool: diff --git a/cmapi/pyproject.toml b/cmapi/pyproject.toml new file mode 100644 index 0000000000..c81c64faae --- /dev/null +++ b/cmapi/pyproject.toml @@ -0,0 +1,22 @@ +[tool.ruff] +line-length = 80 +target-version = "py39" +# Enable common rule sets +select = [ + "E", # pycodestyle errors + "F", # pyflakes: undefined names, unused imports, etc. + "I", # isort: import sorting + "B", # flake8-bugbear: common bugs and anti-patterns + "UP", # pyupgrade: use modern Python syntax + "N", # pep8-naming: naming conventions +] + +ignore = [] + +# Exclude cache and temporary directories +exclude = [ + "__pycache__", +] + +[tool.ruff.format] +quote-style = "single" From 245ccebba6337d1ec31a0bf3fcf79149d758173a Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Wed, 23 Apr 2025 15:49:03 +0000 Subject: [PATCH 09/13] 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 9b0d2264d0..98802931ab 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 @@ def _get_prog_name(cls, name: str) -> str: :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 @@ def _get_sorted_progs( 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 @@ def start_node( :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 @@ def start_node( 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 @@ def stop_node( # 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 4c2e2ff1db..8d8ed14134 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 aa2efe800b..9c171b7091 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 @@ def get_systemd_serv_name(self, service_name): 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 @@ def test_mcs_process_manager(self): ) ) - 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 195c286499..8dd0bac2bb 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 @@ def test_add_remove_read_only_node(self): 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 @@ def test_add_remove_read_only_node(self): # 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() From 7199c974c9a6ea11295adb936693e5634c9764d7 Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Wed, 23 Apr 2025 19:17:20 +0000 Subject: [PATCH 10/13] Zero-diff to trigger build --- cmapi/cmapi_server/node_manipulation.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmapi/cmapi_server/node_manipulation.py b/cmapi/cmapi_server/node_manipulation.py index 8d8ed14134..4277311de3 100644 --- a/cmapi/cmapi_server/node_manipulation.py +++ b/cmapi/cmapi_server/node_manipulation.py @@ -17,12 +17,14 @@ from cmapi_server import helpers from cmapi_server.constants import ( - CMAPI_CONF_PATH, CMAPI_SINGLE_NODE_XML, DEFAULT_MCS_CONF_PATH, LOCALHOSTS, + CMAPI_CONF_PATH, + CMAPI_SINGLE_NODE_XML, + DEFAULT_MCS_CONF_PATH, + LOCALHOSTS, MCS_DATA_PATH, ) from mcs_node_control.models.node_config import NodeConfig - PMS_NODE_PORT = '8620' EXEMGR_NODE_PORT = '8601' From ff25d4e35c39bd1a1772ce69386ef52f52051f41 Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Wed, 23 Apr 2025 21:49:23 +0000 Subject: [PATCH 11/13] Restore edits after cherry-pick --- cmapi/cmapi_server/constants.py | 18 ++++++++-------- cmapi/cmapi_server/controllers/endpoints.py | 2 -- cmapi/cmapi_server/failover_agent.py | 1 - cmapi/cmapi_server/helpers.py | 5 ----- cmapi/cmapi_server/managers/process.py | 14 ++++++------- .../process_dispatchers/container.py | 8 ++++--- cmapi/cmapi_server/test/test_node_manip.py | 1 - cmapi/mcs_node_control/models/node_config.py | 21 +++++++++++-------- 8 files changed, 32 insertions(+), 38 deletions(-) diff --git a/cmapi/cmapi_server/constants.py b/cmapi/cmapi_server/constants.py index 37790b7654..99375a628c 100644 --- a/cmapi/cmapi_server/constants.py +++ b/cmapi/cmapi_server/constants.py @@ -77,17 +77,17 @@ class ProgInfo(NamedTuple): # on top level of process handling # mcs-storagemanager starts conditionally inside mcs-loadbrm, but should be # stopped using cmapi -ALL_MCS_PROGS: dict[str, ProgInfo] = { +ALL_MCS_PROGS: dict[MCSProgs, ProgInfo] = { # workernode starts on primary and non primary node with 1 or 2 added # to subcommand (DBRM_Worker1 - on primary, DBRM_Worker2 - non primary) - MCSProgs.STORAGE_MANAGER.value: ProgInfo(15, 'mcs-storagemanager', '', False, 1), - MCSProgs.WORKER_NODE.value: ProgInfo(13, 'mcs-workernode', 'DBRM_Worker{}', False, 1), - MCSProgs.CONTROLLER_NODE.value: ProgInfo(11, 'mcs-controllernode', 'fg', True), - MCSProgs.PRIM_PROC.value: ProgInfo(5, 'mcs-primproc', '', False, 1), - MCSProgs.EXE_MGR.value: ProgInfo(9, 'mcs-exemgr', '', False, 1), - MCSProgs.WRITE_ENGINE_SERVER.value: ProgInfo(7, 'mcs-writeengineserver', '', False, 3), - MCSProgs.DML_PROC.value: ProgInfo(3, 'mcs-dmlproc', '', False), - MCSProgs.DDL_PROC.value: ProgInfo(1, 'mcs-ddlproc', '', False), + MCSProgs.STORAGE_MANAGER: ProgInfo(15, 'mcs-storagemanager', '', False, 1), + MCSProgs.WORKER_NODE: ProgInfo(13, 'mcs-workernode', 'DBRM_Worker{}', False, 1), + MCSProgs.CONTROLLER_NODE: ProgInfo(11, 'mcs-controllernode', 'fg', True), + MCSProgs.PRIM_PROC: ProgInfo(5, 'mcs-primproc', '', False, 1), + MCSProgs.EXE_MGR: ProgInfo(9, 'mcs-exemgr', '', False, 1), + MCSProgs.WRITE_ENGINE_SERVER: ProgInfo(7, 'mcs-writeengineserver', '', False, 3), + MCSProgs.DML_PROC: ProgInfo(3, 'mcs-dmlproc', '', False), + MCSProgs.DDL_PROC: ProgInfo(1, 'mcs-ddlproc', '', False), } # constants for docker container dispatcher diff --git a/cmapi/cmapi_server/controllers/endpoints.py b/cmapi/cmapi_server/controllers/endpoints.py index 360c1fad8c..047e452d38 100644 --- a/cmapi/cmapi_server/controllers/endpoints.py +++ b/cmapi/cmapi_server/controllers/endpoints.py @@ -435,7 +435,6 @@ def put_config(self): is_primary=node_config.is_primary_node(), use_sudo=use_sudo, timeout=request_timeout, - is_read_only=node_config.is_read_only(), ) except CMAPIBasicError as err: raise_422_error( @@ -705,7 +704,6 @@ def put_shutdown(self): is_primary=node_config.is_primary_node(), use_sudo=use_sudo, timeout=timeout, - is_read_only=node_config.is_read_only(), ) except CMAPIBasicError as err: raise_422_error( diff --git a/cmapi/cmapi_server/failover_agent.py b/cmapi/cmapi_server/failover_agent.py index 1e4f5fdac7..ff92c06b68 100644 --- a/cmapi/cmapi_server/failover_agent.py +++ b/cmapi/cmapi_server/failover_agent.py @@ -97,7 +97,6 @@ def enterStandbyMode(self, test_mode = False): if not test_mode: MCSProcessManager.stop_node( is_primary=nc.is_primary_node(), - is_read_only=nc.is_read_only(), ) logger.info( 'FA.enterStandbyMode(): successfully stopped node.' diff --git a/cmapi/cmapi_server/helpers.py b/cmapi/cmapi_server/helpers.py index e022d35033..eef8ad8031 100644 --- a/cmapi/cmapi_server/helpers.py +++ b/cmapi/cmapi_server/helpers.py @@ -540,11 +540,6 @@ def get_desired_nodes(config=DEFAULT_MCS_CONF_PATH): return [ node.text for node in nodes ] -def get_read_only_nodes(root) -> list[str]: - """Get names of read only nodes from config""" - return [node.text for node in root.findall('./ReadOnlyNodes/Node')] - - def in_maintenance_state(config=DEFAULT_MCS_CONF_PATH): nc = NodeConfig() root = nc.get_current_config_root(config, upgrade=False) diff --git a/cmapi/cmapi_server/managers/process.py b/cmapi/cmapi_server/managers/process.py index 98802931ab..b0807a031d 100644 --- a/cmapi/cmapi_server/managers/process.py +++ b/cmapi/cmapi_server/managers/process.py @@ -7,7 +7,7 @@ import psutil from cmapi_server.exceptions import CMAPIBasicError -from cmapi_server.constants import MCS_INSTALL_BIN, ALL_MCS_PROGS, MCSProgs +from cmapi_server.constants import MCS_INSTALL_BIN, ALL_MCS_PROGS, MCSProgs, ProgInfo from cmapi_server.process_dispatchers.base import BaseDispatcher from cmapi_server.process_dispatchers.systemd import SystemdDispatcher from cmapi_server.process_dispatchers.container import ( @@ -33,7 +33,7 @@ class MCSProcessManager: e.g. re/-start or stop systemd services, run executable. """ CONTROLLER_MAX_RETRY = 30 - mcs_progs = {} + mcs_progs: dict[str, ProgInfo] = {} mcs_version_info = None dispatcher_name = None process_dispatcher: BaseDispatcher = None @@ -48,7 +48,7 @@ def _get_prog_name(cls, name: str) -> str: :rtype: str """ if cls.dispatcher_name == 'systemd': - prog = MCSProgs[name] + prog = MCSProgs(name) return ALL_MCS_PROGS[prog].service_name return name @@ -98,7 +98,8 @@ def _detect_processes(cls) -> None: if cls.mcs_progs: logging.warning('Mcs ProcessHandler already detected processes.') - for prog_name, prog_info in ALL_MCS_PROGS.items(): + for prog, prog_info in ALL_MCS_PROGS.items(): + prog_name = prog.value if os.path.exists(os.path.join(MCS_INSTALL_BIN, prog_name)): cls.mcs_progs[prog_name] = prog_info @@ -454,7 +455,6 @@ def stop_node( is_primary: bool, use_sudo: bool = True, timeout: int = 10, - is_read_only: bool = False, ): """Stop mcs node processes. @@ -464,8 +464,6 @@ def stop_node( :type use_sudo: bool, optional :param timeout: timeout for DMLProc gracefully stop using DBRM, seconds :type timeout: int - :param is_read_only: if true, doesn't stop WriteEngine - :type is_read_only: bool, optional :raises CMAPIBasicError: immediately if one mcs process not stopped """ # Every time try to stop all processes no matter primary it or slave, @@ -481,5 +479,5 @@ def stop_node( def restart_node(cls, is_primary: bool, use_sudo: bool, is_read_only: bool = False): """TODO: For next releases.""" if cls.get_running_mcs_procs(): - cls.stop_node(is_primary, use_sudo, is_read_only) + cls.stop_node(is_primary, use_sudo) cls.start_node(is_primary, use_sudo, is_read_only) diff --git a/cmapi/cmapi_server/process_dispatchers/container.py b/cmapi/cmapi_server/process_dispatchers/container.py index 7db927b327..644c4a7af0 100644 --- a/cmapi/cmapi_server/process_dispatchers/container.py +++ b/cmapi/cmapi_server/process_dispatchers/container.py @@ -11,7 +11,7 @@ import psutil from cmapi_server.constants import ( - IFLAG, LIBJEMALLOC_DEFAULT_PATH, MCS_INSTALL_BIN, ALL_MCS_PROGS + IFLAG, LIBJEMALLOC_DEFAULT_PATH, MCS_INSTALL_BIN, ALL_MCS_PROGS, MCSProgs ) from cmapi_server.exceptions import CMAPIBasicError from cmapi_server.process_dispatchers.base import BaseDispatcher @@ -126,7 +126,8 @@ def _make_cmd(service: str) -> str: :return: command with arguments if needed :rtype: str """ - service_info = ALL_MCS_PROGS[service] + prog = MCSProgs(service) + service_info = ALL_MCS_PROGS[prog] command = os.path.join(MCS_INSTALL_BIN, service) if service_info.subcommand: @@ -188,7 +189,8 @@ def start( env=env_vars ) # TODO: any other way to detect service finished its initialisation? - sleep(ALL_MCS_PROGS[service].delay) + prog = MCSProgs(service) + sleep(ALL_MCS_PROGS[prog].delay) logger.debug(f'Started "{service}".') if is_primary and service == 'DDLProc': diff --git a/cmapi/cmapi_server/test/test_node_manip.py b/cmapi/cmapi_server/test/test_node_manip.py index 8dd0bac2bb..88c8993dbf 100644 --- a/cmapi/cmapi_server/test/test_node_manip.py +++ b/cmapi/cmapi_server/test/test_node_manip.py @@ -7,7 +7,6 @@ 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 BaseNodeManipTestCase, tmp_mcs_config_filename from mcs_node_control.models.node_config import NodeConfig diff --git a/cmapi/mcs_node_control/models/node_config.py b/cmapi/mcs_node_control/models/node_config.py index c71e397032..9d52ad49e3 100644 --- a/cmapi/mcs_node_control/models/node_config.py +++ b/cmapi/mcs_node_control/models/node_config.py @@ -4,24 +4,23 @@ import pwd import re import socket -from os import mkdir, replace, chown +from os import chown, mkdir, replace from pathlib import Path from shutil import copyfile -from xml.dom import minidom # to pick up pretty printing functionality +from xml.dom import minidom # to pick up pretty printing functionality from lxml import etree from cmapi_server.constants import ( - DEFAULT_MCS_CONF_PATH, DEFAULT_SM_CONF_PATH, + DEFAULT_MCS_CONF_PATH, + DEFAULT_SM_CONF_PATH, MCS_MODULE_FILE_PATH, ) + # from cmapi_server.managers.process import MCSProcessManager -from mcs_node_control.models.misc import ( - read_module_id, get_dbroots_list -) +from mcs_node_control.models.misc import get_dbroots_list, read_module_id from mcs_node_control.models.network_ifaces import get_network_interfaces - module_logger = logging.getLogger() @@ -569,11 +568,15 @@ def get_all_dbroots(self, root): return dbroots + def get_read_only_nodes(self, root=None) -> list[str]: + """Get names of read only nodes from config""" + root = root or self.get_current_config_root() + return [node.text for node in root.findall('./ReadOnlyNodes/Node')] + def is_read_only(self, root=None) -> bool: """Checks if this node is in read-only mode""" - from cmapi_server.helpers import get_read_only_nodes # Avoid circular import root = root or self.get_current_config_root() - read_only_nodes = set(get_read_only_nodes(root)) + read_only_nodes = set(self.get_read_only_nodes(root)) my_names = set(self.get_network_addresses_and_names()) return bool(read_only_nodes.intersection(my_names)) \ No newline at end of file From 5359c9822f40ab5b4e942c0f9d18255c5eee4068 Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Fri, 25 Apr 2025 04:26:13 +0000 Subject: [PATCH 12/13] On every node change adjust dbroots in the read-only nodes --- cmapi/cmapi_server/failover_agent.py | 4 +- cmapi/cmapi_server/handlers/cluster.py | 9 +- cmapi/cmapi_server/helpers.py | 2 +- cmapi/cmapi_server/managers/transaction.py | 2 +- cmapi/cmapi_server/node_manipulation.py | 59 +++++++---- cmapi/cmapi_server/test/test_node_manip.py | 101 +++++++++++++++---- cmapi/mcs_node_control/models/node_config.py | 22 ++++ 7 files changed, 150 insertions(+), 49 deletions(-) diff --git a/cmapi/cmapi_server/failover_agent.py b/cmapi/cmapi_server/failover_agent.py index ff92c06b68..864715e090 100644 --- a/cmapi/cmapi_server/failover_agent.py +++ b/cmapi/cmapi_server/failover_agent.py @@ -95,9 +95,7 @@ def enterStandbyMode(self, test_mode = False): try: # TODO: remove test_mode condition and add mock for testing if not test_mode: - MCSProcessManager.stop_node( - is_primary=nc.is_primary_node(), - ) + MCSProcessManager.stop_node(is_primary=nc.is_primary_node()) logger.info( 'FA.enterStandbyMode(): successfully stopped node.' ) diff --git a/cmapi/cmapi_server/handlers/cluster.py b/cmapi/cmapi_server/handlers/cluster.py index 1cc9a1d6ff..ebe13dc60c 100644 --- a/cmapi/cmapi_server/handlers/cluster.py +++ b/cmapi/cmapi_server/handlers/cluster.py @@ -15,7 +15,7 @@ get_current_key, get_version, update_revision_and_manager, ) from cmapi_server.node_manipulation import ( - add_node, add_dbroot, remove_node, switch_node_maintenance, + add_node, add_dbroot, remove_node, switch_node_maintenance, update_dbroots_of_readonly_nodes, ) from mcs_node_control.models.misc import get_dbrm_master from mcs_node_control.models.node_config import NodeConfig @@ -181,11 +181,6 @@ def add_node( host=node, input_config_filename=config, output_config_filename=config ) - else: - logger.debug( - f'Node {node} is read-only, skipping dbroot addition' - ) - except Exception as err: raise CMAPIBasicError('Error while adding node.') from err @@ -228,6 +223,8 @@ def remove_node(node: str, config: str = DEFAULT_MCS_CONF_PATH) -> dict: node, input_config_filename=config, output_config_filename=config ) + with NodeConfig().modify_config(config) as root: + update_dbroots_of_readonly_nodes(root) except Exception as err: raise CMAPIBasicError('Error while removing node.') from err diff --git a/cmapi/cmapi_server/helpers.py b/cmapi/cmapi_server/helpers.py index eef8ad8031..1e707cb29e 100644 --- a/cmapi/cmapi_server/helpers.py +++ b/cmapi/cmapi_server/helpers.py @@ -378,7 +378,7 @@ async def update_config(node: str, headers: dict, body: dict) -> None: ) as response: resp_json = await response.json(encoding='utf-8') response.raise_for_status() - logging.info(f'Node {node} config put successfull.') + logging.info(f'Node {node} config put successful.') except aiohttp.ClientResponseError as err: # TODO: may be better to check if resp status is 422 cause # it's like a signal that cmapi server raised it in diff --git a/cmapi/cmapi_server/managers/transaction.py b/cmapi/cmapi_server/managers/transaction.py index 98de505241..75e70370c2 100644 --- a/cmapi/cmapi_server/managers/transaction.py +++ b/cmapi/cmapi_server/managers/transaction.py @@ -106,7 +106,7 @@ def rollback_transaction(self, nodes: Optional[list] = None) -> None: try: rollback_transaction(self.txn_id, nodes=nodes) self.active_transaction = False - logging.debug(f'Successfull rollback of transaction "{self.txn_id}".') + logging.debug(f'Successful rollback of transaction "{self.txn_id}".') except Exception: logging.error( f'Error while rolling back transaction "{self.txn_id}"', diff --git a/cmapi/cmapi_server/node_manipulation.py b/cmapi/cmapi_server/node_manipulation.py index 4277311de3..2cff380e2d 100644 --- a/cmapi/cmapi_server/node_manipulation.py +++ b/cmapi/cmapi_server/node_manipulation.py @@ -113,8 +113,8 @@ def add_node( if not read_only: _rebalance_dbroots(c_root) _move_primary_node(c_root) - else: - add_dbroots_of_other_nodes(c_root, pm_num) + + update_dbroots_of_readonly_nodes(c_root) except Exception: logging.error( 'Caught exception while adding node, config file is unchanged', @@ -187,8 +187,7 @@ def remove_node( _rebalance_dbroots(c_root) _move_primary_node(c_root) - if is_read_only: - remove_dbroots_of_node(c_root, pm_num) + update_dbroots_of_readonly_nodes(c_root) else: # TODO: # - IMO undefined behaviour here. Removing one single node @@ -262,7 +261,7 @@ def rebalance_dbroots( # # returns the id of the new dbroot on success # raises an exception on error -def add_dbroot(input_config_filename = None, output_config_filename = None, host = None): +def add_dbroot(input_config_filename = None, output_config_filename = None, host = None) -> int: node_config = NodeConfig() if input_config_filename is None: c_root = node_config.get_current_config_root() @@ -1185,26 +1184,54 @@ class NodeNotFoundException(Exception): pass +def get_pm_module_num_to_addr_map(root: etree.Element) -> dict[int, str]: + """Get a mapping of PM module numbers to their IP addresses""" + module_num_to_addr = {} + smc_node = root.find("./SystemModuleConfig") + mod_count = int(smc_node.find("./ModuleCount3").text) + for i in range(1, mod_count + 1): + ip_addr = smc_node.find(f"./ModuleIPAddr{i}-1-3").text + module_num_to_addr[i] = ip_addr + return module_num_to_addr + + +def update_dbroots_of_readonly_nodes(root: etree.Element) -> None: + """Read-only nodes do not have their own dbroots, but they must have all the dbroots of the other nodes + So this function sets list of dbroots of each read-only node to the list of all the dbroots in the cluster + """ + nc = NodeConfig() + pm_num_to_addr = get_pm_module_num_to_addr_map(root) + for ro_node in nc.get_read_only_nodes(root): + # Get PM num by IP address + this_ip_pm_num = None + for pm_num, pm_addr in pm_num_to_addr.items(): + if pm_addr == ro_node: + this_ip_pm_num = pm_num + break + + if this_ip_pm_num is not None: + # Add dbroots of other nodes to this read-only node + add_dbroots_of_other_nodes(root, this_ip_pm_num) + else: # This should not happen + err_msg = f"Could not find PM number for read-only node {ro_node}" + logging.error(err_msg) + raise NodeNotFoundException(err_msg) + + def add_dbroots_of_other_nodes(root: etree.Element, module_num: int) -> None: """Adds all the dbroots listed in the config to this (read-only) node""" existing_dbroots = _get_existing_db_roots(root) sysconf_node = root.find("./SystemModuleConfig") + # Remove existing dbroots from this module + remove_dbroots_of_node(root, module_num) + # Write node's dbroot count - dbroot_count_node = sysconf_node.find(f"./ModuleDBRootCount{module_num}-3") - if dbroot_count_node is not None: - sysconf_node.remove(dbroot_count_node) dbroot_count_node = etree.SubElement( sysconf_node, f"ModuleDBRootCount{module_num}-3" ) dbroot_count_node.text = str(len(existing_dbroots)) - # Remove existing dbroot IDs of this module if present - for i in range(1, 100): - dbroot_id_node = sysconf_node.find(f"./ModuleDBRootID{module_num}-{i}-3") - if dbroot_id_node is not None: - sysconf_node.remove(dbroot_id_node) - # Write new dbroot IDs to the module mapping for i, dbroot_id in enumerate(existing_dbroots, start=1): dbroot_id_node = etree.SubElement( @@ -1221,10 +1248,6 @@ def remove_dbroots_of_node(root: etree.Element, module_num: int) -> None: dbroot_count_node = sysconf_node.find(f"./ModuleDBRootCount{module_num}-3") if dbroot_count_node is not None: sysconf_node.remove(dbroot_count_node) - else: - logging.error( - f"ModuleDBRootCount{module_num}-3 not found in SystemModuleConfig" - ) # Remove existing dbroot IDs for i in range(1, 100): diff --git a/cmapi/cmapi_server/test/test_node_manip.py b/cmapi/cmapi_server/test/test_node_manip.py index 88c8993dbf..be21fbd57e 100644 --- a/cmapi/cmapi_server/test/test_node_manip.py +++ b/cmapi/cmapi_server/test/test_node_manip.py @@ -1,13 +1,13 @@ import logging import socket import unittest -from unittest.mock import ANY, patch +from unittest.mock import patch from lxml import etree from cmapi_server import node_manipulation from cmapi_server.constants import MCS_DATA_PATH -from cmapi_server.node_manipulation import add_dbroots_of_other_nodes, remove_dbroots_of_node +from cmapi_server.node_manipulation import add_dbroots_of_other_nodes, remove_dbroots_of_node, update_dbroots_of_readonly_nodes from cmapi_server.test.unittest_global import BaseNodeManipTestCase, tmp_mcs_config_filename from mcs_node_control.models.node_config import NodeConfig @@ -23,12 +23,18 @@ def test_add_remove_node(self): './test-output0.xml','./test-output1.xml','./test-output2.xml' ) hostaddr = socket.gethostbyname(socket.gethostname()) - node_manipulation.add_node( - self.NEW_NODE_NAME, tmp_mcs_config_filename, self.tmp_files[0] - ) - node_manipulation.add_node( - hostaddr, self.tmp_files[0], self.tmp_files[1] - ) + + with patch('cmapi_server.node_manipulation.update_dbroots_of_readonly_nodes') as mock_update_dbroots_of_readonly_nodes: + node_manipulation.add_node( + self.NEW_NODE_NAME, tmp_mcs_config_filename, self.tmp_files[0] + ) + mock_update_dbroots_of_readonly_nodes.assert_called_once() + mock_update_dbroots_of_readonly_nodes.reset_mock() + + node_manipulation.add_node( + hostaddr, self.tmp_files[0], self.tmp_files[1] + ) + mock_update_dbroots_of_readonly_nodes.assert_called_once() # get a NodeConfig, read test.xml # look for some of the expected changes. @@ -42,10 +48,13 @@ def test_add_remove_node(self): node = root.find("./ExeMgr2/IPAddr") self.assertEqual(node.text, hostaddr) - node_manipulation.remove_node( - self.NEW_NODE_NAME, self.tmp_files[1], self.tmp_files[2], - test_mode=True - ) + with patch('cmapi_server.node_manipulation.update_dbroots_of_readonly_nodes') as mock_update_dbroots_of_readonly_nodes: + node_manipulation.remove_node( + self.NEW_NODE_NAME, self.tmp_files[1], self.tmp_files[2], + test_mode=True + ) + mock_update_dbroots_of_readonly_nodes.assert_called_once() + nc = NodeConfig() root = nc.get_current_config_root(self.tmp_files[2]) node = root.find('./PMS1/IPAddr') @@ -67,8 +76,7 @@ def test_add_remove_read_only_node(self): # Mock _rebalance_dbroots and _move_primary_node (only after the first node is added) with patch('cmapi_server.node_manipulation._rebalance_dbroots') as mock_rebalance_dbroots, \ patch('cmapi_server.node_manipulation._move_primary_node') as mock_move_primary_node, \ - patch('cmapi_server.node_manipulation.add_dbroots_of_other_nodes') as mock_add_dbroots_of_other_nodes, \ - patch('cmapi_server.node_manipulation.remove_dbroots_of_node') as mock_remove_dbroots_of_node: + patch('cmapi_server.node_manipulation.update_dbroots_of_readonly_nodes') as mock_update_dbroots_of_readonly_nodes: # Add a read-only node node_manipulation.add_node( @@ -94,7 +102,8 @@ def test_add_remove_read_only_node(self): mock_rebalance_dbroots.assert_not_called() mock_move_primary_node.assert_not_called() - mock_add_dbroots_of_other_nodes.assert_called_once_with(ANY, 2) + mock_update_dbroots_of_readonly_nodes.assert_called_once() + mock_update_dbroots_of_readonly_nodes.reset_mock() # Test read-only node removal node_manipulation.remove_node( @@ -109,7 +118,7 @@ def test_add_remove_read_only_node(self): mock_rebalance_dbroots.assert_not_called() mock_move_primary_node.assert_not_called() - mock_remove_dbroots_of_node.assert_called_once_with(ANY, 2) + mock_update_dbroots_of_readonly_nodes.assert_called_once() def test_add_dbroots_nodes_rebalance(self): @@ -271,13 +280,23 @@ def test_unassign_dbroot1(self): self.assertTrue(caught_it) -class TestReadOnlyNodeDBRootsManip(unittest.TestCase): - our_module_idx = 2 +class TestDBRootsManipulation(unittest.TestCase): + our_module_idx = 3 + ro_node1_ip = '192.168.1.3' + ro_node2_ip = '192.168.1.4' def setUp(self): - # Mock initial XML structure (add two dbroots) + # Mock initial XML structure (add two nodes and two dbroots) self.root = etree.Element('Columnstore') - etree.SubElement(self.root, 'SystemModuleConfig') + # Add two PM modules with IP addresses + smc = etree.SubElement(self.root, 'SystemModuleConfig') + module_count = etree.SubElement(smc, 'ModuleCount3') + module_count.text = '2' + module1_ip = etree.SubElement(smc, 'ModuleIPAddr1-1-3') + module1_ip.text = '192.168.1.1' + module2_ip = etree.SubElement(smc, 'ModuleIPAddr2-1-3') + module2_ip.text = '192.168.1.2' + system_config = etree.SubElement(self.root, 'SystemConfig') dbroot_count = etree.SubElement(system_config, 'DBRootCount') dbroot_count.text = '2' @@ -286,6 +305,15 @@ def setUp(self): dbroot2 = etree.SubElement(system_config, 'DBRoot2') dbroot2.text = '/data/dbroot2' + def test_get_pm_module_num_to_addr_map(self): + result = node_manipulation.get_pm_module_num_to_addr_map(self.root) + + expected = { + 1: '192.168.1.1', + 2: '192.168.1.2', + } + self.assertEqual(result, expected) + def test_add_dbroots_of_other_nodes(self): '''add_dbroots_of_other_nodes must add dbroots of other nodes into mapping of the node.''' add_dbroots_of_other_nodes(self.root, self.our_module_idx) @@ -325,3 +353,36 @@ def test_remove_dbroots_of_node(self): dbroot2 = self.root.find(f'./SystemModuleConfig/ModuleDBRootID{self.our_module_idx}-2-3') self.assertIsNone(dbroot1) self.assertIsNone(dbroot2) + + def test_update_dbroots_of_readonly_nodes(self): + """Test that update_dbroots_of_readonly_nodes adds all existing dbroots to all existing read-only nodes""" + # Add two new new modules to the XML structure (two already exist) + smc = self.root.find('./SystemModuleConfig') + module_count = smc.find('./ModuleCount3') + module_count.text = '4' + module3_ip = etree.SubElement(smc, 'ModuleIPAddr3-1-3') + module3_ip.text = self.ro_node1_ip + module4_ip = etree.SubElement(smc, 'ModuleIPAddr4-1-3') + module4_ip.text = self.ro_node2_ip + # Add them to ReadOnlyNodes + read_only_nodes = etree.SubElement(self.root, 'ReadOnlyNodes') + for ip in [self.ro_node1_ip, self.ro_node2_ip]: + node = etree.SubElement(read_only_nodes, 'Node') + node.text = ip + + update_dbroots_of_readonly_nodes(self.root) + + # Check that read only nodes have all the dbroots + for ro_module_idx in range(3, 5): + module_count = self.root.find(f'./SystemModuleConfig/ModuleDBRootCount{ro_module_idx}-3') + self.assertIsNotNone(module_count) + self.assertEqual(module_count.text, '2') + + dbroot1 = self.root.find(f'./SystemModuleConfig/ModuleDBRootID{ro_module_idx}-1-3') + dbroot2 = self.root.find(f'./SystemModuleConfig/ModuleDBRootID{ro_module_idx}-2-3') + self.assertIsNotNone(dbroot1) + self.assertIsNotNone(dbroot2) + self.assertEqual(dbroot1.text, '1') + self.assertEqual(dbroot2.text, '2') + + diff --git a/cmapi/mcs_node_control/models/node_config.py b/cmapi/mcs_node_control/models/node_config.py index 9d52ad49e3..c5b374580c 100644 --- a/cmapi/mcs_node_control/models/node_config.py +++ b/cmapi/mcs_node_control/models/node_config.py @@ -1,4 +1,5 @@ import configparser +from contextlib import contextmanager import grp import logging import pwd @@ -7,6 +8,7 @@ from os import chown, mkdir, replace from pathlib import Path from shutil import copyfile +from typing import Optional from xml.dom import minidom # to pick up pretty printing functionality from lxml import etree @@ -136,6 +138,26 @@ def write_config(self, tree, filename=DEFAULT_MCS_CONF_PATH): f.write(self.to_string(tree)) replace(tmp_filename, filename) # atomic replacement + @contextmanager + def modify_config( + self, + input_config_filename: str = DEFAULT_MCS_CONF_PATH, + output_config_filename: Optional[str] = None, + ): + """Context manager to modify the config file + If exception is raised, the config file is not modified and exception is re-raised + If output_config_filename is not provided, the input config file is modified + """ + try: + c_root = self.get_current_config_root(input_config_filename) + yield c_root + except Exception as e: + logging.error(f"modify_config(): Caught exception: '{str(e)}', config file not modified") + raise + else: + output_config_filename = output_config_filename or input_config_filename + self.write_config(c_root, output_config_filename) + def to_string(self, tree): # TODO: try to use lxml to do this to avoid the add'l dependency xmlstr = minidom.parseString(etree.tostring(tree)).toprettyxml( From e7fd6fcf4b074c0bfed8e9c0919e1e425452dcd2 Mon Sep 17 00:00:00 2001 From: Alexander Presnyakov Date: Mon, 28 Apr 2025 19:20:48 +0000 Subject: [PATCH 13/13] Fix logging (trace level) in tests --- cmapi/cmapi_server/logging_management.py | 3 ++- cmapi/cmapi_server/test/unittest_global.py | 10 ++-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/cmapi/cmapi_server/logging_management.py b/cmapi/cmapi_server/logging_management.py index cffcae122b..7979b19ffd 100644 --- a/cmapi/cmapi_server/logging_management.py +++ b/cmapi/cmapi_server/logging_management.py @@ -104,7 +104,8 @@ def enable_console_logging(logger: logging.Logger) -> None: def config_cmapi_server_logging(): # add custom level TRACE only for develop purposes # could be activated using API endpoints or cli tool without relaunching - add_logging_level('TRACE', 5) + if not hasattr(logging, 'TRACE'): + add_logging_level('TRACE', 5) cherrypy._cplogging.LogManager.error = custom_cherrypy_error # reconfigure cherrypy.access log message format # Default access_log_format '{h} {l} {u} {t} "{r}" {s} {b} "{f}" "{a}"' diff --git a/cmapi/cmapi_server/test/unittest_global.py b/cmapi/cmapi_server/test/unittest_global.py index 57ec6c7495..8f78647473 100644 --- a/cmapi/cmapi_server/test/unittest_global.py +++ b/cmapi/cmapi_server/test/unittest_global.py @@ -2,21 +2,14 @@ import os import unittest from contextlib import contextmanager -from datetime import datetime, timedelta from shutil import copyfile from tempfile import TemporaryDirectory import cherrypy -from cryptography.hazmat.backends import default_backend -from cryptography.hazmat.primitives import serialization -from cryptography.hazmat.primitives.asymmetric import rsa -from cryptography import x509 -from cryptography.x509.oid import NameOID -from cryptography.hazmat.primitives import hashes - from cmapi_server import helpers from cmapi_server.constants import CMAPI_CONF_PATH from cmapi_server.controllers.dispatcher import dispatcher, jsonify_error +from cmapi_server.logging_management import config_cmapi_server_logging from cmapi_server.managers.process import MCSProcessManager from cmapi_server.managers.certificate import CertificateManager @@ -80,6 +73,7 @@ def run(self, result=None): ) copyfile(cmapi_config_filename, self.cmapi_config_filename) copyfile(TEST_MCS_CONFIG_FILEPATH, self.mcs_config_filename) + config_cmapi_server_logging() self.app = cherrypy.tree.mount( root=None, config=self.cmapi_config_filename )