Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VMware: Modify conditional expression for VM existence check processing of folder #60679

Open

Conversation

@sky-joker
Copy link
Contributor

commented Aug 16, 2019

SUMMARY

Running a VM clone with vmware_guest failed under the following conditions:.

conditions

For example, create it in the TENANT-B folder with the same name as the VM name in the TENANT-A folder.

スクリーンショット 2019-08-16 19 11 06

playbook

---
- name: VMware VM clone operation
  hosts: localhost
  gather_facts: no
  vars_files:
    - vars/auth.yml
    - vars/vm.yml
  tasks:
    - name: Clone vm from template.
      vmware_guest:
        hostname: "{{ hostname }}"
        username: "{{ username }}"
        password: "{{ password }}"
        validate_certs: no
        datacenter: "{{ datacenter }}"
        folder: "{{ folder }}"
        esxi_hostname: "{{ esxi }}"
        template: "{{ template }}"
        name: "{{ name }}"
        hardware: "{{ hardware }}"
        networks: "{{ vm_networks }}"
        datastore: "{{ datastore }}"
        state: "{{ state }}"

vm.yml

# VM valiables.
datacenter: DC
folder: "/{{ datacenter }}/vm/TENANT-B"
datastore: VM

name: TENANT-VM
template: CentOS7_TMP
esxi: esxi-12.local
hardware:
  num_cpus: 2
  num_cpu_cores_per_socket: 1
  memory_mb: 2048
vm_networks:
  - name: VM Network
state: poweredon

running playbook

