-
Notifications
You must be signed in to change notification settings - Fork 120
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
[Cancelled] Add support for VMware to OpenStack transformation #430
Conversation
:install_drivers => true | ||
} | ||
end | ||
private_class_method :virtv2vwrapper_options_vmwarews2openstack_ssh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the difference between virtv2vwrapper_options_vmwarews2openstack_vddk
and virtv2vwrapper_options_vmwarews2openstack_ssh
just the :transport_method
key in the returned hash?
If so, I think it would be better to have a single method that varied on what type of transport_method
. I worry about someone having to go back and update this some point later and not realizing they have to change both ssh
and vddk
methods.
Something like:
def virtv2vwrapper_options_vmwarews2openstack(task, transport, handle = $evm)
... # all the stuff that's currently in both methods
...
:transport_method => transport,
...
end
def virtv2vwrapper_options_vmwarews2openstack_vddk(task, handle = $evm)
virtv2vwrapper_options_vmwarews2openstack(task, "vddk", handle)
end
def virtv2vwrapper_options_vmwarews2openstack_ssh(task, handle = $evm)
virtv2vwrapper_options_vmwarews2openstack(task, "ssh", handle)
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a little bit more to it. The credentials are different. And BTW, I forgot a field for SSH transport method: ssh_key
which contains the OpenStack provider SSH private key.
Talking about splitting code in methods, there's also work to do on VMware to RHV methods. I'd rather file another PR for refactoring these. And I'm still waiting on #421.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather file another PR for refactoring these
👍
@miq-bot add-label hammer/yes |
This pull request is not mergeable. Please rebase and repush. |
83c6a5d
to
1d34100
Compare
Some comments on commits fabiendupont/manageiq-content@e689633~...7e77723 content/automate/ManageIQ/Transformation/Infrastructure/VM/openstack.class/methods/setdescription.rb
|
Checked commits fabiendupont/manageiq-content@e689633~...7e77723 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 content/automate/ManageIQ/Transformation/Infrastructure/VM/openstack.class/methods/setdescription.rb
|
@miq-bot add-label wip |
This pull request is not mergeable. Please rebase and repush. |
The changes needed for migration to OpenStack have been implemented in #441. Closing this PR. |
This pull request add support for OpenStack target in transformation. To keep the PR small, some refactoring has been deferred. For example, Infrastructure/VM should be moved to VM, as transformations to Infrastructure or Cloud are very similar. Keeping that separation complexifies the code with no value. Thus, the OpenStack-related code is under Infrastructure/VM/OpenStack.
Some additional data checks are performed in assessment:
The only real new method for OpenStack is SetDescription which is simply declared but does nothing (hence no specs provided), until we know how to do it properly (fog based).
The Transformation Hosts utility class welcomes two new methods that build the options hash for virt-v2v-wrapper, either for VDDK, or for SSH.
Depends on:
Associated RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1632355