ansible-collections / community.general

Ansible Community General Collection
https://galaxy.ansible.com/ui/repo/published/community/general/
GNU General Public License v3.0
831 stars 1.53k forks source link

Make optional the deletion of volumes with RAIDType 'None' #8962

Closed pyfontan closed 1 month ago

pyfontan commented 1 month ago

Summary

I am trying to create a volume on HPE servers. The default behaviour of their smart array controllers is to create a 'None' RAIDtype volume for each drive, this volume is automatically deleted as soon as a "true" volume is created on drive. https://support.hpe.com/hpesc/public/docDisplay?docId=a00019059en_us&page=GUID-6A7B774C-9F69-4E41-ABD8-F5CCD120BA2C.html#fnsrc_TABLE_D15_Y1L_1TB_1

But the issue is that the DEL method seems to be not allowed on these kind of drive (Returning a 405 http response code)

Issue Type

Feature Idea

Component Name

redfish_utils

Additional Information

Return of a VolumeCreation command

{                                                                                                   
    "changed": false,
    "invocation": {                                                                                                                                            
        "module_args": {
            "auth_token": null,
            "baseuri": "X.X.X.X", 
            "bios_attributes": {},
            "boot_order": [],
            "category": "Systems",
            "ciphers": null,
            "command": [
                "CreateVolume"
            ],
            "hostinterface_config": {}, 
            "hostinterface_id": null,
            "network_protocols": {},
            "nic_addr": "null",
            "nic_config": {},
            "password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "resource_id": null,
            "secure_boot_enable": true, 
            "service_id": null,
            "sessions_config": {},
            "storage_subsystem_id": "DE07A000",
            "strip_etag_quotes": false, 
            "timeout": 60,
            "username": "XXX",
            "volume_details": {
                "CapacityBytes": "960197124096",
                "Drives": [
                    "/redfish/v1/Systems/1/Storage/DE07A000/Drives/36",
                    "/redfish/v1/Systems/1/Storage/DE07A000/Drives/37"
                ],
                "Name": "SYSTEM",
                "RAIDType": "RAID1"
            },
            "volume_ids": []
        }
    },
    "msg": "HTTP Error 405 on DELETE request to 'https://10.10.250.239/redfish/v1/Systems/1/Storage/DE07A000/Volumes/129', extended message: 'Method Not Allowe
d'"
}

Code of Conduct

ansibullbot commented 1 month ago

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot commented 1 month ago

cc @bhavya06 @jyundt @mraineri @rajeevkallur @renxulei @tomasg2012 @xmadsen click here for bot help

mraineri commented 1 month ago

I'm not sure the history of this block of code to delete volumes is present: https://github.com/ansible-collections/community.general/blob/main/plugins/module_utils/redfish_utils.py#L3796

I wouldn't expect any volumes to be deleted automatically... That seems like bad behavior on Ansible's part... I'd much prefer the "CreateVolume" command only creates volumes, and if a user needs to delete volumes, then they should explicitly send "DeleteVolumes" to do so.

I'd be in favor of removing the block of code I referenced; I'd like to do some history digging as to why it was ever needed in the first place though.

pyfontan commented 1 month ago

@mraineri Sounds good to me.

Nevertheless i wrote this if the decision is to keep this block of code: https://github.com/pyfontan/community.general/commit/722cae8de9652bff157918313dc49f392ce26720

mraineri commented 1 month ago

@pyfontan after speaking with some others, we agree that automatically deleting volumes without a user acknowledging this is in poor practice. However, we think having a switch to request this behavior as part of CreateVolume is okay, but we'd like the default option for storage_none_volume_deletion to be false. If we're going to delete volumes, we'd like the user to clearly indicate it's okay to do so. Would you be able to make a pull request for this based on the changes you've already made?

pyfontan commented 1 month ago

@mraineri Ok, i will change the default value and will do a merge request :+1: