Implement optimistic resource quota utilization computation

.rubocop.yml

@@ -18,6 +18,10 @@ Metrics/ClassLength:
   Exclude:
     - 'app/models/foreman_resource_quota/resource_quota.rb'
 
+Metrics/ModuleLength:
+  Exclude:
+    - 'app/models/concerns/foreman_resource_quota/host_managed_extensions.rb'
+
 Metrics/MethodLength:
   Enabled: false
 

app/controllers/foreman_resource_quota/api/v2/resource_quotas_controller.rb

@@ -64,7 +64,6 @@ def usergroups
         def_param_group :resource_quota do
           param :resource_quota, Hash, required: true, action_aware: true do
             param :name, String, required: true
-            # param :operatingsystem_ids, Array, :desc => N_("Operating system IDs")
           end
         end
 

app/helpers/foreman_resource_quota/resource_quota_helper.rb

@@ -52,20 +52,29 @@ def resource_value_to_string(resource_value, resource_type)
       format(format_text, unit_applied_value, symbol)
     end
 
+    # Use different resource origins to determine host resource utilization.
+    #  - iterates all given hosts and tries do determine their resources utilization
+    # Returns:
+    #   [ <hosts_resources>, <missing_hosts_resources> ]
+    #   for example:
+    #     [
+    #       { "host_a": { cpu_cores: 20, memory_mb: 8196 }, "host_b": { cpu_cores: 15, memory_mb: nil } },
+    #       { "host_c": [ :memory_mb ] },
+    #     ]
     def utilization_from_resource_origins(resources, hosts, custom_resource_origins: nil)
-      utilization_sum = resources.each.with_object({}) { |key, hash| hash[key] = 0 }
+      hosts_resources = create_hosts_resources_hash(hosts, resources)
       missing_hosts_resources = create_missing_hosts_resources_hash(hosts, resources)
       hosts_hash = hosts.index_by(&:name)
       resource_classes = custom_resource_origins || default_resource_origin_classes
       resource_classes.each do |origin_class|
         origin_class.new.collect_resources!(
-          utilization_sum,
+          hosts_resources,
           missing_hosts_resources,
           hosts_hash
         )
       end
 
-      [utilization_sum, missing_hosts_resources]
+      [hosts_resources, missing_hosts_resources]
     end
 
     private
@@ -89,6 +98,25 @@ def create_missing_hosts_resources_hash(hosts, resources)
       hosts.map(&:name).index_with { resources_to_determine.clone }
     end
 
+    # Create a Hash that maps resources and a value to host names.
+    # { <host name>: {<hash of resource values>} }
+    #     for example:
+    #     {
+    #       "host_a": { cpu_cores: nil, disk_gb: nil },
+    #       "host_b": { cpu_cores: nil, disk_gb: nil },
+    #     }
+    # Parameters:
+    #   - hosts: Array of host objects.
+    #   - resources: Array of resources (as symbol, e.g. [:cpu_cores, :disk_gb]).
+    # Returns: Hash with host names as keys and resource-hashs as values.
+    def create_hosts_resources_hash(hosts, resources)
+      return {} if hosts.empty? || resources.empty?
+
+      # Create a hash template with resources mapped to nil
+      resources_to_determine = resources.index_with { |_resource| nil }
+      hosts.map(&:name).index_with { resources_to_determine.dup }
+    end
+
     # Default classes that are used to determine host resources. Determines
     # resources in the order of this list.
     def default_resource_origin_classes

app/models/concerns/foreman_resource_quota/host_managed_extensions.rb

@@ -7,16 +7,22 @@ module HostManagedExtensions
     include ForemanResourceQuota::Exceptions
 
     included do
-      validate :check_resource_quota_capacity
-
-      belongs_to :resource_quota, class_name: '::ForemanResourceQuota::ResourceQuota'
-      has_one :resource_quota_missing_resources, class_name: '::ForemanResourceQuota::ResourceQuotaMissingHost',
-        inverse_of: :missing_host, foreign_key: :missing_host_id, dependent: :destroy
+      validate :verify_resource_quota
+
+      has_one :host_resources, class_name: '::ForemanResourceQuota::HostResources',
+        inverse_of: :host, foreign_key: :host_id, dependent: :destroy
+      has_one :resource_quota_host, class_name: '::ForemanResourceQuota::ResourceQuotaHost',
+        inverse_of: :host, foreign_key: :host_id, dependent: :destroy
+      has_one :resource_quota, class_name: '::ForemanResourceQuota::ResourceQuota',
+        through: :resource_quota_host
       scoped_search relation: :resource_quota, on: :name, complete_value: true, rename: :resource_quota
+
+      # A host shall always have a .host_resources attribute
+      before_validation :build_host_resources, unless: -> { host_resources.present? }
     end
 
-    def check_resource_quota_capacity
-      handle_quota_check
+    def verify_resource_quota
+      handle_quota_check(resource_quota)
       true
     rescue ResourceQuotaException => e
       handle_error('resource_quota_id',
@@ -32,13 +38,27 @@ def check_resource_quota_capacity
         format('An unknown error occured while checking the resource quota capacity: %s', e))
     end
 
+    def resource_quota_id
+      resource_quota&.id
+    end
+
+    def resource_quota_id=(val)
+      if val.blank?
+        resource_quota_host&.destroy
+      else
+        quota = ForemanResourceQuota::ResourceQuota.find_by(id: val)
+        raise ActiveRecord::RecordNotFound, "ResourceQuota with ID \"#{val}\" not found" unless quota
+        self.resource_quota = quota
+      end
+    end
+
     private
 
-    def handle_quota_check
-      return if early_return?
-      quota_utilization = determine_quota_utilization
-      host_resources = determine_host_resources
-      verify_resource_quota_limits(quota_utilization, host_resources)
+    def handle_quota_check(quota)
+      return if early_return?(quota)
+      quota_utilization = determine_quota_utilization(quota)
+      current_host_resources = determine_host_resources(quota.active_resources)
+      check_resource_quota_limits(quota, quota_utilization, current_host_resources)
     end
 
     def handle_error(error_module, error_message, log_message)
@@ -47,60 +67,72 @@ def handle_error(error_module, error_message, log_message)
       false
     end
 
-    def determine_quota_utilization
-      resource_quota.determine_utilization
-      missing_hosts = resource_quota.missing_hosts
+    def determine_quota_utilization(quota)
+      missing_hosts = quota.missing_hosts(exclude: [name])
       unless missing_hosts.empty?
         raise ResourceQuotaUtilizationException,
-          "Resource Quota '#{resource_quota.name}' cannot determine resources for #{missing_hosts.size} hosts."
+          "Resource Quota '#{quota.name}' cannot determine resources for #{missing_hosts.size} hosts."
       end
-      resource_quota.utilization
+      quota.utilization(exclude: [name])
     end
 
-    def determine_host_resources
-      (host_resources, missing_hosts) = call_utilization_helper(resource_quota.active_resources, [self])
-      unless missing_hosts.empty?
+    def determine_host_resources(active_resources)
+      new_host_resources, missing_hosts = call_utilization_helper(active_resources, [self])
+      if missing_hosts.key?(name) || missing_hosts.key?(name.to_sym)
         raise HostResourcesException,
-          "Cannot determine host resources for #{name}"
+          "Cannot determine host resources for #{name}: #{missing_hosts[name]}"
       end
-      host_resources
+      host_resources.resources = new_host_resources
+      host_resources.resources
     end
 
-    def verify_resource_quota_limits(quota_utilization, host_resources)
+    def check_resource_quota_limits(quota, quota_utilization, current_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]
+        max_quota = quota[resource_type]
+        all_hosts_utilization = resource_utilization + current_host_resources[resource_type.to_sym]
         next if all_hosts_utilization <= max_quota
 
