From f7715b559f10fba1009c7f4cd61d5f3c60fa7575 Mon Sep 17 00:00:00 2001 From: Alberto Bellotti Date: Thu, 16 Mar 2017 15:38:19 -0400 Subject: [PATCH 1/3] - Enhanced API to have a task created for provider refreshes and returns the appropriate task_id and task_href in the action result. so a successful "refresh" action now return something like: { "success": true, "message": "Provider id:2 name:'rhev36' refreshing", "task_id": 55, "task_href": "http://localhost:3000/api/tasks/55", "href": "http://localhost:3000/api/providers/2" } - Supports both ext_management_system and provider (i.e. provider_class=provider) - Added spec --- app/controllers/api/providers_controller.rb | 4 +-- app/models/ext_management_system.rb | 5 ++-- app/models/provider.rb | 5 ++-- spec/requests/api/providers_spec.rb | 30 +++++++++++++++++++++ 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/providers_controller.rb b/app/controllers/api/providers_controller.rb index c9331ff2475..0d2c86f1071 100644 --- a/app/controllers/api/providers_controller.rb +++ b/app/controllers/api/providers_controller.rb @@ -139,8 +139,8 @@ def edit_provider(provider, data) def refresh_provider(provider) desc = "#{provider_ident(provider)} refreshing" - provider.refresh_ems - action_result(true, desc) + task_id = provider.refresh_ems(:create_task => true).first + action_result(true, desc, :task_id => task_id) rescue => err action_result(false, err.to_s) end diff --git a/app/models/ext_management_system.rb b/app/models/ext_management_system.rb index 6da193ac2d6..1ded81ad59a 100644 --- a/app/models/ext_management_system.rb +++ b/app/models/ext_management_system.rb @@ -400,14 +400,15 @@ def last_refresh_status end end - def refresh_ems + def refresh_ems(opts = nil) + opts ||= {} if missing_credentials? raise _("no %{table} credentials defined") % {:table => ui_lookup(:table => "ext_management_systems")} end unless authentication_status_ok? raise _("%{table} failed last authentication check") % {:table => ui_lookup(:table => "ext_management_systems")} end - EmsRefresh.queue_refresh(self) + EmsRefresh.queue_refresh(self, nil, opts) end def self.ems_infra_discovery_types diff --git a/app/models/provider.rb b/app/models/provider.rb index 9af033a7ffa..492e0a8bc0c 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -54,13 +54,14 @@ def my_zone end alias_method :zone_name, :my_zone - def refresh_ems + def refresh_ems(opts = nil) + opts ||= {} if missing_credentials? raise _("no %{table} credentials defined") % {:table => ui_lookup(:table => "provider")} end unless authentication_status_ok? raise _("%{table} failed last authentication check") % {:table => ui_lookup(:table => "provider")} end - managers.each { |manager| EmsRefresh.queue_refresh(manager) } + managers.collect { |manager| EmsRefresh.queue_refresh(manager, nil, opts) }.flatten end end diff --git a/spec/requests/api/providers_spec.rb b/spec/requests/api/providers_spec.rb index a82d85ad804..30277eab792 100644 --- a/spec/requests/api/providers_spec.rb +++ b/spec/requests/api/providers_spec.rb @@ -744,6 +744,36 @@ def failed_auth_action(id) expect(response).to have_http_status(:ok) expect_results_to_match_hash("results", [failed_auth_action(p1.id), failed_auth_action(p2.id)]) end + + it "provider refresh are created with a task" do + api_basic_authorize collection_action_identifier(:providers, :refresh) + + provider = FactoryGirl.create(:ext_management_system, sample_vmware.symbolize_keys.except(:type)) + provider.update_authentication(:default => default_credentials.symbolize_keys) + allow_any_instance_of(provider.class).to receive_messages(:authentication_status_ok? => true) + + run_post(providers_url(provider.id), gen_request(:refresh)) + + expect_single_action_result(:success => true, + :message => a_string_matching("Provider .* refreshing"), + :href => providers_url(provider.id), + :task => true) + end + + it "provider refresh for provider_class=provider are created with a task" do + api_basic_authorize collection_action_identifier(:providers, :refresh) + + provider = FactoryGirl.create(:provider_foreman, :zone => @zone, :url => "example.com", :verify_ssl => false) + provider.update_authentication(:default => default_credentials.symbolize_keys) + allow_any_instance_of(provider.class).to receive_messages(:authentication_status_ok? => true) + + run_post(providers_url(provider.id) + '?provider_class=provider', gen_request(:refresh)) + + expect_single_action_result(:success => true, + :message => a_string_matching("Provider .* refreshing"), + :href => providers_url(provider.id), + :task => true) + end end describe 'Providers import VM' do From 30ea3618fa7adb30c060c9aad38842c2fa969919 Mon Sep 17 00:00:00 2001 From: Alberto Bellotti Date: Wed, 22 Mar 2017 16:10:01 -0400 Subject: [PATCH 2/3] Preferred defaulting opts to {} in method declaration, preferring flat_map over collect.flatten --- app/models/ext_management_system.rb | 3 +-- app/models/provider.rb | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/models/ext_management_system.rb b/app/models/ext_management_system.rb index 1ded81ad59a..9d80595e81a 100644 --- a/app/models/ext_management_system.rb +++ b/app/models/ext_management_system.rb @@ -400,8 +400,7 @@ def last_refresh_status end end - def refresh_ems(opts = nil) - opts ||= {} + def refresh_ems(opts = {}) if missing_credentials? raise _("no %{table} credentials defined") % {:table => ui_lookup(:table => "ext_management_systems")} end diff --git a/app/models/provider.rb b/app/models/provider.rb index 492e0a8bc0c..bb08ac530f9 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -54,14 +54,13 @@ def my_zone end alias_method :zone_name, :my_zone - def refresh_ems(opts = nil) - opts ||= {} + def refresh_ems(opts = {}) if missing_credentials? raise _("no %{table} credentials defined") % {:table => ui_lookup(:table => "provider")} end unless authentication_status_ok? raise _("%{table} failed last authentication check") % {:table => ui_lookup(:table => "provider")} end - managers.collect { |manager| EmsRefresh.queue_refresh(manager, nil, opts) }.flatten + managers.flat_map { |manager| EmsRefresh.queue_refresh(manager, nil, opts) } end end From 8d45246ccb07d758e9652a55cbf42df388b557c5 Mon Sep 17 00:00:00 2001 From: Alberto Bellotti Date: Wed, 22 Mar 2017 21:26:56 -0400 Subject: [PATCH 3/3] Preferring alternative to using allow_any_instance_of in providers spec. --- spec/requests/api/providers_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/api/providers_spec.rb b/spec/requests/api/providers_spec.rb index 30277eab792..3b4c246b177 100644 --- a/spec/requests/api/providers_spec.rb +++ b/spec/requests/api/providers_spec.rb @@ -750,7 +750,7 @@ def failed_auth_action(id) provider = FactoryGirl.create(:ext_management_system, sample_vmware.symbolize_keys.except(:type)) provider.update_authentication(:default => default_credentials.symbolize_keys) - allow_any_instance_of(provider.class).to receive_messages(:authentication_status_ok? => true) + provider.authentication_type(:default).update(:status => "Valid") run_post(providers_url(provider.id), gen_request(:refresh)) @@ -765,7 +765,7 @@ def failed_auth_action(id) provider = FactoryGirl.create(:provider_foreman, :zone => @zone, :url => "example.com", :verify_ssl => false) provider.update_authentication(:default => default_credentials.symbolize_keys) - allow_any_instance_of(provider.class).to receive_messages(:authentication_status_ok? => true) + provider.authentication_type(:default).update(:status => "Valid") run_post(providers_url(provider.id) + '?provider_class=provider', gen_request(:refresh))