ionos-cloud / module-ansible

Apache License 2.0
10 stars 8 forks source link

bug: new module version requires additional field creating discrepancy to the docs #179

Closed hegerdes closed 1 year ago

hegerdes commented 1 year ago

Description

The new 7.0 Version requires to set the licence_type field for additional volumes - despite the docs saying otherwise> This also breaks backwards compatibility.

Expected behavior

You do not need a licence_type field for additional HDDs and SSDs

Environment

Ansible version:

ansible [core 2.15.3]
  config file = None
  configured module search path = ['/home/henrik/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/henrik/.local/lib/python3.9/site-packages/ansible
  ansible collection location = /home/henrik/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/henrik/.local/bin/ansible
  python version = 3.9.2 (default, Feb 28 2021, 17:03:44) [GCC 10.2.1 20210110] (/usr/bin/python3)
  jinja version = 3.1.2
  libyaml = True

Module version:

ionoscloudsdk.ionoscloud:7.0.0-beta.1

OS:

RedHat Linux & Windows

Configuration Files

None

How to Reproduce

Upgrade to the new module version
Create a Task with an additional volume:

- name: attach_volumes | Attach additional volumes to server
  ionoscloudsdk.ionoscloud.volume:
    datacenter: "{{ datacenter.id }}"
    server: "{{ provisioned_server.id }}"
    name: "{{ current_vm.name }}-{{ vm_identifier_postfix }}-vol-{{ volume.name }}"
    size: "{{ volume.size | int }}"
    disk_type: "{{ volume.type }}"
    state: "present"
    wait: true
    # Error if this is not present
    # licence_type: 'OTHER'
    wait_timeout: "{{ ionos_timeout }}"
  loop: "{{ [current_vm.volumes }}"
  loop_control:
    loop_var: volume
    index_var: volume_idx

Error and Debug Output

Require field licence_type or image not set.

Additional Notes

The Docs say, that this field is NOT required and it wasn't before. Now either the docs are wrong or the module has a bug.

References

rmocanu-ionos commented 1 year ago

Hello, The reason this seems to be happening is because we removed the default values for the values which can trigger resource replacement to avoid the case when resources are replaced by mistake. This happens because we do not know when a value is set by the user to the default value or not set and the default 'kicks in'. The default value for licence_type used to be 'UNKNOWN', so you can use that to have the same functionality. We will update the docs regarding the required parameters. Thank you for letting us know!

hegerdes commented 1 year ago

It is totally okay to have breaking changes in a major release but if these happen there is a need for a migration guide.

What I understood from your comment is that this behavior (my described bug) is the new way to do it and users need to provide attributes themselve for parameters that trigger a replacement. Is this correct?

If so can you please provide a list of all the affected resources with removed defaults and what it's original value was? It would be really helpful to include this in the release notes or the migration guide.

rmocanu-ionos commented 1 year ago

This is a great idea, we will make a list of every removed default value and add it to the docs. Yes, the users will need to provide values themselves.