-        raise ResourceLimitException, formulate_limit_error(resource_utilization,
+        raise ResourceLimitException, formulate_limit_error(quota.name, resource_utilization,
           all_hosts_utilization, max_quota, resource_type)
       end
     end
 
-    def formulate_limit_error(resource_utilization, all_hosts_utilization, max_quota, resource_type)
-      if resource_utilization < max_quota
+    def formulate_limit_error(quota_name, resource_utilization, all_hosts_utilization, max_quota, resource_type)
+      if resource_utilization <= max_quota
         N_(format("Host exceeds %s limit of '%s'-quota by %s (max. %s)",
           natural_resource_name_by_type(resource_type),
-          resource_quota.name,
+          quota_name,
           resource_value_to_string(all_hosts_utilization - max_quota, resource_type),
           resource_value_to_string(max_quota, resource_type)))
       else
         N_(format("%s limit of '%s'-quota is already exceeded by %s without adding the new host (max. %s)",
           natural_resource_name_by_type(resource_type),
-          resource_quota.name,
+          quota_name,
           resource_value_to_string(resource_utilization - max_quota, resource_type),
           resource_value_to_string(max_quota, resource_type)))
       end
     end
 
-    def early_return?
-      if resource_quota.nil?
+    def formulate_resource_inconsistency_error(quota_name, resource_type, quota_utilization_value, resource_value)
+      N_("Resource Quota '#{quota_name}' inconsistency detected while destroying host '#{name}':\n" \
+         "Resource Quota #{resource_type} current utilization: #{quota_utilization_value}.\n" \
+         "Host resource value: #{resource_value}.\n" \
+         'Skipping.')
+    end
+
+    def formulate_quota_inconsistency_error(quota_name)
+      N_("An error occured adapting the resource quota utilization of '#{quota_name}' " \
+         "while processing host '#{name}'. The resource quota utilization values might be inconsistent.")
+    end
+
+    def early_return?(quota)
+      if quota.nil?
         return true if quota_assigment_optional?
         raise HostResourceQuotaEmptyException, 'must be given.'
       end
-      return true if resource_quota.active_resources.empty?
+      return true if quota.active_resources.empty?
       return true if Setting[:resource_quota_global_no_action] # quota is assigned, but not supposed to be checked
       false
     end
@@ -111,7 +143,13 @@ def quota_assigment_optional?
 
     # Wrap into a function for easier testing
     def call_utilization_helper(resources, hosts)
-      utilization_from_resource_origins(resources, hosts)
+      all_host_resources, missing_hosts = utilization_from_resource_origins(resources, hosts)
+      unless all_host_resources.key?(name)
+        raise HostResourcesException,
+          "Host #{name} was not included when determining host resources."
+      end
+      current_host_resources = all_host_resources[name]
+      [current_host_resources, missing_hosts]
     end
   end
 end

app/models/foreman_resource_quota/host_resources.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+module ForemanResourceQuota
+  class HostResources < ApplicationRecord
+    self.table_name = 'hosts_resources'
+
+    belongs_to :host, class_name: '::Host::Managed'
+    validates :host, { presence: true, uniqueness: true }
+
+    def resources
+      {
+        cpu_cores: cpu_cores,
+        memory_mb: memory_mb,
+        disk_gb: disk_gb,
+      }
+    end
+
+    def resources=(val)
+      allowed_attributes = val.slice(:cpu_cores, :memory_mb, :disk_gb)
+      assign_attributes(allowed_attributes) # Set multiple attributes at once (given a hash)
+    end
+
+    # Returns an array of unknown host resources (returns an empty array if all are known)
+    # For example, completely unknown host resources returns:
+    #   [
+    #     :cpu_cores,
+    #     :memory_mb,
+    #     :disk_gb,
+    #   ]
+    # Consider only the resource_quota's active resources by default.
+    def missing_resources(only_active_resources: true)
+      empty_resources = []
+      resources_to_check = %i[cpu_cores memory_mb disk_gb]
+      resources_to_check = host.resource_quota.active_resources if only_active_resources && host.resource_quota.present?
+
+      resources_to_check.each do |single_resource|
+        empty_resources << single_resource if send(single_resource).nil?
+      end
+
+      empty_resources
+    end
+  end
+end