Skip to content
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

Fix azure_rm_manageddisk caching comparison #624

Merged
merged 1 commit into from
Feb 11, 2022

Conversation

nbr23
Copy link
Contributor

@nbr23 nbr23 commented Aug 30, 2021

SUMMARY

Fix faulty comparison in is_attach_caching_option_different which returned True when caching was "None", and caused disks to be detached / reattached by mistake.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • azure_rm_manageddisk
ADDITIONAL INFORMATION

The following code displays the issue. It runs the same action twice, but the result will show as "changed" for both.
This is because the attach_caching is None, but the comparison in the module is incorrect, leading the module to believe it needs to update the caching parameter.
As a result, the disk gets detached from the VM and reattached, which could have unexpected consequences and possibly lead to issues.

    - name: Create data disk and attach to "testvm" virtualmachine
      azure.azcollection.azure_rm_manageddisk:
        resource_group: "{{ resource_group }}"
        name: 'testvm-data'
        location: eastus
        disk_size_gb: 64
        managed_by: testvm
        storage_account_type: Standard_LRS

    - name: Same action again
      azure.azcollection.azure_rm_manageddisk:
        resource_group: "{{ resource_group }}"
        name: 'testvm-data'
        location: eastus
        disk_size_gb: 64
        managed_by: testvm
        storage_account_type: Standard_LRS

The proposed patch fixes it, and the second run shows as unchanged, respecting indempotency.

Cheers!

@Fred-sun Fred-sun added medium_priority Medium priority ready_for_review The PR has been modified and can be reviewed and merged labels Aug 30, 2021
@xuzhang3
Copy link
Collaborator

LGTM

@xuzhang3 xuzhang3 merged commit b7b43b8 into ansible-collections:dev Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium_priority Medium priority ready_for_review The PR has been modified and can be reviewed and merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants