From ebc424b03eb70ef7e4a50bb0c07b3349950f6f0f Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 11 Sep 2019 19:06:17 +0000 Subject: [PATCH] Fixes #27838 - Refactor cvv export to allow for default cv --- .../content_view_version.rb | 21 +- .../cv_import_export_helper.rb | 80 ++++--- .../content_view/version/export_test.rb | 197 +----------------- 3 files changed, 52 insertions(+), 246 deletions(-) diff --git a/lib/hammer_cli_katello/content_view_version.rb b/lib/hammer_cli_katello/content_view_version.rb index e69be408..7564892a 100644 --- a/lib/hammer_cli_katello/content_view_version.rb +++ b/lib/hammer_cli_katello/content_view_version.rb @@ -372,10 +372,7 @@ def execute end export_json_options[:repositories] = [] else - repositories = fetch_cvv_repositories(cvv) - puppet_check(cvv) - check_repo_type(repositories) - check_repo_download_policy(repositories) + repositories = fetch_exportable_cvv_repositories(cvv) collect_packages(repositories) export_json_options[:repositories] = repositories @@ -480,11 +477,14 @@ def execute publish(cv['id'], export_json['major'], export_json['minor']) else sync_repositories(export_json['repositories'], options['option_organization_id'], - import_tar_params) - publish( - cv['id'], export_json['major'], - export_json['minor'], repos_units(export_json['repositories']) - ) + import_tar_params) + + unless cv['default'] + publish( + cv['id'], export_json['major'], + export_json['minor'], repos_units(export_json['repositories']) + ) + end end return HammerCLI::EX_OK end @@ -498,8 +498,7 @@ def sync_repositories(repositories, organization_id, options) library_repos = index( :repositories, 'organization_id' => organization_id, - 'library' => true, - 'label' => repo['label'] + 'library' => true ) library_repo = library_repos.select do |candidate_repo| diff --git a/lib/hammer_cli_katello/cv_import_export_helper.rb b/lib/hammer_cli_katello/cv_import_export_helper.rb index 3ae9491b..3d492012 100644 --- a/lib/hammer_cli_katello/cv_import_export_helper.rb +++ b/lib/hammer_cli_katello/cv_import_export_helper.rb @@ -3,10 +3,32 @@ module HammerCLIKatello module CVImportExportHelper PUBLISHED_REPOS_DIR = "/var/lib/pulp/published/yum/https/repos/".freeze - def fetch_cvv_repositories(cvv) - cvv['repositories'].collect do |repo| - show(:repositories, 'id' => repo['id'], :full_result => true) + def fetch_exportable_cvv_repositories(cvv) + immediate = [] + non_immediate_names = [] + + cvv['repositories'].each do |repo| + next unless repo['content_type'] == 'yum' + + api_repo = show(:repositories, 'id' => repo['id'], :full_result => true) + + download_policy = if api_repo['library_instance_id'] + library = show(:repositories, 'id' => api_repo['library_instance_id']) + library['download_policy'] + else + api_repo['download_policy'] + end + + if download_policy == 'immediate' + immediate << api_repo + else + non_immediate_names << api_repo['name'] + end end + + warn_repo_download_policy(non_immediate_names) + + return immediate end def find_local_component_id(component_from_export) @@ -23,41 +45,15 @@ def find_local_component_id(component_from_export) found_composite_version.first['id'] end - def puppet_check(cvv) - unless cvv['puppet_modules'].empty? - raise _("The Content View '#{cvv['content_view']['label']}'"\ - " contains Puppet modules, this is not supported at this time."\ - " Please remove the modules, publish a new version"\ - " and try the export again.") - end - end + def warn_repo_download_policy(repository_names) + return if repository_names.empty? - def check_repo_type(repositories) - repositories.select do |repo| - if repo['content_type'] != 'yum' - raise _("The Repository '#{repo['name']}' is a non-yum repository."\ - " Only Yum is supported at this time."\ - " Please remove the repository from the Content View,"\ - " republish and try the export again.") - end - end - end - - def check_repo_download_policy(repositories) - non_immediate = repositories.select do |repo| - unless repo['library_instance_id'].nil? - show(:repositories, 'id' => repo['library_instance_id'])['download_policy'] != 'immediate' - end - end - unless non_immediate.empty? - non_immediate_names = non_immediate.collect { |repo| repo['name'] } - msg = <<~MSG - All exported repositories must be set to an immediate download policy and re-synced. - The following repositories need action: - #{non_immediate_names.join(', ')} - MSG - raise _(msg) - end + msg = <<~MSG + The following repositories could not be exported due to the download policy + not being set to 'immediate': + #{repository_names.join(', ')} + MSG + print_message msg end def collect_packages(repositories) @@ -88,10 +84,12 @@ def import_checks(cv, import_cv, major, minor) " please create the Content View and try the import again.") end - if import_cv['latest_version'].to_f >= version - raise _("The latest version (#{import_cv['latest_version']}) of"\ - " the Content View '#{cv['name']}'"\ - " is greater or equal to the version you are trying to import (#{version})") + unless import_cv['default'] + if import_cv['latest_version'].to_f >= version + raise _("The latest version (#{import_cv['latest_version']}) of"\ + " the Content View '#{cv['name']}'"\ + " is greater or equal to the version you are trying to import (#{version})") + end end unless import_cv['repository_ids'].nil? diff --git a/test/functional/content_view/version/export_test.rb b/test/functional/content_view/version/export_test.rb index ebba978a..dcec2a26 100644 --- a/test/functional/content_view/version/export_test.rb +++ b/test/functional/content_view/version/export_test.rb @@ -16,12 +16,11 @@ ex = api_expects(:content_view_versions, :show) ex.returns( 'id' => '5', - 'repositories' => [{'id' => '2'}], + 'repositories' => [{'id' => '2', 'content_type' => 'yum'}], 'major' => 1, 'minor' => 0, 'content_view' => {'name' => 'cv'}, - 'content_view_id' => 4321, - 'puppet_modules' => [] + 'content_view_id' => 4321 ) ex = api_expects(:content_views, :show) @@ -36,6 +35,7 @@ 'id' => '2', 'label' => 'Test_Repo', 'content_type' => 'yum', + 'download_policy' => 'immediate', 'backend_identifier' => 'Default_Organization-Library-Test_Repo', 'relative_path' => 'Default_Organization/Library/Test_Repo', 'library_instance_id' => '1', @@ -100,197 +100,6 @@ assert_equal(HammerCLI::EX_OK, result.exit_code) end - it "fails export if any repository is set to background" do - params = [ - '--id=5', - '--export-dir=/tmp/exports' - ] - - ex = api_expects(:content_view_versions, :show) - ex.returns( - 'id' => '5', - 'repositories' => [{'id' => '2'}], - 'major' => 1, - 'minor' => 0, - 'content_view' => {'name' => 'cv'}, - 'content_view_id' => 4321, - 'puppet_modules' => [] - ) - - ex = api_expects(:content_views, :show) - ex.returns( - 'id' => '4321', - 'composite' => false - ) - - ex = api_expects(:repositories, :show).with_params('id' => '2') - ex.returns( - 'id' => '2', - 'label' => 'Test_Repo', - 'content_type' => 'yum', - 'backend_identifier' => 'Default_Organization-Library-Test_Repo', - 'relative_path' => 'Default_Organization/Library/Test_Repo', - 'library_instance_id' => '1' - ) - - api_expects(:repositories, :show).with_params('id' => '1').returns( - 'id' => '1', - 'download_policy' => 'background' - ) - - File.expects(:exist?).with('/usr/share/foreman').returns(true) - File.stubs(:exist?).with('/var/log/hammer/hammer.log._copy_').returns(false) - - result = run_cmd(@cmd + params) - assert_equal(HammerCLI::EX_SOFTWARE, result.exit_code) - end - - it "fails export if cvv contains puppet module" do - params = [ - '--id=5', - '--export-dir=/tmp/exports' - ] - - ex = api_expects(:content_view_versions, :show) - ex.returns( - 'id' => '5', - 'repositories' => [{'id' => '2'}], - 'major' => 1, - 'minor' => 0, - 'content_view' => {'name' => 'cv'}, - 'content_view_id' => 4321, - 'puppet_modules' => [{'id' => '1'}] - ) - - ex = api_expects(:content_views, :show) - ex.returns( - 'id' => '4321', - 'composite' => false, - 'puppet_modules' => [{'id' => '1'}] - ) - - ex = api_expects(:repositories, :show).with_params('id' => '2') - ex.returns( - 'id' => '2', - 'label' => 'Test_Repo', - 'content_type' => 'yum', - 'backend_identifier' => 'Default_Organization-Library-Test_Repo', - 'relative_path' => 'Default_Organization/Library/Test_Repo', - 'library_instance_id' => '1' - ) - - File.expects(:exist?).with('/usr/share/foreman').returns(true) - File.stubs(:exist?).with('/var/log/hammer/hammer.log._copy_').returns(false) - - result = run_cmd(@cmd + params) - assert_equal(result.exit_code, 70) - end - - it "fails export if any repository is set to on_demand" do - params = [ - '--id=5', - '--export-dir=/tmp/exports' - ] - - ex = api_expects(:content_view_versions, :show) - ex.returns( - 'id' => '5', - 'repositories' => [{'id' => '2'}, {'id' => '3'}], - 'major' => 1, - 'minor' => 0, - 'content_view' => {'name' => 'cv'}, - 'content_view_id' => 4321, - 'puppet_modules' => [] - ) - - ex = api_expects(:content_views, :show) - ex.returns( - 'id' => '4321', - 'composite' => false - ) - - ex = api_expects(:repositories, :show).with_params('id' => '2') - ex.returns( - 'id' => '2', - 'label' => 'Test_Repo', - 'name' => 'Test_Repo', - 'content_type' => 'yum', - 'backend_identifier' => 'Default_Organization-Library-Test_Repo', - 'relative_path' => 'Default_Organization/Library/Test_Repo', - 'library_instance_id' => '1' - ) - - api_expects(:repositories, :show).with_params('id' => '3').returns( - 'id' => '3', - 'label' => 'Test_Repo_3', - 'name' => 'Test_Repo_3', - 'content_type' => 'yum', - 'backend_identifier' => 'Default_Organization-Library-Test_Repo_3', - 'relative_path' => 'Default_Organization/Library/Test_Repo_3', - 'library_instance_id' => '4' - ) - - api_expects(:repositories, :show).with_params('id' => '1').returns( - 'id' => '1', - 'download_policy' => 'on_demand' - ) - api_expects(:repositories, :show).with_params('id' => '4').returns( - 'id' => '4', - 'download_policy' => 'immediate' - ) - - File.expects(:exist?).with('/usr/share/foreman').returns(true) - File.stubs(:exist?).with('/var/log/hammer/hammer.log._copy_').returns(false) - - result = run_cmd(@cmd + params) - assert_equal(result.err, "Could not export the content view:\n" \ - " Error: All exported repositories must be set to an "\ - "immediate download policy and re-synced.\n" \ - " The following repositories need action:\n" \ - " Test_Repo\n" \ - ) - assert_equal(HammerCLI::EX_SOFTWARE, result.exit_code) - end - - it "fails export if any repository is not yum" do - params = [ - '--id=5', - '--export-dir=/tmp/exports' - ] - - ex = api_expects(:content_view_versions, :show) - ex.returns( - 'id' => '5', - 'repositories' => [{'id' => '2'}], - 'major' => 1, - 'minor' => 0, - 'content_view' => {'name' => 'cv'}, - 'content_view_id' => 4321 - ) - - ex = api_expects(:content_views, :show) - ex.returns( - 'id' => '4321', - 'composite' => false - ) - - ex = api_expects(:repositories, :show).with_params('id' => '2') - ex.returns( - 'id' => '2', - 'label' => 'Test_Repo', - 'content_type' => 'file', - 'backend_identifier' => 'Default_Organization-Library-Test_Repo', - 'relative_path' => 'Default_Organization/Library/Test_Repo', - 'library_instance_id' => '1' - ) - - File.expects(:exist?).with('/usr/share/foreman').returns(true) - File.stubs(:exist?).with('/var/log/hammer/hammer.log._copy_').returns(false) - - result = run_cmd(@cmd + params) - assert_equal(result.exit_code, 70) - end - it "fails export if content view version has no repository" do params = [ '--id=5',