From a1f1b0ffa87db5975cc63bd5e09353662cb803de Mon Sep 17 00:00:00 2001 From: Samir Jha Date: Thu, 26 Sep 2024 19:00:38 +0000 Subject: [PATCH] Refs #37785 - Race condition during container push --- .../registry/registry_proxies_controller.rb | 2 +- .../repository/create_container_push_root.rb | 44 +++++++++++++++++++ .../actions/katello/repository/create_root.rb | 20 +++------ .../registry_proxies_controller_test.rb | 2 +- 4 files changed, 51 insertions(+), 17 deletions(-) create mode 100644 app/lib/actions/katello/repository/create_container_push_root.rb diff --git a/app/controllers/katello/api/registry/registry_proxies_controller.rb b/app/controllers/katello/api/registry/registry_proxies_controller.rb index 3f6f56e3fc3..334672f47ae 100644 --- a/app/controllers/katello/api/registry/registry_proxies_controller.rb +++ b/app/controllers/katello/api/registry/registry_proxies_controller.rb @@ -354,7 +354,7 @@ def create_container_repo_if_needed container_push_name: @container_path_input, container_push_name_format: @container_push_name_format ) - sync_task(::Actions::Katello::Repository::CreateRoot, root, @container_path_input) + sync_task(::Actions::Katello::Repository::CreateContainerPushRoot, root, @container_path_input) end end diff --git a/app/lib/actions/katello/repository/create_container_push_root.rb b/app/lib/actions/katello/repository/create_container_push_root.rb new file mode 100644 index 00000000000..5f5a5f84566 --- /dev/null +++ b/app/lib/actions/katello/repository/create_container_push_root.rb @@ -0,0 +1,44 @@ +module Actions + module Katello + module Repository + class CreateContainerPushRoot < Actions::EntryAction + def plan(root, relative_path = nil) + repository = ::Katello::Repository.new(:environment => root.organization.library, :content_view_version => root.organization.library.default_content_view_version, :root => root) + #Container push may concurrently call root add several times before the db can update. + # If the root already exists, we can skip the creation of the root and repository. + # We acquire a lock on the product to ensure that the root is not created multiple times by different workers. + + root.product.with_lock do + begin + root.save! + rescue ActiveRecord::RecordInvalid => e + if root.is_container_push && e.message.include?("Name has already been taken for this product") + Rails.logger.warn("Skipping root repository creation as container push root repository already exists: #{root.container_push_name}") + return + end + raise e + end + repository.container_repository_name = relative_path if root.docker? && root.is_container_push + repository.relative_path = relative_path || repository.custom_repo_path + begin + repository.save! + rescue ActiveRecord::RecordInvalid => e + if root.is_container_push && e.message.include?("Container Repository Name") && e.message.include?("conflicts with an existing repository") + Rails.logger.warn("Skipping repository creation as container push repository already exists: #{root.container_push_name}") + return + end + raise e + end + end + + action_subject(repository) + plan_action(::Actions::Katello::Repository::Create, repository) + end + + def humanized_name + _("Create Container Push Repository Root") + end + end + end + end +end diff --git a/app/lib/actions/katello/repository/create_root.rb b/app/lib/actions/katello/repository/create_root.rb index 41520f5ff0a..79aef25836d 100644 --- a/app/lib/actions/katello/repository/create_root.rb +++ b/app/lib/actions/katello/repository/create_root.rb @@ -2,23 +2,13 @@ module Actions module Katello module Repository class CreateRoot < Actions::EntryAction - def plan(root, relative_path = nil) - begin - root.save! - rescue ActiveRecord::RecordInvalid => e - if root.is_container_push && e.message.include?("Container Repository Name") && e.message.include?("conflicts with an existing repository") - logger.warn("Skipping repository creation as container push repository already exists: #{root.container_push_name}") - return - end - raise e - end + def plan(root) + root.save! repository = ::Katello::Repository.new(:environment => root.organization.library, - :content_view_version => root.organization.library.default_content_view_version, - :root => root) - repository.container_repository_name = relative_path if root.docker? && root.is_container_push - repository.relative_path = relative_path || repository.custom_repo_path + :content_view_version => root.organization.library.default_content_view_version, + :root => root) + repository.relative_path = repository.custom_repo_path repository.save! - action_subject(repository) plan_action(::Actions::Katello::Repository::Create, repository) end diff --git a/test/controllers/api/registry/registry_proxies_controller_test.rb b/test/controllers/api/registry/registry_proxies_controller_test.rb index c5ba3a7a9a1..7f34f6d8a88 100644 --- a/test/controllers/api/registry/registry_proxies_controller_test.rb +++ b/test/controllers/api/registry/registry_proxies_controller_test.rb @@ -993,7 +993,7 @@ def setup_permissions @controller.instance_variable_set(:@container_path_input, container_push_name) @controller.instance_variable_set(:@container_push_name_format, container_push_name_format) @controller.expects(:sync_task).with( - ::Actions::Katello::Repository::CreateRoot, + ::Actions::Katello::Repository::CreateContainerPushRoot, mock_root_repo, container_push_name ).returns(true)