ansible-collections / ibm.storage_virtualize

GNU General Public License v3.0
3 stars 9 forks source link

Error message for changes to Thin on ibm_svc_manage_volume always triggers when "Thin = False", even if the volume is already thick/generic #32

Open mainline-automation opened 1 year ago

mainline-automation commented 1 year ago
SUMMARY

In new change to ibm_svc_manage_volume, the module is supposed to respond with a message when a task attempts to change the thin provisioned status of a volume. However, if the task is attempting to change a volume to Thin = False, it will always respond with that message.

I have included a fix in my current pull request #31

ISSUE TYPE
COMPONENT NAME

ibm_svc_manage_volume

ANSIBLE VERSION
ansible [core 2.15.4]
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/ansible/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.9/site-packages/ansible
  ansible collection location = /home/ansible/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.9.16 (main, Sep 12 2023, 00:00:00) [GCC 11.3.1 20221121 (Red Hat 11.3.1-4)] (/usr/bin/python3)
  jinja version = 3.1.2
  libyaml = True
COLLECTION VERSION
# /home/ansible/playbooks/flashsystem_setup/collections/ansible_collections
Collection             Version
---------------------- -------
ibm.storage_virtualize 2.1.0
CONFIGURATION
COLLECTIONS_PATHS(/home/ansible/playbooks/flashsystem_setup/ansible.cfg) = ['/home/ansible/playbooks/flashsystem_setup/collections']
CONFIG_FILE() = /home/ansible/playbooks/flashsystem_setup/ansible.cfg
DEFAULT_REMOTE_USER(/home/ansible/playbooks/flashsystem_setup/ansible.cfg) = ansible
GALAXY_SERVER_LIST(/home/ansible/playbooks/flashsystem_setup/ansible.cfg) = ['validated', 'rh-certified', 'community']
OS / ENVIRONMENT

RHEL 9.2, IBM Storwize V7000 724, Storage Virtualize 8.5.0.9

STEPS TO REPRODUCE
- name: Use ibm.storage_virtualize.ibm_svc_manage_volume with a loop to create new volumes from CSV file
    # added_volumes.list is needed from registered variable
    ibm.storage_virtualize.ibm_svc_manage_volume:
      clustername: "{{ clustername }}"
      log_path: /home/ansible/playbooks/flashsystem_setup/pylog2.log
      state: "{{ item.volume_state }}"
      name: "{{ item.name }}"
      size: "{{ item.size }}"
      unit: "{{ item.unit }}"
      thin: "{{ item.thin }}"
      pool: "{{ item.pool }}"
      validate_certs: false
      #token: "{{ api_token }}"
      username: xxxxx
      password: xxxxx
    loop: "{{ volume_csv.list}}"
    register: response
    ignore_errors: true
EXPECTED RESULTS

The expected result is that, when a volume has "False" or "No" for Thin, it would give the error message if it is currently Thin. However, if the volume is already thick/generic, it should just report that the volume already exists.

ACTUAL RESULTS

Volumes that were already Thin = True were ignored when the task ran. Jobs that were already Thin = False triggered the error message

"item": {
                                        "datastore_state": "",
                                        "esx_datastore_name": "",
                                        "esx_datastore_type": "",
                                        "host_fqdn": "esxlab.bpic.mainline.com",
                                        "host_state": "present",
                                        "hostname": "bpicaix04",
                                        "name": "ansible3",
                                        "os": "aix",
                                        "pool": "Pool_01",
                                        "size": "512",
                                        "thin": "False",
                                        "unit": "mb",
                                        "volume_state": "present"
                                    },
                                    "msg": "Update not supported for parameter: ['thin']"
                                },
                                "_ansible_no_log": null,
                                "ansible_facts": {
                                    "transform_list": [
                                        {
                                            "name": "ansible3",
                                            "pool": "Pool_01",
                                            "size": "512",
                                            "thin": "False",
                                            "unit": "mb",
                                            "volume_state": "present"
                                        }
                                    ]
                                },
sumitguptaibm commented 5 months ago

@mainline-automation I think current message correctly indicates that user cannot pass thin (or for that matter, other params such as compressed for changing existing volume's properties), even before testing it's value (true/false). Please let me know your comments, if we can close this?

mainline-automation commented 5 months ago

The issue is that I found that if the volume was already thick/generic and you happened to say "thin=false", it would create an error. So, the volume is already in the state of thin=false, but putting thin=false into the parameters creates an error. It shouldn't react if there's no actual change being made. It should just acknowledge that the volume state is correct and move on.

Beyond that there is the issue that users can do a thick to thin/thin to thick conversion in the GUI but the collection disallows it.

sumitguptaibm commented 5 months ago

@mainline-automation , I'll check on this. If needed, we'll fix it in next (September) release.