From 682568fa489549d2335326297d2e3080d8a70e65 Mon Sep 17 00:00:00 2001 From: Jeremy Lenz Date: Thu, 1 Feb 2024 17:04:17 -0500 Subject: [PATCH 1/4] Fixes #37137 - Revert to old method of creating module streams after registration --- .../katello/concerns/host_managed_extensions.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/app/models/katello/concerns/host_managed_extensions.rb b/app/models/katello/concerns/host_managed_extensions.rb index c483fa6b772..5183477c8b9 100644 --- a/app/models/katello/concerns/host_managed_extensions.rb +++ b/app/models/katello/concerns/host_managed_extensions.rb @@ -326,11 +326,21 @@ def import_enabled_repositories(repos) end def import_module_streams(module_streams) + # create_or_find_by avoids race conditions during concurrent registrations but clogs postgres logs with harmless errors. + # So we'll use create_or_find_by! during registration and first_or_create! otherwise. + registered_time = subscription_facet&.registered_at + use_create_or_find_by = registered_time.nil? || registered_time > 1.minute.ago streams = {} module_streams.each do |module_stream| - stream = AvailableModuleStream.create_or_find_by!(name: module_stream["name"], - context: module_stream["context"], - stream: module_stream["stream"]) + if use_create_or_find_by + stream = AvailableModuleStream.create_or_find_by!(name: module_stream["name"], + context: module_stream["context"], + stream: module_stream["stream"]) + else + stream = AvailableModuleStream.where(name: module_stream["name"], + context: module_stream["context"], + stream: module_stream["stream"]).first_or_create! + end streams[stream.id] = module_stream end sync_available_module_stream_associations(streams) From 756f496acb9991106906df9ab4e93c0c15463308 Mon Sep 17 00:00:00 2001 From: Jeremy Lenz Date: Tue, 20 Feb 2024 18:54:42 -0500 Subject: [PATCH 2/4] Refs #37137 - use insert_all and index_by --- .../concerns/host_managed_extensions.rb | 77 ++++++++++++------- 1 file changed, 50 insertions(+), 27 deletions(-) diff --git a/app/models/katello/concerns/host_managed_extensions.rb b/app/models/katello/concerns/host_managed_extensions.rb index 5183477c8b9..a6f628720d9 100644 --- a/app/models/katello/concerns/host_managed_extensions.rb +++ b/app/models/katello/concerns/host_managed_extensions.rb @@ -325,50 +325,73 @@ def import_enabled_repositories(repos) content_facet.update_repositories_by_paths(paths.compact) end + def available_module_stream_id_from(name:, stream:, context:) + AvailableModuleStream.find_by!(name: name, stream: stream, context: context).id + rescue ActiveRecord::RecordNotFound + Rails.logger.warn("Module stream not found: name: #{name}, stream: #{stream}, context: #{context}") + nil + end + def import_module_streams(module_streams) - # create_or_find_by avoids race conditions during concurrent registrations but clogs postgres logs with harmless errors. - # So we'll use create_or_find_by! during registration and first_or_create! otherwise. - registered_time = subscription_facet&.registered_at - use_create_or_find_by = registered_time.nil? || registered_time > 1.minute.ago - streams = {} - module_streams.each do |module_stream| - if use_create_or_find_by - stream = AvailableModuleStream.create_or_find_by!(name: module_stream["name"], - context: module_stream["context"], - stream: module_stream["stream"]) - else - stream = AvailableModuleStream.where(name: module_stream["name"], - context: module_stream["context"], - stream: module_stream["stream"]).first_or_create! - end - streams[stream.id] = module_stream - end - sync_available_module_stream_associations(streams) + Rails.logger.debug "INSERT_ALL-------------------------------------" + Rails.logger.debug module_streams.first(3) + streams = module_streams.map do |module_stream| + { + name: module_stream["name"], + stream: module_stream["stream"], + context: module_stream["context"] + } + end + newly_added_records = AvailableModuleStream.insert_all( + streams, + unique_by: %w[name stream context], + returning: %w[id name stream context] + ) + Rails.logger.debug "newly_added_records-------------------------------------" + Rails.logger.debug newly_added_records.to_a + # module_streams looks like this + # {"name"=>"389-ds", "stream"=>"1.4", "version"=>"8030020201203210520", "context"=>"e114a9e7", "arch"=>"x86_64", "profiles"=>[], "installed_profiles"=>[], "status"=>"default", "active"=>false} + + indexed_module_streams = module_streams.index_by do |module_stream| + available_module_stream_id_from( + name: module_stream["name"], + stream: module_stream["stream"], + context: module_stream["context"] + ) + end + Rails.logger.debug "INDEXED_MODULE_STREAMS-------------------------------------" + Rails.logger.debug indexed_module_streams + + sync_available_module_stream_associations(indexed_module_streams) end def sync_available_module_stream_associations(new_available_module_streams) upgradable_streams = self.host_available_module_streams.where(:available_module_stream_id => new_available_module_streams.keys) + new_associated_ids = new_available_module_streams.keys.compact old_associated_ids = self.available_module_stream_ids - delete_ids = old_associated_ids - new_available_module_streams.keys + delete_ids = old_associated_ids - new_associated_ids if delete_ids.any? self.host_available_module_streams.where(:available_module_stream_id => delete_ids).delete_all end - new_ids = new_available_module_streams.keys - old_associated_ids - new_ids.each do |new_id| + new_ids = new_associated_ids - old_associated_ids + + hams_to_create = new_ids.map do |new_id| module_stream = new_available_module_streams[new_id] status = module_stream["status"] # Set status to "unknown" only if the active field is in use and set to false and the module is enabled if enabled_module_stream_inactive?(module_stream) status = "unknown" end - self.host_available_module_streams.create!(host_id: self.id, - available_module_stream_id: new_id, - installed_profiles: module_stream["installed_profiles"], - status: status) - end - + { + host_id: self.id, + available_module_stream_id: new_id, + installed_profiles: module_stream["installed_profiles"], + status: status + } + end + HostAvailableModuleStream.insert_all(hams_to_create) if hams_to_create.any? upgradable_streams.each do |hams| module_stream = new_available_module_streams[hams.available_module_stream_id] shared_keys = hams.attributes.keys & module_stream.keys From 8982ebc169b8fd72eea86e4af44dd0a38a93ae67 Mon Sep 17 00:00:00 2001 From: Jeremy Lenz Date: Tue, 27 Feb 2024 16:53:24 -0500 Subject: [PATCH 3/4] Refs #37137 - cache AvailableModuleStreams --- .../concerns/host_managed_extensions.rb | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/app/models/katello/concerns/host_managed_extensions.rb b/app/models/katello/concerns/host_managed_extensions.rb index a6f628720d9..c10f6be51ed 100644 --- a/app/models/katello/concerns/host_managed_extensions.rb +++ b/app/models/katello/concerns/host_managed_extensions.rb @@ -326,15 +326,15 @@ def import_enabled_repositories(repos) end def available_module_stream_id_from(name:, stream:, context:) - AvailableModuleStream.find_by!(name: name, stream: stream, context: context).id - rescue ActiveRecord::RecordNotFound - Rails.logger.warn("Module stream not found: name: #{name}, stream: #{stream}, context: #{context}") - nil + @indexed_available_module_streams ||= Katello::AvailableModuleStream.all.index_by do |available_module_stream| + "#{available_module_stream.name}-#{available_module_stream.stream}-#{available_module_stream.context}" + end + @indexed_available_module_streams["#{name}-#{stream}-#{context}"]&.id end def import_module_streams(module_streams) - Rails.logger.debug "INSERT_ALL-------------------------------------" - Rails.logger.debug module_streams.first(3) + # module_streams looks like this + # {"name"=>"389-ds", "stream"=>"1.4", "version"=>"8030020201203210520", "context"=>"e114a9e7", "arch"=>"x86_64", "profiles"=>[], "installed_profiles"=>[], "status"=>"default", "active"=>false} streams = module_streams.map do |module_stream| { name: module_stream["name"], @@ -342,15 +342,11 @@ def import_module_streams(module_streams) context: module_stream["context"] } end - newly_added_records = AvailableModuleStream.insert_all( + AvailableModuleStream.insert_all( streams, unique_by: %w[name stream context], returning: %w[id name stream context] ) - Rails.logger.debug "newly_added_records-------------------------------------" - Rails.logger.debug newly_added_records.to_a - # module_streams looks like this - # {"name"=>"389-ds", "stream"=>"1.4", "version"=>"8030020201203210520", "context"=>"e114a9e7", "arch"=>"x86_64", "profiles"=>[], "installed_profiles"=>[], "status"=>"default", "active"=>false} indexed_module_streams = module_streams.index_by do |module_stream| available_module_stream_id_from( @@ -359,15 +355,13 @@ def import_module_streams(module_streams) context: module_stream["context"] ) end - Rails.logger.debug "INDEXED_MODULE_STREAMS-------------------------------------" - Rails.logger.debug indexed_module_streams sync_available_module_stream_associations(indexed_module_streams) end def sync_available_module_stream_associations(new_available_module_streams) - upgradable_streams = self.host_available_module_streams.where(:available_module_stream_id => new_available_module_streams.keys) new_associated_ids = new_available_module_streams.keys.compact + upgradable_streams = self.host_available_module_streams.where(:available_module_stream_id => new_associated_ids) old_associated_ids = self.available_module_stream_ids delete_ids = old_associated_ids - new_associated_ids From 1f8225e72be8bf9a6f4cf1b9e2f53f77a8b06229 Mon Sep 17 00:00:00 2001 From: Jeremy Lenz Date: Wed, 28 Feb 2024 10:59:54 -0500 Subject: [PATCH 4/4] Refs #37137 - fix test --- .../katello/concerns/host_managed_extensions.rb | 14 +++++++------- .../concerns/host_managed_extensions_test.rb | 5 +++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/app/models/katello/concerns/host_managed_extensions.rb b/app/models/katello/concerns/host_managed_extensions.rb index c10f6be51ed..b0d2210dc56 100644 --- a/app/models/katello/concerns/host_managed_extensions.rb +++ b/app/models/katello/concerns/host_managed_extensions.rb @@ -342,12 +342,13 @@ def import_module_streams(module_streams) context: module_stream["context"] } end - AvailableModuleStream.insert_all( - streams, - unique_by: %w[name stream context], - returning: %w[id name stream context] - ) - + if streams.any? + AvailableModuleStream.insert_all( + streams, + unique_by: %w[name stream context], + returning: %w[id name stream context] + ) + end indexed_module_streams = module_streams.index_by do |module_stream| available_module_stream_id_from( name: module_stream["name"], @@ -355,7 +356,6 @@ def import_module_streams(module_streams) context: module_stream["context"] ) end - sync_available_module_stream_associations(indexed_module_streams) end diff --git a/test/models/concerns/host_managed_extensions_test.rb b/test/models/concerns/host_managed_extensions_test.rb index b6b05af85f8..ac95bc1f40e 100644 --- a/test/models/concerns/host_managed_extensions_test.rb +++ b/test/models/concerns/host_managed_extensions_test.rb @@ -367,6 +367,10 @@ def test_import_modules_with_active_field assert_equal 2, @foreman_host.host_available_module_streams.unknown.count end + def clear_cached_available_module_streams + @foreman_host.instance_variable_set(:@indexed_available_module_streams, nil) + end + def test_import_modules_with_update modules_json = [make_module_json("enabled21111", "enabled")] prior_count = HostAvailableModuleStream.count @@ -384,6 +388,7 @@ def test_import_modules_with_update assert_empty @foreman_host.reload.host_available_module_streams assert_equal prior_count, HostAvailableModuleStream.count + clear_cached_available_module_streams @foreman_host.import_module_streams([make_module_json("xxxx", "enabled", 'blah', ["default"])]) assert_equal "enabled", @foreman_host.reload.host_available_module_streams.first.status assert_equal ["default"], @foreman_host.reload.host_available_module_streams.first.installed_profiles