-
Notifications
You must be signed in to change notification settings - Fork 14
Hotfix to enhance code clarity, and update configurations for high availability (HA) systems #69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ailability (HA) systems
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances code clarity and updates resource configurations for high availability systems in SAP HANA deployments on Azure by improving filesystem handling, refining resource properties, and updating test case setups. Key changes include updates to filesystem freeze methods, enhanced resource configuration defaults, and modifications to test commands and validation logic.
Reviewed Changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/roles/ha_db_hana/resource_migration_test.py | Updated command templating and added a new resource ID retrieval command for HANA. |
| tests/roles/ha_db_hana/block_network_test.py | Replaced ping with nc for network blocking tests and updated in-code comments accordingly. |
| tests/modules/get_pcmk_properties_db_test.py | Added sensitive parameters for testing scenario, ensuring they are skipped during processing. |
| tests/modules/filesystem_freeze_test.py | Adjusted mount point expectations in filesystem freeze tests. |
| src/vars/input-api.yaml | Updated default values and command templating for HANA DB HA test cases. |
| src/roles/misc/tasks/test-case-setup.yml & rescue.yml | Minor updates in variable setups for improved clarity in test case configuration. |
| src/roles/ha_db_hana/tasks/resource-migration.yml | Introduced a new block to retrieve HANA resource id with fallback handling. |
| src/roles/ha_db_hana/tasks/files/constants.yaml | Updated resource default timeout and delay values, and renamed resource key for clarity. |
| src/roles/ha_db_hana/tasks/block-network.yml | Modified network connectivity check to use nc instead of ping. |
| src/module_utils/commands.py | Modified the FREEZE_FILESYSTEM lambda to accept a mount point parameter. |
| docs/HIGH_AVAILABILITY.md | Revised documentation instructions with updated documentation links. |
| src/modules/get_pcmk_properties_scs.py, src/modules/get_pcmk_properties_db.py | Updated parsing logic to skip sensitive data in NV pair elements and refined resource configuration retrieval. |
| src/modules/filesystem_freeze.py | Adjusted _find_filesystem to return a tuple and updated corresponding command invocations. |
| src/modules/check_indexserver.py | Updated expected properties structure for SUSE to a list format for consistency. |
Files not reviewed (1)
- tests/roles/mock_data/cibadmin.txt: Language not supported
Comments suppressed due to low confidence (1)
src/roles/ha_db_hana/tasks/files/constants.yaml:62
- The value for pcmk_delay_max was changed from '30s' to '15' without a unit. Verify that this numeric value is processed as intended downstream and that the absence of a unit does not cause issues.
pcmk_delay_max: "15"
…g bash as the shell executable
KimForss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two questions, see comments
… in HA configuration
dennispadia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
hdamecharla
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
dennispadia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
…ailability (HA) systems (Azure#69)
…ailability (HA) systems (Azure#69)
…ailability (HA) systems (#69)
This pull request introduces several updates across multiple files, focusing on improving functionality, enhancing configurability, and addressing issues in high availability (HA) setups for SAP HANA on Azure. Key changes include updates to filesystem handling, resource configurations, and test case setups.
Filesystem and Mount Handling:
_find_filesystemmethod insrc/modules/filesystem_freeze.pyto return both the filesystem and mount point, improving clarity and enabling more precise operations.FREEZE_FILESYSTEMcommand insrc/module_utils/commands.pyto accept a mount point as a parameter, aligning with the updated_find_filesystemmethod.Resource Configuration Enhancements:
src/modules/get_pcmk_properties_db.pyto include specific types likesbd_stonithandfence_agentfor better granularity.RESOURCE_DEFAULTSinconstants.yamlto refine timeout values and add specific configurations forsbd_stonith. [1] [2]Test Case and Validation Improvements:
resource-migration.yml, improving flexibility across different setups.input-api.yaml, centralizing configuration and reducing duplication. [1] [2]Security and Data Handling:
_parse_nvpair_elementsin bothget_pcmk_properties_db.pyandget_pcmk_properties_scs.pyto skip sensitive parameters likepasswdandpassword, enhancing security. [1] [2]Documentation and Miscellaneous:
HIGH_AVAILABILITY.mdto reference the correct documentation link.azure-mgmt-networkpackage inazure-lb.yml, likely due to redundancy or a shift in dependency management.