Fixes #37114: Download packages during upgrade before maintenance mode

Only clean package cache when downloading packages
Only clean metadata when cleaning dnf cache

definitions/procedures/packages/update.rb

@@ -7,11 +7,12 @@ 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 :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)
+      package_manager.clean_cache(:assumeyes => assumeyes_val) if @clean_cache
       opts = { :assumeyes => assumeyes_val, :dnf_options => @dnf_options }
       packages_action(:update, @packages, opts)
     rescue ForemanMaintain::Error::ExecutionError => e
@@ -27,11 +28,15 @@ def necessary?
     end
 
     def description
-      if @dnf_options.include?('--downloadonly')
+      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

definitions/scenarios/upgrade_to_capsule_6_16.rb

@@ -38,6 +38,14 @@ class PreMigrations < Abstract
     end
 
     def compose
+      add_step(Procedures::Repositories::Setup.new(:version => '6.16'))
+      modules_to_enable = ["satellite-capsule:#{el_short_name}"]
+      add_step(Procedures::Packages::EnableModules.new(:module_names => modules_to_enable))
+      add_step(Procedures::Packages::Update.new(
+        :assumeyes => true,
+        :dnf_options => ['--downloadonly']
+      ))
+
       add_steps(find_procedures(:pre_migrations))
     end
   end
@@ -53,13 +61,8 @@ def set_context_mapping
     end
 
     def compose
-      add_step(Procedures::Repositories::Setup.new(:version => '6.16'))
-      modules_to_enable = ["satellite-capsule:#{el_short_name}"]
-      add_step(Procedures::Packages::EnableModules.new(:module_names => modules_to_enable))
-      add_step(Procedures::Packages::Update.new(:assumeyes => true,
-        :dnf_options => ['--downloadonly']))
       add_step(Procedures::Service::Stop.new)
-      add_step(Procedures::Packages::Update.new(:assumeyes => true))
+      add_step(Procedures::Packages::Update.new(:assumeyes => true, :clean_cache => false))
       add_step_with_context(Procedures::Installer::Upgrade)
     end
   end

definitions/scenarios/upgrade_to_capsule_6_16_z.rb

@@ -38,6 +38,13 @@ class PreMigrations < Abstract
     end
 
     def compose
+      add_step(Procedures::Repositories::Setup.new(:version => '6.16'))
+      modules_to_enable = ["satellite-capsule:#{el_short_name}"]
+      add_step(Procedures::Packages::EnableModules.new(:module_names => modules_to_enable))
+      add_step(Procedures::Packages::Update.new(
+        :assumeyes => true,
+        :dnf_options => ['--downloadonly']
+      ))
       add_steps(find_procedures(:pre_migrations))
     end
   end
@@ -53,13 +60,8 @@ def set_context_mapping
     end
 
     def compose
-      add_step(Procedures::Repositories::Setup.new(:version => '6.16'))
-      modules_to_enable = ["satellite-capsule:#{el_short_name}"]
-      add_step(Procedures::Packages::EnableModules.new(:module_names => modules_to_enable))
-      add_step(Procedures::Packages::Update.new(:assumeyes => true,
-        :dnf_options => ['--downloadonly']))
       add_step(Procedures::Service::Stop.new)
-      add_step(Procedures::Packages::Update.new(:assumeyes => true))
+      add_step(Procedures::Packages::Update.new(:assumeyes => true, :clean_cache => false))
       add_step_with_context(Procedures::Installer::Upgrade)
     end
   end

definitions/scenarios/upgrade_to_katello_nightly.rb

@@ -54,10 +54,12 @@ def compose
       add_step(Procedures::Repositories::Setup.new(:version => 'nightly'))
       modules_to_enable = ["katello:#{el_short_name}", "pulpcore:#{el_short_name}"]
       add_step(Procedures::Packages::EnableModules.new(:module_names => modules_to_enable))
-      add_step(Procedures::Packages::Update.new(:assumeyes => true,
-        :dnf_options => ['--downloadonly']))
+      add_step(Procedures::Packages::Update.new(
+        :assumeyes => true,
+        :dnf_options => ['--downloadonly']
+      ))
       add_step(Procedures::Service::Stop.new)
