Add V2V for VMware to oVirt / RHV #301

ghost · 2018-05-23T12:56:27Z

This PR aims at merging the code maintained in https://github.com/fdupont-redhat/v2v-automate/ in the manageiq-content repository, so that it can be shipped with the appliance.

Related PR: #312

ghost · 2018-05-23T12:57:01Z

@miq-bot add-label transformation

ghost · 2018-05-23T12:57:16Z

@miq-bot add-label wip

mkanoor · 2018-05-24T18:06:26Z

content/automate/ManageIQ/System/CommonMethods/MiqAe.class/__class__.yaml

@@ -0,0 +1,53 @@
+---
+object_type: class


@fdupont-redhat
This class already exists in master, are there any changes to it.

Nope. But apparently, Git think there is...

@fdupont-redhat
When the first PR was created the directory for MiqAe didn't have the class prefix.
The actual name should have been MiqAe.class but it got stored in the content repo as MiqAe.
I am not sure how you created the class, if you look at other class directories under

https://github.com/ManageIQ/manageiq-content/tree/master/content/automate/ManageIQ/System/CommonMethods

You will see that MiqAe stands out.
Can we fix this issue in a separate PR which might help reduce the # of files in this PR?

ghost · 2018-05-25T15:09:46Z

@mkanoor, @gmcculloug, @tinaafitz, my Ruby skills hit a limit with the last errors:

Lint/RescueException - If I understand correctly, I simply have to replace Exception with StandardError in my rescue statements. Am I right ?
Style/RedundantBegin - I have no clue.
Naming/ClassAndModuleCamelCase - The name of the class is consistent with the name of the automate method. Should I change it to comply with Rubocop ? Then, it wouldn't be consistent with the rest of automate methods...
Detected cloudforms - It is due to the message we have decided to put in the description of the VM upon migration. Should we change it ?

Thanks for your help.

mkanoor · 2018-05-29T18:58:19Z

content/automate/ManageIQ/Transformation/Common.class/__methods__/acquiretransformationhost.rb

@@ -22,7 +22,7 @@ def main
              destination_ems = task.transformation_destination(source_cluster).ext_management_system
              raise "Invalid destination EMS type: #{destination_ems.emstype}. Aborting." unless destination_ems.emstype == "rhevm"

-              transformation_host = ManageIQ::Automate::Transformation::TransformationHosts::Common::Utils.get_transformation_host(destination_ems, 'vddk', factory_config)
+              transformation_host = ManageIQ::Automate::Transformation::TransformationHosts::Common::Utils.get_transformation_host(destination_ems, @handle.get_state_var(:transformation_method), factory_config)


@fdupont-redhat
Should the transformation_method be set in request/task options hash, since we want that information to persist. The state_var is temporary and will disappear when the task ends. The Request/Task options would persist.

@mkanoor
The transformation method is set in the AssessTransformation method. Currently, it's hardcoded to 'vddk', but in future versions 'ssh' will also be available and provided by the task or transformation plan, or even the transformation mapping. We still don't know which of these objects will hold the information.

@fdupont-redhat
Do you want to open a git issue on moving this to the request or task options instead of leaving it in state vars?

Shouldn't I wait for this PR to be merged before ? If not, I'll open the issue.

mkanoor · 2018-05-29T18:59:32Z

...tomate/ManageIQ/Transformation/TransformationHosts/ovirt_host.class/__methods__/rolecheck.rb

@@ -9,7 +9,7 @@ def initialize(handle = $evm)
            end

            def main
-              playbook = "/usr/share/doc/ovirt-ansible-v2v-conversion-host-1.0.0/examples/conversion_host_check.yml"
+              playbook = "/usr/share/doc/ovirt-ansible-v2v-conversion-host-1.2.0/examples/conversion_host_check.yml"


@fdupont-redhat
How do these playbooks get into the appliance, is there a seeding process for each ansible enabled appliance?

@mkanoor
The playbooks are not on the appliance, but on the RHV Manager. They are provided by the ovirt-ansible-v2v-conversion-host package. So nothing to do on ManageIQ side.

mkanoor · 2018-05-30T14:16:58Z

content/automate/ManageIQ/Transformation/Common.class/__methods__/acquiretransformationhost.rb

+              source_vm = task.source
+              @handle.log(:info, "Source VM: #{source_vm.name}")
+              source_cluster = source_vm.ems_cluster
+              @handle.log(:info, "Source Cluster: #{source_cluster.name}")


@fdupont-redhat
You dont need the .name
You can just change it to

@handle.log(:info, "Source Cluster: #{source_cluster}")

By default the to_s method that is indirectly called here will look for an attribute called name.
https://github.com/ManageIQ/manageiq-automation_engine/blob/a05c976f0983e18aea96f5d9609c931f75d7ad31/lib/miq_automation_engine/engine/miq_ae_method_service/miq_ae_service_object.rb#L20

