ER-80651: SmartQuota notification rules configuration - #272
Open
Saksham-Nautiyal wants to merge 11 commits into
Open
ER-80651: SmartQuota notification rules configuration#272Saksham-Nautiyal wants to merge 11 commits into
Saksham-Nautiyal wants to merge 11 commits into
Conversation
Add list/create/update/delete/delete-all helper methods on SmartQuota
for the per-quota PAPI notification-rules endpoints
(/platform/7/quota/quotas/{QuotaId}/notifications[/{NotificationId}]).
The isilon-sdk Python bindings (v9.5.0-v9.12.0) never generated typed
methods for these endpoints -- only the global
/platform/7/quota/settings/notifications endpoints were generated --
even though the raw PAPI schema (src/ansible/isilon_sdk/papi_schemas)
confirms the per-quota endpoints exist on every OneFS version 9.0-9.13.
Helpers route through quota_api_instance.api_client.call_api, the same
generic mechanism every generated SDK method uses internally, keeping
all calls within the authenticated SDK client (no raw HTTP).
Also discovered: PUT .../notifications/{NotificationId} only accepts
action fields (action_alert, action_email_owner, action_email_address,
email_template, holdoff, schedule) -- condition/threshold are
immutable after creation and require delete+recreate.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Add the quota_notification_rules list parameter (id, condition, threshold, action_alert, action_email_owner, action_email_address, state) to the smartquota module's argument spec and DOCUMENTATION, per ER-80651. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Add reconcile_quota_notification_rules to SmartQuota, computing the minimal create/update/delete actions to converge current notification rules to the requested quota_notification_rules list: - New rules (no id) are created. - Rules with a matching id and only action-field changes are updated in place via PUT. - Rules with a matching id and a condition/threshold change are deleted and recreated (PAPI immutability, per AC-2 update). - Rules with state=absent and a matching id are deleted. - An empty list deletes all existing rules for the quota. - Exact matches and unknown ids under state=absent are no-ops. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Append the current notification_rules list to the quota details returned by _process_final_quota_details on every state=present invocation where the quota exists (FR-5 query requirement), whether or not quota_notification_rules was supplied. Also update EXAMPLES and RETURN documentation for the new parameter/field. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Add _handle_notification_rules, invoked from perform_module_operation only when quota_notification_rules is present in module params: - Existing quota: reconciles using the already-known quota id (works under check_mode too, since reconcile's mutating calls already guard on self.module.check_mode). - Freshly created quota (not check_mode): re-resolves the new quota's id via get_quota_details before reconciling. - Freshly created quota under check_mode: id is not yet known, so reconciliation is skipped for this invocation (combined create+notify-under-check_mode is validation-story AC-9 scope). - quota_notification_rules omitted entirely: no-op, preserving existing playbook behavior (NFR-1). Full test_smartquota.py suite: 48 passed, same 15 pre-existing failures as baseline main (confirmed via git stash). Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Fixed two root causes of order-dependent test failures: 1. get_smartquota_args was a shared mutable class-level dict. Tests calling .update() leaked stale keys (user_name, group_name, provider_type, etc.) into subsequent tests. Added autouse fixture to deepcopy the base args dict before each test. 2. utils.isi_sdk is a single shared MagicMock across all tests. Its auto-cached child mocks (e.g., quota_api_instance.create_quota_quota) leaked mutations across tests in this file. Added autouse fixture to reset isi_sdk before each test, scoped to TestSmartQuota only. These fixes are isolated to test_smartquota.py and do not affect the shared powerscale_unit_base.py or other test modules. All 63 smartquota tests now pass (down from 15 failures). Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…te hardening for smartquota notification rules Implements ER-80651-smartquota-notification-rules-validation (Phases 1-5): - FR-10/FR-11: Validate quota_notification_rules before any API call. Enum choices for condition/threshold are enforced by Ansible's argument_spec; action-field presence (at least one of action_alert, action_email_owner, action_email_address) is now validated explicitly via _validate_notification_rules(), called at the top of perform_module_operation(). - FR-12: reconcile_quota_notification_rules() now fails with a descriptive error when an update or delete references a rule id that does not exist for the quota, instead of silently creating a new rule (update case) or no-op'ing (delete case). - FR-8: check_mode support already existed at the individual create/update/delete method level; added reconcile-level tests proving no write API call is made under check_mode for create/update/delete/delete-all scenarios. - FR-9: reconcile_quota_notification_rules() accepts an optional diff_dict, populated with 'before'/'after' notification-rule state. perform_module_operation() wires this into self.result['diff'] when self.module._diff is set, following the before/after diff convention used elsewhere in this collection (see job.py). - FR-13: _handle_quota_creation() now tracks whether the quota was created in this invocation (_quota_just_created); if notification-rule creation subsequently fails, the error message explicitly states the quota was created successfully but its rules were not. - Updated DOCUMENTATION/RETURN/EXAMPLES in smartquota.py, regenerated docs/modules/smartquota.rst, and added notification-rule and combined-create/diff-preview example tasks to playbooks/modules/smartquota.yml. - Added 21 new unit tests (test_smartquota.py); updated 1 existing test to reflect the new FR-12 fail-fast behavior. 86 tests pass with 92% statement coverage on smartquota.py. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
During integration testing on PowerScale OneFS 9.15, discovered that the API requires a 'holdoff' parameter (time in seconds before sending another notification) for notification rules with condition='exceeded'. This was not in the original ER-80651 implementation. Changes: - Added 'holdoff' to quota_notification_rules DOCUMENTATION suboptions - Added 'holdoff' to argument_spec (type: int) - Added 'holdoff' to create body in create_quota_notification_rule() - Added 'holdoff' to NOTIFICATION_ACTION_FIELDS for comparison - Updated example playbook to include holdoff=3600 - Added unit test for holdoff parameter - Updated existing test to include holdoff All 87 unit tests pass. Integration test on Pulse PowerScale array successful with holdoff parameter. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Fixed line length violations by breaking long lines - Fixed continuation line indentation issues - Maintained Ansible module structure (imports after documentation) - All ansible-lint checks pass - Python syntax validation passes Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Fixed continuation line indentation in smartquota.py - Fixed line length in test_smartquota.py - Added sanity ignore file for expected E402 errors - All pep8 issues now resolved Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Replace underscore with descriptive variable name 'ignored_quota' to comply with pylint naming conventions. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements per-quota notification rule configuration for PowerScale SmartQuotas, enabling users to define custom notification actions (alerts, emails) when quota thresholds are exceeded, denied, violated, or expired.
Features Implemented
Additional Discovery
During integration testing on OneFS 9.15, discovered that the API requires a
holdoffparameter (time in seconds before sending another notification) for notification rules withcondition: "exceeded". This parameter has been added to the implementation.Changes
quota_notification_rulesparameter to smartquota moduleholdoffparameter support (required for OneFS 9.15+)Test plan
Generated with Devin