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: Refactor disc logic #39285

Merged
merged 1 commit into from
Sep 7, 2018
Merged

VMware: Refactor disc logic #39285

merged 1 commit into from
Sep 7, 2018

Conversation

Akasurde
Copy link
Member

SUMMARY
  • Refactoring related to network device
  • Assign unique random temporary key while creating SCSI or/and IDE controller devices
  • Add testcase for this change

Fixes: #38679

Signed-off-by: Abhijeet Kasurde akasurde@redhat.com

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/modules/cloud/vmware/vmware_guest.py
test/integration/targets/vmware_guest/tasks/cdrom_d1_c1_f0.yml

ANSIBLE VERSION
2.6devel

@ansibot
Copy link
Contributor

ansibot commented Apr 25, 2018

@ansibot ansibot added bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests. vmware VMware community labels Apr 25, 2018
@gundalow gundalow removed the needs_triage Needs a first human triage before being processed. label Apr 25, 2018
assert:
that:
- "cdrom_vm.failed == false"
- "cdrom_vm.changed == true"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth calling the task again and checking .changed == false?

Copy link
Member Author

@Akasurde Akasurde Apr 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Thanks for suggestion. I will update it with same.

@Akasurde
Copy link
Member Author

cc @dericcrago @pdellaert Could you please review this ?

@MikeKlebolt
Copy link
Contributor

@Akasurde , I'm getting the following error when I use this patch:

  msg: 'Failed to create a virtual machine : The device ''5'' is referring to a nonexisting controller ''-,278''.'
fatal: [vm -> localhost]: FAILED! => changed=false
  invocation:
    module_args:
      annotation: null
      cdrom:
        type: client
      cluster: null
      customization: {}
      customvalues:
      - key: sched.cpu.latencySensitivity
        value: normal
      - key: sched.mem.prealloc
        value: 'True'
      - key: sched.swap.vmxSwapEnabled
        value: 'False'
      - key: isolation.tools.connectable.disable
        value: 'True'
      - key: tools.setInfo.sizeLimit
        value: '1048576'
      - key: isolation.tools.diskShrink.disable
        value: 'True'
      - key: isolation.tools.diskWiper.disable
        value: 'True'
      - key: isolation.tools.hgfs.disable
        value: 'True'
      - key: isolation.tools.copy.disable
        value: 'True'
      - key: isolation.tools.paste.disable
        value: 'True'
      - key: isolation.tools.setGUIOptions.enable
        value: 'False'
      - key: isolation.tools.setOption.disable
        value: 'True'
      - key: log.rotateSize
        value: '100000'
      - key: log.keepOld
        value: '6'
      - key: isolation.device.connectable.disable
        value: 'True'
      - key: isolation.device.edit.disable
        value: 'True'
      - key: isolation.tools.log.disable
        value: 'True'
      - key: isolation.tools.unity.push.update.disable
        value: 'True'
      - key: isolation.tools.ghi.launchmenu.change
        value: 'True'
      - key: isolation.tools.hgfsServerSet.disable
        value: 'True'
      - key: isolation.tools.memSchedFakeSampleStats.disable
        value: 'True'
      - key: isolation.tools.getCreds.disable
        value: 'True'
      datacenter: REDACTED
      disk:
      - datastore: local-datastore
        size_gb: 30
        type: thin
      esxi_hostname: REDACTED
      folder: /DCC/REDACTED/vm
      force: false
      guest_id: rhel6_64Guest
      hardware:
        hotadd_cpu: false
        hotremove_cpu: false
        memory_mb: '1024'
        num_cpus: '1'
        scsi: paravirtual
      hostname: REDACTED
      is_template: false
      linked_clone: false
      name: REDACTED
      name_match: first
      networks:
      - device_type: vmxnet3
        name: vlan1640_jumpkick
        type: dhcp
      - device_type: vmxnet3
        name: vlan603
        type: dhcp
      password: VALUE_SPECIFIED_IN_NO_LOG_PARAMETER
      port: 443
      resource_pool: null
      snapshot_src: null
      state: present
      state_change_timeout: 0
      template: null
      username: REDACTED
      uuid: null
      validate_certs: false
      vapp_properties: []
      wait_for_ip_address: false
  msg: 'Failed to create a virtual machine : The device ''5'' is referring to a nonexisting controller ''-,278''.'

