ansible-collections / community.vmware

Ansible Collection for VMware
GNU General Public License v3.0
337 stars 332 forks source link

vmware_guest_snapshot module works even if the folder parameter points to a non-existant folder. #856

Open sg1970 opened 3 years ago

sg1970 commented 3 years ago
SUMMARY

https://docs.ansible.com/ansible/latest/collections/community/vmware/vmware_guest_snapshot_module.html

according to the cryptic document linked above the "Folder" parameter of the vmware_guest_snapshot module says that it is "Destination folder, absolute or relative path to find an existing guest". What does that really mean ? I ask this because I can get away by specifying any existing OR non-existing folder name for the snapshot to be created. there is no difference. The only constraint is that the vm name needs to be correct and some dummy folder name specified.

Can someone please clarify in a non-cryptic manner what this means and what is correct value that needs to be passed and the implications if any of passing wrong value for the folder parameter?

ISSUE TYPE
COMPONENT NAME

vmware_guest_snapshot

ANSIBLE VERSION

$ ansible --version
ansible 2.9.10
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/my_username/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.7/dist-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.7.8 (default, Jun 29 2020, 05:46:05) [GCC 5.4.0 20160609]
CONFIGURATION
$ ansible-config dump --only-changed
DEFAULT_TIMEOUT(/etc/ansible/ansible.cfg) = 10
HOST_KEY_CHECKING(/etc/ansible/ansible.cfg) = False
OS / ENVIRONMENT

VCenter version: 6.7.0. Build: 17712750

OS Version: $ cat /etc/*release DISTRIB_ID=Ubuntu DISTRIB_RELEASE=16.04 DISTRIB_CODENAME=xenial DISTRIB_DESCRIPTION="Ubuntu 16.04.6 LTS" NAME="Ubuntu" VERSION="16.04.6 LTS (Xenial Xerus)" ID=ubuntu ID_LIKE=debian PRETTY_NAME="Ubuntu 16.04.6 LTS" VERSION_ID="16.04" HOME_URL="http://www.ubuntu.com/" SUPPORT_URL="http://help.ubuntu.com/" BUG_REPORT_URL="http://bugs.launchpad.net/ubuntu/" VERSION_CODENAME=xenial UBUNTU_CODENAME=xenial

STEPS TO REPRODUCE

https://docs.ansible.com/ansible/latest/collections/community/vmware/vmware_guest_snapshot_module.html

Use any example from the above link and change the folder parameter to a dummy folder value like /junk
EXPECTED RESULTS

The task should error but does not.

ACTUAL RESULTS
VM Snapshot is created for the VM.
ansibullbot commented 3 years ago

Files identified in the description:

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

click here for bot help

ansibullbot commented 3 years ago

cc @Akasurde @Tomorrow9 @goneri @lparkes @nerzhul @pgbidkar @warthog9 click here for bot help

sky-joker commented 3 years ago

Hi @sg1970

Folder parameter is a path for VM you’d like to create a snapshot.
When an absolute path should begin a datacenter name.

Absolute Path Example

/DC01/vm

The vm is a standard path that is stored VMs.

Relative path Example

/DC01/vm/demo

You can write the following like for a relative path based the above path.

/demo

or

demo

or

vm/demo

and so on

By the way, the module will be searched VMs recursive from a specified folder.

Does that answer your question?

sg1970 commented 3 years ago

@sky-joker Thanks for taking the time to answer. But unfortunately I don't think it answers my question. My main point is I can provide any junk value for the folder: parameter and it will work ( BTW this junk folder is not visible on vcenter because it does not exist). However if I leave out the folder parameter it throws an error ( error msg = 'parameters are required together: name, folder' ). It does not make any sense. I mean what is the point of the folder parameter if I can randomly provide any junk value ?

I hope that clarifies my question.

    - name: Create a VM snapshot
      vmware_guest_snapshot:
        hostname: "{{ vsphere_vcenter_hostname }}"
        username: "{{ vsphere_username }}"
        password: "{{ vsphere_password }}"
        datacenter: "{{ vsphere_datacenter_name }}"
        folder: "/junk_loc123456/"  ## <-- this is a junk folder name that does not exist on VCenter and yet the module works and a snapshot is created.
        name: "{{ vsphere_vmname }}"
        state: present
        snapshot_name: "{{vsphere_vmname}}_{{date_time}}"
        description: "{{vsphere_vmname}}_{{date_time}}_Testing"
        validate_certs: no
      delegate_to: localhost
Akasurde commented 3 years ago

@sg1970 Folder is required as an additional value to determine the uniqueness of VM since VMware allows multiple objects with the same name (under different folder structures).

Historically, the folder value was optional and due to which a different set of problems were introduced. So, we made it required with name parameter.

I hope this answers your question.

sg1970 commented 3 years ago

@Akasurde thanks for the reply. So the folder param is required but is not used by the vmware_guest_snapshot module ? If so are there any un-intended consequences of specifying a bogus non existent folder name when using vmware_guest_snapshot to create snapshots and delete them ?

Akasurde commented 3 years ago

It is not like folder parameter not used entirely. Basically, when we encounter two virtual machines with the same name we use folder value to determine which VM the user wants.

There are no un-intended consequences of specifying a bogus folder value that I know of (unless you have duplicate VM names).

mariolenz commented 3 years ago

@Akasurde

There are no un-intended consequences of specifying a bogus folder value that I know of (unless you have duplicate VM names).

Nevertheless, this is quite a weird behavior. If you specify a VM A in folder B without having a folder B, I think the module should crash. Everything else doesn't make sense to me. It just feels wrong.

And as you say yourself, there are un-intended consequences when you do have duplicate VM names.

sg1970 commented 3 years ago

@mariolenz Agree with you. If a foldername is required then the module (vmware_guest_snapshot) should first check if the folder exists and if so only act if the VM specified in the name parameter is present under that folder only and not go looking for it elsewhere.

slightly offtopic: Also are there other modules that have the similar behavior !! ?

SipSeb commented 2 years ago

I just stumbled upon this bug when searching an explanation for a strange behavior of my playbook. And yes, other modules have this problem, too.

In my case, I want to check for an existing machine before setting up a new one, using the vmware_guest_info module. And I have the same machine name in different folders. So I specify DC1/cloud/test as folder, and it finds the machine in DC1/cloud/dev folder. This is obviously wrong.

When trying to create a new machine in the desired folder (using the vmware_guest module), it alters the machine found with the same name in the other folder instead. There's an old bug in the main ansible repo which already said the same thing back in 2019.

Currently, I don't know how to work around this.