-
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
Add VM Transformation state machine #252
Add VM Transformation state machine #252
Conversation
cc @bzwei |
# phase (entry, exit, error). This updates the details with live | ||
# data from the method, and also sets sane defaults. | ||
case @handle.root['ae_status_state'] | ||
when 'on_entry' |
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.
This main
method should be broken down into smaller methods. A good place to start is converting these when
blocks for the case statement into separate methods.
# state machines. Where running a nested state machines, there can be | ||
# more than one active states: one per level of nesting. The real | ||
# active state is the one with the deepest ancestry. | ||
def current_state(states) |
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.
@fdupont-redhat
Couldn't you get the current state from root['ae_state']?
We are setting it here https://github.com/ManageIQ/manageiq-automation_engine/blob/master/lib/miq_automation_engine/engine/miq_ae_engine/miq_ae_state_machine.rb#L63
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.
In the context of nested state machines, root [ae_state]
contains the name of the current state in the current state machine. And nothing prevents from having two states with the same name when using nested state machines. So, here I take the state ancestry depth as a criteria to distinguish the real running state.
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.
@fdupont-redhat
So do you need the fully qualified instance name that is being used at that moment? We have that information we can set it as an extra attribute in the root object.
@@ -0,0 +1,119 @@ | |||
module Transformation |
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.
@fdupont-redhat To stay consistent with other ManageIQ methods please add ManageIQ and Automate
https://github.com/ManageIQ/manageiq-content/blob/master/content/automate/ManageIQ/AutomationManagement/AnsibleTower/Operations/StateMachines/Job.class/__methods__/launch_ansible_job.rb#L8
progress['percent'] = reconcile_children_percent('', progress['states']) | ||
end | ||
# We record the progress as a task option. | ||
task.set_option(:progress, progress) |
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.
we have new method task.update_transformation_progress(progress)
end | ||
# Then, we set the update time to now, which is also finish time | ||
# when the state is finished. | ||
state_hash['updated_on'] = Time.now.utc |
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.
The task itself has updated_on
column. Should not need this one.
I guess you're renaming "rhev" to either ovirt or rhv:) |
Removed the default on_{entry,exit,error} on the class. Added specific max_retries at state machine instance level.
Checked commits fabiendupont/manageiq-content@6ac472d~...0ab65ba with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 **
|
@mkanoor Can you give this one last review? |
…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.
…nsformation Add VM Transformation state machine (cherry picked from commit d45d46f)
Gaprindashvili backport details:
|
This state machine describes the states to transform a virtual machine. It uses nested state machines to easily change the behavior based on source and destination EMS, as well as transformation method.
The
update_status.rb
method also add tracking information, such as start and update times, and percentage of progress. The percentage is either provided by the method itself, or defaults to the number of retries on the max retries of the state.It also take into account the nesting by creating an artificial ancestry. The ancestry requires the state machine designer to add the following argument to the URI:
?state_ancestry=${#state_ancestry}/${#ae_state}"
.