Skip to content

Commit

Permalink
fixes #13618 - cache expensive vmware api calls
Browse files Browse the repository at this point in the history
  • Loading branch information
timogoebel committed Jan 30, 2017
1 parent 9182590 commit 0520693
Show file tree
Hide file tree
Showing 17 changed files with 291 additions and 26 deletions.
15 changes: 13 additions & 2 deletions app/controllers/api/v2/compute_resources_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class ComputeResourcesController < V2::BaseController
before_action :find_resource, :only => [:show, :update, :destroy, :available_images, :associate,
:available_clusters, :available_flavors, :available_folders,
:available_networks, :available_resource_pools, :available_security_groups, :available_storage_domains,
:available_zones, :available_storage_pods]
:available_zones, :available_storage_pods, :reload_cache]

api :GET, "/compute_resources/", N_("List all compute resources")
param_group :taxonomy_scope, ::Api::V2::BaseController
Expand Down Expand Up @@ -41,6 +41,7 @@ def show
param :server, String, :desc => N_("for VMware")
param :set_console_password, :bool, :desc => N_("for Libvirt and VMware only")
param :display_type, %w(VNC SPICE), :desc => N_('for Libvirt only')
param :caching_enabled, :bool, :desc => N_('enable caching, for VMware only')
param_group :taxonomies, ::Api::V2::BaseController
end
end
Expand Down Expand Up @@ -162,11 +163,21 @@ def associate
render 'api/v2/hosts/index', :layout => 'api/v2/layouts/index_layout'
end

api :PUT, "/compute_resources/:id/reload_cache/", N_("Reload Compute Resource Cache")
param :id, :identifier, :required => true
def reload_cache
if @compute_resource.respond_to?(:reload_cache) && @compute_resource.reload_cache
render_message(_('Successfully cleared the cache.'))
else
render_message(_('Failed to clear the cache.'), :status => :unprocessable_entity)
end
end

private

def action_permission
case params[:action]
when 'available_images', 'available_clusters', 'available_flavors', 'available_folders', 'available_networks', 'available_resource_pools', 'available_security_groups', 'available_storage_domains', 'available_zones', 'associate', 'available_storage_pods'
when 'available_images', 'available_clusters', 'available_flavors', 'available_folders', 'available_networks', 'available_resource_pools', 'available_security_groups', 'available_storage_domains', 'available_zones', 'associate', 'available_storage_pods', 'reload_cache'
:view
else
super
Expand Down
18 changes: 16 additions & 2 deletions app/controllers/compute_resources_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class ComputeResourcesController < ApplicationController

AJAX_REQUESTS = [:template_selected, :cluster_selected, :resource_pools]
before_action :ajax_request, :only => AJAX_REQUESTS
before_action :find_resource, :only => [:show, :edit, :associate, :update, :destroy, :ping] + AJAX_REQUESTS
before_action :find_resource, :only => [:show, :edit, :associate, :update, :destroy, :ping, :reload_cache] + AJAX_REQUESTS

#This can happen in development when removing a plugin
rescue_from ActiveRecord::SubclassNotFound do |e|
Expand Down Expand Up @@ -73,6 +73,20 @@ def destroy
end
end

def reload_cache
if @compute_resource.respond_to?(:reload_cache) && @compute_resource.reload_cache
process_success(
:success_msg => _('Successfully reloaded the cache.'),
:success_redirect => @compute_resource
)
else
process_error(
:error_msg => _('Failed to reload the cache.'),
:redirect => @compute_resource
)
end
end

#ajax methods
def provider_selected
@compute_resource = ComputeResource.new_provider :provider => params[:provider]
Expand Down Expand Up @@ -125,7 +139,7 @@ def action_permission
case params[:action]
when 'associate'
'edit'
when 'ping', 'template_selected', 'cluster_selected', 'resource_pools'
when 'ping', 'template_selected', 'cluster_selected', 'resource_pools', 'reload_cache'
'view'
else
super
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ def compute_resource_params_filter
filter.permit :datacenter,
:pubkey_hash,
:server,
:uuid
:uuid,
:caching_enabled

add_taxonomix_params_filter(filter)
end
Expand Down
57 changes: 39 additions & 18 deletions app/models/compute_resources/foreman/model/vmware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
module Foreman::Model
class Vmware < ComputeResource
include ComputeResourceConsoleCommon
include ComputeResourceCaching

validates :user, :password, :server, :datacenter, :presence => true
before_create :update_public_key
Expand Down Expand Up @@ -53,41 +54,53 @@ def max_memory
end

def datacenters
name_sort(client.datacenters.all)
cache.cache(:datacenters) do
name_sort(client.datacenters.all)
end
end

def cluster(cluster)
dc.clusters.get(cluster)
cache.cache(:"cluster-#{cluster}") do
available_clusters.get(cluster)
end
end

def clusters
if dc.clusters.nil?
if available_clusters.nil?
Rails.logger.info "Datacenter #{dc.try(:name)} returned zero clusters"
return []
end
dc.clusters.map(&:full_path).sort
available_clusters.map(&:full_path).sort
end

def datastores(opts = {})
if opts[:storage_domain]
name_sort(dc.datastores.get(opts[:storage_domain]))
cache.cache(:"datastores-#{opts[:storage_domain]}") do
name_sort(dc.datastores.get(opts[:storage_domain]))
end
else
name_sort(dc.datastores.all(:accessible => true))
cache.cache(:datastores) do
name_sort(dc.datastores.all(:accessible => true))
end
end
end

def storage_pods(opts = {})
if opts[:storage_pod]
begin
dc.storage_pods.get(opts[:storage_pod])
rescue RbVmomi::VIM::InvalidArgument
{} # Return an empty storage pod hash if vsphere does not support the feature
cache.cache(:"storage_pods-#{opts[:storage_pod]}") do
begin
dc.storage_pods.get(opts[:storage_pod])
rescue RbVmomi::VIM::InvalidArgument
{} # Return an empty storage pod hash if vsphere does not support the feature
end
end
else
begin
name_sort(dc.storage_pods.all())
rescue RbVmomi::VIM::InvalidArgument
[] # Return an empty set of storage pods if vsphere does not support the feature
cache.cache(:storage_pods) do
begin
name_sort(dc.storage_pods.all())
rescue RbVmomi::VIM::InvalidArgument
[] # Return an empty set of storage pods if vsphere does not support the feature
end
end
end
end
Expand All @@ -97,20 +110,28 @@ def available_storage_pods(storage_pod = nil)
end

def folders
dc.vm_folders.sort_by{|f| [f.path, f.name]}
cache.cache(:folders) do
dc.vm_folders.sort_by{|f| [f.path, f.name]}
end
end

def networks(opts = {})
name_sort(dc.networks.all(:accessible => true))
cache.cache(:networks) do
name_sort(dc.networks.all(:accessible => true))
end
end

def resource_pools(opts = {})
cluster = cluster(opts[:cluster_id])
name_sort(cluster.resource_pools.all(:accessible => true))
cache.cache(:resource_pools) do
name_sort(cluster.resource_pools.all(:accessible => true))
end
end

def available_clusters
name_sort(dc.clusters)
cache.cache(:clusters) do
name_sort(dc.clusters)
end
end

def available_folders
Expand Down
13 changes: 13 additions & 0 deletions app/models/concerns/compute_resource_caching.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module ComputeResourceCaching
extend ActiveSupport::Concern

def reload_cache
cache.reload
end

private

def cache
@cache ||= ComputeResourceCache.new(self)
end
end
89 changes: 89 additions & 0 deletions app/services/compute_resource_cache.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# This class caches attributes for a compute resource in
# rails cache to speed up slow or expensive API calls
class ComputeResourceCache
attr_accessor :compute_resource, :cache_duration

delegate :logger, :to => ::Rails

def initialize(compute_resource, cache_duration: 180.minutes)
self.compute_resource = compute_resource
self.cache_duration = cache_duration
end

# Tries to retrieve the value for a given key from the cache
# and returns the retrieved value. If the cache is empty,
# the given block is executed and the block's return stored
# in the cache. This value is then returned by this method.
def cache(key, &block)
return get_uncached_value(key, &block) unless cache_enabled?
cached_value = read(key)
return cached_value if cached_value
return unless block_given?
uncached_value = get_uncached_value(key, &block)
write(key, uncached_value)
uncached_value
end

def delete(key)
logger.debug "Deleting from compute resource cache: #{key}"
Rails.cache.delete(cache_key + key.to_s)
end

def read(key)
logger.debug "Reading from compute resource cache: #{key}"
Rails.cache.read(cache_key + key.to_s, cache_options)
end

def write(key, value)
logger.debug "Storing in compute resource cache: #{key}"
Rails.cache.write(cache_key + key.to_s, value, cache_options)
end

def reload
# Rolls the cache_scope to reload the cache as not all
# cache implementations (eg. memcached) support deleting
# keys by a regex
Rails.cache.delete(cache_scope_key)
true
rescue StandardError => e
Foreman::Logging.exception('Failed to reload compute resource cache', e)
false
end

def cache_scope
Rails.cache.fetch(cache_scope_key, cache_options) do
Foreman.uuid
end
end

def cache_enabled?
compute_resource.persisted? && compute_resource.caching_enabled
end

private

def get_uncached_value(key, &block)
return unless block_given?
start_time = Time.now.utc
result = compute_resource.instance_eval(&block)
end_time = Time.now.utc
duration = end_time - (start_time).round(4)
logger.info("Loaded compute resource data for #{key} in #{duration} seconds")
result
end

def cache_key
"compute_resource_#{compute_resource.id}-#{cache_scope}/"
end

