Commit c5d195e7 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre Committed by Nick Thomas

Resolve "Geo: Wiki repository verification is broken"

parent 5a15ca02
...@@ -2,8 +2,6 @@ module Geo ...@@ -2,8 +2,6 @@ module Geo
class RepositoryVerifySecondaryService class RepositoryVerifySecondaryService
include Gitlab::Geo::ProjectLogHelpers include Gitlab::Geo::ProjectLogHelpers
delegate :project, to: :registry
def initialize(registry, type) def initialize(registry, type)
@registry = registry @registry = registry
@type = type.to_sym @type = type.to_sym
...@@ -21,6 +19,8 @@ module Geo ...@@ -21,6 +19,8 @@ module Geo
attr_reader :registry, :type attr_reader :registry, :type
delegate :project, to: :registry
def should_verify_checksum? def should_verify_checksum?
return false if resync? return false if resync?
...@@ -40,15 +40,15 @@ module Geo ...@@ -40,15 +40,15 @@ module Geo
end end
def verify_checksum def verify_checksum
checksum = project.repository.checksum checksum = repository.checksum
if mismatch?(checksum) if mismatch?(checksum)
update_registry!(failure: "#{type.to_s.capitalize} checksum mismatch: #{repository_path}") update_registry!(failure: "#{type.to_s.capitalize} checksum mismatch: #{repository.disk_path}")
else else
update_registry!(checksum: checksum) update_registry!(checksum: checksum)
end end
rescue ::Gitlab::Git::Repository::NoRepository, ::Gitlab::Git::Repository::ChecksumError, Timeout::Error => e rescue ::Gitlab::Git::Repository::NoRepository, ::Gitlab::Git::Repository::ChecksumError, Timeout::Error => e
update_registry!(failure: "Error verifying #{type.to_s.capitalize} checksum: #{repository_path}", exception: e) update_registry!(failure: "Error verifying #{type.to_s.capitalize} checksum: #{repository.disk_path}", exception: e)
end end
def mismatch?(checksum) def mismatch?(checksum)
...@@ -62,28 +62,18 @@ module Geo ...@@ -62,28 +62,18 @@ module Geo
} }
if failure if failure
log_error(failure, exception, type: type, repository_full_path: path_to_repo) log_error(failure, exception, type: type, repository_full_path: repository.path_to_repo)
end end
registry.update!(attrs) registry.update!(attrs)
end end
def repository_path def repository
case type @repository ||=
when :repository case type
registry.project.disk_path when :repository then project.repository
when :wiki when :wiki then project.wiki.repository
registry.project.wiki.disk_path end
end
end
def path_to_repo
case type
when :repository
project.repository.path_to_repo
when :wiki
project.wiki.repository.path_to_repo
end
end end
end end
end end
---
title: Geo - Fix wiki repository verification on a secondary node
merge_request: 5315
author:
type: fixed
...@@ -10,12 +10,14 @@ describe Geo::RepositoryVerifySecondaryService, :geo do ...@@ -10,12 +10,14 @@ describe Geo::RepositoryVerifySecondaryService, :geo do
end end
shared_examples 'verify checksums for repositories/wikis' do |type| shared_examples 'verify checksums for repositories/wikis' do |type|
let(:repository) { find_repository(type) }
subject(:service) { described_class.new(registry, type) } subject(:service) { described_class.new(registry, type) }
it 'does not calculate the checksum when not running on a secondary' do it 'does not calculate the checksum when not running on a secondary' do
allow(Gitlab::Geo).to receive(:secondary?) { false } allow(Gitlab::Geo).to receive(:secondary?) { false }
expect(registry.project.repository).not_to receive(:checksum) expect(repository).not_to receive(:checksum)
service.execute service.execute
end end
...@@ -23,7 +25,7 @@ describe Geo::RepositoryVerifySecondaryService, :geo do ...@@ -23,7 +25,7 @@ describe Geo::RepositoryVerifySecondaryService, :geo do
it 'does not verify the checksum if resync is needed' do it 'does not verify the checksum if resync is needed' do
registry.assign_attributes("resync_#{type}" => true) registry.assign_attributes("resync_#{type}" => true)
expect(registry.project.repository).not_to receive(:checksum) expect(repository).not_to receive(:checksum)
service.execute service.execute
end end
...@@ -31,7 +33,7 @@ describe Geo::RepositoryVerifySecondaryService, :geo do ...@@ -31,7 +33,7 @@ describe Geo::RepositoryVerifySecondaryService, :geo do
it 'does not verify the checksum if primary was never verified' do it 'does not verify the checksum if primary was never verified' do
repository_state.assign_attributes("#{type}_verification_checksum" => nil) repository_state.assign_attributes("#{type}_verification_checksum" => nil)
expect(registry.project.repository).not_to receive(:checksum) expect(repository).not_to receive(:checksum)
service.execute service.execute
end end
...@@ -40,24 +42,31 @@ describe Geo::RepositoryVerifySecondaryService, :geo do ...@@ -40,24 +42,31 @@ describe Geo::RepositoryVerifySecondaryService, :geo do
repository_state.assign_attributes("#{type}_verification_checksum" => 'my_checksum') repository_state.assign_attributes("#{type}_verification_checksum" => 'my_checksum')
registry.assign_attributes("#{type}_verification_checksum_sha" => 'my_checksum') registry.assign_attributes("#{type}_verification_checksum_sha" => 'my_checksum')
expect(registry.project.repository).not_to receive(:checksum) expect(repository).not_to receive(:checksum)
service.execute service.execute
end end
it 'sets checksum when the checksum matches' do it 'sets checksum when the checksum matches' do
expect(registry.project.repository).to receive(:checksum).and_return('my_checksum') expect(repository).to receive(:checksum).and_return('my_checksum')
expect { service.execute }.to change(registry, "#{type}_verification_checksum_sha") expect { service.execute }.to change(registry, "#{type}_verification_checksum_sha")
.from(nil).to('my_checksum') .from(nil).to('my_checksum')
end end
it 'keeps track of failure when the checksum mismatch' do it 'keeps track of failure when the checksum mismatch' do
expect(registry.project.repository).to receive(:checksum).and_return('other_checksum') expect(repository).to receive(:checksum).and_return('other_checksum')
expect { service.execute }.to change(registry, "last_#{type}_verification_failure") expect { service.execute }.to change(registry, "last_#{type}_verification_failure")
.from(nil).to(/#{Regexp.quote(type.to_s.capitalize)} checksum mismatch/) .from(nil).to(/#{Regexp.quote(type.to_s.capitalize)} checksum mismatch/)
end end
def find_repository(type)
case type
when :repository then project.repository
when :wiki then project.wiki.repository
end
end
end end
describe '#execute' do describe '#execute' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment