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

Rescue migration error and update status #16705

Merged
merged 1 commit into from
Dec 21, 2017

Conversation

bzwei
Copy link
Contributor

@bzwei bzwei commented Dec 20, 2017

https://bugzilla.redhat.com/show_bug.cgi?id=1448023

vm_migrate_task needs to rescue migration error and finish with error.

VMWare migration already raises error when a problem is detected. We just need to rescue it in the task.
RHV migration needs to enhance to monitor the migration status and raise error accordingly. The enhancement will be done in manageiq-providers-ovirt.

@bzwei
Copy link
Contributor Author

bzwei commented Dec 20, 2017

@miq-bot assign @gmcculloug
@miq-bot add_label bug
@tinaafitz @pkliczewski please review

@bzwei
Copy link
Contributor Author

bzwei commented Dec 20, 2017

@miq-bot add_labels automate, fine/yes, gaprindashvili/yes

@tinaafitz
Copy link
Member

@bzwei Looks good.

vm.relocate(host, respool, datastore, nil, disk_transform)
end
rescue => err
update_and_notify_parent(:state => 'finished', :status => 'error', :message => "Failed. Reason(#{err.message})")
Copy link
Member

Choose a reason for hiding this comment

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

Minor but it would be more consistent for the message to use [ ] around the err.message.

:message => "Failed. Reason:[#{err.message}]")

@pkliczewski
Copy link
Contributor

pkliczewski commented Dec 21, 2017

@bzwei This PR looks good assuming that we can make vm.migrate synchronous for oVirt but it would be not that simple to do it.

@miq-bot
Copy link
Member

miq-bot commented Dec 21, 2017

Checked commit bzwei@377f422 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@gmcculloug gmcculloug merged commit 805e023 into ManageIQ:master Dec 21, 2017
@gmcculloug gmcculloug added this to the Sprint 76 Ending Jan 1, 2018 milestone Dec 21, 2017
simaishi pushed a commit that referenced this pull request Jan 3, 2018
Rescue migration error and update status
(cherry picked from commit 805e023)
@simaishi
Copy link
Contributor

simaishi commented Jan 3, 2018

Gaprindashvili backport details:

$ git log -1
commit d881b204c351f9314a46b98befdeef12dc509ec0
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Thu Dec 21 09:30:51 2017 -0500

    Merge pull request #16705 from bzwei/migrate_with_error
    
    Rescue migration error and update status
    (cherry picked from commit 805e02333a8bfa3b697b691057d1d0a506b43037)

@bzwei bzwei deleted the migrate_with_error branch January 4, 2018 01:08
@bzwei bzwei restored the migrate_with_error branch January 4, 2018 01:08
simaishi pushed a commit that referenced this pull request Jan 10, 2018
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit c69473642892a0ffb30fc0c797196e99177dee0f
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Thu Dec 21 09:30:51 2017 -0500

    Merge pull request #16705 from bzwei/migrate_with_error
    
    Rescue migration error and update status
    (cherry picked from commit 805e02333a8bfa3b697b691057d1d0a506b43037)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1478518

@bzwei bzwei deleted the migrate_with_error branch January 10, 2018 14:54
d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
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.

6 participants