diff --git a/features/mirror_spec.rb b/features/mirror_spec.rb index a2a30f77a..f2152de18 100644 --- a/features/mirror_spec.rb +++ b/features/mirror_spec.rb @@ -1,23 +1,37 @@ -require File.expand_path('../support/command_rspec_helper', __FILE__) +require File.expand_path('support/command_rspec_helper', __dir__) describe 'mirror' do - let(:files) { %w'repomd.xml repomd.xml.asc repomd.xml.key *-primary.xml.gz *-filelists.xml.gz *-other.xml.gz' } + let(:path) { '/var/lib/rmt/public/repo/SUSE/Products/SLE-Product-SLES/15-SP5/x86_64' } before do - `/usr/bin/rmt-cli repos enable 3114` + `/usr/bin/rmt-cli repos enable 5664` `/usr/bin/rmt-cli mirror` end after do - `/usr/bin/rmt-cli repos disable 3114` + `/usr/bin/rmt-cli repos disable 5664` # cleanup files - FileUtils.rm_r('/var/lib/rmt/public/repo/SUSE/Updates/SLE-Product-SLES/15') + FileUtils.rm_r(path) end - it do - files.each do |filename_pattern| - expect(`find /var/lib/rmt/public/ -name \'#{filename_pattern}\'`).to include(filename_pattern.gsub('*', '')) + let(:metadata_files) { %w[repomd.xml repomd.xml.asc repomd.xml.key *-primary.xml.gz *-filelists.xml.gz *-other.xml.gz] } + + it 'has valid metadata mirrored' do + metadata_files.each do |filename_pattern| + expect(`find #{File.join(path, 'product', 'repodata')} -maxdepth 1 -name \'#{filename_pattern}\'`).to include(filename_pattern.delete('*')) end end + + let(:license_files) { %w[license.txt directory.yast license.de.txt] } + + it 'has licenses correctly mirrored' do + license_files.each do |filename_pattern| + expect(`find #{File.join(path, 'product.license')} -maxdepth 1 -name \'#{filename_pattern}\'`).to include(filename_pattern.delete('*')) + end + end + + it 'has rpms correctly mirrored' do + expect(`find #{File.join(path, 'product', 'x86_64')} -maxdepth 1 -name sles-release-*.rpm`).to include('sles-release') + end end diff --git a/lib/rmt/mirror/base.rb b/lib/rmt/mirror/base.rb index 03b49454a..c83472a0d 100644 --- a/lib/rmt/mirror/base.rb +++ b/lib/rmt/mirror/base.rb @@ -29,6 +29,7 @@ def mirror raise RMT::Mirror::Exception.new(_('Error while mirroring repository: %{error}' % { error: e.message })) ensure cleanup_temp_dirs + cleanup_stale_metadata end protected @@ -130,18 +131,27 @@ def need_to_download?(ref) true end - def move_directory(source:, destination:) - FileUtils.mv(source, destination, force: true) - FileUtils.chmod(0o755, destination) + def cleanup_stale_metadata + # A bug introduced in 2.16 writes metadata into its own directory if it exists + # resulting in a directory structure like repodata/repodata. + # see: https://github.com/SUSE/rmt/issues/1136 + FileUtils.remove_entry(repository_path('repodata', 'repodata')) if Dir.exist?(repository_path('repodata', 'repodata')) + + # With 1.0.0 a backup mechanism was introduced creating .old_* backups of metadata which was never really used + # we remove these files now from the mirrored repositories + # see: https://github.com/SUSE/rmt/pull/1120/files#diff-69bc4fdeb7aa7ceab24bec11c65a184357e5b71317125516edfa2d819653a969L131 + # NOTE: In an short amount of time we had the .old_* changed to .backup_* but this was never released. + glob_old_backups = Dir.glob(repository_path('.old_*')) + + glob_old_backups.each do |old| + FileUtils.remove_entry(old) + end rescue StandardError => e - raise RMT::Mirror::Exception.new(_('Error while moving directory %{src} to %{dest}: %{error}') % { - src: source, - dest: destination, - error: e.message - }) + logger.debug("Can not remove stale metadata directory: #{e}") end def move_files(glob:, destination:) + FileUtils.mkpath(destination) unless Dir.exist?(destination) FileUtils.mv(Dir.glob(glob), destination, force: true) rescue StandardError => e raise RMT::Mirror::Exception.new(_('Error while moving files %{glob} to %{dest}: %{error}') % { diff --git a/lib/rmt/mirror/license.rb b/lib/rmt/mirror/license.rb index 7b58c88b4..363fb9942 100644 --- a/lib/rmt/mirror/license.rb +++ b/lib/rmt/mirror/license.rb @@ -35,7 +35,8 @@ def mirror_implementation download_enqueued - move_directory(source: temp(:license), destination: repository_path) + glob_licenses = File.join(temp(:license), '*') + move_files(glob: glob_licenses, destination: repository_path) rescue RMT::Downloader::Exception => e raise RMT::Mirror::Exception.new(_('Error while mirroring license files: %{error}') % { error: e.message }) end diff --git a/lib/rmt/mirror/repomd.rb b/lib/rmt/mirror/repomd.rb index 54943f0ce..2374763cd 100644 --- a/lib/rmt/mirror/repomd.rb +++ b/lib/rmt/mirror/repomd.rb @@ -6,7 +6,7 @@ def mirror_implementation create_repository_path create_temp_dir(:metadata) - if repository_url.ends_with?('product') + if repository_url.ends_with?('product/') licenses = RMT::Mirror::License.new(repository: repository, logger: logger, mirroring_base_dir: mirroring_base_dir) licenses.mirror end @@ -14,7 +14,8 @@ def mirror_implementation metadata_files = mirror_metadata mirror_packages(metadata_files) - move_directory(source: File.join(temp(:metadata), 'repodata'), destination: repository_path('repodata')) + glob_metadata = File.join(temp(:metadata), 'repodata', '*') + move_files(glob: glob_metadata, destination: repository_path('repodata')) end protected diff --git a/package/obs/rmt-server.changes b/package/obs/rmt-server.changes index f4c9d68d5..a97d0c635 100644 --- a/package/obs/rmt-server.changes +++ b/package/obs/rmt-server.changes @@ -1,15 +1,11 @@ +------------------------------------------------------------------- Thu April 18 09:27:00 UTC 2024 - Adnilson Delgado - Version 2.17 : - * Improve CLI mirroring summary information by adding the mirror repositories, the file count and size. This fixes issue #702. - -------------------------------------------------------------------- -Thu April 17 15:22:00 UTC 2024 - Yogalakshmi Arunachalam - -- Adding Uptime tracking capability - * Uptime data is included in the data sent to SCC via RMT - * https://jira.suse.com/browse/PED-7982 - * https://jira.suse.com/browse/PED-8018 + * Improve CLI mirroring summary information by adding the mirror repositories, the file count and size. (gh#702) + * Copy metadata content to repodata/ and not create a seperate subdirectory repodata/repodata (gh#1136) + * Adding Uptime tracking capability + * Uptime data is included in the data sent to SCC via RMT (PED-7982, PED-8018) ------------------------------------------------------------------- Thu April 11 15:22:00 UTC 2024 - Felix Schnizlein @@ -20,18 +16,6 @@ Thu April 11 15:22:00 UTC 2024 - Felix Schnizlein directories are obsolete and can be removed savely. * Add support for debian repositories using flat or nested structures (jsc#PED-3684) -------------------------------------------------------------------- -Thu April 07 13:23:00 UTC 2024 - Felix Schnizlein - -- Version 2.16 (unreleased): - * Support bzip2 compressed repositories (bsc#1222122) - -------------------------------------------------------------------- -Thu Mar 07 15:33:00 UTC 2024 - Likhitha Priya - -- Version 2.16: - * Add support for debian repositories using flat or nested structures - ------------------------------------------------------------------- Wed Oct 04 13:23:00 UTC 2023 - Felix Schnizlein diff --git a/spec/lib/rmt/mirror/base_spec.rb b/spec/lib/rmt/mirror/base_spec.rb index ae61a0b97..da66560a4 100644 --- a/spec/lib/rmt/mirror/base_spec.rb +++ b/spec/lib/rmt/mirror/base_spec.rb @@ -36,12 +36,14 @@ it 'mirrors repositories and licenses' do expect(base).to receive(:mirror_implementation) expect(base).to receive(:cleanup_temp_dirs) + expect(base).to receive(:cleanup_stale_metadata) base.mirror end it 'throws an exception if mirroring fails' do allow(base).to receive(:mirror_implementation).and_raise(RMT::Mirror::Exception) expect(base).to receive(:cleanup_temp_dirs) + expect(base).to receive(:cleanup_stale_metadata) expect { base.mirror }.to raise_error(RMT::Mirror::Exception) end end @@ -200,35 +202,27 @@ end end - describe '#move_directory' do - let(:src) { '/source/path' } + describe '#move_files' do + let(:src) { '/source/path/*' } let(:dest) { '/destination/path' } - let(:backup) { '/destination/.backup_path' } - - it 'moves content from source to destination' do - expect(FileUtils).to receive(:mv).with(src, dest, force: true) - expect(FileUtils).to receive(:chmod).with(0o755, dest) - base.move_directory(source: src, destination: dest) - end - - it 'fails on file system errors' do - allow(FileUtils).to receive(:mv).with(src, dest, force: true).and_raise(StandardError) + it 'creates the destination directory if not yet existing' do + allow(Dir).to receive(:exist?).with(dest).and_return(false) + expect(FileUtils).to receive(:mkpath).with(dest) + expect(FileUtils).to receive(:mv).with(Dir.glob(src), dest, force: true) - expect { base.move_directory(source: src, destination: dest) }.to raise_exception(/Error while moving directory/) + base.move_files(glob: src, destination: dest) end - end - - describe '#move_files' do - let(:src) { '/source/path' } - let(:dest) { '/destination/path' } - it 'copies content from source to destination without backup' do + it 'copies content from source to destination' do + allow(Dir).to receive(:exist?).with(dest).and_return(true) expect(FileUtils).to receive(:mv).with(Dir.glob(src), dest, force: true) + base.move_files(glob: src, destination: dest) end it 'fails on file system errors' do + allow(Dir).to receive(:exist?).with(dest).and_return(true) allow(FileUtils).to receive(:mv).with(Dir.glob(src), dest, force: true).and_raise(StandardError) expect { base.move_files(glob: src, destination: dest) }.to raise_exception(/Error while moving files/) @@ -286,4 +280,62 @@ expect(base.need_to_download?(package)).to be(false) end end + + describe '#cleanup_stale_metadata' do + let(:duplicated_repodata_path) { base.repository_path('repodata', 'repodata') } + let(:old_backup_glob) { base.repository_path('.old_*') } + let(:found_old_backup_files) do + [ + base.repository_path('.old_repodata'), + base.repository_path('.old_license') + ] + end + + context 'with repodata/repodata/' do + it 'removes the directory' do + allow(Dir).to receive(:exist?).with(duplicated_repodata_path).and_return(true) + allow(Dir).to receive(:glob).with(old_backup_glob).and_return([]) + + expect(FileUtils).to receive(:remove_entry).with(duplicated_repodata_path) + + base.cleanup_stale_metadata + end + end + + context 'with .old_repodata' do + it 'removes stale .old_* backups' do + allow(Dir).to receive(:exist?).with(duplicated_repodata_path).and_return(false) + allow(Dir).to receive(:glob).with(old_backup_glob).and_return(found_old_backup_files) + + expect(FileUtils).to receive(:remove_entry).with(found_old_backup_files[0]) + expect(FileUtils).to receive(:remove_entry).with(found_old_backup_files[1]) + + base.cleanup_stale_metadata + end + end + + context 'filesystem error' do + it 'does not raise' do + allow(Dir).to receive(:exist?).with(duplicated_repodata_path).and_return(false) + allow(Dir).to receive(:glob).with(old_backup_glob).and_return(found_old_backup_files) + allow(FileUtils).to receive(:remove_entry).with(found_old_backup_files[1]).and_raise(StandardError) + + expect(FileUtils).to receive(:remove_entry).with(found_old_backup_files[0]) + expect(base.logger).to receive(:debug) + + base.cleanup_stale_metadata + end + end + + context 'without stale metadata' do + it 'does not delete any directory' do + allow(Dir).to receive(:exist?).with(duplicated_repodata_path).and_return(false) + allow(Dir).to receive(:glob).with(old_backup_glob).and_return([]) + + expect(FileUtils).not_to receive(:remove_entry) + + base.cleanup_stale_metadata + end + end + end end diff --git a/spec/lib/rmt/mirror/license_spec.rb b/spec/lib/rmt/mirror/license_spec.rb index f92da1234..aaef5c791 100644 --- a/spec/lib/rmt/mirror/license_spec.rb +++ b/spec/lib/rmt/mirror/license_spec.rb @@ -67,7 +67,7 @@ allow(license).to receive(:download_cached!).with('directory.yast', to: '/tmp/foo').and_return(licenses_ref) expect(license).to receive(:download_enqueued) expect(license).to receive(:enqueue).with(duck_type(:local_path)).exactly(11).times - expect(license).to receive(:move_directory).with(source: license.temp(:license), destination: license.repository_path) + expect(license).to receive(:move_files).with(glob: File.join(license.temp(:license), '*'), destination: license.repository_path) license.mirror_implementation end diff --git a/spec/lib/rmt/mirror/repomd_spec.rb b/spec/lib/rmt/mirror/repomd_spec.rb index 22847fc67..5a120ee79 100644 --- a/spec/lib/rmt/mirror/repomd_spec.rb +++ b/spec/lib/rmt/mirror/repomd_spec.rb @@ -5,8 +5,8 @@ let(:logger) { RMT::Logger.new('/dev/null') } - - let(:repository_url) { 'https://updates.suse.com/sample/repository/15.4/product' } + # Remember that RMT forces a trialing slash on all repository URLS! + let(:repository_url) { 'https://updates.suse.com/sample/repository/15.4/product/' } let(:repository) do create :repository, name: 'SUSE Linux Enterprise Server 15 SP4', @@ -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_directory).with(source: 'a/repodata', destination: repomd.repository_path('repodata')) + expect(repomd).to receive(:move_files).with(glob: 'a/repodata/*', destination: repomd.repository_path('repodata')) 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_directory).with(source: 'a/repodata', destination: repomd.repository_path('repodata')) + expect(repomd).to receive(:move_files).with(glob: 'a/repodata/*', destination: repomd.repository_path('repodata')) expect(licenses).not_to receive(:mirror)