So, that will be the same anytime I use .name, isn't it ? Because, a quick grep tells me it is used a lot, and not only in this PR.

@fdupont-redhat
You dont have to make this change, if you just used #{vmdb_object} you will get the name attribute.

…s the first element of an array. Using x[:options][:my_key] instead.

…s installation.

mkanoor · 2018-05-30T18:44:59Z

content/automate/ManageIQ/System/CommonMethods/MiqAe.class/__methods__/weightedupdatestatus.rb

@@ -0,0 +1,140 @@
+module ManageIQ


@fdupont-redhat
Didn't this file get moved when you moved the MiqAe to MiqAe.class. This is not a new file.
Have you rebased after your PR got merged?

I have made changes to handle generic error messages. I confirm that I have rebased.

The existing files are named with underscore characters: weighted_update_status.rb and weighted_update_status.yaml. This PR is adding additional files without underscores in the names.

@fdupont-redhat Please correct.

You're correct. I had renamed the files for consistency with the method name, which has no underscore.

mkanoor · 2018-05-30T18:45:32Z

...ent/automate/ManageIQ/System/CommonMethods/MiqAe.class/__methods__/weightedupdatestatus.yaml

@@ -0,0 +1,73 @@
+---


@fdupont-redhat
Even this one was in the MiqAe move to MiqAe.class

miq-bot · 2018-05-31T13:07:55Z

Some comments on commits fabiendupont/manageiq-content@b9469bf~...20ce583

content/automate/ManageIQ/Transformation/Infrastructure/VM/rhevm.class/methods/setdescription.rb

💣 💥 🔥 🚒 - 17 - Detected cloudforms

miq-bot · 2018-05-31T13:08:15Z

Checked commits fabiendupont/manageiq-content@b9469bf~...20ce583 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
19 files checked, 12 offenses detected

content/automate/ManageIQ/Transformation/Common.class/methods/acquiretransformationhost.rb

❗ - Line 35, Col 11 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

content/automate/ManageIQ/Transformation/Common.class/methods/assesstransformation.rb

❗ - Line 75, Col 11 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

content/automate/ManageIQ/Transformation/Infrastructure/VM/Common.class/methods/poweroff.rb

❗ - Line 30, Col 15 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

content/automate/ManageIQ/Transformation/Infrastructure/VM/Common.class/methods/poweron.rb

❗ - Line 16, Col 15 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

content/automate/ManageIQ/Transformation/Infrastructure/VM/rhevm.class/methods/checkpoweredon.rb

❗ - Line 22, Col 15 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

content/automate/ManageIQ/Transformation/Infrastructure/VM/rhevm.class/methods/checkvmininventory.rb

❗ - Line 41, Col 15 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

content/automate/ManageIQ/Transformation/Infrastructure/VM/rhevm.class/methods/setdescription.rb

❗ - Line 19, Col 15 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

content/automate/ManageIQ/Transformation/Infrastructure/VM/vmwarews.class/methods/collapsesnapshots.rb

❗ - Line 25, Col 15 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

content/automate/ManageIQ/Transformation/Infrastructure/VM/vmwarews.class/methods/setmigrated.rb

❗ - Line 39, Col 15 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

content/automate/ManageIQ/Transformation/Infrastructure/VM/vmwarews.class/methods/utils.rb

❗ - Line 55, Col 17 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

content/automate/ManageIQ/Transformation/TransformationHosts/ovirt_host.class/methods/vmchecktransformed_vmwarews2rhevm_vddk.rb

❗ - Line 70, Col 13 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

content/automate/ManageIQ/Transformation/TransformationHosts/ovirt_host.class/methods/vmtransform_vmwarews2rhevm_vddk.rb

❗ - Line 90, Col 13 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

gmcculloug · 2018-05-31T13:41:59Z

The remaining rubocop errors use the implicit rescue style and is consistent with the existing code base. Rubocop changed their default setting for this check and I prefer keeping them this way for consistency.

The recent rubocop change is being discussed here: ManageIQ/guides#310

Fryguy · 2018-05-31T18:08:08Z

and I prefer keeping them this way for consistency.

Then go vote, @gmcculloug ! 😉

gmcculloug · 2018-05-31T20:08:32Z

Thanks @fdupont-redhat.

I opened issue #315 to address refactoring and spec concerns.

Add V2V for VMware to oVirt / RHV (cherry picked from commit dc72f1f)

simaishi · 2018-06-01T12:46:05Z

Gaprindashvili backport details:

$ git log -1
commit 35ca01c522188967b71d0d9877a9db9ba40c7f5b
Author: Greg McCullough <[email protected]>
Date:   Thu May 31 16:08:42 2018 -0400

    Merge pull request #301 from fdupont-redhat/v2v_gaprindashvili-3
    
    Add V2V for VMware to oVirt / RHV
    (cherry picked from commit dc72f1fed8e5faac9e4dca0ad3c6a4982dc5d49f)

