diff --git a/app/services/fact_importer.rb b/app/services/fact_importer.rb index 4035c478f8..30bd1ff5fa 100644 --- a/app/services/fact_importer.rb +++ b/app/services/fact_importer.rb @@ -28,17 +28,17 @@ def initialize(host, facts = {}) @error = false @host = host @facts = normalize(facts) - @counters = {} + @counters = { added: 0, updated: 0, deleted: 0 } end # expect a facts hash - def import! + def import!(additive: false) # This function uses its own transactions that should not be included # in the transaction that handles fact values ensure_fact_names ActiveRecord::Base.transaction do - delete_removed_facts + delete_removed_facts unless additive update_facts add_new_facts end diff --git a/app/services/host_fact_importer.rb b/app/services/host_fact_importer.rb index fe26e2a499..205ca97610 100644 --- a/app/services/host_fact_importer.rb +++ b/app/services/host_fact_importer.rb @@ -11,7 +11,7 @@ def create_new_record_when_facts_are_uploaded? Setting[:create_new_host_when_facts_are_uploaded] end - def import_facts(facts, source_proxy = nil) + def import_facts(facts, source_proxy = nil, additive: false) return false if !create_new_record_when_facts_are_uploaded? && host.new_record? # we are not importing facts for hosts in build state (e.g. waiting for a re-installation) @@ -24,7 +24,7 @@ def import_facts(facts, source_proxy = nil) facts_importer = Foreman::Plugin.fact_importer_registry.get(type).new(host, facts) telemetry_observe_histogram(:importer_facts_import_duration, facts.size, type: type) telemetry_duration_histogram(:importer_facts_import_duration, 1000, type: type) do - facts_importer.import! + facts_importer.import!(additive: additive) end skipping_orchestration do diff --git a/test/unit/fact_importer_test.rb b/test/unit/fact_importer_test.rb index f66c06652a..9b0a8f25ea 100644 --- a/test/unit/fact_importer_test.rb +++ b/test/unit/fact_importer_test.rb @@ -143,6 +143,25 @@ def fact_name_class assert_equal 0, importer.counters[:added] end + test 'additive import preserves existing facts not in the new set' do + # baseline: two facts in DB + assert_equal '2.6.9', value('kernelversion') + assert_equal '10.0.19.33', value('ipaddress') + + # additive import of a single new fact — existing facts must survive + @importer = FactImporter.new(host, 'uptime' => '1 day') + allow_transactions_for @importer + @importer.stubs(:fact_name_class).returns(FactName) + @importer.import!(additive: true) + + assert_equal '2.6.9', value('kernelversion') + assert_equal '10.0.19.33', value('ipaddress') + assert_equal '1 day', value('uptime') + assert_equal 0, @importer.counters[:deleted] + assert_equal 1, @importer.counters[:added] + assert_equal 0, @importer.counters[:updated] + end + test 'importer updates fact values' do assert_equal '2.6.9', value('kernelversion') assert_equal '10.0.19.33', value('ipaddress') diff --git a/test/unit/host_fact_importer_test.rb b/test/unit/host_fact_importer_test.rb index 63fe01c790..7a2b81f9d2 100644 --- a/test/unit/host_fact_importer_test.rb +++ b/test/unit/host_fact_importer_test.rb @@ -47,7 +47,7 @@ class HostFactImporterTest < ActiveSupport::TestCase test 'import_facts only needs operatingsystem and operatingsystemrelease fact' do host = Host.import_host('host', 'puppet') - assert HostFactImporter.new(host).import_facts(:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS') + assert HostFactImporter.new(host).import_facts({:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS'}) end test 'should import facts from json of a new host when certname is not specified' do @@ -70,7 +70,7 @@ class HostFactImporterTest < ActiveSupport::TestCase test 'domain updated from facts' do host = FactoryBot.create(:host, :with_operatingsystem) FactoryBot.create(:domain, :name => 'foo.bar') - assert HostFactImporter.new(host).import_facts(:domain => 'foo.bar', :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name) + assert HostFactImporter.new(host).import_facts({:domain => 'foo.bar', :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name}) assert_equal 'foo.bar', host.domain.to_s end @@ -78,9 +78,9 @@ class HostFactImporterTest < ActiveSupport::TestCase domain = FactoryBot.create(:domain) host = FactoryBot.create(:host, :managed, :domain => domain) FactoryBot.create(:domain, :name => 'foo.bar') - assert HostFactImporter.new(host).import_facts(:domain => domain.name, :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name) + assert HostFactImporter.new(host).import_facts({:domain => domain.name, :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name}) Setting[:ignore_facts_for_domain] = true - assert HostFactImporter.new(host).import_facts(:domain => 'foo.bar', :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name) + assert HostFactImporter.new(host).import_facts({:domain => 'foo.bar', :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name}) assert_equal domain.name, host.domain.name Setting[:ignore_facts_for_domain] = Setting.find_by_name('ignore_facts_for_domain').default @@ -236,13 +236,13 @@ class HostFactImporterTest < ActiveSupport::TestCase test 'comment updated from facts when present' do host = Host.import_host('host') - assert HostFactImporter.new(host).import_facts(foreman_comment: 'new comment', operatingsystemrelease: '6.7', operatingsystem: 'CentOS') + assert HostFactImporter.new(host).import_facts({foreman_comment: 'new comment', operatingsystemrelease: '6.7', operatingsystem: 'CentOS'}) assert_equal 'new comment', host.comment end test 'operatingsystem updated from facts' do host = Host.import_host('host') - assert HostFactImporter.new(host).import_facts(:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS') + assert HostFactImporter.new(host).import_facts({:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS'}) assert_equal 'CentOS 6.7', host.operatingsystem.to_s end @@ -251,7 +251,7 @@ class HostFactImporterTest < ActiveSupport::TestCase os2 = FactoryBot.create(:operatingsystem) medium = os1.media.first host = FactoryBot.create(:host, :operatingsystem => os1, :medium => medium) - HostFactImporter.new(host).import_facts(:operatingsystem => os2.name, :operatingsystemrelease => os2.major.to_s) + HostFactImporter.new(host).import_facts({:operatingsystem => os2.name, :operatingsystemrelease => os2.major.to_s}) assert_equal host.operatingsystem, os2 assert_nil host.medium @@ -264,7 +264,7 @@ class HostFactImporterTest < ActiveSupport::TestCase medium.operatingsystems << os2 host = FactoryBot.create(:host, :operatingsystem => os1, :medium => medium) - HostFactImporter.new(host).import_facts(:operatingsystem => os2.name, :operatingsystemrelease => os2.major.to_s) + HostFactImporter.new(host).import_facts({:operatingsystem => os2.name, :operatingsystemrelease => os2.major.to_s}) assert_equal host.operatingsystem, os2 assert_equal host.medium, medium @@ -272,9 +272,9 @@ class HostFactImporterTest < ActiveSupport::TestCase test 'operatingsystem not updated from facts when ignore_facts_for_operatingsystem false' do host = Host.import_host('host') - assert HostFactImporter.new(host).import_facts(:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS') + assert HostFactImporter.new(host).import_facts({:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS'}) Setting[:ignore_facts_for_operatingsystem] = true - assert HostFactImporter.new(host).import_facts(:operatingsystemrelease => '6.8', :operatingsystem => 'CentOS') + assert HostFactImporter.new(host).import_facts({:operatingsystemrelease => '6.8', :operatingsystem => 'CentOS'}) assert_equal 'CentOS 6.7', host.operatingsystem.to_s Setting[:ignore_facts_for_operatingsystem] = Setting.find_by_name('ignore_facts_for_operatingsystem').default @@ -360,7 +360,7 @@ class HostFactImporterTest < ActiveSupport::TestCase payload[:object].operatingsystem.name == 'CentOS' end - HostFactImporter.new(host).import_facts(operatingsystemrelease: '6.7', operatingsystem: 'CentOS') + HostFactImporter.new(host).import_facts({operatingsystemrelease: '6.7', operatingsystem: 'CentOS'}) end end end