V2V - State machines - Initial work #248

ghost · 2018-02-08T22:37:53Z

This is the initial work to integrate the state machines and methods required to transform virtual machines.
There are three main areas:

Transformation Host:
- Enable/disable role (provision host)
- Methods to assign and track transformation hosts in tasks
Transformation Request:
- Approve the request, i.e. select the VM to be transformed and create tasks
- Schedule tasks based on dependencies, capacity, throttling rules...
Transformation Task:
- Execute the transformation
- Track completion and provide machine readable progress for UI

ghost · 2018-02-08T22:38:18Z

@miq-bot add_label v2v

miq-bot · 2018-02-08T22:39:12Z

@fdupont-redhat Cannot apply the following label because they are not recognized: v2v

gmcculloug · 2018-02-08T22:39:22Z

@bzwei Please review

…nly about the role enblement.

mkanoor · 2018-02-12T15:22:59Z

.../ManageIQ/Transformation/TransformationHosts/ovirt_host.class/__methods__/tagasdisabled.yaml

+    scope: instance
+    language: ruby
+    location: inline
+    options: {}


@fdupont-redhat Is this missing the embedded methods data since your method is using the embedded methods

Where do you see it using an embedded method ? That one doesn't. Anyway, I see I forgot to rename the module from ConversionHost to TransformationHost.

@fdupont-redhat in your earlier ruby code you had

if @debug ManageIQ::Automate::System::CommonMethods::MIQ_AE::Debug.new.dump_object ManageIQ::Automate::System::CommonMethods::MIQ_AE::Debug.new.dump_root end

which would have needed the embedded methods, now that you have removed it, its not relevant.

mkanoor · 2018-02-12T15:24:00Z

...rmation/TransformationHosts/ovirt_host.class/__methods__/transformvm_vmware2redhat_vddk.yaml

+    scope: instance
+    language: ruby
+    location: inline
+    options: {}


@fdupont-redhat This might also be missing the embedded_methods names.

mkanoor · 2018-02-12T15:41:53Z

content/automate/ManageIQ/Transformation/TransformationHosts/Common.class/__methods__/utils.rb

+        end
+
+        def get_runners_count_by_host(host)
+          return @handle.vmdb(:vm).all.select { |v| v.custom_get('Transformation Host') == host.name }.size


@fdupont-redhat dont need the return here

@fdupont-redhat This can be an expensive call fetching all the vms and then fetching/comparing host name.
Its good in the sense that it doesn't require locking the host record to add/remove vms being transformed

If you have a less expensive approach, I take.

@fdupont-redhat You could lookup the CustomAttribute records that has the matching name and value properties and get the count from there. If needed you can limit the result set to just VMs by only returning ones with resource_type => VmOrTemplate.

Maybe @handle.vmdb(:custom_attribute).where(:name => 'Transformation Host', :value => host.name).size

gmcculloug · 2018-02-12T15:44:52Z

@fdupont-redhat It would be good if you could run through the rubocop warning/errors and address those as a first pass.

mkanoor · 2018-02-12T15:45:57Z

content/automate/ManageIQ/Transformation/TransformationHosts/Common.class/__methods__/utils.rb

+          return @handle.vmdb(:vm).all.select { |v| v.custom_get('Transformation Host') == host.name }.size
+        end
+
+        def get_runners_count_by_ems(ems)


@fdupont-redhat

def get_runners_count_by_ems get_transformation_hosts.inject(0){|sum,host| sum + get_runners_count_by_host(host) } end

mkanoor · 2018-02-12T15:57:05Z

content/automate/ManageIQ/Transformation/TransformationHosts/Common.class/__methods__/utils.rb

+            @handle.log(:info, "No transformation host found.")
+          else
+            @handle.log(:info, "Least busy transformation host: #{transformation_host}")
+            vm.custom_set('Transformation Host', transformation_host.guid)


@fdupont-redhat are transformation_host.guid and transformation_host.name the same, since in line # 19 you are using name.

It will be changed, as the transformation host will be part of the options hash of the transformation task, rather that the VM.

mkanoor · 2018-02-12T15:58:21Z

content/automate/ManageIQ/Transformation/TransformationHosts/Common.class/__methods__/utils.rb

+        end
+
+        def unset_transformation_host(vm)
+          vm.custom_set('Transformation Host', nil)


@fdupont-redhat can the custom attribute be deleted instead of being set to nil. Will the nil logic be used elsewhere to compare completed transformations.