miq-bot added the v2v label May 23, 2018

miq-bot changed the title ~~Add V2V for VMware to oVirt / RHV~~ [WIP] Add V2V for VMware to oVirt / RHV May 23, 2018

miq-bot added the wip label May 23, 2018

gmcculloug added the gaprindashvili/yes label May 23, 2018

gmcculloug self-assigned this May 23, 2018

gmcculloug requested review from mkanoor and tinaafitz May 23, 2018 13:08

mkanoor reviewed May 24, 2018

View reviewed changes

mkanoor reviewed May 29, 2018

View reviewed changes

mkanoor reviewed May 30, 2018

View reviewed changes

gmcculloug added the enhancement label May 30, 2018

fabiendupont added 8 commits May 30, 2018 18:48

Add V2V code for gaprindashvili-3 release

b9469bf

Remove empty scaffold of cloud conversion

aac64a8

Replace the 'puts' call with 'handle.log'.

9a4de23

Fix rubcocop errors

eb7f092

Fix rubcocop errors - cont'd

d077995

Fix rubocop errors - cont'd

b71eafd

Fix disks and networks enumeration: x.get_option(:my_key) only return…

876ecbd

…s the first element of an array. Using x[:options][:my_key] instead.

Add an attribute to the virt-v2v-wrapper options hash to force driver…

4a5e5dc

…s installation.

ghost force-pushed the v2v_gaprindashvili-3 branch from 0f55a86 to 4a5e5dc Compare May 30, 2018 17:26

Fix rubocop errors - cont'd

efdd8c9

mkanoor reviewed May 30, 2018

View reviewed changes

fabiendupont added 3 commits May 31, 2018 13:56

Fix disk size handling. Remove state that enforce virtio-scsi driver.

1a77e7a

Removed unused method

f365d21

Fix rubocop error - cont'd

20ce583

gmcculloug changed the title ~~[WIP] Add V2V for VMware to oVirt / RHV~~ Add V2V for VMware to oVirt / RHV May 31, 2018

gmcculloug removed the wip label May 31, 2018

mkanoor approved these changes May 31, 2018

View reviewed changes

gmcculloug merged commit dc72f1f into ManageIQ:master May 31, 2018

gmcculloug added this to the Sprint 87 Ending Jun 4, 2018 milestone May 31, 2018

gmcculloug mentioned this pull request May 31, 2018

Refactor and provide spec tests for the changes in PR 301 #315

Closed

simaishi pushed a commit that referenced this pull request Jun 1, 2018

Merge pull request #301 from fdupont-redhat/v2v_gaprindashvili-3

35ca01c

Add V2V for VMware to oVirt / RHV (cherry picked from commit dc72f1f)

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Jun 1, 2018

ghost deleted the v2v_gaprindashvili-3 branch July 31, 2018 07:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add V2V for VMware to oVirt / RHV #301

Add V2V for VMware to oVirt / RHV #301

ghost commented May 23, 2018 •

edited by gmcculloug

Loading

ghost commented May 23, 2018

ghost commented May 23, 2018

mkanoor May 24, 2018

ghost May 25, 2018

mkanoor May 30, 2018

ghost commented May 25, 2018

mkanoor May 29, 2018

ghost May 29, 2018

mkanoor May 30, 2018

ghost May 30, 2018

mkanoor May 29, 2018

ghost May 29, 2018

mkanoor May 30, 2018

ghost May 30, 2018 •

edited by ghost

Loading

mkanoor May 30, 2018

mkanoor May 30, 2018

ghost May 31, 2018 •

edited by ghost

Loading

gmcculloug May 31, 2018

ghost May 31, 2018

mkanoor May 30, 2018

miq-bot commented May 31, 2018

miq-bot commented May 31, 2018

gmcculloug commented May 31, 2018

Fryguy commented May 31, 2018

gmcculloug commented May 31, 2018

simaishi commented Jun 1, 2018

Add V2V for VMware to oVirt / RHV #301

Add V2V for VMware to oVirt / RHV #301

Conversation

ghost commented May 23, 2018 • edited by gmcculloug Loading

ghost commented May 23, 2018

ghost commented May 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented May 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost May 30, 2018 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost May 31, 2018 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented May 31, 2018

miq-bot commented May 31, 2018

gmcculloug commented May 31, 2018

Fryguy commented May 31, 2018

gmcculloug commented May 31, 2018

simaishi commented Jun 1, 2018

ghost commented May 23, 2018 •

edited by gmcculloug

Loading

ghost May 30, 2018 •

edited by ghost

Loading

ghost May 31, 2018 •

edited by ghost

Loading