diff --git a/.rubocop.yml b/.rubocop.yml index 605b0b336..e3761e41a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -122,3 +122,6 @@ Style/OptionalBooleanParameter: Style/StderrPuts: Enabled: false + +Metrics/ParameterLists: + Max: 6 diff --git a/definitions/procedures/packages/update.rb b/definitions/procedures/packages/update.rb index f353f93b0..eddf4d13a 100644 --- a/definitions/procedures/packages/update.rb +++ b/definitions/procedures/packages/update.rb @@ -7,13 +7,18 @@ class Update < ForemanMaintain::Procedure param :warn_on_errors, 'Do not interrupt scenario on failure', :flag => true, :default => false param :dnf_options, 'Extra dnf options if any', :array => true, :default => [] + param :download_only, 'Download and cache packages only', :flag => true, :default => false param :clean_cache, 'If true will cause a DNF cache clean', :flag => true, :default => true end def run assumeyes_val = @assumeyes.nil? ? assumeyes? : @assumeyes package_manager.clean_cache(:assumeyes => assumeyes_val) if @clean_cache - opts = { :assumeyes => assumeyes_val, :dnf_options => @dnf_options } + opts = { + :assumeyes => assumeyes_val, + :options => @dnf_options, + :download_only => @download_only, + } packages_action(:update, @packages, opts) rescue ForemanMaintain::Error::ExecutionError => e if @warn_on_errors @@ -28,15 +33,11 @@ def necessary? end def description - if download_only? + if @download_only "Download package(s) #{@packages.join(', ')}" else "Update package(s) #{@packages.join(', ')}" end end - - def download_only? - @dnf_options.include?('--downloadonly') - end end end diff --git a/definitions/scenarios/packages.rb b/definitions/scenarios/packages.rb index a86019cf9..534859cfa 100644 --- a/definitions/scenarios/packages.rb +++ b/definitions/scenarios/packages.rb @@ -115,22 +115,16 @@ def compose end add_step_with_context(Procedures::Packages::UnlockVersions) + add_step_with_context( + Procedures::Packages::Update, + :force => true, + :warn_on_errors => true, + :download_only => context.get(:downloadonly) + ) if context.get(:downloadonly) - add_step_with_context( - Procedures::Packages::Update, - :force => true, - :warn_on_errors => true, - :dnf_options => ['--downloadonly'] - ) add_step_with_context(Procedures::Packages::LockVersions) else - add_step_with_context( - Procedures::Packages::Update, - :force => true, - :warn_on_errors => true - ) - add_step_with_context(Procedures::Installer::Run) end diff --git a/definitions/scenarios/upgrade_to_capsule_6_16.rb b/definitions/scenarios/upgrade_to_capsule_6_16.rb index 7add0ebcb..aa5db4cf6 100644 --- a/definitions/scenarios/upgrade_to_capsule_6_16.rb +++ b/definitions/scenarios/upgrade_to_capsule_6_16.rb @@ -63,7 +63,7 @@ def compose end add_step(Procedures::Packages::Update.new( :assumeyes => true, - :dnf_options => ['--downloadonly'] + :download_only => true )) add_step(Procedures::Service::Stop.new) add_step(Procedures::Packages::Update.new(:assumeyes => true, :clean_cache => false)) diff --git a/definitions/scenarios/upgrade_to_capsule_6_16_z.rb b/definitions/scenarios/upgrade_to_capsule_6_16_z.rb index 41ea9b8ec..754289ac6 100644 --- a/definitions/scenarios/upgrade_to_capsule_6_16_z.rb +++ b/definitions/scenarios/upgrade_to_capsule_6_16_z.rb @@ -45,7 +45,7 @@ def compose end add_step(Procedures::Packages::Update.new( :assumeyes => true, - :dnf_options => ['--downloadonly'] + :download_only => true )) add_steps(find_procedures(:pre_migrations)) end diff --git a/definitions/scenarios/upgrade_to_katello_nightly.rb b/definitions/scenarios/upgrade_to_katello_nightly.rb index 4f7fd5631..fca96fad6 100644 --- a/definitions/scenarios/upgrade_to_katello_nightly.rb +++ b/definitions/scenarios/upgrade_to_katello_nightly.rb @@ -60,7 +60,7 @@ def compose end add_step(Procedures::Packages::Update.new( :assumeyes => true, - :dnf_options => ['--downloadonly'] + :download_only => true )) add_step(Procedures::Service::Stop.new) add_step(Procedures::Packages::Update.new(:assumeyes => true, :clean_cache => false)) diff --git a/definitions/scenarios/upgrade_to_satellite_6_16.rb b/definitions/scenarios/upgrade_to_satellite_6_16.rb index a74674b2c..8b1a85ce3 100644 --- a/definitions/scenarios/upgrade_to_satellite_6_16.rb +++ b/definitions/scenarios/upgrade_to_satellite_6_16.rb @@ -65,7 +65,7 @@ def compose end add_step(Procedures::Packages::Update.new( :assumeyes => true, - :dnf_options => ['--downloadonly'] + :download_only => true )) add_step(Procedures::Service::Stop.new) add_step(Procedures::Packages::Update.new(:assumeyes => true, :clean_cache => false)) diff --git a/definitions/scenarios/upgrade_to_satellite_6_16_z.rb b/definitions/scenarios/upgrade_to_satellite_6_16_z.rb index 42fdb6306..397d6ec03 100644 --- a/definitions/scenarios/upgrade_to_satellite_6_16_z.rb +++ b/definitions/scenarios/upgrade_to_satellite_6_16_z.rb @@ -45,7 +45,7 @@ def compose end add_step(Procedures::Packages::Update.new( :assumeyes => true, - :dnf_options => ['--downloadonly'] + :download_only => true )) add_steps(find_procedures(:pre_migrations)) end diff --git a/lib/foreman_maintain/concerns/system_helpers.rb b/lib/foreman_maintain/concerns/system_helpers.rb index a9abd3f18..416dcba82 100644 --- a/lib/foreman_maintain/concerns/system_helpers.rb +++ b/lib/foreman_maintain/concerns/system_helpers.rb @@ -100,13 +100,17 @@ def server? end def packages_action(action, packages, options = {}) - options.validate_options!(:assumeyes, :dnf_options) + options.validate_options!(:assumeyes, :options, :download_only) case action when :install package_manager.install(packages, :assumeyes => options[:assumeyes]) when :update - package_manager.update(packages, :assumeyes => options[:assumeyes], - :dnf_options => options[:dnf_options]) + package_manager.update( + packages, + :assumeyes => options[:assumeyes], + :options => options[:options], + :download_only => options[:download_only] + ) when :remove package_manager.remove(packages, :assumeyes => options[:assumeyes]) else diff --git a/lib/foreman_maintain/package_manager/apt.rb b/lib/foreman_maintain/package_manager/apt.rb index e5b02d7f1..2c37872cd 100644 --- a/lib/foreman_maintain/package_manager/apt.rb +++ b/lib/foreman_maintain/package_manager/apt.rb @@ -19,10 +19,12 @@ def remove(packages, assumeyes: false) apt_action('remove', packages, :assumeyes => assumeyes) end - def update(packages = [], assumeyes: false) + # rubocop:disable Lint/UnusedMethodArgument + def update(packages = [], assumeyes: false, options: [], download_only: false) action = packages.any? ? '--only-upgrade install' : 'upgrade' - apt_action(action, packages, :assumeyes => assumeyes) + apt_action(action, packages, :assumeyes => assumeyes, :download_only => download_only) end + # rubocop:enable Lint/UnusedMethodArgument def clean_cache(assumeyes: false) apt_action('clean', :assumeyes => assumeyes) @@ -66,10 +68,12 @@ def reboot_required? [status, output] end - def apt_action(action, packages, with_status: false, assumeyes: false, valid_exit_statuses: [0]) + # rubocop:disable Layout/LineLength + def apt_action(action, packages, with_status: false, assumeyes: false, valid_exit_statuses: [0], download_only: false) apt_options = [] packages = [packages].flatten(1) apt_options << '-y' if assumeyes + apt_options << '--download-only' if download_only apt_options_s = apt_options.empty? ? '' : ' ' + apt_options.join(' ') packages_s = packages.empty? ? '' : ' ' + packages.join(' ') if with_status @@ -80,5 +84,6 @@ def apt_action(action, packages, with_status: false, assumeyes: false, valid_exi :interactive => !assumeyes, :valid_exit_statuses => valid_exit_statuses) end end + # rubocop:enable Layout/LineLength end end diff --git a/lib/foreman_maintain/package_manager/dnf.rb b/lib/foreman_maintain/package_manager/dnf.rb index 74bcd6aed..e7e4c02f0 100644 --- a/lib/foreman_maintain/package_manager/dnf.rb +++ b/lib/foreman_maintain/package_manager/dnf.rb @@ -59,8 +59,14 @@ def remove(packages, assumeyes: false) dnf_action('remove', packages, assumeyes: assumeyes) end - def update(packages = [], assumeyes: false, dnf_options: []) - dnf_action('update', packages, assumeyes: assumeyes, dnf_options: dnf_options) + def update(packages = [], assumeyes: false, options: [], download_only: false) + dnf_action( + 'update', + packages, + assumeyes: assumeyes, + dnf_options: options, + download_only: download_only + ) end def check_update(packages: nil, with_status: false) @@ -128,10 +134,11 @@ def reboot_required? private # rubocop:disable Metrics/LineLength, Metrics/ParameterLists - def dnf_action(action, packages, with_status: false, assumeyes: false, dnf_options: [], valid_exit_statuses: [0]) + def dnf_action(action, packages, with_status: false, assumeyes: false, dnf_options: [], valid_exit_statuses: [0], download_only: false) packages = [packages].flatten(1) dnf_options << '-y' if assumeyes + dnf_topions << '--downloadonly' if download_only dnf_options << '--disableplugin=foreman-protector' command = ['dnf', dnf_options.join(' '), action] diff --git a/test/definitions/procedures/update_package_test.rb b/test/definitions/procedures/update_package_test.rb index 809cafbfe..de2929ecf 100644 --- a/test/definitions/procedures/update_package_test.rb +++ b/test/definitions/procedures/update_package_test.rb @@ -7,16 +7,16 @@ ForemanMaintain.stubs(:el?).returns(true) procedure = Procedures::Packages::Update.new procedure.expects(:packages_action).with(:update, [], - { :assumeyes => false, :dnf_options => [] }) + { :assumeyes => false, :options => [], :download_only => false }) result = run_procedure(procedure) assert result.success? end it 'updates all packages with --downloadonly' do ForemanMaintain.stubs(:el?).returns(true) - procedure = Procedures::Packages::Update.new(:dnf_options => ['--downloadonly']) + procedure = Procedures::Packages::Update.new(:download_only => true) procedure.expects(:packages_action).with(:update, [], - { :assumeyes => false, :dnf_options => ["--downloadonly"] }) + { :assumeyes => false, :options => [], :download_only => true }) result = run_procedure(procedure) assert result.success? end