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: Support multiple CDROM for Windows #58951

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
5 participants
@oatakan
Copy link

commented Jul 11, 2019

Add support for multi cdrom implementation. This scenario will help enable the use case to create a Windows template on VMware Vcenter from ISO files using Ansible playbook only. In this scenario, first cdrom is windows installation media, second cdrom is an autogenerated iso file that contains Autounattend,xml file to enable automatic installation of Windows without user interaction or other tools. See this galaxy role with this implementation: https://galaxy.ansible.com/oatakan/windows_vcenter_template.

Note that this installation will break backwards compatibility as it moves cdrom attribute from 'dict' to 'list'. Not sure how to implement to allow either for backwards compatibility, suggestions are welcome here to help move this into the project.

SUMMARY

This is needed to support the use case to create a Windows template from scratch.

Fixes #45407

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

vmware guest.py: add support for multiple cdroms

ADDITIONAL INFORMATION
Update vmware_guest.py
Add support for multi cdrom implementation. This scenario will help enable the use case to create a Windows template on VMware Vcenter from ISO files using Ansible playbook only. In this scenario, first cdrom is windows installation media, second cdrom is an autogenerated iso file that contains Autounattend,xml file to enable automatic installation of Windows without user interaction or other tools. See this galaxy role with this implementation: https://galaxy.ansible.com/oatakan/windows_vcenter_template.

Note that this installation will break compatibility as it moves cdrom attribute from 'dict' to 'list'. Not sure how to implement to allow either for backwards compatibility, suggestions are welcome here to help move this into the project.
@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

@oatakan, just so you are aware we have a dedicated Working Group for vmware.
You can find other people interested in this in #ansible-vmware on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

lib/ansible/modules/cloud/vmware/vmware_guest.py:1035:38: singleton-comparison Comparison to None should be 'expr is not None'
lib/ansible/modules/cloud/vmware/vmware_guest.py:1066:23: singleton-comparison Comparison to True should be just 'not expr'
lib/ansible/modules/cloud/vmware/vmware_guest.py:1069:28: singleton-comparison Comparison to True should be just 'not expr'

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/cloud/vmware/vmware_guest.py:1035:60: E711 comparison to None should be 'if cond is not None:'
lib/ansible/modules/cloud/vmware/vmware_guest.py:1066:72: E712 comparison to True should be 'if cond is not True:' or 'if not cond:'
lib/ansible/modules/cloud/vmware/vmware_guest.py:1069:74: E712 comparison to True should be 'if cond is not True:' or 'if not cond:'

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E309 version_added for new option (cdrom) should be '2.5'. Currently '2.9'

click here for bot help

@Akasurde Akasurde requested review from goneri, Akasurde and jillr Jul 11, 2019

@Akasurde

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

@oatakan Thanks for the PR. Could you please take a look at #51120 ? I think these two PRs are trying to solve the same problem. Correct me if I am wrong. Thanks.

@Akasurde Akasurde removed the needs_triage label Jul 11, 2019

@Akasurde Akasurde changed the title Update vmware_guest.py VMware: Support multiple CDROM for Windows Jul 11, 2019

@oatakan

This comment has been minimized.

Copy link
Author

commented Jul 11, 2019

@oatakan Thanks for the PR. Could you please take a look at #51120 ? I think these two PRs are trying to solve the same problem. Correct me if I am wrong. Thanks.

@Akasurde Thanks for your quick response. I didn't originally see that PR. After reviewing, it accomplishes the same. One thing I'm not sure if that PR is set to be included in 2.9? Can you confirm? If so, we can close this one for sure.

Also for PR #51120, I think there should be default values so that user doesn't have to specify type 'controller_type', 'controller_number' and 'unit_number' so that it can be simpler from end-user perspective. I haven't tested it yet but it also needs to be tested to see if it supports addition/deletion scenarios properly which this PR does currently.

@jillr

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

@oatakan #51120 is close to being able to be merged for 2.9, if you would be able to review and test it that would be very helpful to ensure it covers this use case well.

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.