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

Launch an URL returned by an automate button #10118

Merged
merged 9 commits into from
Nov 28, 2016

Conversation

martinpovolny
Copy link
Member

@martinpovolny martinpovolny commented Jul 28, 2016

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

Purpose or Intent

Extension to custom buttons where:

  1. the custom button runs an automate method through the queue (using MiqTask)
  2. the customer-provided automate method stuff would do some potentially complex stuff and would return an URL (through SystemConsole model).
  3. the UI opens browser window on that URL or even open a HTML5 VNC client to a provided websocket URL (not in this PR).

A simple usecase is opening a custom web page that is not available immediately, but needs to be made ready by the automate script.

The greater idea is than an automate script would somehow build a chain of proxy servers for VNC connection and the ManageIQ UI would open a HTML5 VNC client that would connect through that tunnel.

button_url

Example usage from automate:

  1. create a custom button with the check box "Open URL" checked
  2. attach a method to it with the text below:
  $evm.root['vm'].remote_console_url = 'http://www.redhat.com'

press the button.

FIXME (for a follow up PR?)

  1. When deleting from SystemConsole we might need to do some cleanups (websocket proxy) or 2nd level proxy (Console proxy level 2 #11273). However it makes no sense to use the proxies together with this on a single VM so this is not a big issue.
  2. Might need to review how user is passed through and what can be set by the set_remote_console_url method exposed to automate.

@gmcculloug
Copy link
Member

We do something like this for the Dynamic dialog fields in automate. We run an automate resolution and get a workspace back app/models/dynamic_dialog_field_value_processor.rb#L8. Then pass the root attributes app/models/dynamic_dialog_field_value_processor.rb#L14 to be processed here app/models/dialog_field_text_box.rb#L61.

Does that help?

@martinpovolny
Copy link
Member Author

@gmcculloug : If I read it correctly, then that is sync. I need to do it async through the MiqTask (and wait_for_task on the UI side).

Things like setting up a tunnel might take some time and the UI should not hang while waiting for the automate.

Also the setup of a tunnel to a VNC endpoint on a hypevisor node would be done by a different appliance. The UI appliance probably won't be able to reach the hypervisor nodes directly at a service provider deployment.

Does that make sense?

@gmcculloug
Copy link
Member

@martinpovolny I think what you really want to do here is pass something to the automate method that it can update instead of trying to pull back data from the automate method.

For example you could pass the ID of the resource you are running against and if that supports custom_attributes the automate script could set the required data there. Otherwise, we could do something like expose the MiqTask object to the automate service model and allow the automate method to set values into the Task.

@@ -1,4 +1,6 @@
module MiqAeMethodService
class MiqAeServiceVmInfra < MiqAeServiceVm
expose :set_remote_console_url
Copy link
Member

Choose a reason for hiding this comment

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

Based on the active record model method this belongs in miq_ae_service_vm.rb. (Should be removed from miq_ae_service_vm_infra.rb and miq_ae_service_vm_or_template.rb.

@martinpovolny martinpovolny force-pushed the button_launch_url branch 2 times, most recently from fb6c3f3 to 1f54960 Compare September 22, 2016 11:28
@martinpovolny martinpovolny removed the wip label Sep 22, 2016
@martinpovolny martinpovolny changed the title [WIP] Launch an URL returned by an automate button Launch an URL returned by an automate button Sep 22, 2016
@martinpovolny martinpovolny changed the title Launch an URL returned by an automate button [WIP] Launch an URL returned by an automate button Sep 22, 2016
@martinpovolny martinpovolny force-pushed the button_launch_url branch 2 times, most recently from d468d52 to a68a360 Compare October 17, 2016 12:14
@martinpovolny martinpovolny changed the title [WIP] Launch an URL returned by an automate button Launch an URL returned by an automate button Oct 17, 2016
@martinpovolny martinpovolny force-pushed the button_launch_url branch 2 times, most recently from 79996a9 to f966f96 Compare October 17, 2016 19:08
@martinpovolny martinpovolny removed the wip label Oct 17, 2016
@@ -250,6 +251,17 @@ def applies_to_class_model(applies_to_class)
applies_to_class == "ServiceTemplate" ? Service : applies_to_class.constantize
end

def custom_button_done
url = SystemConsole.where(:vm => params[:id]).first.try(:url)
Copy link
Contributor

Choose a reason for hiding this comment

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

@martinpovolny @gmcculloug
Is this meant only for VM's or can it be used with other objects like storage, network, host etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

only VMs at this point.

Copy link
Contributor

Choose a reason for hiding this comment

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

@martinpovolny
Will we have a 'ur'l as a custom attribute available for vm objects which the Automate Method would set?

Copy link
Member Author

Choose a reason for hiding this comment

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

@mkanoor : that sounds good. How do I do that? @gmcculloug suggested that I should pass an ID of some resource and work with that. Can you show me how I would do what you suggest?

Copy link
Member

Choose a reason for hiding this comment

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

Change to: url = SystemConsole.find_by(:vm => params[:id]).try(:url)

You are currently setting the url when calling set_remote_console_url from automate. Do you need to store the url somewhere else since it already resides in the SystemConsole instance associated to the VM?

If so automate supports custom_get and custom_set methods. lib/miq_automation_engine/service_models/miq_ae_service_vm_or_template.rb#L140

Note: These custom attributes are visible in the VM summary screen. Here is an example of what it would look like to set a custom attribute named url:
image

@skateman
Copy link
Member

@martinpovolny I'm getting this error in the automate.log:

[----] E, [2016-10-23T21:58:05.450688 #24350:2b16aae6f550] ERROR -- : MiqAeServiceModelBase.ar_method raised: <ActiveRecord::RecordInvalid>: <Validation failed: Url secret has already been taken>
[----] E, [2016-10-23T21:58:05.450768 #24350:2b16aae6f550] ERROR -- : /home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/validations.rb:78:in `raise_validation_error'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/validations.rb:50:in `save!'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/attribute_methods/dirty.rb:30:in `save!'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/transactions.rb:324:in `block in save!'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/transactions.rb:395:in `block in with_transaction_returning_status'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/connection_adapters/abstract/database_statements.rb:232:in `block in transaction'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/connection_adapters/abstract/transaction.rb:189:in `within_new_transaction'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/connection_adapters/abstract/database_statements.rb:232:in `transaction'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/transactions.rb:211:in `transaction'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/transactions.rb:392:in `with_transaction_returning_status'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/transactions.rb:324:in `save!'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/suppressor.rb:45:in `save!'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-5.0.0.1/lib/active_record/persistence.rb:51:in `create!'
/home/dhalasz/Repositories/ManageIQ/manageiq/app/models/vm.rb:97:in `set_remote_console_url'
/home/dhalasz/Repositories/ManageIQ/manageiq/lib/miq_automation_engine/engine/miq_ae_service_model_base.rb:269:in `public_send'
/home/dhalasz/Repositories/ManageIQ/manageiq/lib/miq_automation_engine/engine/miq_ae_service_model_base.rb:269:in `block in object_send'
/home/dhalasz/Repositories/ManageIQ/manageiq/lib/miq_automation_engine/engine/miq_ae_service_model_base.rb:288:in `ar_method'
/home/dhalasz/Repositories/ManageIQ/manageiq/lib/miq_automation_engine/engine/miq_ae_service_model_base.rb:298:in `ar_method'
/home/dhalasz/Repositories/ManageIQ/manageiq/lib/miq_automation_engine/engine/miq_ae_service_model_base.rb:267:in `object_send'
/home/dhalasz/Repositories/ManageIQ/manageiq/lib/miq_automation_engine/engine/miq_ae_service_model_base.rb:122:in `block (2 levels) in expose'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/2.3.0/drb/drb.rb:1624:in `perform_without_block'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/2.3.0/drb/drb.rb:1584:in `perform'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/2.3.0/drb/drb.rb:1657:in `block (2 levels) in main_loop'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/2.3.0/drb/drb.rb:1653:in `loop'
/home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/2.3.0/drb/drb.rb:1653:in `block in main_loop'
[----] E, [2016-10-23T21:58:05.455019 #24350:2b16ac1643a4] ERROR -- : <AEMethod openurl> The following error occurred during method evaluation:
[----] E, [2016-10-23T21:58:05.455498 #24350:2b16ac1643a4] ERROR -- : <AEMethod openurl>   DRb::DRbUnknownError: ActiveRecord
[----] E, [2016-10-23T21:58:05.456382 #24350:2b16ac1643a4] ERROR -- : <AEMethod openurl>   /home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/2.3.0/drb/drb.rb:1149:in `method_missing'
[----] E, [2016-10-23T21:58:05.459874 #24350:2b16ac1643a4] ERROR -- : Method STDERR: /home/dhalasz/.rbenv/versions/2.3.1/lib/ruby/2.3.0/drb/drb.rb:1149:in `method_missing': ActiveRecord (DRb::DRbUnknownError)
[----] E, [2016-10-23T21:58:05.460288 #24350:2b16ac1643a4] ERROR -- : Method STDERR:    from <code: $evm.root['vm'].set_remote_console_url(:url => 'http://www.seznam.cz', :userid => $evm.root['user_id'])>:5:in `<main>'
[----] I, [2016-10-23T21:58:05.465484 #24350:2b16aae5b104]  INFO -- : <AEMethod [/miq-Marketplace/System/Request/OpenURL]> Ending

@martinpovolny
Copy link
Member Author

@skateman : can you, please, check again?

@skateman
Copy link
Member

@martinpovolny looks good, aren't we planning to add a spinner while we wait for the task?

@martinpovolny
Copy link
Member Author

Would be good to get some feedback from the automate people.

I think that the functionality is a good one but I am not so sure about how I actually did it. It works but if feels hacky.

The question by @mkanoor if it's only valid for VMs suggests it might make sense for more entities. But the SystemConsole model is only related to VMs.

@@ -1,5 +1,9 @@
module MiqAeMethodService
class MiqAeServiceVm < MiqAeServiceVmOrTemplate
def set_remote_console_url(url)
Copy link
Member

Choose a reason for hiding this comment

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

Following the rubocop warning this method should be named remote_console_url=
def remote_console_url=(url)

Would also need to update the method in the VM model which would need to take two parameters.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@@ -91,4 +91,16 @@ def running_processes
end
pl
end

def set_remote_console_url(url, user_id)
Copy link
Member

Choose a reason for hiding this comment

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

Should be renamed:
def remote_console_url=(url, user_id)

Copy link
Member Author

Choose a reason for hiding this comment

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

really? how would I call such method?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I did. I don't know what magic happens here, a method with = with more that one arg sounds strange, but w/o the names matching I was getting errors

@miq-bot
Copy link
Member

miq-bot commented Nov 27, 2016

<github_pr_commenter_batch />Some comments on commits martinpovolny/manageiq@c9de5ff~...c02e8ea

spec/controllers/application_controller/buttons_spec.rb

  • ⚠️ - 55 - Detected expect_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

@miq-bot
Copy link
Member

miq-bot commented Nov 27, 2016

Checked commits martinpovolny/manageiq@c9de5ff~...c02e8ea with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
8 files checked, 0 offenses detected
Everything looks good. 🏆

@gmcculloug
Copy link
Member

Look goos, thanks.

@gmcculloug gmcculloug merged commit 6502d59 into ManageIQ:master Nov 28, 2016
@gmcculloug gmcculloug added this to the Sprint 50 Ending Dec 5, 2016 milestone Nov 28, 2016
@simaishi simaishi removed the blocker label Nov 29, 2016
@simaishi
Copy link
Contributor

simaishi commented Jan 6, 2017

Euwe backport conflict:

$ git cherry-pick -e -x -m 1 6502d59
error: could not apply 6502d59... Merge pull request #10118 from martinpovolny/button_launch_url
$ git diff
diff --cc app/controllers/application_controller/buttons.rb
index cf250b6,54095da..0000000
--- a/app/controllers/application_controller/buttons.rb
+++ b/app/controllers/application_controller/buttons.rb
@@@ -264,12 -276,15 +276,19 @@@ module ApplicationController::Button
        options[:target_id] = obj.id
        options[:target_kls] = obj.class.name
        dialog_initialize(button.resource_action, options)
+     elsif button.options && button.options.key?(:open_url) && button.options[:open_url]
+       task_id = button.invoke_async(obj)
+       initiate_wait_for_task(:task_id => task_id, :action => :custom_button_done)
      else
        begin
++<<<<<<< HEAD
 +        button.invoke(obj)    # Run the task
++=======
+         button.invoke(obj)
++>>>>>>> 6502d59... Merge pull request #10118 from martinpovolny/button_launch_url
        rescue StandardError => bang
          add_flash(_("Error executing: \"%{task_description}\" %{error_message}") %
-           {:task_description => params[:desc], :error_message => bang.message}, :error) # Push msg and error flag
+           {:task_description => params[:desc], :error_message => bang.message}, :error)
        else
          add_flash(_("\"%{task_description}\" was executed") % {:task_description => params[:desc]})
        end

@simaishi
Copy link
Contributor

simaishi commented Jan 6, 2017

@martinpovolny Please resolve conflict and make Euwe-specific PR (referencing this one) or suggest other PRs to backport.

@simaishi
Copy link
Contributor

Backported to Euwe via #13449

@martinpovolny martinpovolny deleted the button_launch_url branch November 28, 2017 18:51
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.

8 participants