This is how you do it. There are only three methods exposed custom_keys, custom_get and custom_set.

See /app/models/mixins/custom_attribute_mixin.rb#L82

It will be changed, as the transformation host will be part of the options hash of the transformation task, rather that the VM.

mkanoor · 2018-02-12T16:07:57Z

content/automate/ManageIQ/Transformation/TransformationHosts/Common.class/__methods__/utils.rb

+            max_runners = factory_config['transformation_host_max_runners'] if max_runners.blank?
+            next if runners_count == max_runners
+            ( transformation_host = host ; break ) if runners_count == 0
+            if transformation_host.nil? || runners_count < min_runners


@fdupont-redhat How is the min_runners being set, it starts out as nil and only gets set inside of the if block to the runner count. The || logic seems odd.

mkanoor · 2018-02-12T17:10:05Z

...eIQ/Transformation/TransformationHosts/StateMachines/TransformationHostRole.class/check.yaml

+  fields:
+  - State1:
+      value: "/Transformation/TransformationHost/${#transformation_host_type}/RoleCheck"
+      on_error: "/Transformation/TransformationHost/${#transformation_host_type}/MarkAsDisabled"


@fdupont-redhat
We dont support paths in on_error/on_exit/on_entry slots?

Well, a typo while copying. It should be /Transformation/TransformationHost/${#transformation_host_type}.MarkAsDisabled.

mkanoor · 2018-02-12T17:10:55Z

...ate/ManageIQ/Transformation/TransformationHosts/ovirt_host.class/__methods__/roleenable.yaml

+      datatype: string
+      priority: 0
+      owner: 
+      default_value: bastion.v2v.example.com


@fdupont-redhat the default value should be empty

mkanoor · 2018-02-12T17:11:18Z

...ate/ManageIQ/Transformation/TransformationHosts/ovirt_host.class/__methods__/roleenable.yaml

+      datatype: string
+      priority: 1
+      owner: 
+      default_value: http://bastion.v2v.example.com/vddk/VMware-vix-disklib-6.5.2-6195444.x86_64.tar.gz


@fdupont-redhat please check default value

…sformation Host role.

gmcculloug · 2018-02-14T18:57:54Z

@fdupont-redhat Can you cleanup the two Style/YodaCondition rubocop issues and un-wip if this is ready?

Not sure why we are seeing the other rubocop about parameters at the moment.

ghost · 2018-02-14T20:12:43Z

@gmcculloug These errors are strange, as this syntax is found in all the methods that implement the module layout. For example: https://github.com/ManageIQ/manageiq-content/blob/master/content/automate/ManageIQ/Cloud/Orchestration/Operations/Methods.class/__methods__/available_tenants.rb.

gmcculloug · 2018-02-14T20:28:08Z

Correct, the YodaCondition is a more recent rubocop check so we were not seeing that when the new method type was introduced.

Ultimately we will want to go through and correct them all, but the goal is not to introduce more warnings.

ghost · 2018-02-14T20:58:24Z

@gmcculloug WIP label removed and all lights green on rubocop side.

gmcculloug · 2018-02-14T21:05:56Z

@tinaafitz @mkanoor Can you give this another look after changes?

ghost · 2018-02-14T21:16:56Z

Forgot to add the request entrypoints for the role check/enable/disable state machines.
@miq-bot add_label wip

@tinaafitz @lfu What would you advise for the request names ?

lfu · 2018-02-14T22:10:21Z

...eIQ/Transformation/TransformationHosts/StateMachines/TransformationHostRole.class/check.yaml

+  fields:
+  - State1:
+      value: "/Transformation/TransformationHosts/${#transformation_host_type}/RoleCheck"
+      on_error: "/Transformation/TransformationHosts/${#transformation_host_type}.MarkAsDisabled"


Where is MarkAsDisabled defined?

There is a TagAsEnabled? Does this needs to be renamed.

You're right. It is TagAsDisabled. I have renamed it.

lfu · 2018-02-14T22:10:44Z

...eIQ/Transformation/TransformationHosts/StateMachines/TransformationHostRole.class/check.yaml

+      value: "/Transformation/TransformationHosts/${#transformation_host_type}/RoleCheck"
+      on_error: "/Transformation/TransformationHosts/${#transformation_host_type}.MarkAsDisabled"
+  - State2:
+      value: "/Transformation/TransformationHosts/${#transformation_host_type}.MarkAsEnabled"


@fdupont-redhat Does this need a METHOD:: since its a call to a class method

@fdupont-redhat Also is this TagAsEnabled?

You're right. It is TagAsDisabled. I have renamed it.

@mkanoor METHOD:: is not needed because on_error expects method(s). I don't know how it is coded internally, but experimentally it works without it. If you think I should make it explicit, I'll change it.

miq-bot · 2018-02-15T09:44:31Z

Checked commits fabiendupont/manageiq-content@2082d92~...8ef9427 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
20 files checked, 1 offense detected

**

💣 💥 🔥 🚒 - Linter/Yaml - missing config files

…dpoints for custom buttons.

ghost · 2018-02-15T13:34:41Z

@miq-bot remove_label wip

tinaafitz

@fdupont-redhat Looks good.

…Q/manageiq-content/pull/248. Removed code for sysprep state, as we won't use them. Moved the code to a sub directory to ease import.

…machines_init V2V - State machines - Initial work (cherry picked from commit 15b3a0e)

simaishi · 2018-05-29T21:44:07Z

Gaprindashvili backport details:

$ git log -1
commit cfa1f77759fdd22e75ff7e19a770cb6d9bd0d0da
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Thu Feb 15 15:55:17 2018 -0500

    Merge pull request #248 from fdupont-redhat/v2v_transformation_state_machines_init
    
    V2V - State machines - Initial work
    (cherry picked from commit 15b3a0e8e859ced52cd66c6ff0ecce91cf47a581)

gmcculloug self-assigned this Feb 8, 2018

gmcculloug added enhancement v2v labels Feb 8, 2018

gmcculloug requested a review from tinaafitz February 8, 2018 22:39

gmcculloug requested a review from mkanoor February 8, 2018 22:39

miq-bot added the wip label Feb 8, 2018

Added the TransformationHosts state machines and methods. This is mai…

2082d92

…nly about the role enblement.

mkanoor reviewed Feb 12, 2018

View reviewed changes

Removed code used for actual transformation, focusing this PR on Tran…

c2737ac

…sformation Host role.

ghost changed the title ~~[WIP] V2V - State machines - Initial work~~ V2V - State machines - Initial work Feb 14, 2018

miq-bot removed the wip label Feb 14, 2018

Fixing rubocop warnings.

a47af7e

miq-bot changed the title ~~V2V - State machines - Initial work~~ [WIP] V2V - State machines - Initial work Feb 14, 2018

miq-bot added the wip label Feb 14, 2018

Fixing rubocop warnings.

2257d2f

lfu reviewed Feb 14, 2018

View reviewed changes

Minor fixes on methods/instances paths. Added request instances as en…

8ef9427

…dpoints for custom buttons.

miq-bot changed the title ~~[WIP] V2V - State machines - Initial work~~ V2V - State machines - Initial work Feb 15, 2018

miq-bot removed the wip label Feb 15, 2018

tinaafitz approved these changes Feb 15, 2018

View reviewed changes

mkanoor approved these changes Feb 15, 2018

View reviewed changes

gmcculloug merged commit 15b3a0e into ManageIQ:master Feb 15, 2018

gmcculloug added this to the Sprint 80 Ending Feb 26, 2018 milestone Feb 15, 2018

ghost mentioned this pull request Mar 22, 2018

Cleanup after merges of PR#248 and #252 fabiendupont/v2v-automate#2

Merged

gmcculloug added the gaprindashvili/yes label May 16, 2018

simaishi pushed a commit that referenced this pull request May 29, 2018

Merge pull request #248 from fdupont-redhat/v2v_transformation_state_…

cfa1f77

…machines_init V2V - State machines - Initial work (cherry picked from commit 15b3a0e)

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels May 29, 2018

ghost deleted the v2v_transformation_state_machines_init branch July 11, 2018 07:07

V2V - State machines - Initial work #248

V2V - State machines - Initial work #248

Conversation

ghost commented Feb 8, 2018

ghost commented Feb 8, 2018

miq-bot commented Feb 8, 2018

gmcculloug commented Feb 8, 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

Choose a reason for hiding this comment

gmcculloug commented Feb 12, 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

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

gmcculloug commented Feb 14, 2018

ghost commented Feb 14, 2018

gmcculloug commented Feb 14, 2018

ghost commented Feb 14, 2018

gmcculloug commented Feb 14, 2018

ghost commented Feb 14, 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

miq-bot commented Feb 15, 2018

ghost commented Feb 15, 2018

tinaafitz left a comment

Choose a reason for hiding this comment

simaishi commented May 29, 2018