Wip: Resolve hosts relation

app/models/concerns/foreman_resource_quota/host_managed_extensions.rb

@@ -12,8 +12,10 @@ module HostManagedExtensions
       has_one :host_resources, class_name: '::ForemanResourceQuota::HostResources',
         inverse_of: :host, foreign_key: :host_id, dependent: :destroy
       has_one :resource_quota, class_name: '::ForemanResourceQuota::ResourceQuota',
-        through: :host_resources
+        through: :host_resources, foreign_key: :resource_quota_id
       scoped_search relation: :resource_quota, on: :name, complete_value: true, rename: :resource_quota
+
+      after_save :save_host_resources
     end
 
     def verify_resource_quota
@@ -40,11 +42,13 @@ def host_resources
 
     def resource_quota=(val)
       if val.is_a?(ForemanResourceQuota::ResourceQuota)
+        # self.host_resources.resource_quota = val
         super(val)
       else
-        resource_quota = ForemanResourceQuota::ResourceQuota.find_by(id: val)
-        if resource_quota
-          super(resource_quota)
+        quota = ForemanResourceQuota::ResourceQuota.find_by(id: val)
+        if quota
+          # self.host_resources.resource_quota = quota
+          super(quota)
         else
           errors.add(:resource_quota, "not found for ID: #{val}")
         end
@@ -76,20 +80,12 @@ def determine_quota_utilization(quota)
     end
 
     def determine_host_resources(active_resources)
-      # TODO: Evaluate, check host resources or just rely on host.host_resources?
-      #   - Problem:
-      #     on every save, the host resources are determined which makes the process slower
-      #   - Solution:
-      #     Check if resources exist, only determine if they don't
-      #   - Potential Issue:
-      #     A user adds another hard disk and that is not "recognized", just when re-calculating the whole quota resources
-      all_host_resources, missing_hosts = call_utilization_helper(active_resources, [self])
-      current_host_resources = all_host_resources[self.name]
-      if missing_hosts.has_key?(self.name)
+      new_host_resources, missing_hosts = call_utilization_helper(active_resources, [self])
+      if missing_hosts.has_key?(self.name) or missing_hosts.has_key?(self.name.to_sym)
         raise HostResourcesException,
           "Cannot determine host resources for #{name}: #{missing_hosts[name]}"
       end
-      self.host_resources.resources = current_host_resources
+      self.host_resources.resources = new_host_resources
       self.host_resources.resources
     end
 
@@ -150,56 +146,18 @@ def quota_assigment_optional?
 
     # Wrap into a function for easier testing
     def call_utilization_helper(resources, hosts)
-      utilization_from_resource_origins(resources, hosts)
-    end
-
-    # TODO: Might not be necessary anymore?
-    def add_host_capacity_to_quota(quota)
-      return if quota.nil?
-
-      update_quota_with_host_resources(quota) do |quota_resource_utilization, resource_value, _|
-        quota_resource_utilization + resource_value
-      end
-    end
-
-    # TODO: Might not be necessary anymore?
-    def remove_host_capacity_from_quota(quota)
-      return if quota.nil?
-
-      update_quota_with_host_resources(quota) do |quota_resource_utilization, resource_value, resource_type|
-        quota_resource_utilization - helper_resource_value_subtraction(
-          quota.name,
-          resource_value,
-          quota_resource_utilization,
-          resource_type
-        )
-      end
-    end
-
-    # TODO: Might not be necessary anymore?
-    def update_quota_with_host_resources(quota)
-      host_resources = determine_host_resources(quota.active_resources)
-      new_utilization = quota.utilization
-
-      host_resources.each do |resource_type, resource_value|
-        new_utilization[resource_type] ||= 0
-        new_utilization[resource_type] = yield(new_utilization[resource_type], resource_value, resource_type)
+      all_host_resources, missing_hosts = utilization_from_resource_origins(resources, hosts)
+      if not all_host_resources.has_key?(self.name)
+        raise HostResourcesException,
+          "Host #{name} was not included when determining host resources."
       end
