Merge pull request #1261 from SUSE/rmt-cleanup-old-metadata

Rmt cleanup old metadata

lib/rmt/mirror/base.rb

@@ -155,8 +155,9 @@ def cleanup_stale_metadata
     logger.debug("Can not remove stale metadata directory: #{e}")
   end
 
-  def move_files(glob:, destination:)
+  def move_files(glob:, destination:, clean_before: false)
     FileUtils.mkpath(destination) unless Dir.exist?(destination)
+    FileUtils.rm_rf(Dir.glob(File.join(destination, '*'))) if clean_before
     FileUtils.mv(Dir.glob(glob), destination, force: true)
   rescue StandardError => e
     raise RMT::Mirror::Exception.new(_('Error while moving files %{glob} to %{dest}: %{error}') % {

lib/rmt/mirror/repomd.rb

@@ -15,7 +15,7 @@ def mirror_implementation
     mirror_packages(metadata_files)
 
     glob_metadata = File.join(temp(:metadata), 'repodata', '*')
-    move_files(glob: glob_metadata, destination: repository_path('repodata'))
+    move_files(glob: glob_metadata, destination: repository_path('repodata'), clean_before: true)
   end
 
   protected

package/obs/rmt-server.changes

@@ -13,6 +13,7 @@ Wed Oct 30 09:01:32 UTC 2024 - Natnael Getahun <[email protected]>
     * Allow SLE Micro system to access SLES repositories (bsc#1230419)
   * Skip system token rotation in read-only APIs
   * Enable RMT to handle the new dl.suse.com CDN domain (bsc#1234641)
+  * Clean up RMT metadata after repo update (bsc#1233308)
 
 -------------------------------------------------------------------
 Wed Aug 21 15:28:43 UTC 2024 - Jesús Bermúdez Velázquez <[email protected]>

spec/lib/rmt/mirror/base_spec.rb

@@ -230,6 +230,54 @@
 
       expect { base.move_files(glob: src, destination: dest) }.to raise_exception(/Error while moving files/)
     end
+
+    it 'removes existing files when destination directory is empty' do
+      allow(Dir).to receive(:exist?).with(dest).and_return(false)
+      allow(Dir).to receive(:glob).with(src).and_return(%w[/source/path/newfile1.txt /source/path/newfile2.txt])
+      allow(Dir).to receive(:glob).with(File.join(dest, '*')).and_return([])
+      expect(FileUtils).to receive(:mkpath).with(dest)
+      expect(FileUtils).to receive(:rm_rf).with([])
+      expect(FileUtils).to receive(:mv).with(%w[/source/path/newfile1.txt /source/path/newfile2.txt], dest, force: true)
+
+      base.move_files(glob: src, destination: dest, clean_before: true)
+    end
+
+    it 'handles errors during removal of existing files' do
+      allow(Dir).to receive(:exist?).with(dest).and_return(true)
+      existing_files = ['/destination/path/file1.txt']
+
+      allow(Dir).to receive(:glob).with(File.join(dest, '*')).and_return(existing_files)
+      allow(FileUtils).to receive(:rm_rf).with(existing_files).and_raise(StandardError.new('Remove failed'))
+
+      expect do
+        base.move_files(glob: src, destination: dest, clean_before: true)
+      end.to raise_exception(/Error while moving files/)
+    end
+
+    it 'does not remove existing files when clean_before is false' do
+      allow(Dir).to receive(:exist?).with(dest).and_return(true)
+      allow(Dir).to receive(:glob).with(src).and_return(%w[/source/path/newfile1.txt /source/path/newfile2.txt])
+      allow(Dir).to receive(:glob).with(File.join(dest, '*')).and_return(%w[/destination/path/existing1.txt /destination/path/existing2.txt])
+
+      expect(FileUtils).not_to receive(:rm_rf)
+      expect(FileUtils).to receive(:mv).with(%w[/source/path/newfile1.txt /source/path/newfile2.txt], dest, force: true)
+
+      base.move_files(glob: src, destination: dest, clean_before: false)
+    end
+
+    it 'removes existing files when clean_before is true' do
+      allow(Dir).to receive(:exist?).with(dest).and_return(true)
+      existing_files = %w[/destination/path/file1.txt /destination/path/file2.txt]
+      source_files = %w[/source/path/newfile1.txt /source/path/newfile2.txt]
+
+      allow(Dir).to receive(:glob).with(File.join(dest, '*')).and_return(existing_files)
+      allow(Dir).to receive(:glob).with(src).and_return(source_files)
+
+      expect(FileUtils).to receive(:rm_rf).with(existing_files)
+      expect(FileUtils).to receive(:mv).with(source_files, dest, force: true)
+
+      base.move_files(glob: src, destination: dest, clean_before: true)
+    end
   end
 
   describe '#need_to_download?' do

spec/lib/rmt/mirror/repomd_spec.rb

@@ -52,7 +52,7 @@
       expect(licenses).to receive(:mirror)
       expect(repomd).to receive(:mirror_metadata)
       expect(repomd).to receive(:mirror_packages)
-      expect(repomd).to receive(:move_files).with(glob: 'a/repodata/*', destination: repomd.repository_path('repodata'))
+      expect(repomd).to receive(:move_files).with(glob: 'a/repodata/*', destination: repomd.repository_path('repodata'), clean_before: true)
 
       repomd.mirror_implementation
     end
@@ -65,7 +65,7 @@
         expect(repomd).to receive(:create_temp_dir).with(:metadata)
         expect(repomd).to receive(:mirror_metadata)
         expect(repomd).to receive(:mirror_packages)
-        expect(repomd).to receive(:move_files).with(glob: 'a/repodata/*', destination: repomd.repository_path('repodata'))
+        expect(repomd).to receive(:move_files).with(glob: 'a/repodata/*', destination: repomd.repository_path('repodata'), clean_before: true)
 
         expect(licenses).not_to receive(:mirror)