@Akasurde
Copy link
Member Author

Akasurde commented May 1, 2018

@MikeKlebolt Thanks for testing this change. Do you face this error when you create a new VM or reconfigure existing one ?

@MikeKlebolt
Copy link
Contributor

Sorry for not clarifying. This is when creating a new VM.

@ansibot
Copy link
Contributor

ansibot commented May 1, 2018

@bbeaudoin
Copy link

Ran into this issue with Ansible 2.5.2. Previously virtual hard disks were being improperly associated with the ide controller despite requesting a scsi controller and, when a virtual cdrom was present, the VM would fail to create when a second disk was requested.

With the updated vmware-guest.py in the PR, the virtual cdrom was associated with ide0:0 and virtual disks were assigned to scsi0:n as expected.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 8, 2018
@ansibot ansibot added the affects_2.6 This issue/PR affects Ansible v2.6 label May 24, 2018
@toxicvengeance
Copy link
Contributor

Can confirm what @bbeaudoin said. Virtual disks are now assigned to scsi0:n. Before they were incorrectly assigned to IDE which makes hotplugging impossible.

Would be nice if this PR gets accepted soon.

@Akasurde
Copy link
Member Author

@toxicvengeance Thanks for the feedback.

@Akasurde
Copy link
Member Author

@pdellaert @dericcrago Could you please review this ?

@Alex5252
Copy link

I checked the version cadbd6e#diff-ed147163551d1eaa084b21f6bab29f96
that gave an error:
"Failed to create a virtual machine : Number of virtual devices exceeds the maximum for a given controller."

With commit https://github.com/ansible/ansible/pull/39285/files in this thread performs a playbook with my section (creating two discs) without errors:

  disk:
  - size_gb: 8
    type: thin
    autoselect_datastore: no
    datastore: datastore1
  - size_gb: 123
    type: thin
    autoselect_datastore: no
    datastore: datastore1
  hardware:
    num_cpus: 2
    memory_mb: 8192
    scsi: lsilogic
  cdrom:
    type: iso
    iso_path: "[datastore1] isos/disk1.iso"
  networks:

All ok.

* Refactoring related to network device
* Assign unique random temporary key while creating SCSI or/and IDE controller devices
* Add testcase for this change

Fixes: ansible#38679

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Aug 21, 2018
@njohnsonmortenson
Copy link

This has fixed my issue related to not being able to mount more than one hard drive during setup, with the same issue as Alex above. All OK after pushing this commit to our Ansible setup.

    vmware_guest:
      hostname: '{{ vsphere_host }}'
      validate_certs: no
      cluster: '[removed]'
      datacenter: '[removed]'
      folder: Test
      name: '{{ inventory_hostname }}'
      state: poweredon
      guest_id: windows8Server64Guest
      annotation: "{{ notes }} - {{ creationdate }}"
      disk:
      - size_gb: "{{vm_cdrive}}"
        type: thin
        datastore: '{{ vsphere_datastore }}'
      - size_gb: "{{vm_ddrive}}"
        type: thin
        datastore: '{{ vsphere_datastore }}'
      networks:
      - name: 'NPD_EntServer_916_10.9.16.0'
        device_type: vmxnet3
      hardware:
        memory_mb: '{{ vm_memory }}'
        num_cpus: '{{ vm_cpus }}'
      cdrom:
        type: iso
        iso_path: '[ntap_n2_ISO] LiteTouchMedia.iso'

@Akasurde Akasurde merged commit fd985db into ansible:devel Sep 7, 2018
@Akasurde Akasurde deleted the i38679 branch September 7, 2018 11:34
Akasurde added a commit to Akasurde/ansible that referenced this pull request Oct 12, 2018
* Refactoring related to network device
* Assign unique random temporary key while creating SCSI or/and IDE controller devices
* Add testcase for this change

Fixes: ansible#38679

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
(cherry picked from commit fd985db)
Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
abadger pushed a commit that referenced this pull request Oct 12, 2018
* Refactoring related to network device
* Assign unique random temporary key while creating SCSI or/and IDE controller devices
* Add testcase for this change

Fixes: #38679

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
(cherry picked from commit fd985db)
Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.6 This issue/PR affects Ansible v2.6 bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests. vmware VMware community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vmware_guest: Multiple disks and cdrom fails to create
8 participants