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

Fixes for Live Migrate UI #8521

Conversation

mansam
Copy link
Contributor

@mansam mansam commented May 6, 2016

Update: Working on the Evacuate pull request with @sseago revealed a number of problems that applied to Live Migrate that this pull request is intended to resolve.

  • Move Live Migrate button to the openstack-specific single vm toolbar
  • Gracefully flash an error if Live Migrate is selected from the
    list view for an instance that doesn't support it.
  • Disable and show a title message on the Live Migrate button rather than skipping it if the action is unsupported for a given instance.
  • Show the human readable error message rather than the entire Fog exception when Migrate fails
  • Fix incorrect handling of parameters from the front end
  • Improve angular style of the live_migrate form partial.

@mansam
Copy link
Contributor Author

mansam commented May 6, 2016

@miq-bot add_label darga/yes

@dclarizio
Copy link

@h-kataria please review

@h-kataria
Copy link
Contributor

h-kataria commented May 9, 2016

@mansam got an error when i pressed Migrate selected Instance button for a Amazon Instance from a list view:

[----] I, [2016-05-09T17:27:25.946819 #1774:14fe880]  INFO -- :   Parameters: {"miq_grid_checks"=>"1r2130", "pressed"=>"instance_live_migrate"}
[----] F, [2016-05-09T17:27:26.032345 #1774:14fe880] FATAL -- : Error caught: [NoMethodError] undefined method `is_available' for #<ManageIQ::Providers::Amazon::CloudManager::Vm:0x007fc3c3437ef0>
/home/hkataria/.rvm/gems/ruby-2.2.3/bundler/gems/rails-d7e4083437a9/activemodel/lib/active_model/attribute_methods.rb:433:in `method_missing'
/home/hkataria/dev/manageiq/app/controllers/vm_cloud_controller.rb:99:in `live_migrate'
/home/hkataria/dev/manageiq/app/controllers/application_controller/explorer.rb:88:in `x_button'

Also an error when tried to Migrate from Openstack Instance summary screen or list view

[----] I, [2016-05-09T17:29:51.762216 #1774:14fe880]  INFO -- :   Parameters: {"miq_grid_checks"=>"1r1813", "pressed"=>"instance_live_migrate", "id"=>"1000000001813"}
[----] F, [2016-05-09T17:29:51.909662 #1774:14fe880] FATAL -- : Error caught: [NoMethodError] undefined method `is_available' for #<ManageIQ::Providers::Openstack::CloudManager::Vm:0x007fc3dc473540>

@h-kataria
Copy link
Contributor

h-kataria commented May 10, 2016

@mansam another error when i press submit button on Migrate screen after checking a checkbox "Block migration"

=================================
[----] F, [2016-05-10T10:24:40.467855 #6733:7bad374] FATAL -- : Error caught: [RuntimeError] Invalid input
/home/hkataria/dev/manageiq/app/controllers/application_controller.rb:2305:in `find_by_id_filtered'
/home/hkataria/dev/manageiq/app/controllers/vm_cloud_controller.rb:152:in `live_migrate_vm'
/home/hkataria/.rvm/gems/ruby-2.2.3/bundler/gems/rails-d7e4083437a9/actionpack/lib/action_controller/metal/basic_implicit_render.rb:4:in `send_action'

@chessbyte chessbyte closed this May 10, 2016
@chessbyte chessbyte reopened this May 10, 2016
@mansam
Copy link
Contributor Author

mansam commented May 10, 2016

@h-kataria I have a set of fixes for a number of things with Live Migrate coming soon.

* Move Live Migrate button to the openstack-specific single-vm bar
* Gracefully flash an error if Live Migrate is selected from the
  list view for an instance that doesn't support it.
@mansam mansam force-pushed the improve-handling-of-instances-that-dont-support-live-migrate branch from b34496a to b87f7ef Compare May 11, 2016 19:10
@mansam mansam changed the title Improve handling of instances that don't support Live Migrate Fixes for Live Migrate UI May 11, 2016
@mansam
Copy link
Contributor Author

mansam commented May 11, 2016

@h-kataria The committed updates should fix the problem you were experiencing as well as a few others I became aware of.

@h-kataria
Copy link
Contributor

@mansam when i try to migrate an instance, i see following error in log & and i get redirected back to previous screen list/details screen with a flash message "Unable to live migrate Instance "BradTest2": No route to host - connect(2) for 10.3.57.7:5000 (Errno::EHOSTUNREACH)":
[----] F, [2016-05-11T16:19:04.720126 #30717:1b25d30] FATAL -- : Error caught: [Errno::EHOSTUNREACH] No route to host - connect(2) for 10.3.57.7:5000
/home/hkataria/.rvm/gems/ruby-2.2.3/gems/excon-0.49.0/lib/excon/socket.rb:132:in connect_nonblock' /home/hkataria/.rvm/gems/ruby-2.2.3/gems/excon-0.49.0/lib/excon/socket.rb:132:inrescue in block in connect'
/home/hkataria/.rvm/gems/ruby-2.2.3/gems/excon-0.49.0/lib/excon/socket.rb:111:in `block in connect'

instance_live_migrate

@sseago
Copy link
Contributor

sseago commented May 11, 2016

@h-kataria Hmm. That sounds like something is wrong with your openstack networking. "no route to host - connect(2) for 10.3.57.7:5000" -- it seems like it's trying to make a keystone API call to 10.3.57.7 and it can't reach it. Can manageiq reach both infra and cloud provider endpoints?

@h-kataria
Copy link
Contributor

@sseago i don't have an environment set up to connect to host, but in case it can't reach the host, it should still not show errors in log, it should only show a flash message in UI.

@mansam
Copy link
Contributor Author

mansam commented May 11, 2016

@h-kataria My code is just catching the exception and parsing out the error into that message flash. The logging isn't coming from the UI code.

@sseago
Copy link
Contributor

sseago commented May 11, 2016

Is it showing the error in log and the flash message now? I would have thought that this sort of thing would go to the logs as well as the UI, but presumably only at an 'info' level (or maybe 'debug')?

@mansam
Copy link
Contributor Author

mansam commented May 11, 2016

@h-kataria Can you check whether the network call to live_migrate_form_fields is succeeding in the browser console?

@h-kataria
Copy link
Contributor

@mansam getting 500 Internal Server Error on live_migrate_form_fields transaction

@mansam
Copy link
Contributor Author

mansam commented May 11, 2016

Okay, that's why you see it in the logs even though the migrate call catches it. Needs exception handling for getting the host list when there's a networking failure like that.

@mansam
Copy link
Contributor Author

mansam commented May 11, 2016

(that said, your migrate call is still going to fail due to whatever that networking issue is)

@mansam
Copy link
Contributor Author

mansam commented May 11, 2016

@h-kataria it should now handle that exception case more gracefully when loading the list of hosts.

@h-kataria
Copy link
Contributor

👍

@miq-bot
Copy link
Member

miq-bot commented May 11, 2016

Checked commits mansam/manageiq@43d639c~...41e6610 with ruby 2.2.3, rubocop 0.37.2, and haml-lint 0.16.1
7 files checked, 18 offenses detected

app/controllers/vm_cloud_controller.rb

app/views/vm_common/_live_migrate.html.haml

  • 🔴 Warn - Line 10, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 10, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 10, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 10, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 10, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 17, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 17, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 17, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 17, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 25, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 25, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 25, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 34, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 34, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 34, Col - - Operator => should be surrounded by a single space.
  • 🔴 Warn - Line 47, Col - - Operator => should be surrounded by a single space.

@sseago
Copy link
Contributor

sseago commented May 12, 2016

This fixes the UI issues I hit earlier. Works for me now.

@h-kataria h-kataria added this to the Sprint 41 Ending May 30, 2016 milestone May 12, 2016
@h-kataria h-kataria merged commit ad14e24 into ManageIQ:master May 12, 2016
chessbyte pushed a commit that referenced this pull request May 12, 2016
…at-dont-support-live-migrate

Fixes for Live Migrate UI
(cherry picked from commit ad14e24)
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

6 participants