Skip to content

Conversation

devanshjainms
Copy link
Contributor

This pull request introduces enhanced validation for required cluster resources, particularly focusing on the new azureevents resource. It ensures that required resources are checked and appropriate warnings are generated when any are missing. The changes also expand test coverage for resource validation and improve robustness in command execution mocks.

Resource validation enhancements:

  • Added a _check_required_resources method to get_pcmk_properties.py to check for required resources defined in RESOURCE_DEFAULTS and append warnings to the result message if any are missing.
  • Integrated _check_required_resources into the resource validation flow for both HANA (get_pcmk_properties_db.py) and SCS (get_pcmk_properties_scs.py) modules, ensuring required resources are checked during validation. [1] [2]

Support for new resource type:

  • Added azureevents as a required resource in the RESOURCE_DEFAULTS section of constants.yaml for both SUSE and REDHAT OS types, including its meta attributes and operations. [1] [2]
  • Updated RESOURCE_CATEGORIES in get_pcmk_properties_db.py to include the azureevents resource type.

Testing improvements:

  • Expanded tests in get_pcmk_properties_test.py to cover scenarios where required resources are missing, present, or optional, ensuring the new validation logic works as expected.
  • Improved the mock command execution logic to handle edge cases where the command argument is missing or of an unexpected type.

Logging improvements:

  • Imported the logging module in get_pcmk_properties.py to support warning messages when required resources are missing.

@devanshjainms devanshjainms requested a review from a team as a code owner October 14, 2025 07:16
@devanshjainms devanshjainms requested review from KimForss, Copilot and hdamecharla and removed request for Copilot October 14, 2025 07:16
…ired parameters and update warning messages for missing resources
@Copilot Copilot AI review requested due to automatic review settings October 14, 2025 08:09
Copy link

@Copilot Copilot AI left a 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 pull request introduces comprehensive validation for required cluster resources, particularly adding support for the new azureevents resource type. The changes implement systematic checking of required resources and generate appropriate warnings when resources are missing from the cluster configuration.

  • Adds new _check_required_resources method to validate presence of required cluster resources
  • Integrates the new azureevents resource into the validation framework for both SUSE and REDHAT systems
  • Expands test coverage to validate the new resource checking functionality

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/module_utils/get_pcmk_properties.py Implements core _check_required_resources method with logging and warning generation
src/modules/get_pcmk_properties_db.py Adds azureevents resource category and integrates required resource checking
src/modules/get_pcmk_properties_scs.py Integrates required resource checking into SCS validation flow
src/roles/ha_db_hana/tasks/files/constants.yaml Defines azureevents resource configuration for both SUSE and REDHAT OS types
tests/module_utils/get_pcmk_properties_test.py Adds comprehensive tests for required resource validation scenarios

@hdamecharla hdamecharla self-assigned this Oct 14, 2025
hdamecharla
hdamecharla previously approved these changes Oct 14, 2025
Copy link
Member

@hdamecharla hdamecharla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

KimForss
KimForss previously approved these changes Oct 14, 2025
@devanshjainms devanshjainms dismissed stale reviews from KimForss and hdamecharla via af35d6f October 14, 2025 16:15
Copy link
Contributor

@dhruvmicrosoft dhruvmicrosoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@devanshjainms devanshjainms merged commit b9b61ad into Azure:development-oct-2025 Oct 14, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants