dcnm_vrf: Pydantic integration (replaces #400) #550
Conversation
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
1. plugins/module_utils/vrf/dcnm_vrf_v12.py - push_diff_attach json.dumps() the payload before passing to send_to_controller() With the change to send_to_controller in the last commit, integration test is failing due to new requirement that payload format is the responsibility of the caller.
1. plugins/module_utils/vrf/dcnm_vrf_v12.py - push_diff_delete json.dumps() the payload before passing to send_to_controller() With the change to send_to_controller in the last commit, integration test is failing due to new requirement that payload format is the responsibility of the caller.
1. plugins/module_utils/vrf/dcnm_vrf_v12.py - push_diff_deploy json.dumps() the payload before passing to send_to_controller() With the change to send_to_controller in the last commit, integration test is failing due to new requirement that payload format is the responsibility of the caller.
1. plugins/module_utils/vrf/dcnm_vrf_v12.py - push_diff_detach json.dumps() the payload before passing to send_to_controller() With the change to send_to_controller in the last commit, integration test is failing due to new requirement that payload format is the responsibility of the caller.
1. plugins/module_utils/vrf/dcnm_vrf_v12.py - push_diff_undeploy json.dumps() the payload before passing to send_to_controller() With the change to send_to_controller in the last commit, integration test is failing due to new requirement that payload format is the responsibility of the caller.
1. plugins/module_utils/vrf/dcnm_vrf_v12.py - push_diff_create_update json.dumps() the payload before passing to send_to_controller() With the change to send_to_controller in the last commit, integration test is failing due to new requirement that payload format is the responsibility of the caller.
1. plugins/module_utils/vrf/dcnm_vrf_v12.py - diff_merge_create Replace call to dcnm_send + handle_response, with call to send_to_controller.
This is an experimental commit. Leaving original code, commented-out, until integration tests are verified to pass. 1. plugins/module_utils/vrf/dcnm_vrf_v12.py - push_diff_attach Refactor lanAttachList update code into new method. - update_lan_attach_list New method which updates the lanAttachList. Refactored out of push_diff_attach.
Fix below error ERROR: plugins/module_utils/vrf/dcnm_vrf_v12.py:3147:1: W293: blank line contains whitespace
No functional changes in this commit. Remove commented code after verifying integration tests pass.
Note, we are retaining the original code - renamed to diff_for_attach_deploy_orig - since doing so makes the diff much easier to read. We will remove this in the following commit.
1. plugins/module_utils/vrf/dcnm_vrf_v12.py
- diff_for_attach_deploy
- Initial refactor
- Simplify logic
- Leverage new helper methods
- _prepare_attach_for_deploy
- _extension_values_match
- _deployment_status_match
Fix the errors below: ERROR: plugins/module_utils/vrf/dcnm_vrf_v12.py:676:8: logging-fstring-interpolation: Use lazy % formatting in logging functions ERROR: plugins/module_utils/vrf/dcnm_vrf_v12.py:746:14: f-string-without-interpolation: Using an f-string that does not have any interpolated variables
Remove original diff_for_attach_deploy method after verifying that integration tests pass after refactoring.
- populate_have_create Avoid potential KeyError by using dict.pop(“vrfStatus”, None) instead of del dict[“vrfStatus”]
We are retaining the original version of populate_have_attach — renamed to populate_have_attach_orig — so that the diff is easier to read.
We will remove this in the next commit after verifying integration tests still pass.
- populate_have_attach
- refactor
- simplify
- Rather than update attach dict in place, populate a new attach_dict
- This removes the need to delete keys not used in the payload
- leverage new method _update_vrf_lite_extension
- Small logic change for determining attach_state
Remove original version of populate_have_attach after verifying integration tests pass.
- _update_vrf_lite_extension Return deepcopy of updated attach dict.
get_diff_query_for_vrfs_in_want - Remove one level of indentation by leveraging a lookup hash - Combine conditionals in for loop - Update log messages - Return a deepcopy of the query diff
1. format_diff - Refactor into the following methods - format_diff_attach - format_diff_create - format_diff_deploy - Simplify the logic vs the original code We’re retaining the original method, renamed to format_diff_orig. We’ll remove it in the next commit after verifying integration tests pass.
No functional changes in this commit. - diff_for_attach_deploy Update code comments for better clarity.
No functional changes in this commit. - format_diff_orig Remove method after verifying integration tests pass with refactored methods.
1. get_diff_delete Refactor into the following two methods: - _get_diff_delete_with_config Detach, undeploy, and delete the VRFs specified in self.config - _get_diff_delete_without_config Detach, undeploy, and delete all VRFs. We are retaining the original method, renamed to get_diff_delete_orig, until we’ve verified that integration tests pass.
1. get_diff_replaced - Simplify logic We are retaining the original method, renamed to get_diff_replaced_orig, until we’ve verified that integration tests pass.
Remove the following original methods after verifying that integration tests pass. - get_diff_delete_orig - get_diff_replace_orig
- update_vrf_attach_vrf_lite_extensions - Simplify logic This is part 1 of what is likely a multi-part refactor. We are retaining the original method, renamed to update_vrf_attach_vrf_lite_extensions_orig, until we verify that integration tests still pass.
1. update_vrf_attach_vrf_lite_extensions_orig Remove method after verifying integration tests pass.
NOTE: We are retaining the original methods (renamed to *_orig) until we’ve verified that integration tests pass. This commit infuses Pyydatic deeper into the dcnm_vrf’s methods. The methods below are all within class NdfcVrfV12. 1. update_lan_attach_list - call get_vrf_lite_objects_model instead of get_vrf_list_objects - operate on the list[ExtensionPrototypeValue] (rather than original list[dict]) - pass lite (now a list of ExtensionPrototypeValue) to update_vrf_attach_vrf_lite_extensions 2. update_vrf_attach_vrf_lite_extensions - Modify to work with list of ExtensionPrototypeValue - call get_extension_values_from_lite_objects with list of ExtensionPrototypeValue - get_extension_values_from_lite_objects now returns a list of VrfLiteConnProtoItem - Use this list of VrfLiteConnProtoItem to update nbr_dict 3. get_extension_values_from_lite_objects This method now operates on Pydantic models exclusively. 4. log_list_of_models It is anticipated that we will be working with lists of models extensively. This utility method logs a list of pydantic models by calling json.dumps(model.model_dump()) on each model in the list. It is currently called from all of the above methods.
Remove original methods after verifying intergration tests pass. Removed the following: - get_extension_values_from_lite_objects_orig - update_vrf_attach_vrf_lite_extensions_orig - update_lan_attach_list_orig Updated docstrings for the replacement methods - get_extension_values_from_lite_objects - update_vrf_attach_vrf_lite_extensions - update_lan_attach_list
1. _update_vrf_lite_extension - call self.get_vrf_lite_objects_model instead of self.get_vrf_lite_objects - leverage vrf_lite model (ControllerResponseVrfsSwitchesV12) to update attach - Rather than hardcode AUTO_VRF_LITE_FLAG to “false”, try to assign the value from the switch (vrf_lite_conn_model.auto_vrf_lite_flag). Assign “false” only if the switch does not provide a value. - Add debug logs.
1. get_vrf_lite_objects_model - Return a list of VrfsSwitchesDataItem models - Renamed to get_list_of_vrfs_switches_data_item_model 2. get_vrf_lite_objects - Remove unused method 3. Multiple methods - Update calls to renamed get_vrf_lite_objects_model to call updated name get_list_of_vrfs_switches_data_item_model - Update accesses to the model given the new returned value is a list of models. 4. Multiple methods - Update debug log to call log_list_of_models when logging the list of VrfsSwitchesDataItem models
Implement automatic allocation of DOT1Q IDs for VRF Lite extensions when not explicitly provided in the playbook. This feature calls the NDFC resource reservation API to allocate DOT1Q IDs dynamically. Changes: - Add get_vrf_lite_dot1q_id() method to call resource reservation API - Update property_values_match() to accept optional skip_prop parameter - Modify update_attach_params_extension_values() to: - Accept serial_number and vrf_name parameters - Auto-allocate DOT1Q ID when not provided in playbook - Update _extension_values_match() to skip DOT1Q_ID comparison when auto-allocated - Update transmute_attach_params_to_payload() to pass additional parameters Addresses Feature #3 from FEATURE_PORTING_PLAN.md (Issues #210, #467) Reference: develop branch commit c475d35 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Update the porting plan to reflect completion of VRF Lite DOT1Q auto-allocation feature. All required changes have been implemented and committed. Progress update: - Total completed features: 3 of 11 - Feature #3 (VRF Lite DOT1Q Auto-Allocation) completed with commit ef52212 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Implement support for IPv6 redistribute direct route-map configuration to provide IPv6 equivalent of the existing IPv4 redist_direct_rmap parameter. Changes: - Add v6_redist_direct_rmap to PlaybookVrfModelV12 - Default value: 'FABRIC-RMAP-REDIST-SUBNET' - Maps to v6VrfRouteMap in template config - Add v6_redist_direct_rmap to VrfTemplateConfigV12 - Alias: v6VrfRouteMap - Automatically serializes to controller payload - Add v6_redist_direct_rmap to dcnm_vrf_v2 module arguments - Type: str - Required: false - Default: 'FABRIC-RMAP-REDIST-SUBNET' The Pydantic models handle automatic serialization/deserialization between playbook parameters (v6_redist_direct_rmap) and controller API fields (v6VrfRouteMap). Addresses Feature #4 from FEATURE_PORTING_PLAN.md (Issue #492) Reference: develop branch commit 349bbeb 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Update the porting plan to reflect completion of IPv6 redistribute route-map feature. All required changes have been implemented and committed. Progress update: - Total completed features: 4 of 11 - Feature #4 (IPv6 Redistribute Route Map) completed with commit c75ee14 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Improve handling of empty string vs None for the instanceValues field
to prevent errors when processing VRF attachments with empty instance
values.
Changes:
- Update condition in diff_for_attach_deploy() to explicitly check for
both None and empty string ("") values
- Before: if want_attach.get("instanceValues") and have_lan_attach_model.instance_values:
- After: Explicitly check (is not None and != "") for both want and have
This prevents attempting to parse empty strings as JSON and ensures
consistent handling of missing/empty instanceValues.
Addresses Feature #5 from FEATURE_PORTING_PLAN.md (Issue #522)
Reference: develop branch commit 3acfab8
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Update the porting plan to reflect completion of empty instanceValues handling feature. All required changes have been implemented and committed. Progress update: - Total completed features: 5 of 11 - Feature #5 (Empty InstanceValues Handling) completed with commit 4141f8a 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Prevent VRF deletion if networks are still attached to the VRF. This ensures data integrity and provides clear error messages to users when attempting to delete a VRF with attached networks. Changes: - Add GET_NET_VRF path to dcnm_vrf_paths for querying networks by VRF - Add check_network_attachments() method to validate no networks attached - Call check_network_attachments() before VRF deletion in both: - push_to_remote() method - push_to_remote_model() method - Provides clear error message directing users to use dcnm_network module The check queries the controller API to retrieve all networks attached to the VRF. If any networks exist, deletion is blocked with fail_json and the user is instructed to remove network attachments first. Addresses Feature #6 from FEATURE_PORTING_PLAN.md (Issue #456) Reference: develop branch commit 28c16fe 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Update the porting plan to reflect completion of network attachment check during VRF deletion feature. All required changes have been implemented and committed. Progress update: - Total completed features: 6 of 11 - Feature #6 (Network Attachment Check During Deletion) completed with commit 40aa7bf 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Improve cleanup of orphaned VRF resources to handle multiple VRFs at once and process both TOP_DOWN_VRF_VLAN and TOP_DOWN_L3_DOT1Q resource pools. Changes: - Update release_orphaned_resources() signature to accept vrf_del_list parameter instead of single vrf - Add support for multiple resource pools (TOP_DOWN_VRF_VLAN and TOP_DOWN_L3_DOT1Q) - Update filtering logic to check if entityName is in vrf_del_list - Add validation checks for ipAddress and switchName to avoid deleting invalid Fabric-scoped resources - Update call sites to pass entire VRF deletion list instead of looping and calling once per VRF - Add debug logging for VRF deletion list and resource pool processing This enhancement improves efficiency by processing all VRFs in a single operation and ensures proper cleanup of both VLAN and DOT1Q resources (e.g., VRF Lite extensions). Addresses Feature #7 from FEATURE_PORTING_PLAN.md Reference: develop branch commits (various cleanup improvements) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Update the porting plan to reflect completion of orphaned resources cleanup enhancement feature. All required changes have been implemented and committed. Progress update: - Total completed features: 7 of 11 - Feature #7 (Orphaned Resources Cleanup Enhancement) completed with commit 5f0eba0 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Improve attach state determination to use explicit bool conversion
with default value. This ensures consistent behavior when isLanAttached
is missing from the controller response.
Changes:
- Update get_have_deploy() to use explicit attach_state variable
- Change from: deploy = attach.get("isLanAttached")
- Change to: attach_state = bool(attach.get("isLanAttached", False))
deploy = attach_state
- Provides explicit False default when isLanAttached is missing
- Ensures attach_state is always a boolean rather than potentially None
This prevents subtle bugs that could occur if the controller response
doesn't include the isLanAttached field.
Addresses Feature #8 from FEATURE_PORTING_PLAN.md (Issue #522 partial)
Reference: develop branch commit 3acfab8
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
…efinement) as completed Document completion of Feature #8 with commit hash d4b5c4f and implementation details. Updated attach state determination in get_have_deploy() to use explicit bool conversion with default value for improved robustness when handling controller responses. Progress: 8 of 11 features completed (73%) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Feature #9 (VRF Deletion Failure Fix from commit faeae9b) was already implemented as part of Feature #7 (Orphaned Resources Cleanup Enhancement - commit 5f0eba0). All fixes from the reference commit are present: - Validation for ipAddress and switchName to prevent deletion of invalid resources - Signature updated to accept vrf_del_list instead of single vrf - Call sites updated to build list and call once - Debug logging added throughout cleanup process Progress: 9 of 11 features completed (82%) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Port fixes from commit 416fa1a to handle "Fail" messages in NDFC response DATA. Changes: - Added search_nested_json() utility function to dcnm.py - Recursively searches nested dicts/lists for a search string - Case-insensitive search of all string values - Returns True if found, False otherwise - Updated dcnm_vrf_v12.py: - Imported search_nested_json from dcnm module - Added check in handle_response() to search for "fail" in response DATA - Sets fail=True and changed=False when "fail" found in DATA This prevents the module from incorrectly succeeding when the controller returns a "Fail" status buried in nested response DATA fields. Fixes issues #324, #457 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Feature #10 (Response Data "Fail" Message Handling) successfully implemented with commit 3647b9c. Added search_nested_json() utility function and integrated it into handle_response() to detect "Fail" messages in nested response DATA fields. This prevents the module from incorrectly succeeding when the controller returns failures embedded in response DATA despite a 200 OK HTTP status. Progress: 10 of 11 features completed (91%) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Feature #11 (Import Statement Cleanup) verified as already complete. All imports in dcnm_vrf_v12.py are: - Properly formatted by isort - All imported functions are actively used - No unused imports exist Final Status: 11 of 11 features completed (100%) All features from dcnm_vrf.py (develop branch) have been successfully ported to the new Pydantic-based dcnm_vrf_v2.py implementation. Features completed: 1. L3VNI Without VLAN Support 2. Deploy Flag Handling Fix 3. VRF Lite DOT1Q Auto-Allocation 4. IPv6 Redistribute Route Map 5. Empty InstanceValues Handling 6. Network Attachment Check During Deletion 7. Orphaned Resources Cleanup Enhancement 8. Attach State Logic Refinement 9. VRF Deletion Failure Fix (included in #7) 10. Response Data "Fail" Message Handling 11. Import Statement Cleanup 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Updated unit tests to account for new network attachment validation added in Feature #6 (Network Attachment Check During Deletion). Changes: - Added empty_network_data fixture with proper DATA field structure - Added mock_pools_top_down_l3_dot1q fixture for DOT1Q pool cleanup - Updated all deletion-related tests to mock network attachment check - Updated resource cleanup tests to mock both VLAN and DOT1Q pools Fixtures added: - empty_network_data: Returns empty DATA array for network check - mock_pools_top_down_l3_dot1q: Empty DOT1Q pool for cleanup Tests fixed: - test_dcnm_vrf_v2_12_delete_std - test_dcnm_vrf_v2_12_delete_std_lite - test_dcnm_vrf_v2_12_delete_dcnm_only - test_dcnm_vrf_v2_12_delete_failure - test_dcnm_vrf_v2_12_override_with_deletions - test_dcnm_vrf_v2_12_lite_override_with_deletions All 5 previously failing tests now pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add empty_vrf_lite_data fixture with required REQUEST_PATH field to satisfy Pydantic model validation - Reorder mock sequence to put mock_vrf_lite_obj before empty_network_data for correct VRF Lite processing flow - Update test assertions to use response[1] instead of response[0] to account for network attachment check response in the result array - All 45 VRF v2 unit tests now pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Both of the tests below already exist in this file, with identical contents. test_dcnm_vrf_v2_12_check_mode test_dcnm_vrf_v2_12_merged_new
Fixing assert for this test.
ControllerResponseInt -> ControllerResponseGetIntV12
This seems to have been missing in the version merged from the develop branch.
Fix the errors below: ERROR: Found 3 ignores issue(s) which need to be resolved: ERROR: tests/sanity/ignore-2.16.txt:1:1: File 'plugins/action/tests/unit/ndfc_pc_members_validate.py' does not exist ERROR: tests/sanity/ignore-2.16.txt:127:1: Duplicate 'import-3.10' ignore for path 'plugins/module_utils/common/sender_requests.py' first found on line 18 ERROR: tests/sanity/ignore-2.16.txt:128:1: Duplicate 'import-3.11' ignore for path 'plugins/module_utils/common/sender_requests.py' first found on line 19
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This PR is based on #400 and integrates all PRs related to dcnm_vrf.py that have been merged into the develop branch back into dcnm_vrf_v2.py. Hence, dcnm_vrf_v2.py is now at feature-parity with dcnm_vrf.py in the develop branch.
Below is the content of #400 describing the objectives for dcnm_vrf_v2.py.
dcnm_vrfintegration tests for readabilityCaveats
Details
Will update as work progresses.
Added files
plugins/module_utils/common/enums/ansible.pyEnumerations related to Ansible.
plugins/module_utils/common/enums/bgp.pyplugins/module_utils/common/enums/request.pyEnumerations related to HTTP requests
plugins/module_utils/vrf/vrf_controller_to_playbook_v12.pyVrfControllerToPlaybookV12ModelSerialize NDFC version 12 controller payload fields to fields used in a dcnm_vrf playbook.
plugins/module_utils/vrf/vrf_controller_to_playbook.pyVrfControllerToPlaybookModelSerialize payload fields (common to NDFC versions 11 and 12) to fields
used in a dcnm_vrf playbook.
plugins/module_utils/vrf/vrf_playbook_model.pyValidation model for dcnm_vrf playbooks
plugins/module_utils/common/models/ipv4_cidr_host.pyModel to validate a CIDR-format IPv4 host address.
Changed files
All integration tests
"{{ var }}"was used in asserts, change tovarplugins/modules/dcnm_vrf.py