Skip to content

Fix storage map accumulation and optimize datastore filtering#49

Open
albertoflorez wants to merge 1 commit into
redhat-cop:mainfrom
albertoflorez:fix/storage-map
Open

Fix storage map accumulation and optimize datastore filtering#49
albertoflorez wants to merge 1 commit into
redhat-cop:mainfrom
albertoflorez:fix/storage-map

Conversation

@albertoflorez

Copy link
Copy Markdown
Contributor

Fixes a critical bug in `mtv_management` role where storage maps were incorrectly accumulating datastores from previous iterations when using `mtv_management_multiple_storage_maps_overrides`.

Technical Details

Bug 1: Variable Name Inconsistency

  • File: `roles/mtv_management/tasks/_mtv_storage_map.yml:5`
  • Issue: Initialized `mtv_management_mtv_storagemaps` but referenced `mtv_management_mtv_storagemap_maps` in process tasks
  • Impact: Dictionary was never reset between loop iterations, causing accumulation
  • Fix: Changed initialization to use `mtv_management_mtv_storagemap_maps`

Bug 2: Dictionary Key Collision

  • File: `roles/mtv_management/tasks/_mtv_storage_map_process_datastore.yml:57,64`
  • Issue: Used `datastore.selfLink` as dictionary key
  • Impact: When same datastore mapped to multiple storage classes, last one overwrote previous entries
  • Fix: Changed key to `datastore.selfLink + '-' + storageClass` for uniqueness

Optimization: Pre-filtering Datastores

  • Files: `roles/mtv_management/tasks/_mtv_storage_map.yml`
  • Issue: Looped through ALL datastores (~100+) then filtered with complex conditionals
  • Impact: 80% unnecessary iterations when using overrides
  • Fix: Extract allowed/excluded IDs upfront, filter before loop (matching `_mtv_network_map.yml` pattern)

Changes

  • `_mtv_storage_map.yml`:
    • Fixed variable initialization
    • Added `Extract included datastore IDs` task
    • Added `Extract excluded datastore IDs` task
    • Modified loop filtering for VMware/Ovirt datastores
  • `_mtv_storage_map_process_datastore.yml`:
    • Updated dictionary key to include storageClass
    • Removed redundant conditional logic (now handled by pre-filtering)

Testing

Validated with configuration containing:

  • multiple storage classes
  • multiple datastores with overlapping mappings
  • Result: Each storage map contains only its designated datastores with correct storageClass

Breaking Changes

None. Fully backward compatible with existing configurations."

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.

1 participant