From 36135969a8818fdf78397de554dde27fbc2f38b9 Mon Sep 17 00:00:00 2001 From: Bastian Schmidt Date: Sat, 3 Feb 2024 20:17:55 +0100 Subject: [PATCH] Fix host creation resource origin * Add new resource origin ComputeAttributesOrigin (available during host creation) * In ResourceOrigins, iterate hosts by name instead of id (new host has no id) * Add resource quota exception classes * Add host managed extensions test * Add host managed factory extensions * Fix resource to string and tests * Fix deepCopy null error * Fix autoload filepaths --- .../resource_quota_helper.rb | 118 +++++++-- app/lib/foreman_resource_quota/exceptions.rb | 17 ++ .../host_managed_extensions.rb | 85 +++++-- .../foreman_resource_quota/resource_quota.rb | 20 +- .../foreman_resource_quota/resource_origin.rb | 47 ++-- .../compute_attributes_origin.rb | 46 ++++ .../compute_resource_origin.rb | 51 ++-- .../resource_origins/facts_origin.rb | 6 +- .../resource_origins/vm_attributes_origin.rb | 2 +- lib/foreman_resource_quota/engine.rb | 9 +- lib/foreman_resource_quota/register.rb | 14 +- .../host_resource_quota_extensions.rb | 9 + test/helpers/hosts_helper_test.rb | 34 +-- test/helpers/resource_quota_helper_test.rb | 231 +++++++++++++++++- .../concerns/host_managed_extension_test.rb | 46 ++++ test/models/resource_quota_test.rb | 106 ++++---- test/unit/resource_quota_test.rb | 2 +- webpack/helper.js | 18 +- 18 files changed, 669 insertions(+), 192 deletions(-) create mode 100644 app/lib/foreman_resource_quota/exceptions.rb create mode 100644 app/services/foreman_resource_quota/resource_origins/compute_attributes_origin.rb create mode 100644 test/factories/host_resource_quota_extensions.rb create mode 100644 test/models/concerns/host_managed_extension_test.rb diff --git a/app/helpers/foreman_resource_quota/resource_quota_helper.rb b/app/helpers/foreman_resource_quota/resource_quota_helper.rb index f397bbd..cbd44b2 100644 --- a/app/helpers/foreman_resource_quota/resource_quota_helper.rb +++ b/app/helpers/foreman_resource_quota/resource_quota_helper.rb @@ -2,40 +2,118 @@ module ForemanResourceQuota module ResourceQuotaHelper + include ResourceOrigin + FACTOR_B_TO_KB = 1024 FACTOR_B_TO_MB = 1024 * 1024 FACTOR_B_TO_GB = 1024 * FACTOR_B_TO_MB - def natural_resource_name_by_type(type) - type_names = { cpu_cores: 'CPU cores', memory_mb: 'Memory (MB)', disk_gb: 'Disk space (GB)' } - type_names[type] + def natural_resource_name_by_type(resource_type) + type_names = { cpu_cores: 'CPU cores', memory_mb: 'Memory', disk_gb: 'Disk space' } + key = resource_type.is_a?(String) ? resource_type.to_sym : resource_type + raise "No natural name for unknown resource type '#{resource_type}'" unless type_names.key?(key) + type_names[key] end - def build_missing_resources_per_host_list(hosts, quota_utilization) - # missing_res_per_host := { : [] } - # for example: { 1: [ :disk_gb ], 2: [ :cpu_cores, :disk_gb ] } - return {} if hosts.empty? || quota_utilization.empty? + def units_by_type(resource_type) + type_units = { + cpu_cores: [ + { symbol: 'cores', factor: 1 }, + ], + memory_mb: [ + { symbol: 'MB', factor: 1 }, + { symbol: 'GB', factor: FACTOR_B_TO_KB }, + { symbol: 'TB', factor: FACTOR_B_TO_MB }, + ], + disk_gb: [ + { symbol: 'GB', factor: 1 }, + { symbol: 'TB', factor: FACTOR_B_TO_KB }, + { symbol: 'PB', factor: FACTOR_B_TO_MB }, + ], + } + key = resource_type.is_a?(String) ? resource_type.to_sym : resource_type + raise "No units for unknown resource type '#{resource_type}'" unless type_units.key?(key) + type_units[key] + end - missing_res_per_host = hosts.map(&:id).index_with { [] } - missing_res_per_host.each_key do |host_id| - quota_utilization.each do |resource| - missing_res_per_host[host_id] << resource unless resource.nil? - end + def find_largest_unit(resource_value, units) + units.reverse_each do |unit| + return unit.values_at(:symbol, :factor) if resource_value >= unit[:factor] end - missing_res_per_host + units[0].values_at(:symbol, :factor) + end + + def resource_value_to_string(resource_value, resource_type) + (symbol, factor) = find_largest_unit(resource_value, units_by_type(resource_type)) + unit_applied_value = (resource_value / factor).round(1) + format_text = if (unit_applied_value % 1).zero? + '%.0f %s' + else + '%.1f %s' + end + format(format_text, unit_applied_value, symbol) end def utilization_from_resource_origins(resources, hosts, use_compute_resource: true, use_vm_attributes: true, - use_facts: true) - utilization = resources.each.with_object({}) { |key, hash| hash[key] = 0 } - missing_res_per_host = build_missing_resources_per_host_list(hosts, resources) + use_compute_attributes: true, use_facts: true) + utilization_sum = resources.each.with_object({}) { |key, hash| hash[key] = 0 } + missing_hosts_resources = create_missing_hosts_resources_hash(hosts, resources) + hosts_hash = hosts.index_by(&:name) if use_compute_resource - ResourceOrigin::ComputeResourceOrigin.new.collect_resources!(utilization, missing_res_per_host) + ResourceOrigin::ComputeResourceOrigin.new.collect_resources!( + utilization_sum, + missing_hosts_resources, + hosts_hash + ) + end + if use_vm_attributes + ResourceOrigin::VMAttributesOrigin.new.collect_resources!( + utilization_sum, + missing_hosts_resources, + hosts_hash + ) + end + if use_compute_attributes + ResourceOrigin::ComputeAttributesOrigin.new.collect_resources!( + utilization_sum, + missing_hosts_resources, + hosts_hash + ) + end + if use_facts + ResourceOrigin::FactsOrigin.new.collect_resources!( + utilization_sum, + missing_hosts_resources, + hosts_hash + ) end - ResourceOrigin::VMAttributesOrigin.new.collect_resources!(utilization, missing_res_per_host) if use_vm_attributes - ResourceOrigin::FactsOrigin.new.collect_resources!(utilization, missing_res_per_host) if use_facts - [utilization, missing_res_per_host] + [utilization_sum, missing_hosts_resources] + end + + private + + # Create a hash that maps resources to host names. + # Parameters: + # - hosts: An array of host objects. + # - resources: List of resources to be determined per host. + # Returns: + # - missing_hosts_resources := { : [] } + # for example: + # { + # "host_a": { + # [ :cpu_cores, :disk_gb ] + # }, + # "host_b": { + # [ :cpu_cores, :disk_gb ] + # }, + def create_missing_hosts_resources_hash(hosts, resources) + return {} if hosts.empty? || resources.empty? + + resources_to_determine = resources.compact + return {} if resources_to_determine.empty? + + hosts.map(&:name).index_with { resources_to_determine.clone } end end end diff --git a/app/lib/foreman_resource_quota/exceptions.rb b/app/lib/foreman_resource_quota/exceptions.rb new file mode 100644 index 0000000..e3015e4 --- /dev/null +++ b/app/lib/foreman_resource_quota/exceptions.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module ForemanResourceQuota + module Exceptions + class HostResourceQuotaEmptyException < Foreman::Exception + end + + class ResourceLimitException < Foreman::Exception + end + + class HostResourcesException < Foreman::Exception + end + + class ResourceQuotaUtilizationException < Foreman::Exception + end + end +end 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 442a208..603964c 100644 --- a/app/models/concerns/foreman_resource_quota/host_managed_extensions.rb +++ b/app/models/concerns/foreman_resource_quota/host_managed_extensions.rb @@ -3,7 +3,8 @@ module ForemanResourceQuota module HostManagedExtensions extend ActiveSupport::Concern - include ResourceQuotaHelper + include ForemanResourceQuota::ResourceQuotaHelper + include ForemanResourceQuota::Exceptions included do validate :check_resource_quota_capacity @@ -12,50 +13,80 @@ module HostManagedExtensions scoped_search relation: :resource_quota, on: :name, complete_value: true, rename: :resource_quota end - # rubocop: disable Metrics/AbcSize def check_resource_quota_capacity - return errors.empty? if early_return? + return true if early_return? - resource_quota.determine_utilization([self]) - unless resource_quota.missing_hosts.empty? - errors.add(:resource_quota, - N_(format('An error occured reading host resources. Check the foreman error log.'))) - return false - end - - verify_resource_quota_limits(resource_quota.utilization) - errors.empty? + quota_utilization = determine_quota_utilization + host_resources = determine_host_resources + verify_resource_quota_limits(quota_utilization, host_resources) + true + rescue Foreman::Exception => e + Rails.logger.error(N_(format('An error occured while checking the resource quota capacity: %s', e))) + errors.add(:resource_quota, e.bare_message) + false + rescue StandardError => e + Rails.logger.error(N_(format('An unknown error occured while checking the resource quota capacity: %s', e))) + errors.add(:base, e.message) + false end - # rubocop: enable Metrics/AbcSize private - def verify_resource_quota_limits(quota_utilization) - quota_utilization.each do |type, utilization| - next if utilization.nil? - max_quota = resource_quota[type] - next if utilization <= max_quota - errors.add(:resource_quota, N_(format("Host resources exceed quota '%s' for %s: %s > %s", - resource_quota_name, - natural_resource_name_by_type(type), - utilization, - max_quota))) - break + def verify_resource_quota_limits(quota_utilization, host_resources) + quota_utilization.each do |resource_type, resource_utilization| + next if resource_utilization.nil? + + max_quota = resource_quota[resource_type] + all_hosts_utilization = resource_utilization + host_resources[resource_type.to_sym] + next if all_hosts_utilization <= max_quota + + raise ResourceLimitException, formulate_limit_error(resource_utilization, + all_hosts_utilization, max_quota, resource_type) + end + end + + def determine_host_resources + (host_resources, missing_hosts) = utilization_from_resource_origins(resource_quota.active_resources, [self]) + unless missing_hosts.empty? + raise HostResourcesException, + "Cannot determine host resources for #{missing_hosts}" end + host_resources + end + + def determine_quota_utilization + resource_quota.determine_utilization + missing_hosts = resource_quota.missing_hosts + unless missing_hosts.empty? + raise UtilizationException, + "Resource Quota '#{resource_quota.name}' cannot determine resources for #{missing_hosts.size} hosts." + end + resource_quota.utilization + end + + def formulate_limit_error(resource_utilization, all_hosts_utilization, max_quota, resource_type) + error_text = if resource_utilization < max_quota + "Host exceeds %s limit of '%s'-quota by %s (max. %s)" + else + "%s limit of '%s'-quota exceeded by %s (max. %s)" + end + N_(format(error_text, natural_resource_name_by_type(resource_type), + resource_quota.name, + resource_value_to_string(all_hosts_utilization - max_quota, resource_type), + resource_value_to_string(max_quota, resource_type))) end def early_return? if resource_quota.nil? return true if quota_assigment_optional? - errors.add(:resource_quota, 'must be given.') - return true + raise HostResourceQuotaEmptyException, 'must be given.' end return true if Setting[:resource_quota_global_no_action] # quota is assigned, but not supposed to be checked false end def quota_assigment_optional? - owner.resource_quota_is_optional || owner.admin || Setting[:resource_quota_global_optional_user_assignment] + owner.resource_quota_is_optional || Setting[:resource_quota_optional_assignment] end end end diff --git a/app/models/foreman_resource_quota/resource_quota.rb b/app/models/foreman_resource_quota/resource_quota.rb index b07d72d..b17e816 100644 --- a/app/models/foreman_resource_quota/resource_quota.rb +++ b/app/models/foreman_resource_quota/resource_quota.rb @@ -38,7 +38,7 @@ def number_of_usergroups usergroups.size end - def determine_utilization(additional_hosts: []) + def determine_utilization(additional_hosts = []) quota_hosts = (hosts | (additional_hosts)) self.utilization, self.missing_hosts = call_utilization_helper(quota_hosts) @@ -52,13 +52,6 @@ def to_label name end - private - - # Wrap into a function for better testing - def call_utilization_helper(quota_hosts) - utilization_from_resource_origins(active_resources, quota_hosts) - end - def active_resources resources = [] %i[cpu_cores memory_mb disk_gb].each do |res| @@ -67,11 +60,18 @@ def active_resources resources end + private + + # Wrap into a function for better testing + def call_utilization_helper(quota_hosts) + utilization_from_resource_origins(active_resources, quota_hosts) + end + def print_warning(missing_hosts, hosts) - warn_text = "Could not determines resources for #{utilization_hash.missing_hosts.size} hosts:" + warn_text = "Could not determines resources for #{missing_hosts.size} hosts:" missing_hosts.each do |host_id, missing_resources| missing_host = hosts.find { |obj| obj.id == host_id } - warn_text << " '#{missing_host.name}': '#{missing_resources}'\n" + warn_text << " '#{missing_host.name}': '#{missing_resources}'\n" unless missing_host.nil? end Rails.logger.warn warn_text end diff --git a/app/services/foreman_resource_quota/resource_origin.rb b/app/services/foreman_resource_quota/resource_origin.rb index d82123c..99e7772 100644 --- a/app/services/foreman_resource_quota/resource_origin.rb +++ b/app/services/foreman_resource_quota/resource_origin.rb @@ -9,22 +9,23 @@ class ResourceOrigin disk_gb: :extract_disk_gb, }.freeze - def collect_resources!(resources_sum, missing_res_per_host) - return if missing_res_per_host.empty? + def collect_resources!(resources_sum, missing_hosts_resources, host_objects) + return if missing_hosts_resources.empty? - relevant_hosts = Host::Managed.where(id: missing_res_per_host.keys).includes(host_eager_name) - relevant_hosts = relevant_hosts.compact + relevant_hosts = load_hosts_eagerly(missing_hosts_resources, host_objects, host_attribute_eager_name) host_values = collect_attribute_from_hosts(relevant_hosts, host_attribute_name) - host_values.each do |host_id, attribute_content| - missing_res_per_host[host_id].reverse_each do |resource_name| - process_resource!(resources_sum, missing_res_per_host, resource_name, host_id, - attribute_content) + host_values.each do |host_name, attribute_content| + missing_hosts_resources[host_name].reverse_each do |resource_name| + resource_value = process_resource(resource_name, attribute_content) + next unless resource_value + resources_sum[resource_name] += resource_value + missing_hosts_resources[host_name].delete(resource_name) end - missing_res_per_host.delete(host_id) if missing_res_per_host[host_id].empty? + missing_hosts_resources.delete(host_name) if missing_hosts_resources[host_name].empty? end end - def host_eager_name + def host_attribute_eager_name raise NotImplementedError end @@ -46,23 +47,33 @@ def extract_disk_gb(param) private + def load_hosts_eagerly(missing_hosts_resources, host_objects, eager_attribute) + relevant_hosts = Host::Managed.where(name: missing_hosts_resources.keys).includes(eager_attribute) + relevant_hosts = relevant_hosts.compact + if relevant_hosts.size < missing_hosts_resources.size # Add non-eagerly loaded host objects + relevant_hosts_names = relevant_hosts.map(&:name) + (missing_hosts_resources.keys - relevant_hosts_names).each do |missing_host_name| + relevant_hosts << host_objects[missing_host_name] + end + end + relevant_hosts + end + def collect_attribute_from_hosts(host_list, attribute_name) - host_values = [] + host_values = {} host_list.each do |host| - host_attribute = host.send(attribute_name) - host_values << [host.id, host_attribute] if host_attribute.present? + attribute_value = host.send(attribute_name) + host_values[host.name] = attribute_value if attribute_value.present? rescue StandardError # skip hosts whose attribute couldn't be collected end host_values end - def process_resource!(resources_sum, missing_res_per_host, resource_name, host_id, attribute_content) + def process_resource(resource_name, attribute_content) resource_value = method(RESOURCE_FUNCTION_MAP[resource_name]).call(attribute_content) - return unless resource_value - - resources_sum[resource_name] += resource_value - missing_res_per_host[host_id].delete(resource_name) + return nil unless resource_value + resource_value end end end diff --git a/app/services/foreman_resource_quota/resource_origins/compute_attributes_origin.rb b/app/services/foreman_resource_quota/resource_origins/compute_attributes_origin.rb new file mode 100644 index 0000000..106f49c --- /dev/null +++ b/app/services/foreman_resource_quota/resource_origins/compute_attributes_origin.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module ForemanResourceQuota + module ResourceOrigin + class ComputeAttributesOrigin < ResourceOrigin + def host_attribute_eager_name + :compute_attributes + end + + def host_attribute_name + :compute_attributes + end + + def extract_cpu_cores(param) + param[:cpus].to_i + rescue StandardError + nil + end + + def extract_memory_mb(param) + param[:memory].to_i / ResourceQuotaHelper::FACTOR_B_TO_MB + rescue StandardError + nil + end + + def extract_disk_gb(param) + param[:volumes_attributes].values.sum { |disk| parse_storage_string(disk[:capacity]) } + rescue StandardError + nil + end + + private + + def parse_storage_string(storage_str) + case storage_str[-1].upcase + when 'G' + storage_str.to_i + when 'T' + storage_str[0..-2].to_i * ResourceQuotaHelper::FACTOR_B_TO_MB + when 'M' + (storage_str[0..-2].to_f / ResourceQuotaHelper::FACTOR_B_TO_MB).ceil + end + end + end + end +end diff --git a/app/services/foreman_resource_quota/resource_origins/compute_resource_origin.rb b/app/services/foreman_resource_quota/resource_origins/compute_resource_origin.rb index 203b511..2d80ae0 100644 --- a/app/services/foreman_resource_quota/resource_origins/compute_resource_origin.rb +++ b/app/services/foreman_resource_quota/resource_origins/compute_resource_origin.rb @@ -10,7 +10,7 @@ def extract_cpu_cores(param) end def extract_memory_mb(param) - param.memory.to_i / FACTOR_B_TO_MB + param.memory.to_i / ResourceQuotaHelper::FACTOR_B_TO_MB rescue StandardError nil end @@ -20,8 +20,8 @@ def extract_disk_gb(_) nil end - def collect_resources!(resources_sum, missing_res_per_host) - compute_resource_to_hosts = group_hosts_by_compute_resource(missing_res_per_host.keys) + def collect_resources!(resources_sum, missing_hosts_resources, _host_objects) + compute_resource_to_hosts = group_hosts_by_compute_resource(missing_hosts_resources.keys) compute_resource_to_hosts.each do |compute_resource_id, hosts| next if compute_resource_id == :nil_compute_resource @@ -30,27 +30,29 @@ def collect_resources!(resources_sum, missing_res_per_host) next if host_vms.empty? hosts.each do |host| - process_host_vm!(resources_sum, missing_res_per_host, host, host_vms, vm_id_attr) + vm = host_vms.find { |obj| obj.send(vm_id_attr) == host.uuid } + next unless vm + process_host_vm!(resources_sum, missing_hosts_resources, host.name, vm) end end end - def process_host_vm!(resources_sum, missing_res_per_host, host, host_vms, vm_id_attr) - vm = host_vms.find { |obj| obj.send(vm_id_attr) == host.uuid } - return unless vm - - missing_res_per_host[host.id].reverse_each do |resource_name| # reverse: delete items while iterating - process_resource!(resources_sum, missing_res_per_host, resource_name, host.id, vm) - end - missing_res_per_host.delete(host.id) if missing_res_per_host[host.id].empty? - end - - def group_hosts_by_compute_resource(host_ids) - Host::Managed.where(id: host_ids).includes(:compute_resource).group_by do |host| + # Groups hosts with the same compute resource. + # Parameters: An array of host names. + # Returns: A hash where keys represent compute resource IDs and values are the list of host objects. + def group_hosts_by_compute_resource(host_names) + Host::Managed.where(name: host_names).includes(:compute_resource).group_by do |host| host.compute_resource&.id || :nil_compute_resource end end + # Filters VMs of a compute resource, given a list of hosts. + # Parameters: + # - hosts: An array of host objects. + # - compute_resource_id: ID of the compute resource. + # Returns: + # - filtered_vms: An array of filtered virtual machine objects. + # - id_attr: Attribute used for identifying virtual machines (either :vmid or :id). def filter_vms_by_hosts(hosts, compute_resource_id) host_uuids = hosts.map(&:uuid) vms = ComputeResource.find_by(id: compute_resource_id).vms.all @@ -58,6 +60,23 @@ def filter_vms_by_hosts(hosts, compute_resource_id) filtered_vms = vms.select { |obj| host_uuids.include?(obj.send(id_attr)) } # reduce from all vms [filtered_vms, id_attr] end + + # Processes a host's virtual machines and updates resource allocation. + # Parameters: + # - resources_sum: Hash containing total resources sum. + # - missing_hosts_resources: Hash containing missing resources per host. + # - host_name: Name of the host. + # - vm: Compute resource VM object of given host. + # Returns: None. + def process_host_vm!(resources_sum, missing_hosts_resources, host_name, host_vm) + missing_hosts_resources[host_name].reverse_each do |resource_name| + resource_value = process_resource(resource_name, host_vm) + next unless resource_value + resources_sum[resource_name] += resource_value + missing_hosts_resources[host_name].delete(resource_name) + end + missing_hosts_resources.delete(host_name) if missing_hosts_resources[host_name].empty? + end end end end diff --git a/app/services/foreman_resource_quota/resource_origins/facts_origin.rb b/app/services/foreman_resource_quota/resource_origins/facts_origin.rb index 05c60e4..0511829 100644 --- a/app/services/foreman_resource_quota/resource_origins/facts_origin.rb +++ b/app/services/foreman_resource_quota/resource_origins/facts_origin.rb @@ -21,7 +21,7 @@ class FactsOrigin < ResourceOrigin /^facter_disks::(\w+)::size_bytes$/, ].freeze - def host_eager_name + def host_attribute_eager_name :fact_values end @@ -39,7 +39,7 @@ def extract_cpu_cores(param) def extract_memory_mb(param) common_keys = param.keys & FACTS_KEYS_MEMORY_B - return (param[common_keys.first].to_i / FACTOR_B_TO_MB).to_i if common_keys.any? + return (param[common_keys.first].to_i / ResourceQuotaHelper::FACTOR_B_TO_MB).to_i if common_keys.any? nil rescue StandardError nil @@ -61,7 +61,7 @@ def extract_disk_gb(param) end def sum_disk_space(facts, keys) - keys.map { |key| facts[key].to_i }.sum / FACTOR_B_TO_GB unless keys.empty? + keys.map { |key| facts[key].to_i }.sum / ResourceQuotaHelper::FACTOR_B_TO_GB unless keys.empty? end end end diff --git a/app/services/foreman_resource_quota/resource_origins/vm_attributes_origin.rb b/app/services/foreman_resource_quota/resource_origins/vm_attributes_origin.rb index b9e32d3..f520e0c 100644 --- a/app/services/foreman_resource_quota/resource_origins/vm_attributes_origin.rb +++ b/app/services/foreman_resource_quota/resource_origins/vm_attributes_origin.rb @@ -3,7 +3,7 @@ module ForemanResourceQuota module ResourceOrigin class VMAttributesOrigin < ResourceOrigin - def host_eager_name + def host_attribute_eager_name :compute_resource end diff --git a/lib/foreman_resource_quota/engine.rb b/lib/foreman_resource_quota/engine.rb index 4fbb909..f550c87 100644 --- a/lib/foreman_resource_quota/engine.rb +++ b/lib/foreman_resource_quota/engine.rb @@ -4,11 +4,12 @@ module ForemanResourceQuota class Engine < ::Rails::Engine engine_name 'foreman_resource_quota' - config.autoload_paths += Dir["#{config.root}/app/services/foreman_resource_quota"] - config.autoload_paths += Dir["#{config.root}/app/helpers/foreman_resource_quota"] - config.autoload_paths += Dir["#{config.root}/app/controllers/foreman_resource_quota"] config.autoload_paths += Dir["#{config.root}/app/models/"] - config.autoload_paths += Dir["#{config.root}/app/views/foreman_resource_quota"] + config.autoload_paths += Dir["#{config.root}/app/controllers/"] + config.autoload_paths += Dir["#{config.root}/app/views/"] + config.autoload_paths += Dir["#{config.root}/app/services/foreman_resource_quota/"] + config.autoload_paths += Dir["#{config.root}/app/helpers/foreman_resource_quota/"] + config.autoload_paths += Dir["#{config.root}/app/lib/foreman_resource_quota/"] # Add db migrations initializer 'foreman_resource_quota.load_app_instance_data' do |app| diff --git a/lib/foreman_resource_quota/register.rb b/lib/foreman_resource_quota/register.rb index 9b92fd4..0ab2621 100644 --- a/lib/foreman_resource_quota/register.rb +++ b/lib/foreman_resource_quota/register.rb @@ -65,14 +65,14 @@ # add UI user/usergroup/hosts extension extend_page 'users/_form' do |cx| - cx.add_pagelet :user_tabs, + cx.add_pagelet :main_tabs, id: :quota_user_tab, name: N_('Resource Quota'), resource_type: :user, partial: 'users/form_quota_tab' end extend_page 'usergroups/_form' do |cx| - cx.add_pagelet :usergroup_tabs, + cx.add_pagelet :main_tabs, id: :quota_usergroup_tab, name: N_('Resource Quota'), resource_type: :usergroup, @@ -88,12 +88,12 @@ # Add global Foreman settings settings do category :provisioning do - setting 'resource_quota_global_optional_user_assignment', + setting 'resource_quota_optional_assignment', type: :boolean, - default: true, - full_name: N_('Global Resource Quota optional assignment'), - description: N_('Overwrite user-specific configurations and make the Resource Quota assignment - during host deployment optional for everyone.') + default: false, + full_name: N_('Resource Quota optional assignment'), + description: N_('Make the assignment of a Resource quota, during the host creation process, optional for everyone. + If this is true, user-specific "optional assignment" configurations are neglected.') setting 'resource_quota_global_no_action', type: :boolean, default: true, diff --git a/test/factories/host_resource_quota_extensions.rb b/test/factories/host_resource_quota_extensions.rb new file mode 100644 index 0000000..f06c9dd --- /dev/null +++ b/test/factories/host_resource_quota_extensions.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.modify do + factory :host do + trait :with_resource_quota do + resource_quota { FactoryBot.create(:resource_quota) } + end + end +end diff --git a/test/helpers/hosts_helper_test.rb b/test/helpers/hosts_helper_test.rb index 40af056..90d6919 100644 --- a/test/helpers/hosts_helper_test.rb +++ b/test/helpers/hosts_helper_test.rb @@ -7,22 +7,28 @@ class HostsHelperTest < ActionView::TestCase include ::FormHelper include ForemanResourceQuota::HostsHelper - test 'host edit page form' do - @host = FactoryBot.create :host - quotas = [] - quotas << (FactoryBot.create :resource_quota) - quotas << (FactoryBot.create :resource_quota) - quotas << (FactoryBot.create :resource_quota) - as_admin { quotas.each(&:save!) } + context 'optional quota assignment at host creation' do + def setup + Setting[:resource_quota_optional_assignment] = true + end + + test 'host edit page form' do + @host = FactoryBot.create :host + quotas = [] + quotas << (FactoryBot.create :resource_quota) + quotas << (FactoryBot.create :resource_quota) + quotas << (FactoryBot.create :resource_quota) + as_admin { quotas.each(&:save!) } - form = '' - as_admin do - form = form_for(@host) do |f| - resource_quota_select(f, ResourceQuota.all) + form = '' + as_admin do + form = form_for(@host) do |f| + resource_quota_select(f, ResourceQuota.all) + end + end + quotas.each do |quota| + assert form[quota.name] end - end - quotas.each do |quota| - assert form[quota.name] end end end diff --git a/test/helpers/resource_quota_helper_test.rb b/test/helpers/resource_quota_helper_test.rb index 98a20c7..94b18e4 100644 --- a/test/helpers/resource_quota_helper_test.rb +++ b/test/helpers/resource_quota_helper_test.rb @@ -6,20 +6,227 @@ module ForemanResourceQuota class ResourceQuotaHelperTest < ActiveSupport::TestCase include ForemanResourceQuota::ResourceQuotaHelper - test 'resource quota natural name by type' do - assert_equal 'CPU cores', natural_resource_name_by_type(:cpu_cores) - assert_equal 'Memory (MB)', natural_resource_name_by_type(:memory_mb) - assert_equal 'Disk space (GB)', natural_resource_name_by_type(:disk_gb) + describe 'natural resource by type' do + test 'CPU type' do + assert_equal 'CPU cores', natural_resource_name_by_type(:cpu_cores) + end + + test 'memory type' do + assert_equal 'Memory', natural_resource_name_by_type(:memory_mb) + end + + test 'disk space type' do + assert_equal 'Disk space', natural_resource_name_by_type(:disk_gb) + end + + test 'raises error for unknown type' do + assert_raises Exception do + natural_resource_name_by_type(:disk_space) + end + end + end + + describe 'units by type' do + test 'CPU type' do + units = [{ symbol: 'cores', factor: 1 }] + assert_equal units, units_by_type(:cpu_cores) + end + + test 'memory type' do + units = [ + { symbol: 'MB', factor: 1 }, + { symbol: 'GB', factor: FACTOR_B_TO_KB }, + { symbol: 'TB', factor: FACTOR_B_TO_MB }, + ] + assert_equal units, units_by_type(:memory_mb) + end + + test 'disk space type' do + units = [ + { symbol: 'GB', factor: 1 }, + { symbol: 'TB', factor: FACTOR_B_TO_KB }, + { symbol: 'PB', factor: FACTOR_B_TO_MB }, + ] + assert_equal units, units_by_type(:disk_gb) + end + + test 'raises error for unknown type' do + assert_raises Exception do + units_by_type(:disk_space) + end + end + end + + context 'optional quota assignment at host creation' do + def setup + Setting[:resource_quota_optional_assignment] = true + end + + test 'builds missing resource per host list' do + hosts = [] + hosts << (FactoryBot.create :host) + hosts << (FactoryBot.create :host) + quota_utilization = %i[cpu_cores memory_mb disk_gb] + missing_host_res = create_missing_hosts_resources_hash(hosts, quota_utilization) + assert_equal quota_utilization, missing_host_res[hosts[0].name] + assert_equal quota_utilization, missing_host_res[hosts[1].name] + end + end + + describe 'resource to unit string' do + test 'CPU Cores to resource string' do + assert_equal '123 cores', resource_value_to_string(123, :cpu_cores) + end + + test 'CPU Cores to resource string (float)' do + assert_equal '123.5 cores', resource_value_to_string(123.5, :cpu_cores) + end + + test 'CPU Cores to resource string (large)' do + assert_equal '51239 cores', resource_value_to_string(51_239, :cpu_cores) + end + + test 'memory to resource string' do + assert_equal '1023 MB', resource_value_to_string(1023, :memory_mb) + end + + test 'memory to resource string (float)' do + assert_equal '1023.5 MB', resource_value_to_string(1023.5, :memory_mb) + end + + test 'memory to resource string (GB)' do + assert_equal '5 GB', resource_value_to_string((1024 * 5.3).round, :memory_mb) + end + + test 'memory to resource string (TB)' do + assert_equal '123 TB', resource_value_to_string(1024 * 1024 * 123, :memory_mb) + end + + test 'disk space to resource string' do + assert_equal '5 GB', resource_value_to_string(5, :disk_gb) + end + + test 'disk space to resource string (float)' do + assert_equal '5.5 GB', resource_value_to_string(5.5, :disk_gb) + end + + test 'disk space to resource string (TB)' do + assert_equal '532 TB', resource_value_to_string(1024 * 532, :disk_gb) + end + + test 'disk space to resource string (PB)' do + assert_equal '823 PB', resource_value_to_string(1024 * 1024 * 823, :disk_gb) + end end - test 'builds missing resource per host list' do - hosts = [] - hosts << (FactoryBot.create :host) - hosts << (FactoryBot.create :host) - quota_utilization = %i[cpu_cores memory_mb disk_gb] - missing_host_res = build_missing_resources_per_host_list(hosts, quota_utilization) - assert_equal quota_utilization, missing_host_res[hosts[0].id] - assert_equal quota_utilization, missing_host_res[hosts[1].id] + describe 'find largest unit' do + test 'find largest unit for CPU Cores' do + (symbol, factor) = find_largest_unit(19, units_by_type(:cpu_cores)) + assert_equal 'cores', symbol + assert_equal 1, factor + end + + test 'find largest unit for CPU Cores (large)' do + (symbol, factor) = find_largest_unit(512_301, units_by_type(:cpu_cores)) + assert_equal 'cores', symbol + assert_equal 1, factor + end + + test 'find largest unit for memory MB' do + (symbol, factor) = find_largest_unit(50, units_by_type(:memory_mb)) + assert_equal 'MB', symbol + assert_equal 1, factor + end + + test 'find largest unit for memory MB (large)' do + (symbol, factor) = find_largest_unit(1023, units_by_type(:memory_mb)) + assert_equal 'MB', symbol + assert_equal 1, factor + end + + test 'find largest unit for memory GB' do + (symbol, factor) = find_largest_unit(1024, units_by_type(:memory_mb)) + assert_equal 'GB', symbol + assert_equal 1024, factor + end + + test 'find largest unit for memory GB (mid)' do + (symbol, factor) = find_largest_unit(39 * 1024, units_by_type(:memory_mb)) + assert_equal 'GB', symbol + assert_equal 1024, factor + end + + test 'find largest unit for memory GB (large)' do + (symbol, factor) = find_largest_unit((1024 * 1024) - 1, units_by_type(:memory_mb)) + assert_equal 'GB', symbol + assert_equal 1024, factor + end + + test 'find largest unit for memory TB' do + (symbol, factor) = find_largest_unit(1024 * 1024, units_by_type(:memory_mb)) + assert_equal 'TB', symbol + assert_equal 1024 * 1024, factor + end + + test 'find largest unit for memory TB (mid)' do + (symbol, factor) = find_largest_unit(1024 * 1024 * 49, units_by_type(:memory_mb)) + assert_equal 'TB', symbol + assert_equal 1024 * 1024, factor + end + + test 'find largest unit for memory TB (large)' do + (symbol, factor) = find_largest_unit(1024 * 1024 * 1026, units_by_type(:memory_mb)) + assert_equal 'TB', symbol + assert_equal 1024 * 1024, factor + end + + test 'find largest unit for disk space GB' do + (symbol, factor) = find_largest_unit(5, units_by_type(:disk_gb)) + assert_equal 'GB', symbol + assert_equal 1, factor + end + + test 'find largest unit for disk space GB (large)' do + (symbol, factor) = find_largest_unit(1023, units_by_type(:disk_gb)) + assert_equal 'GB', symbol + assert_equal 1, factor + end + + test 'find largest unit for disk space TB' do + (symbol, factor) = find_largest_unit(1024, units_by_type(:disk_gb)) + assert_equal 'TB', symbol + assert_equal 1024, factor + end + + test 'find largest unit for disk space TB (mid)' do + (symbol, factor) = find_largest_unit(1024 * 59, units_by_type(:disk_gb)) + assert_equal 'TB', symbol + assert_equal 1024, factor + end + + test 'find largest unit for disk space TB (large)' do + (symbol, factor) = find_largest_unit((1024 * 1024) - 1, units_by_type(:disk_gb)) + assert_equal 'TB', symbol + assert_equal 1024, factor + end + + test 'find largest unit for disk space PB' do + (symbol, factor) = find_largest_unit(1024 * 1024, units_by_type(:disk_gb)) + assert_equal 'PB', symbol + assert_equal 1024 * 1024, factor + end + + test 'find largest unit for disk space PB (mid)' do + (symbol, factor) = find_largest_unit(1024 * 1024 * 98, units_by_type(:disk_gb)) + assert_equal 'PB', symbol + assert_equal 1024 * 1024, factor + end + + test 'find largest unit for disk space PB (large)' do + (symbol, factor) = find_largest_unit(1024 * 1024 * 1025, units_by_type(:disk_gb)) + assert_equal 'PB', symbol + assert_equal 1024 * 1024, factor + end end end end diff --git a/test/models/concerns/host_managed_extension_test.rb b/test/models/concerns/host_managed_extension_test.rb new file mode 100644 index 0000000..7e81f84 --- /dev/null +++ b/test/models/concerns/host_managed_extension_test.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'test_plugin_helper' + +module ForemanResourceQuota + class HostManagedExtensionTest < ActiveSupport::TestCase + include ForemanResourceQuota::Exceptions + + describe 'host create validation' do + test 'should succeed with resource quota' do + assert FactoryBot.create(:host, :with_resource_quota) + end + + test 'should fail without resource quota' do + assert_raises(ActiveRecord::RecordInvalid) { FactoryBot.create(:host) } + end + end + + describe 'resource quota capacity' do + def setup + @host = FactoryBot.create(:host, :with_resource_quota) + @quota = @host.resource_quota + end + + test 'should validate empty quota' do + assert @host.check_resource_quota_capacity + end + + test 'should fail determining host resources' do + @quota.update cpu_cores: 1 # TODO: Here, check why the host resources can be determined? + assert_raises(ActiveRecord::RecordInvalid) { @host.check_resource_quota_capacity } + end + + test 'single host invalid capacity' do + # TODO: Write further tests + assert_equal 'CPU cores', 1 + end + test 'multi host valid capacity' do + assert_equal 'CPU cores', 1 + end + test 'multi host invalid capacity' do + assert_equal 'CPU cores', 1 + end + end + end +end diff --git a/test/models/resource_quota_test.rb b/test/models/resource_quota_test.rb index c584e57..4032cec 100644 --- a/test/models/resource_quota_test.rb +++ b/test/models/resource_quota_test.rb @@ -4,62 +4,66 @@ module ForemanResourceQuota class ResourceQuotaTest < ActiveSupport::TestCase - def setup - @quota = FactoryBot.create :resource_quota - @user = FactoryBot.create :user - @usergroup = FactoryBot.create :usergroup - @host = FactoryBot.create :host - end + context 'optional quota assignment at host creation' do + def setup + Setting[:resource_quota_optional_assignment] = true - test 'hosts relation' do - @quota.hosts << @host - as_admin { @quota.save! } - assert_equal @host.id, @quota.hosts[0].id - assert_equal @quota.id, @host.resource_quota.id - end - test 'users relation' do - @quota.users << @user - as_admin { @quota.save! } - assert_equal @user.id, @quota.users[0].id - assert_equal @quota.id, @user.resource_quotas[0].id - end - test 'usergroups relation' do - @quota.usergroups << @usergroup - as_admin { @quota.save! } - assert_equal @usergroup.id, @quota.usergroups[0].id - assert_equal @quota.id, @usergroup.resource_quotas[0].id - end + @quota = FactoryBot.create :resource_quota + @user = FactoryBot.create :user + @usergroup = FactoryBot.create :usergroup + @host = FactoryBot.create :host + end - test 'number of hosts' do - second_host = FactoryBot.create :host - third_host = FactoryBot.create :host - @quota.hosts << [@host, second_host, third_host] - assert_equal 3, @quota.number_of_hosts - end - test 'number of users' do - second_user = FactoryBot.create :user - third_user = FactoryBot.create :user - @quota.users << [@user, second_user, third_user] - assert_equal 3, @quota.number_of_users - end - test 'number of usergroups' do - second_usergroup = FactoryBot.create :usergroup - third_usergroup = FactoryBot.create :usergroup - @quota.usergroups << [@usergroup, second_usergroup, third_usergroup] - assert_equal 3, @quota.number_of_usergroups - end + test 'hosts relation' do + @quota.hosts << @host + as_admin { @quota.save! } + assert_equal @host.id, @quota.hosts[0].id + assert_equal @quota.id, @host.resource_quota.id + end + test 'users relation' do + @quota.users << @user + as_admin { @quota.save! } + assert_equal @user.id, @quota.users[0].id + assert_equal @quota.id, @user.resource_quotas[0].id + end + test 'usergroups relation' do + @quota.usergroups << @usergroup + as_admin { @quota.save! } + assert_equal @usergroup.id, @quota.usergroups[0].id + assert_equal @quota.id, @usergroup.resource_quotas[0].id + end + + test 'number of hosts' do + second_host = FactoryBot.create :host + third_host = FactoryBot.create :host + @quota.hosts << [@host, second_host, third_host] + assert_equal 3, @quota.number_of_hosts + end + test 'number of users' do + second_user = FactoryBot.create :user + third_user = FactoryBot.create :user + @quota.users << [@user, second_user, third_user] + assert_equal 3, @quota.number_of_users + end + test 'number of usergroups' do + second_usergroup = FactoryBot.create :usergroup + third_usergroup = FactoryBot.create :usergroup + @quota.usergroups << [@usergroup, second_usergroup, third_usergroup] + assert_equal 3, @quota.number_of_usergroups + end - test 'determine utilization' do - exp_utilization = { cpu_cores: 1, memory_mb: 1, disk_gb: 2 } - exp_missing_hosts = [] - @quota.hosts << @host - as_admin { @quota.save! } + test 'determine utilization' do + exp_utilization = { cpu_cores: 1, memory_mb: 1, disk_gb: 2 } + exp_missing_hosts = [] + @quota.hosts << @host + as_admin { @quota.save! } - @quota.stub(:call_utilization_helper, [exp_utilization, exp_missing_hosts]) do - @quota.determine_utilization + @quota.stub(:call_utilization_helper, [exp_utilization, exp_missing_hosts]) do + @quota.determine_utilization + end + assert_equal exp_utilization.transform_keys(&:to_s), @quota.utilization + assert_equal exp_missing_hosts, @quota.missing_hosts end - assert_equal exp_utilization.transform_keys(&:to_s), @quota.utilization - assert_equal exp_missing_hosts, @quota.missing_hosts end end end diff --git a/test/unit/resource_quota_test.rb b/test/unit/resource_quota_test.rb index 60dcedb..2c6e19f 100644 --- a/test/unit/resource_quota_test.rb +++ b/test/unit/resource_quota_test.rb @@ -4,6 +4,6 @@ module ForemanResourceQuota class ResourceQuotaTest < ActiveSupport::TestCase - # TODO: Add tests here + # TODO: Remove test/unit folder (https://guides.rubyonrails.org/testing.html#rails-sets-up-for-testing-from-the-word-go) end end diff --git a/webpack/helper.js b/webpack/helper.js index 0e2088c..63d07b4 100644 --- a/webpack/helper.js +++ b/webpack/helper.js @@ -56,15 +56,17 @@ const areReactElementsEqual = (element1, element2) => { * @returns {void} - The function modifies the destination hash in place. */ function deepCopy(dest, src) { - Object.keys(dest).forEach(key => { - if (src.hasOwnProperty(key)) { - if (typeof dest[key] === 'object' && typeof src[key] === 'object') { - deepCopy(dest[key], src[key]); - } else if (dest[key] !== src[key]) { - dest[key] = src[key]; + if (dest) { + Object.keys(dest).forEach(key => { + if (src.hasOwnProperty(key)) { + if (typeof dest[key] === 'object' && typeof src[key] === 'object') { + deepCopy(dest[key], src[key]); + } else if (dest[key] !== src[key]) { + dest[key] = src[key]; + } } - } - }); + }); + } } /**