Modules are not idempotent

[smnmtzgr]

I think the modules of this ansible collection are not correct. A lot of them are not working as Ansible expects: they are not idempotent.

E.g. there shouldnt be "state: create" and "state: modify". "state: create" should be enough. At the moment executing modules with "state: create" creates the resource at first execution and if it gets executed again we get an error that the resource already exists. Thats not how Ansible modules should work. If they work correctly they give a changed=false response (if the resource already exists) when executed the 2nd or additional times. And if the resource already exists it should still check if all given module arguments are still in sync with the real state on the device. So create and modify should be the same...

I've tested it with the module dellemc.datadomain.mtree for example.

I also think that the naming "state: create" and "state: destroy" is not really a good choice. It would be better to use "state: present" (create/modify the object) and "state: absent". Thats more "Ansible-like". The same is the case for "state: rename". This shouldn`t be needed -> "state: present" should do the work.

E.g. you could check the documentation of the Netapp ONTAP modules: https://docs.ansible.com/ansible/latest/collections/netapp/ontap/na_ontap_aggregate_module.html#ansible-collections-netapp-ontap-na-ontap-aggregate-module

Here you can see that it has all this functionality of creating, deleting, renaming, adding, modifying things... but there are only the two states "state: present" and "state: absent". And the tasks can be run multiple times leading to the same result.

I've also noticed that "check-mode" (--check parameter for ansible-playbook) is not correctly implemented by the datadomain-modules. When using --check here the task is completely skipped. Thats not how it should work. The correct thing is: --check should be like a "dry-run". It should show if the task would change things or not (green/yellow output), but the change shouldnt be really executed.

[smnmtzgr]

ping: @kshirs2 / @gk4delltech

[kshirs2]

Hello Simon, Thanks for evaluating the data domain collection and providing valuable feedback.

For Idempotency - The Data domain module uses ssh commands instead of rest API as it has yet to evolve to the point where it would do all tasks. Since ssh commands and their syntax are different for each operation, it would be challenging to draft a command for each action and run it together. Ex. nfs has different commands for creating/destroying export, adding/removing clients, and adding/changing options. So if you use the present state, you must run all three/four commands one after another based on whether the resource/setting exists. I am not saying that it is impossible to achieve idempotency, but it would not be easy. I am looking at this as an enhancement request for future releases rather than an issue. check-mode With the current code, it does skip the plays. It's like it is due to different ssh commands for each action. We will take care of this in future releases. I will close these issues as a collection is built like it and track them as enhancement requests. I will let you know the progress of it.

[smnmtzgr]

@kshirs2 thanks for your answer. But I think thats no good approach at all. In this state I wouldn't recommend any customers to use this ansible datadomain-collection.

Please consider the following things:

• Idempotency is not "good to have", I think it is just as Ansible should work! It is very bad when running tasks multiple times lead to different results. (e.g. running a "state: create" task two times leads to an error with your modules...)
• I dont think it is impossible to implement the things you've mentioned. Thats exactly one of the things we should solve with ansible. The logic if something should be created or modified should be in the modules. It is the task of the module-programmer to hide/solve the complexity in the ansible modules to give the users an easy to use interface (with ansible). Thats not the case with your module-collection.
• To solve it you could just do a "lookup" before doing anything. And then your modules should decide what to do: create or modify. And then they could execute the needed commands.
• Same with check-mode... I think this should be easy to implement. I dont understand whats the problem here -> just query whats the actual state on the device. And then decide what to do.

[laugmanuel]

I definitely support all the points @smnmtzgr mentions! This is not a nice to have enhancement request, but a requirement for enterprise Ansible support!

Interestingly, Dell itself recognizes this for other Storage stuff: https://infohub.delltechnologies.com/l/dell-powermax-ansible-modules-best-practices-1/idempotency-and-ansible

[thomas-merz]

I also agree that idempotency is a must-have not a nice-to-have.

[kshirs2]

Hello, We are working on it and will update the repository once it's achieved. Meanwhile, we will add notes in the documentation for idempotency so users know this collection does not support it.

[smnmtzgr]

@kshirs2 thanks this sounds good. If you need test-candidates don't hesitate to ask for help.

[DSI-BenThomas]

Any progress on this? @kshirs2

[kshirs1]

Sorry I could not reply earlier. Right now, there has yet to be progress. This module is based on commands and each command has its combination to get the job done. However, I am checking all possibilities to make it idempotent. I'll make sure to update you soon. I appreciate your patience.

[kshirs2]

Hi, I have been trying to make the modules to idempotent in branch iden-1.0.2. I want to request your help to evaluate and provide feedback. Thank you. @laugmanuel @smnmtzgr @thomas-merz @DSI-BenThomas