(venv) [root@3600ad0bdbd6 vmware]# ansible-playbook main.yml
 [WARNING]: No inventory was parsed, only implicit localhost is available

 [WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

 [WARNING]: Found variable using reserved name: name


PLAY [VMware VM clone operation] *******************************************************************************************************************************************

TASK [Clone vm from template.] *********************************************************************************************************************************************
ok: [localhost]

PLAY RECAP *****************************************************************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

Clone is not executed because it is determined that the VM already exists.

Cause

This is due to a problem with the vmware conditional expression at module_utils.

If exists vm one or more, i think we should run the processing of finding VM from folder.
After modifying the code as follows, the clone was executed.

--- before/vmware.py  2019-08-16 10:27:54.930884918 +0000
+++ after/vmware.py 2019-08-16 10:28:08.866842735 +0000
@@ -904,7 +904,7 @@

             # get_managed_objects_properties may return multiple virtual machine,
             # following code tries to find user desired one depending upon the folder specified.
-            if len(vms) > 1:
+            if len(vms) >= 1:
                 # We have found multiple virtual machines, decide depending upon folder value
                 if self.params['folder'] is None:
                     self.module.fail_json(msg="Multiple virtual machines with same name [%s] found, "
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

module_uitls/vmware.py

@ansibot

This comment has been minimized.

@Akasurde Akasurde self-assigned this Aug 16, 2019

@Akasurde Akasurde requested review from Akasurde, jillr and goneri Aug 16, 2019

@Akasurde Akasurde changed the title Modify conditional expression for VM existence check processing of folder VMware: Modify conditional expression for VM existence check processing of folder Aug 16, 2019

@jillr
jillr approved these changes Aug 16, 2019
@jillr
Copy link
Contributor

left a comment

Change lgtm, could you also please update the tests?

@ansibot ansibot removed the needs_triage label Aug 16, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Aug 17, 2019

@sky-joker sky-joker force-pushed the sky-joker:modify_conditional_expression_of_module_utils_vmware branch 4 times, most recently to 3d1429a Aug 17, 2019

@sky-joker

This comment has been minimized.

Copy link
Contributor Author

commented Aug 17, 2019

As a result of the integration tests, it was found that some modules were affected.
One of the causes is that the folder parameter is not implemented in the module.

@Akasurde
Copy link
Member

left a comment

@sky-joker Thanks for the contribution. Could you please take a look into the additional tests in #60271 and update this PR ? Thanks.

@@ -10,6 +10,7 @@
password: "{{ vcenter_password }}"
name: "{{ virtual_machines[0].name }}"
datacenter: "{{ dc1 }}"
folder: "{{ f0 }}"

This comment has been minimized.

Copy link
@Akasurde

Akasurde Aug 19, 2019

Member

I think this should be {{ virtual_machines[0].folder }}

@@ -8,6 +8,8 @@
hostname: "{{ vcenter_hostname }}"
username: "{{ vcenter_username }}"
password: "{{ vcenter_password }}"
datacenter: "{{ dc1 }}"
folder: "{{ f0 }}"

This comment has been minimized.

Copy link
@Akasurde

Akasurde Aug 19, 2019

Member
Suggested change
folder: "{{ f0 }}"
folder: "{{ item.folder }}"
@sky-joker

This comment has been minimized.

Copy link
Contributor Author

commented Aug 19, 2019

Hi, @Akasurde

Thank you for pointing it out!
I will modify the pointed out part.

Thanks

@sky-joker

This comment has been minimized.

Copy link
Contributor Author

commented Aug 19, 2019

@Akasurde

I checked #60271
The following fixes were required to successfully test vmware_guest_move:.

--- before/vmware.py    2019-08-19 23:12:34.023167127 +0900
+++ after/vmware.py 2019-08-19 23:12:43.984366423 +0900
@@ -990,6 +990,8 @@
                     actual_vm_folder_path = self.get_vm_path(content=self.content, vm_name=vms[0])
                     if actual_vm_folder_path.endswith(self.params['folder']):
                         vm_obj = vms[0]
+                else:
+                    vm_obj = vms[0]
         elif 'moid' in self.params and self.params['moid']:
             vm_obj = VmomiSupport.templateOf('VirtualMachine')(self.params['moid'], self.si._stub)

The reason why else is required is that the vmware_guest_move modules do not implement folder parameter.
It was confirmed that this bug can be solved by using module_utils/vmware.py of #60271.

However, the following challenges remain:.
#60271 (comment)

Thanks

@ansibot ansibot removed the small_patch label Aug 20, 2019

@sky-joker sky-joker force-pushed the sky-joker:modify_conditional_expression_of_module_utils_vmware branch Aug 20, 2019

@sky-joker sky-joker force-pushed the sky-joker:modify_conditional_expression_of_module_utils_vmware branch 2 times, most recently from 674e685 to 407d599 Aug 31, 2019

@ansibot ansibot removed the needs_rebase label Aug 31, 2019

@sky-joker sky-joker force-pushed the sky-joker:modify_conditional_expression_of_module_utils_vmware branch from 70587e4 to 407d599 Aug 31, 2019

@sky-joker

This comment has been minimized.

Copy link
Contributor Author

commented Aug 31, 2019

A change of cdrom_d1_c1_f0.yml is required to test code modified to exact match.
But, changing cdrom_d1_c1_f0.yml results in a conflict and the integration test fails.
Do I need to create a separate PR to resolve this issue?

@agowa338

This comment has been minimized.

Copy link
Contributor

commented Aug 31, 2019

You need to rebase in order to resolve the merge conflict. This isn't what you mean, is it?

@sky-joker sky-joker force-pushed the sky-joker:modify_conditional_expression_of_module_utils_vmware branch from 407d599 to 39002c6 Sep 1, 2019

@ansibot ansibot added the needs_rebase label Sep 1, 2019

@sky-joker sky-joker force-pushed the sky-joker:modify_conditional_expression_of_module_utils_vmware branch from 39002c6 to d9f8527 Sep 1, 2019

@sky-joker

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2019

Thank you @agowa338 for teaching me :)

@ansibot ansibot removed the needs_rebase label Sep 1, 2019

@sky-joker sky-joker force-pushed the sky-joker:modify_conditional_expression_of_module_utils_vmware branch from 1e4e476 to d472113 Sep 1, 2019

@sky-joker sky-joker force-pushed the sky-joker:modify_conditional_expression_of_module_utils_vmware branch from d472113 to a674ecf Sep 1, 2019

@sky-joker

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2019

Modified vm folder path comparison condition, and added an error message that is display when / is included in the datacenter name.

@sky-joker

This comment has been minimized.

Copy link
Contributor Author

commented Sep 3, 2019

ready_for_review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.