-
-      quota.utilization = new_utilization
-    rescue ResourceQuotaException => e
-      Rails.logger.warn("#{formulate_quota_inconsistency_error(quota.name)}\n#{e.bare_message}")
+      current_host_resources = all_host_resources[self.name]
+      return current_host_resources, missing_hosts
     end
 
-    # TODO: Might not be necessary anymore?
-    def helper_resource_value_subtraction(quota_name, host_resource_value, quota_value, resource_type)
-      return host_resource_value if quota_value >= host_resource_value
-
-      # Log inconsistency warning and don't subtract anything from quota utilization value
-      Rails.logger.warn(formulate_quota_inconsistency_error(quota_name, resource_type,
-        new_utilization[resource_type],
-        host_resource_value))
-      0
+    def save_host_resources
+      host_resources_item = host_resources
+      host_resources_item.save if host_resources_item.changed?
     end
   end
 end

app/models/foreman_resource_quota/host_resources.rb

@@ -5,7 +5,10 @@ class HostResources < ApplicationRecord
     self.table_name = 'hosts_resources'
 
     belongs_to :resource_quota, class_name: 'ResourceQuota'
-    belongs_to :host, class_name: '::Host::Managed'
+    belongs_to :host, class_name: '::Host::Managed', unqiue: true
+    // Talk to Manisha regarding mico-service SQL relation
+    -> For some reasion, when adding a host like my_quota.hosts << [some_host_a, some_host_b] results in NEW HostResources entries
+    -> See the test on the right
 
     def resources
       {
@@ -17,7 +20,7 @@ def resources
 
     def resources=(val)
       allowed_attributes = val.slice(:cpu_cores, :memory_mb, :disk_gb)
-      update(allowed_attributes)
+      assign_attributes(allowed_attributes) # Set multiple attributes at once (given a hash)
     end
 
     # Return a Array of unknown host resources (returns an empty Array if all are known)

app/models/foreman_resource_quota/resource_quota.rb

@@ -22,6 +22,8 @@ class ResourceQuota < ApplicationRecord
 
     validates :name, presence: true, uniqueness: true
 
+    after_save :save_hosts_resources
+
     scoped_search on: :name, complete_value: true
     scoped_search on: :id, complete_enabled: false, only_explicit: true, validator: ScopedSearch::Validators::INTEGER
 
@@ -52,12 +54,13 @@ def number_of_missing_hosts
     #   - exclude: an Array of host names to exclude from the utilization
     def missing_hosts(exclude: [])
       missing_hosts = {}
-      query = active_resources.map { |col| [col, nil] }.to_h
-      hosts_resources.where(query).each do |host_resources_item|
-        host_name = host_resources_item.host.name
-        next if exclude.include?(host_name)
-        missing_resources = host_resources_item.missing_resources(use_active_resources: true)
-        missing_hosts[host_name] = missing_resources
+      active_resources.each do |single_resource|
+        hosts_resources.where(single_resource => nil).find_each do |host_resources_item|
+          host_name = host_resources_item.host.name
+          next if exclude.include?(host_name)
+          missing_hosts[host_name] ||= []
+          missing_hosts[host_name] << single_resource
+        end
       end
       missing_hosts
     end
@@ -73,7 +76,11 @@ def missing_hosts(exclude: [])
     # Parameters:
     #   - exclude: an Array of host names to exclude from the utilization
     def utilization(exclude: [])
-      current_utilization = {}
+      current_utilization = {
+        cpu_cores: nil,
+        memory_mb: nil,
+        disk_gb: nil,
+      }
 
       self.active_resources.each do |resource|
         current_utilization[resource] = 0
@@ -105,9 +112,11 @@ def update_hosts_resources(hosts_resources_hash)
       update_hosts = hosts.where(name: hosts_resources_hash.keys)
       update_hosts_ids = update_hosts.pluck(:name, :id).to_h
       hosts_resources_hash.each do |host_name, resources|
+        # Update the host_resources without loading the whole host object
         host_resources_item = self.hosts_resources.find_by(host_id: update_hosts_ids[host_name])
         if host_resources_item
           host_resources_item.resources = resources
+          host_resources_item.save
         else
           Rails.logger.warn "HostResources not found for host_name: #{host_name}"
         end
@@ -119,7 +128,6 @@ def determine_utilization(additional_hosts = [])
       all_host_resources, missing_hosts_resources = call_utilization_helper(quota_hosts)
       self.update_hosts_resources(all_host_resources)
 
-      # TODO: Set the host_resources accordingly?
       Rails.logger.warn create_hosts_resources_warning(missing_hosts_resources) unless missing_hosts.empty?
     rescue StandardError => e
       Rails.logger.error("An error occured while determining resources for quota '#{name}': #{e}")
@@ -151,5 +159,9 @@ def create_hosts_resources_warning(missing_hosts_resources)
         warn_text << "  '#{host_name}': '#{missing_resources}'\n" unless missing_resources.empty?
       end
     end
+
+    def save_hosts_resources
+      hosts_resources.each { |host_resource| host_resource.save if host_resource.changed? }
+    end
   end
 end

db/migrate/20240611142813_create_hosts_resources.rb

@@ -1,11 +1,11 @@
 class CreateHostsResources < ActiveRecord::Migration[6.1]
   def change
     create_table :hosts_resources do |t|
-      t.references :resource_quota, foreign_key: true
       t.references :host, foreign_key: true, unique: true, null: false
-      t.integer :cpu_cores
-      t.integer :memory_mb
-      t.integer :disk_gb
+      t.references :resource_quota, foreign_key: true, default: nil
+      t.integer :cpu_cores, default: nil
+      t.integer :memory_mb, default: nil
+      t.integer :disk_gb, default: nil
 
       t.timestamps
     end

test/controllers/api/v2/resource_quotas_test.rb

@@ -123,7 +123,7 @@ def setup
 
         test 'should show utilization' do
           exp_utilization = { cpu_cores: 10, memory_mb: 20 }
-          stub_quota_utilization(exp_utilization, {})
+          stub_quota_utilization(exp_utilization)
           get :utilization, params: { resource_quota_id: @quota.id }, session: set_session_user
           assert_response :success
           show_response = ActiveSupport::JSON.decode(@response.body)
@@ -134,7 +134,7 @@ def setup
 
         test 'should show missing_hosts' do
           exp_missing_hosts = { 'some_host' => %i[cpu_cores memory_mb] }
-          stub_quota_utilization({}, exp_missing_hosts)
+          stub_quota_missing_hosts(exp_missing_hosts)
           get :missing_hosts, params: { resource_quota_id: @quota.id }, session: set_session_user
           assert_response :success
           show_response = ActiveSupport::JSON.decode(@response.body)

test/factories/foreman_resource_quota_factories.rb

@@ -5,5 +5,23 @@
     sequence(:name) { |n| "test resource quota#{n}" }
     sequence(:description) { |n| "resource quota description#{n}" }
   end
-  # TODO: Evaluate adding fixtures for resource origins
+
+  trait :with_existing_host_resources do
+    transient do
+      host_resources { [] }
+    end
+
+    after(:create) do |quota, evaluator|
+      quota.cpu_cores = nil
+      quota.memory_mb = nil
+      quota.disk_gb = nil
+      host = FactoryBot.create(:host, resource_quota: quota)
+      host.host_resources.resources = evaluator.host_resources
+      host.save!
+      quota.cpu_cores = evaluator.cpu_cores
+      quota.memory_mb = evaluator.memory_mb
+      quota.disk_gb = evaluator.disk_gb
+      quota.save!
+    end
+  end
 end