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

Add default cleanup state machine for VM transformation #378

Merged

Conversation

ghost
Copy link

@ghost ghost commented Jul 28, 2018

This PR provides the default state machine for VM transformation cleanup. The state machine is triggered from an automate request, as designed in #357. The request creation passes a variable in $evm.root: service_template_transformation_plan_task_id.

The process is as follow:

  1. Assess the transformation cleanup. It basically sets the required state vars.
  2. Kill virt-v2v. The PID of the process and the transformation host information is available in the task. It simply needs a remote command execution to kill the process. virt-v2v-wrapper handles cleaning up the destination provider when the virt-v2v command fails, so nothing to do on ManageIQ side.
  3. Power on the VM if it was powered on when the migration started.

Associated RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1599997

@ghost ghost force-pushed the v2v_cleanup_add_default_state_machine branch from 1ecbebb to bbfc783 Compare July 28, 2018 21:46
@ghost
Copy link
Author

ghost commented Jul 28, 2018

@miq-bot add-label transformation
@miq-bot add-label enhancement
@miq-bot add-label gaprindashvili/yes
@miq-bot add-label blocker

raise 'No task found. Exiting' if task.nil?
@handle.log(:info, "Task: #{task.inspect}") if @debug

destination_ems = @handle.vmdb(:ext_management_system).find_by(:id => task.get_option(:destination_ems_id))
Copy link
Contributor

Choose a reason for hiding this comment

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

@fdupont-redhat
Can you check if the destination_ems is present before setting the state var.

destination_vm = @handle.vmdb(:vm).find_by(:id => task.get_option(:destination_vm_id))
destination_vm.start
if @handle.root['service_template_transformation_plan_task'].blank?
task = @handle.vmdb(:service_template_transformation_plan_task).find_by(:id => @handle.root['service_template_transformation_plan_task_id'])
Copy link
Contributor

Choose a reason for hiding this comment

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

@fdupont-redhat Check for task being nil

Copy link
Author

Choose a reason for hiding this comment

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

I check if task is nil, as well as VM.

# Retrieve transformation host
transformation_host = @handle.vmdb(:host).find_by(:id => task.get_option(:transformation_host_id))

# Skip if virtv2v has not started yet or is already finished
Copy link
Contributor

Choose a reason for hiding this comment

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

@fdupont-redhat
Can all of these skips be in a separate function called validate or valid and you call the remote command only if its valid.

Copy link
Member

@gmcculloug gmcculloug left a comment

Choose a reason for hiding this comment

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

@fdupont-redhat Two minor cleanup items.

def main
task = @handle.vmdb(:service_template_transformation_plan_task).find_by(:id => @handle.root['service_template_transformation_plan_task_id'])
raise 'No task found. Exiting' if task.nil?
@handle.log(:info, "Task: #{task.inspect}") if @debug
Copy link
Member

Choose a reason for hiding this comment

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

@debug is referenced here but never defined.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed with default value to false.

module Common
class VMCheckTransformed
def initialize(handle = $evm)
@debug = true
Copy link
Member

Choose a reason for hiding this comment

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

and @debug is referenced here but never used.

@miq-bot
Copy link
Member

miq-bot commented Jul 30, 2018

Checked commits fabiendupont/manageiq-content@bbfc783~...01b9fa9 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 2 offenses detected

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

content/automate/ManageIQ/Transformation/TransformationHosts/Common.class/methods/killvirtv2v.rb

@gmcculloug gmcculloug merged commit fe3fe98 into ManageIQ:master Jul 30, 2018
@gmcculloug gmcculloug added this to the Sprint 91 Ending Jul 30, 2018 milestone Jul 30, 2018
@ghost ghost deleted the v2v_cleanup_add_default_state_machine branch July 31, 2018 07:12
@ghost ghost mentioned this pull request Jul 31, 2018
simaishi pushed a commit that referenced this pull request Aug 1, 2018
…tate_machine

Add default cleanup state machine for VM transformation
(cherry picked from commit fe3fe98)

https://bugzilla.redhat.com/show_bug.cgi?id=1608758
@simaishi
Copy link
Contributor

simaishi commented Aug 1, 2018

Gaprindashvili backport details:

$ git log -1
commit a1db90493bec152af05c2ffbfed122dd46d3d385
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Mon Jul 30 18:45:34 2018 -0400

    Merge pull request #378 from fdupont-redhat/v2v_cleanup_add_default_state_machine
    
    Add default cleanup state machine for VM transformation
    (cherry picked from commit fe3fe985101d38eee66c050562d3a7de8666e6ea)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1608758

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants