From f57d44d0a8db9c890a76c4ca3f422feca6e53b95 Mon Sep 17 00:00:00 2001 From: Bastian Schmidt Date: Tue, 19 Nov 2024 15:09:16 +0100 Subject: [PATCH 1/4] Add foreman-tasks and theforeman-rubocop dependencies * foreman-tasks >= 10.0, < 11 * theforeman-rubocop ~> 0.1.0 --- foreman_resource_quota.gemspec | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/foreman_resource_quota.gemspec b/foreman_resource_quota.gemspec index 562838f..5de8202 100644 --- a/foreman_resource_quota.gemspec +++ b/foreman_resource_quota.gemspec @@ -15,4 +15,8 @@ Gem::Specification.new do |s| s.description = 'Foreman Plug-in to manage resource usage among users.' s.files = Dir['{app,config,db,lib,locale,webpack}/**/*'] + ['LICENSE', 'Rakefile', 'README.md', 'package.json'] + + s.add_dependency 'foreman-tasks', '>= 10.0', '< 11' + + s.add_development_dependency 'theforeman-rubocop', '~> 0.1.0' end From c8d8853adaf3751798cca6bf6903a878b8b7cbfe Mon Sep 17 00:00:00 2001 From: Bastian Schmidt Date: Wed, 20 Nov 2024 16:00:17 +0100 Subject: [PATCH 2/4] Add RecurringLogic to update HostResources Add task RefreshResourceQuotaUtilization which calls determine_utilization on every ResourceQuota. The command iterates all hosts of a ResourceQuota and sets the host.host_resources which is used for the ResourceQuota.utilization. This guarantees that Foreman does not miss changes to HostResources, when they are not performed via the Foreman UI. Moreover, it increases performance since Foreman Resource Quota does not re-compute a ResourceQuota's utilization on host deployment and, instead, takes the last-computed sum of HostResources. However, this comes with the downside that HostResources might be missed when edited out-of Foreman UI and new hosts are deployed before the task is executed. --- .rubocop.yml | 4 ++ .../refresh_resource_quota_utilization.rb | 25 ++++++++++++ lib/foreman_resource_quota/engine.rb | 39 +++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 lib/foreman_resource_quota/async/refresh_resource_quota_utilization.rb diff --git a/.rubocop.yml b/.rubocop.yml index f19eac4..e81de53 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -43,3 +43,7 @@ Rails/SkipsModelValidations: Style/FormatStringToken: Enabled: false + +Rails/DynamicFindBy: + Exclude: + - "lib/foreman_resource_quota/engine.rb" diff --git a/lib/foreman_resource_quota/async/refresh_resource_quota_utilization.rb b/lib/foreman_resource_quota/async/refresh_resource_quota_utilization.rb new file mode 100644 index 0000000..3720ba6 --- /dev/null +++ b/lib/foreman_resource_quota/async/refresh_resource_quota_utilization.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module ForemanResourceQuota + module Async + class RefreshResourceQuotaUtilization < ::Actions::EntryAction + include ::Actions::RecurringAction + + def run + ResourceQuota.all.each do |quota| + quota.determine_utilization + rescue e + logger.error N_(format("An error occured determining the utilization of '%s'-quota: %s", quota.name, e)) + end + end + + def logger + action_logger + end + + def rescue_strategy_for_self + Dynflow::Action::Rescue::Fail + end + end + end +end diff --git a/lib/foreman_resource_quota/engine.rb b/lib/foreman_resource_quota/engine.rb index 0e96649..c5be3fb 100644 --- a/lib/foreman_resource_quota/engine.rb +++ b/lib/foreman_resource_quota/engine.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'foreman_tasks' + module ForemanResourceQuota class Engine < ::Rails::Engine engine_name 'foreman_resource_quota' @@ -42,10 +44,47 @@ class Engine < ::Rails::Engine Rails.logger.warn "ForemanResourceQuota: skipping engine hook (#{e})" end + # Register ForemanTasks-based recurring logic/scheduled tasks + initializer 'foreman_resource_quota.register_scheduled_tasks', before: :finisher_hook do |_app| + action_paths = [ForemanResourceQuota::Engine.root.join('lib/foreman_resource_quota/async')] + ::ForemanTasks.dynflow.config.eager_load_paths.concat(action_paths) + + # Skip object creation if the admin user is not present + # skip database manipulations while tables do not exist, like in migrations + if ActiveRecord::Base.connection.data_source_exists?(ForemanTasks::Task.table_name) && + User.unscoped.find_by_login(User::ANONYMOUS_ADMIN).present? + # Register the scheduled tasks + ::ForemanTasks.dynflow.config.on_init(false) do |_world| + ForemanResourceQuota::Engine.register_scheduled_task( + ForemanResourceQuota::Async::RefreshResourceQuotaUtilization, + '0 1 * * *' + ) + end + end + rescue ActiveRecord::NoDatabaseError => e + Rails.logger.warn "ForemanResourceQuota: skipping ForemanTasks registration hook (#{e})" + end + initializer 'foreman_resource_quota.register_gettext', after: :load_config_initializers do |_app| locale_dir = File.join(File.expand_path('../..', __dir__), 'locale') locale_domain = 'foreman_resource_quota' Foreman::Gettext::Support.add_text_domain locale_domain, locale_dir end + + # Helper to register ForemanTasks + def self.register_scheduled_task(task_class, cronline) + return if ::ForemanTasks::RecurringLogic.joins(:tasks) + .merge(::ForemanTasks::Task.where(label: task_class.name)) + .exists? + ::ForemanTasks::RecurringLogic.transaction(isolation: :serializable) do + User.as_anonymous_admin do + recurring_logic = ::ForemanTasks::RecurringLogic.new_from_cronline(cronline) + recurring_logic.save! + recurring_logic.start(task_class) + end + end + rescue ActiveRecord::TransactionIsolationError => e + Rails.logger.warn "ForemanResourceQuota: skipping RecurringLogic registration hook (#{e})" + end end end From 8e20c3bb1a7b81cf7b4ac4f7403ed82b1bca0681 Mon Sep 17 00:00:00 2001 From: Bastian Schmidt Date: Tue, 26 Nov 2024 16:13:00 +0100 Subject: [PATCH 3/4] Add job test for RefreshResourceQuotaUtilization --- lib/foreman_resource_quota/engine.rb | 2 +- ...refresh_resource_quota_utilization_test.rb | 45 +++++++++++++++++++ test/test_plugin_helper.rb | 5 +++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 test/jobs/refresh_resource_quota_utilization_test.rb diff --git a/lib/foreman_resource_quota/engine.rb b/lib/foreman_resource_quota/engine.rb index c5be3fb..cfe4510 100644 --- a/lib/foreman_resource_quota/engine.rb +++ b/lib/foreman_resource_quota/engine.rb @@ -51,7 +51,7 @@ class Engine < ::Rails::Engine # Skip object creation if the admin user is not present # skip database manipulations while tables do not exist, like in migrations - if ActiveRecord::Base.connection.data_source_exists?(ForemanTasks::Task.table_name) && + if ActiveRecord::Base.connection.data_source_exists?(::ForemanTasks::Task.table_name) && User.unscoped.find_by_login(User::ANONYMOUS_ADMIN).present? # Register the scheduled tasks ::ForemanTasks.dynflow.config.on_init(false) do |_world| diff --git a/test/jobs/refresh_resource_quota_utilization_test.rb b/test/jobs/refresh_resource_quota_utilization_test.rb new file mode 100644 index 0000000..ee6639c --- /dev/null +++ b/test/jobs/refresh_resource_quota_utilization_test.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'test_plugin_helper' +require 'foreman_tasks/test_helpers' + +class RefreshResourceQuotaUtilizationTest < ActiveSupport::TestCase + include ForemanTasks::TestHelpers::WithInThreadExecutor + + setup do + User.current = User.find_by(login: 'secret_admin') + Setting[:resource_quota_global_no_action] = false + Setting[:resource_quota_optional_assignment] = false + User.current.resource_quota_is_optional = false + + stub_host_utilization({ cpu_cores: 2, memory_mb: 1024 * 4, disk_gb: 60 }, {}) + @quota = FactoryBot.create(:resource_quota, cpu_cores: 20, memory_mb: 1024 * 30, disk_gb: 512) + + @host_a = FactoryBot.create(:host, resource_quota: @quota) + @host_b = FactoryBot.create(:host, resource_quota: @quota) + @host_c = FactoryBot.create(:host, resource_quota: @quota) + @host_d = FactoryBot.create(:host, resource_quota: @quota) + @host_e = FactoryBot.create(:host, resource_quota: @quota) + @quota.reload + end + + test 'single resource quota utilization should be updated' do + assert_equal({ cpu_cores: 5 * 2, memory_mb: 5 * 1024 * 4, disk_gb: 5 * 60 }, @quota.utilization) + new_host_utilization = { cpu_cores: 3, memory_mb: 1024 * 5, disk_gb: 61 } + quota_hosts_resources = { + @host_a.name => new_host_utilization, + @host_b.name => new_host_utilization, + @host_c.name => new_host_utilization, + @host_d.name => new_host_utilization, + @host_e.name => new_host_utilization, + } + stub_quota_utilization_helper(quota_hosts_resources, {}) + ForemanTasks.sync_task(ForemanResourceQuota::Async::RefreshResourceQuotaUtilization) + @quota.reload + assert_equal({ + cpu_cores: 5 * new_host_utilization[:cpu_cores], + memory_mb: 5 * new_host_utilization[:memory_mb], + disk_gb: 5 * new_host_utilization[:disk_gb], + }, @quota.utilization) + end +end diff --git a/test/test_plugin_helper.rb b/test/test_plugin_helper.rb index 7b692ee..bf3fbd8 100644 --- a/test/test_plugin_helper.rb +++ b/test/test_plugin_helper.rb @@ -40,6 +40,11 @@ def stub_quota_missing_hosts(return_missing_hosts) .returns(return_missing_hosts) end +def stub_quota_utilization_helper(return_hosts_resources, return_missing_hosts) + ForemanResourceQuota::ResourceQuota.any_instance.stubs(:call_utilization_helper) + .returns([return_hosts_resources, return_missing_hosts]) +end + def stub_host_utilization(return_utilization, return_missing_hosts) Host::Managed.any_instance.stubs(:call_utilization_helper).returns([return_utilization, return_missing_hosts]) end From 3d09d9323b5ea5b7e93a7e9c0d0fbd8f4fe848da Mon Sep 17 00:00:00 2001 From: Bastian Schmidt Date: Tue, 26 Nov 2024 18:07:43 +0100 Subject: [PATCH 4/4] Save HostResources after host is saved A Hosts's HostResources are determined during the validation step but not saved, because the Host must exist before its HostResources can be stored (not-null database constraint). We add a routine which saves the HostResources immediately after a Host is saved in case its Host.host_resources changed --- .../foreman_resource_quota/host_managed_extensions.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/concerns/foreman_resource_quota/host_managed_extensions.rb b/app/models/concerns/foreman_resource_quota/host_managed_extensions.rb index 989a555..3d9793f 100644 --- a/app/models/concerns/foreman_resource_quota/host_managed_extensions.rb +++ b/app/models/concerns/foreman_resource_quota/host_managed_extensions.rb @@ -19,6 +19,7 @@ module HostManagedExtensions # A host shall always have a .host_resources attribute before_validation :build_host_resources, unless: -> { host_resources.present? } + after_save :save_host_resources, if: -> { host_resources.changed? } end def verify_resource_quota @@ -141,6 +142,10 @@ def quota_assigment_optional? owner.resource_quota_is_optional || Setting[:resource_quota_optional_assignment] end + def save_host_resources + host_resources.save + end + # Wrap into a function for easier testing def call_utilization_helper(resources, hosts) all_host_resources, missing_hosts = utilization_from_resource_origins(resources, hosts)