diff --git a/src/module_utils/commands.py b/src/module_utils/commands.py index 93ee3cc2..69d8f49f 100644 --- a/src/module_utils/commands.py +++ b/src/module_utils/commands.py @@ -66,3 +66,21 @@ } CIB_ADMIN = lambda scope: ["cibadmin", "--query", "--scope", scope] + +RECOMMENDATION_MESSAGES = { + "priority-fencing-delay": ( + "The 'priority-fencing-delay' setting is not configured. " + "In a two-node cluster, configure priority-fencing-delay to enhance the " + "highest-priority node's survival odds during a fence race condition. " + "For more details on the setup, check official cluster pacemaker configuration " + "documentation in learn.microsoft.com" + ), + "azureevents": ( + "The Azure scheduled events resource is not configured. " + "It is advised to setup this agent in your cluster to monitor the Instance Metadata " + "Service (IMDS) for platform maintenance events, allowing it to proactively drain " + "resources or initiate a clean failover before Azure maintenance impacts the node. " + "For more details on the setup, check official cluster pacemaker configuration " + "documentation in learn.microsoft.com" + ), +} diff --git a/src/module_utils/get_pcmk_properties.py b/src/module_utils/get_pcmk_properties.py index 322bc12d..efeea03a 100644 --- a/src/module_utils/get_pcmk_properties.py +++ b/src/module_utils/get_pcmk_properties.py @@ -11,16 +11,17 @@ BaseHAClusterValidator: Base validator class for cluster configurations. """ +import logging from abc import ABC try: from ansible.module_utils.sap_automation_qa import SapAutomationQA from ansible.module_utils.enums import OperatingSystemFamily, Parameters, TestStatus - from ansible.module_utils.commands import CIB_ADMIN + from ansible.module_utils.commands import CIB_ADMIN, RECOMMENDATION_MESSAGES except ImportError: from src.module_utils.sap_automation_qa import SapAutomationQA from src.module_utils.enums import OperatingSystemFamily, Parameters, TestStatus - from src.module_utils.commands import CIB_ADMIN + from src.module_utils.commands import CIB_ADMIN, RECOMMENDATION_MESSAGES class BaseHAClusterValidator(SapAutomationQA, ABC): @@ -79,6 +80,7 @@ def __init__( self.fencing_mechanism = fencing_mechanism self.constants = constants self.cib_output = cib_output + self.missing_required_items = [] def _get_expected_value(self, category, name): """ @@ -190,6 +192,15 @@ def _create_parameter( status = self._determine_parameter_status(value, expected_config) + if status == TestStatus.WARNING.value and not value: + self._handle_missing_required_parameter( + expected_config=expected_config, + name=name, + category=category, + subcategory=subcategory, + op_name=op_name, + ) + display_expected_value = None if expected_config is None: display_expected_value = "" @@ -289,6 +300,47 @@ def _determine_parameter_status(self, value, expected_config): else TestStatus.ERROR.value ) + def _handle_missing_required_parameter( + self, expected_config, name, category, subcategory=None, op_name=None + ): + """ + Handle warnings for missing required parameters. + Logs warning message and updates result when a required parameter has no value. + + :param expected_config: The expected configuration (tuple or dict) + :type expected_config: tuple or dict + :param name: The parameter name + :type name: str + :param category: The parameter category + :type category: str + :param subcategory: The parameter subcategory, defaults to None + :type subcategory: str, optional + :param op_name: The operation name (if applicable), defaults to None + :type op_name: str, optional + """ + is_required = False + if isinstance(expected_config, tuple) and len(expected_config) == 2: + is_required = expected_config[1] + elif isinstance(expected_config, dict): + is_required = expected_config.get("required", False) + + if is_required: + param_display_name = f"{op_name}_{name}" if op_name else name + category_display = f"{category}_{subcategory}" if subcategory else category + self.missing_required_items.append( + { + "type": "parameter", + "name": name, + "display_name": param_display_name, + "category": category_display, + } + ) + warning_msg = ( + f"Required parameter '{param_display_name}' in category '{category_display}' " + + "has no value configured." + ) + self.log(logging.WARNING, warning_msg) + def _parse_nvpair_elements(self, elements, category, subcategory=None, op_name=None): """ Parse nvpair elements and create parameter dictionaries. @@ -511,6 +563,8 @@ def validate_from_constants(self): overall_status = TestStatus.ERROR.value elif warning_parameters: overall_status = TestStatus.WARNING.value + elif self.result.get("status") == TestStatus.WARNING.value: + overall_status = TestStatus.WARNING.value else: overall_status = TestStatus.SUCCESS.value @@ -520,7 +574,10 @@ def validate_from_constants(self): "status": overall_status, } ) - self.result["message"] += "HA Parameter Validation completed successfully. " + self.result["message"] += "HA parameter validation completed successfully. " + recommendation_message = self._generate_recommendation_message() + if recommendation_message: + self.result["message"] += recommendation_message def _validate_basic_constants(self, category): """ @@ -618,6 +675,72 @@ def _validate_resource_constants(self): """ return [] + def _check_required_resources(self): + """ + Check if required resources are present in the cluster. + Adds warnings to result message for missing required resources. + """ + if "RESOURCE_DEFAULTS" not in self.constants: + return + + try: + if self.cib_output: + resource_scope = self._get_scope_from_cib("resources") + else: + resource_scope = self.parse_xml_output( + self.execute_command_subprocess(CIB_ADMIN(scope="resources")) + ) + if resource_scope is None: + return + + for resource_type, resource_config in ( + self.constants["RESOURCE_DEFAULTS"].get(self.os_type, {}).items() + ): + if not isinstance(resource_config, dict): + continue + if resource_config.get("required", False): + if resource_type in self.RESOURCE_CATEGORIES: + xpath = self.RESOURCE_CATEGORIES[resource_type] + elements = resource_scope.findall(xpath) + if not elements: + self.missing_required_items.append( + {"type": "resource", "name": resource_type, "xpath": xpath} + ) + self.result["status"] = TestStatus.WARNING.value + except Exception as ex: + self.result["message"] += f"Error checking required resources: {str(ex)} " + + def _generate_recommendation_message(self): + """ + Generate recommendation message based on missing required items. + Uses centralized RECOMMENDATION_MESSAGES dictionary for consistent messaging. + + :return: Formatted recommendation message + :rtype: str + """ + recommendations = [] + + for item in self.missing_required_items: + if item["name"] in RECOMMENDATION_MESSAGES: + recommendations.append(RECOMMENDATION_MESSAGES[item["name"]]) + else: + if item["type"] == "parameter": + recommendations.append( + f"The '{item['display_name']}' parameter in category " + f"'{item['category']}' is not configured." + ) + elif item["type"] == "resource": + recommendations.append( + f"The required resource '{item['name']}' is not found in cluster config." + ) + + if recommendations: + recommendation_header = "\n\nRecommendation for warnings:\n" + recommendation_body = "\n".join(recommendations) + return recommendation_header + recommendation_body + + return "" + def _validate_constraint_constants(self): """ Validate constraint constants with offline validation support. diff --git a/src/modules/get_azure_lb.py b/src/modules/get_azure_lb.py index 73c3f159..b2a7f31a 100644 --- a/src/modules/get_azure_lb.py +++ b/src/modules/get_azure_lb.py @@ -247,17 +247,17 @@ def get_load_balancers_details(self) -> None: parameters = [] def check_parameters(entity, parameters_dict, entity_type): - for key, expected_value in parameters_dict.items(): + for key, value_object in parameters_dict.items(): parameters.append( Parameters( category=entity_type, id=entity["name"], name=key, value=str(entity[key]), - expected_value=str(expected_value), + expected_value=str(value_object.get("value", "")), status=( TestStatus.SUCCESS.value - if entity[key] == expected_value + if entity[key] == value_object.get("value", "") else TestStatus.ERROR.value ), ).to_dict() diff --git a/src/modules/get_pcmk_properties_db.py b/src/modules/get_pcmk_properties_db.py index 843da940..b5e5c133 100644 --- a/src/modules/get_pcmk_properties_db.py +++ b/src/modules/get_pcmk_properties_db.py @@ -171,6 +171,7 @@ class HAClusterValidator(BaseHAClusterValidator): "azurelb": ".//primitive[@type='azure-lb']", "angi_filesystem": ".//primitive[@type='SAPHanaFilesystem']", "angi_hana": ".//primitive[@type='SAPHanaController']", + "azureevents": ".//primitive[@type='azure-events-az']", } def __init__( @@ -227,6 +228,7 @@ def _validate_resource_constants(self): """ Resource validation with HANA-specific logic and offline validation support. Validates resource constants by iterating through expected parameters. + Also checks for required resources. :return: A list of parameter dictionaries :rtype: list @@ -243,6 +245,7 @@ def _validate_resource_constants(self): if resource_scope is not None: parameters.extend(self._parse_resources_section(resource_scope)) + self._check_required_resources() except Exception as ex: self.result["message"] += f"Error validating resource constants: {str(ex)} " diff --git a/src/modules/get_pcmk_properties_scs.py b/src/modules/get_pcmk_properties_scs.py index c4d61349..b0e954be 100644 --- a/src/modules/get_pcmk_properties_scs.py +++ b/src/modules/get_pcmk_properties_scs.py @@ -224,6 +224,7 @@ def _validate_resource_constants(self): """ Resource validation with SCS-specific logic and offline validation support. Validates resource constants by iterating through expected parameters. + Also checks for required resources. :return: A list of parameter dictionaries :rtype: list @@ -240,6 +241,7 @@ def _validate_resource_constants(self): if resource_scope is not None: parameters.extend(self._parse_resources_section(resource_scope)) + self._check_required_resources() except Exception as ex: self.result["message"] += f"Error validating resource constants: {str(ex)} " diff --git a/src/roles/ha_db_hana/tasks/files/constants.yaml b/src/roles/ha_db_hana/tasks/files/constants.yaml index c3cd5660..e936e8df 100644 --- a/src/roles/ha_db_hana/tasks/files/constants.yaml +++ b/src/roles/ha_db_hana/tasks/files/constants.yaml @@ -19,6 +19,9 @@ CRM_CONFIG_DEFAULTS: stonith-enabled: value: "true" required: false + stonith-timeout: + value: ["210", "210s"] + required: false concurrent-fencing: value: "true" required: false @@ -451,6 +454,39 @@ RESOURCE_DEFAULTS: timeout: value: ["20", "20s"] required: false + + azureevents: + required: true + meta_attributes: + allow-unhealthy-nodes: + value: "true" + required: false + failure-timeout: + value: "120s" + required: false + operations: + monitor: + interval: + value: ["10", "10s"] + required: false + timeout: + value: ["240", "240s"] + required: false + start: + interval: + value: ["0", "0s"] + required: false + timeout: + value: ["10", "10s"] + required: false + stop: + interval: + value: ["0", "0s"] + required: false + timeout: + value: ["10", "10s"] + required: false + REDHAT: fence_agent: required: false @@ -708,6 +744,38 @@ RESOURCE_DEFAULTS: value: ["20", "20s"] required: false + azureevents: + required: true + meta_attributes: + allow-unhealthy-nodes: + value: "true" + required: false + failure-timeout: + value: "120s" + required: false + operations: + monitor: + interval: + value: ["10", "10s"] + required: false + timeout: + value: ["240", "240s"] + required: false + start: + interval: + value: ["0", "0s"] + required: false + timeout: + value: ["10", "10s"] + required: false + stop: + interval: + value: ["0", "0s"] + required: false + timeout: + value: ["10", "10s"] + required: false + # === OS Parameters === # Run command as root. Format of command is: "parent_key child_key" # Example: sysctl net.ipv4.tcp_timestamps diff --git a/src/templates/report.html b/src/templates/report.html index c5410a96..c8a70984 100644 --- a/src/templates/report.html +++ b/src/templates/report.html @@ -228,6 +228,11 @@ background-color: #ddd; } + .test-message { + white-space: pre-wrap; + word-wrap: break-word; + } + /* Legend Styles */ .legend { position: absolute; @@ -397,7 +402,7 @@

System Information

{% endif %} Message - {{ test_case_result.TestCaseMessage }} + {{ test_case_result.TestCaseMessage }} {% if test_case_result.PackageVersions %} @@ -438,7 +443,6 @@

System Information

const fixedJsonString = jsonString .replace(/False/g, 'false') .replace(/True/g, 'true') - .replace(/\\n/g, '') .replace(/None/g, 'null') .replace(/'([^']+)'(?=\s*:)/g, '"$1"') .replace(/:\s*'([^']*)'(?=\s*[,\}])/g, function (match, value) { @@ -511,13 +515,28 @@

System Information

const messages = document.getElementsByClassName('newlinemessages') for (let message of messages) { try { - message.innerHTML = JSON.parse(message.textContent.trim().replace(/\\n/g, '').replace(/'/g, '"')).join('
') + message.innerHTML = JSON.parse(message.textContent.trim().replace(/'/g, '"')).join('
') } catch (error) { console.error('Error formatting messages:', error) } } } + // Function to format test case messages (preserve newlines) + function formatTestMessages() { + const messages = document.getElementsByClassName('test-message') + for (let message of messages) { + try { + let text = message.textContent + text = text.replace(/\\n/g, '\n') + text = text.replace(/^["']|["']$/g, '') + message.textContent = text + } catch (error) { + console.error('Error formatting test message:', error) + } + } + } + // Function to toggle the details of a test case function toggleDetails(element) { var details = element.nextElementSibling @@ -730,6 +749,7 @@

System Information

} formatJsonObjects() formatNewLineMessages() + formatTestMessages() formatPackageList() parseClusterData() document.addEventListener('DOMContentLoaded', () => { diff --git a/tests/module_utils/get_pcmk_properties_test.py b/tests/module_utils/get_pcmk_properties_test.py index 02b28315..a4d14d86 100644 --- a/tests/module_utils/get_pcmk_properties_test.py +++ b/tests/module_utils/get_pcmk_properties_test.py @@ -31,6 +31,7 @@ + """ @@ -91,8 +92,12 @@ "REDHAT": { "stonith-enabled": {"value": "true", "required": False}, "cluster-name": {"value": "hdb_HDB", "required": False}, + "stonith-timeout": {"value": "900", "required": False}, + }, + "azure-fence-agent": { + "priority": {"value": "10", "required": False}, + "stonith-timeout": {"value": "210", "required": False}, }, - "azure-fence-agent": {"priority": {"value": "10", "required": False}}, "sbd": {"pcmk_delay_max": {"value": "30", "required": False}}, }, "RSC_DEFAULTS": { @@ -223,10 +228,12 @@ def mock_execute_command(*args, **kwargs): Mock function to replace execute_command_subprocess. """ command = args[0] if args else kwargs.get("command", []) + if not command or not isinstance(command, (list, str)): + return "" command_str = " ".join(command) if isinstance(command, list) else str(command) if "sysctl" in command_str: return DUMMY_OS_COMMAND - if len(command) >= 2 and command[-1] in mock_xml_outputs: + if isinstance(command, list) and len(command) >= 2 and command[-1] in mock_xml_outputs: return mock_xml_outputs[command[-1]] return "" @@ -274,8 +281,8 @@ def test_get_expected_value_fence_config(self, validator): Test _get_expected_value method with fence configuration. """ validator.fencing_mechanism = "azure-fence-agent" - expected = validator._get_expected_value("crm_config", "priority") - assert expected == ("10", False) + expected = validator._get_expected_value("crm_config", "stonith-timeout") + assert expected == ("210", False) def test_get_resource_expected_value_instance_attributes(self, validator): """ @@ -429,3 +436,61 @@ def test_get_scope_from_cib_invalid_scope(self, validator_with_cib): """ scope_element = validator_with_cib._get_scope_from_cib("invalid_scope") assert scope_element is None + + def test_check_required_resources_missing(self, validator): + """ + Test _check_required_resources method when required resource is missing. + """ + validator.constants["RESOURCE_DEFAULTS"]["REDHAT"]["required_missing_resource"] = { + "required": True, + "meta_attributes": {}, + } + validator.RESOURCE_CATEGORIES["required_missing_resource"] = ( + ".//primitive[@type='NonExistent']" + ) + validator._check_required_resources() + + # Check that missing resource was tracked + assert len(validator.missing_required_items) > 0 + assert any( + item["type"] == "resource" and item["name"] == "required_missing_resource" + for item in validator.missing_required_items + ) + assert validator.result["status"] == TestStatus.WARNING.value + + def test_check_required_resources_present(self, validator): + """ + Test _check_required_resources method when required resource is present. + Uses CIB output to avoid command execution for more reliable testing. + """ + constants = DUMMY_CONSTANTS.copy() + constants["RESOURCE_DEFAULTS"] = { + "REDHAT": {"sbd_stonith": {"required": True, "meta_attributes": {}}} + } + + validator_with_cib = TestableBaseHAClusterValidator( + os_type=OperatingSystemFamily.REDHAT, + sid="HDB", + virtual_machine_name="vmname", + constants=constants, + fencing_mechanism="sbd", + cib_output=DUMMY_XML_FULL_CIB, + ) + validator_with_cib._check_required_resources() + assert ( + "Required resource 'sbd_stonith' not found" not in validator_with_cib.result["message"] + ) + + def test_check_required_resources_optional(self, validator): + """ + Test _check_required_resources method with optional resource missing. + """ + validator.constants["RESOURCE_DEFAULTS"]["REDHAT"]["optional_resource"] = { + "required": False, + "meta_attributes": {}, + } + validator.RESOURCE_CATEGORIES["optional_resource"] = ".//primitive[@type='NonExistent']" + + initial_message = validator.result["message"] + validator._check_required_resources() + assert "Required resource 'optional_resource'" not in validator.result["message"]