-
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
Restore VM power state upon transformation #336
Conversation
… This will prevent power on VMs that where down.
@@ -11,8 +11,10 @@ def initialize(handle = $evm) | |||
|
|||
def main | |||
task = @handle.root['service_template_transformation_plan_task'] | |||
destination_vm = @handle.vmdb(:vm).find_by(:id => task.get_option(:destination_vm_id)) | |||
destination_vm.start | |||
if task.get_ption(:source_vm_power_state) == 'on' |
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 I think this should be task.get_option
.
@@ -69,6 +69,9 @@ def main | |||
} | |||
@handle.set_state_var(:factory_config, factory_config) | |||
|
|||
# Store source VM power state, as we well power it off |
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 I think you probably meant to say, "....as we will power it off"
destination_vm.start | ||
if task.get_ption(:source_vm_power_state) == 'on' | ||
destination_vm = @handle.vmdb(:vm).find_by(:id => task.get_option(:destination_vm_id)) | ||
destination_vm.start |
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 Do you need to be concerned if the find_by doesn't find the VM?
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 don't think so as the previous state in the state machine waits for the VM to be in the VMDB.
@handle.root["ae_result"] = "retry" | ||
@handle.root["ae_retry_interval"] = "15.seconds" | ||
if task.get_option(:source_vm_power_state) == 'on' | ||
destination_vm = @handle.vmdb(:vm).find_by(:id => task.get_option(:destination_vm_id)) |
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 Should we protect against the case where we don't find the destination_vm?
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 don't think so as one of the previous state in the state machine waits for the VM to be in the VMDB.
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 Looks good.
@mkanoor Please review.
Checked commits fabiendupont/manageiq-content@2ebafaf~...3fb4d2e with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
Restore VM power state upon transformation (cherry picked from commit 941ab32) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1594828
Gaprindashvili backport details:
|
This will prevent power on VMs that where down.
Hopefully, it fixes #335.
Associated BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1594788