def cache_scope_key
"compute_resource_#{compute_resource.id}-cache_scope_key"
end

def cache_options
{
:expires_in => cache_duration,
:race_condition_ttl => 1.minute
}
end
end
4 changes: 2 additions & 2 deletions app/services/foreman/access_permissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@

permission_set.security_block :compute_resources do |map|
ajax_actions = [:test_connection]
map.permission :view_compute_resources, {:compute_resources => [:index, :show, :auto_complete_search, :ping, :available_images],
map.permission :view_compute_resources, {:compute_resources => [:index, :show, :auto_complete_search, :ping, :available_images, :reload_cache],
:"api/v1/compute_resources" => [:index, :show],
:"api/v2/compute_resources" => [:index, :show, :available_images, :available_clusters, :available_folders,
:available_flavors, :available_networks, :available_resource_pools,
:available_security_groups, :available_storage_domains, :available_zones,
:available_storage_pods]
:available_storage_pods, :reload_cache]
}
map.permission :create_compute_resources, {:compute_resources => [:new, :create].push(*ajax_actions),
:"api/v1/compute_resources" => [:create],
Expand Down
2 changes: 1 addition & 1 deletion app/views/api/v2/compute_resources/vmware.json.rabl
Original file line number Diff line number Diff line change
@@ -1 +1 @@
attributes :user, :datacenter, :server, :set_console_password
attributes :user, :datacenter, :server, :set_console_password, :caching_enabled
2 changes: 2 additions & 0 deletions app/views/compute_resources/form/_vmware.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@
<%= checkbox_f f, :set_console_password, :checked => f.object.set_console_password?,
:label => _("Console passwords"),
:help_inline => _("Set a randomly generated password on the display connection") %>
<%= checkbox_f f, :caching_enabled, :label => _("Enable caching"),
:help_inline => _("Cache slow calls to VMWare to speed up page rendering") %>
<%= f.hidden_field(:pubkey_hash) if f.object.uuid.present? %>
1 change: 1 addition & 0 deletions app/views/compute_resources/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<% title_actions display_link_if_authorized(_("Associate VMs"),
hash_for_associate_compute_resource_path(:compute_resource_id => @compute_resource).merge(:auth_object => @compute_resource, :permission => 'edit_compute_resources'),
:title => _("Associate VMs to Foreman hosts"), :method => :put, :class=>"btn btn-default"),
(display_link_if_authorized(_('Reload Cache'), hash_for_reload_cache_compute_resource_path(@compute_resource).merge(:auth_object => @compute_resource), :method => :put, :class => "btn btn-default") if @compute_resource.respond_to?(:reload_cache) && @compute_resource.caching_enabled),
link_to_if_authorized(_('Edit'), hash_for_edit_compute_resource_path(@compute_resource).merge(:auth_object => @compute_resource), :class => "btn btn-default") %>

<ul class="nav nav-tabs" data-tabs="tabs">
Expand Down
4 changes: 4 additions & 0 deletions app/views/compute_resources/show/_vmware.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@
<td><%= _("Console passwords") %></td>
<td><%= @compute_resource.set_console_password? ? _("Enabled") : _("Disabled") %></td>
</tr>
<tr>
<td><%= _("Caching") %></td>
<td><%= @compute_resource.caching_enabled ? _("Enabled") : _("Disabled") %></td>
</tr>
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@
get 'resource_pools'
post 'ping'
put 'associate'
put 'reload_cache'
end
constraints(:id => /[^\/]+/) do
resources :vms, :controller => "compute_resources_vms" do
Expand Down
1 change: 1 addition & 0 deletions config/routes/api/v2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@
get 'available_clusters/(:cluster_id)/available_resource_pools', :to => 'compute_resources#available_resource_pools', :on => :member
get :available_zones, :on => :member
put :associate, :on => :member
put :reload_cache, :on => :member
(resources :locations, :only => [:index, :show]) if SETTINGS[:locations_enabled]
(resources :organizations, :only => [:index, :show]) if SETTINGS[:organizations_enabled]
resources :compute_attributes, :only => [:create, :update]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddCachingEnabledToComputeResource < ActiveRecord::Migration
def change
add_column :compute_resources, :caching_enabled, :boolean, default: true
end
end
12 changes: 12 additions & 0 deletions test/controllers/api/v2/compute_resources_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,18 @@ def teardown
assert !available_storage_domains.empty?
end

context 'cache reloading' do
test 'should reload cache if supported' do
put :reload_cache, { :id => compute_resources(:vmware).to_param }
assert_response :success
end

test 'should fail if unsupported' do
put :reload_cache, { :id => compute_resources(:ovirt).to_param }
assert_response :unprocessable_entity
end
end

context 'ec2' do
setup do
@ec2_object = Object.new
Expand Down
Loading

0 comments on commit 0520693

Please sign in to comment.