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 Live Migrate actions to the task queue. #208

Merged
merged 4 commits into from
Jan 31, 2017

Conversation

mansam
Copy link
Contributor

@mansam mansam commented Jan 20, 2017

Use the task queue for Live Migrate instead of calling the action directly from the UI.

Depends on ManageIQ/manageiq#13491 which contains the backend changes.

@mansam mansam changed the title Add Live Migrate actions to the task queue. [WIP] Add Live Migrate actions to the task queue. Jan 20, 2017
@mansam mansam closed this Jan 23, 2017
@mansam mansam reopened this Jan 23, 2017
@mansam mansam force-pushed the queue-live-migrate-actions branch from 348eaf6 to a7f07c6 Compare January 23, 2017 23:56
@mansam mansam changed the title [WIP] Add Live Migrate actions to the task queue. Add Live Migrate actions to the task queue. Jan 25, 2017
@mansam
Copy link
Contributor Author

mansam commented Jan 25, 2017

@miq-bot remove_label wip,pending_backend

@mzazrivec
Copy link
Contributor

This is what I get when I run the instance migration on a OpenStack cloud provider:

No route matches {:action=>"wait_for_task", :controller=>"vm", ...} [vm/live_migrate_vm]

}
task_id = @record.class.live_migrate_queue(session[:userid], @record, options)
unless task_id.kind_of?(Integer)
add_flash(_("Instance live migration task %{id} failed.") % {:id => task_id.to_s}, :error)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm having trouble understanding the above logic. If task_id is not integer, we're gonna render a flash error
telling that a task with id task_id failed? What will task_id contain at this point? Will it really be the task's id?
Then it would be an integer, right? But at the same time it wouldn't pass the unless task_id.kind_of?(Integer) condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are of course right, the id in the flash error is mistaken.

@mansam
Copy link
Contributor Author

mansam commented Jan 26, 2017

I don't get that route problem. The route is defined in this patch, and when I run it locally the requests to wait_for_task succeed.

@mansam mansam force-pushed the queue-live-migrate-actions branch from f3d2441 to 22a205f Compare January 26, 2017 18:14
@mzazrivec
Copy link
Contributor

The routing error I shown above will show when you navigate to OpenStack Cloud Provider ->
its Instances, and from the list view (i.e. not on the instance summary page) you'll select an Instance
and run the migration.

@mansam
Copy link
Contributor Author

mansam commented Jan 27, 2017

Ah, I see. Fix incoming.

@mansam mansam force-pushed the queue-live-migrate-actions branch from 22a205f to 946e9de Compare January 30, 2017 19:16
@miq-bot
Copy link
Member

miq-bot commented Jan 30, 2017

Checked commits mansam/manageiq-ui-classic@cbe6c13~...946e9de with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 1 offense detected

app/controllers/application_controller/ci_processing.rb

  • ❕ - Line 540, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for live_migrate_finished is too high. [34.34/20]

@mzazrivec mzazrivec self-assigned this Jan 31, 2017
@mzazrivec mzazrivec added this to the Sprint 54 Ending Feb 13, 2017 milestone Jan 31, 2017
@mzazrivec mzazrivec merged commit 08c7a71 into ManageIQ:master Jan 31, 2017
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.

4 participants