-      add_step(Procedures::Packages::Update.new(:assumeyes => true))
+      add_step(Procedures::Packages::Update.new(:assumeyes => true, :clean_cache => false))
       add_step_with_context(Procedures::Installer::Upgrade)
     end
   end

definitions/scenarios/upgrade_to_satellite_6_16.rb

@@ -38,6 +38,13 @@ class PreMigrations < Abstract
     end
 
     def compose
+      add_step(Procedures::Repositories::Setup.new(:version => '6.16'))
+      modules_to_enable = ["satellite:#{el_short_name}"]
+      add_step(Procedures::Packages::EnableModules.new(:module_names => modules_to_enable))
+      add_step(Procedures::Packages::Update.new(
+        :assumeyes => true,
+        :dnf_options => ['--downloadonly']
+      ))
       add_steps(find_procedures(:pre_migrations))
     end
   end
@@ -54,13 +61,8 @@ def set_context_mapping
     end
 
     def compose
-      add_step(Procedures::Repositories::Setup.new(:version => '6.16'))
-      modules_to_enable = ["satellite:#{el_short_name}"]
-      add_step(Procedures::Packages::EnableModules.new(:module_names => modules_to_enable))
-      add_step(Procedures::Packages::Update.new(:assumeyes => true,
-        :dnf_options => ['--downloadonly']))
       add_step(Procedures::Service::Stop.new)
-      add_step(Procedures::Packages::Update.new(:assumeyes => true))
+      add_step(Procedures::Packages::Update.new(:assumeyes => true, :clean_cache => false))
       add_step_with_context(Procedures::Installer::Upgrade)
       add_step(Procedures::Installer::UpgradeRakeTask)
     end

definitions/scenarios/upgrade_to_satellite_6_16_z.rb

@@ -38,6 +38,13 @@ class PreMigrations < Abstract
     end
 
     def compose
+      add_step(Procedures::Repositories::Setup.new(:version => '6.16'))
+      modules_to_enable = ["satellite:#{el_short_name}"]
+      add_step(Procedures::Packages::EnableModules.new(:module_names => modules_to_enable))
+      add_step(Procedures::Packages::Update.new(
+        :assumeyes => true,
+        :dnf_options => ['--downloadonly']
+      ))
       add_steps(find_procedures(:pre_migrations))
     end
   end
@@ -53,13 +60,8 @@ def set_context_mapping
     end
 
     def compose
-      add_step(Procedures::Repositories::Setup.new(:version => '6.16'))
-      modules_to_enable = ["satellite:#{el_short_name}"]
-      add_step(Procedures::Packages::EnableModules.new(:module_names => modules_to_enable))
-      add_step(Procedures::Packages::Update.new(:assumeyes => true,
-        :dnf_options => ['--downloadonly']))
       add_step(Procedures::Service::Stop.new)
-      add_step(Procedures::Packages::Update.new(:assumeyes => true))
+      add_step(Procedures::Packages::Update.new(:assumeyes => true, :clean_cache => false))
       add_step_with_context(Procedures::Installer::Upgrade)
       add_step(Procedures::Installer::UpgradeRakeTask)
     end

lib/foreman_maintain/package_manager/dnf.rb

@@ -92,7 +92,7 @@ def list_installed_packages(queryformat = '%{NAME}-%{VERSION}-%{RELEASE}.%{ARCH}
     end
 
     def clean_cache(assumeyes: false)
-      dnf_action('clean', 'all', :assumeyes => assumeyes)
+      dnf_action('clean', 'metadata', :assumeyes => assumeyes)
     end
 
     def module_enabled?(name)

test/lib/package_manager/dnf_test.rb

@@ -148,7 +148,7 @@ def with_lock_config(protector_enabled: false)
     describe 'clean_cache' do
       it 'invokes dnf to clean cache' do
         expect_execute!(
-          'dnf -y --disableplugin=foreman-protector clean all',
+          'dnf -y --disableplugin=foreman-protector clean metadata',
           :interactive => false
         )
         subject.clean_cache(:assumeyes => true)