diff --git a/.gitlab/issue_templates/Geo Replicate a new Git repository type.md b/.gitlab/issue_templates/Geo Replicate a new Git repository type.md index 5cb99ebcb17ef421ecf07c73d3920a82d19b17e5..71a962d178954e1ab643734f0470dd4ca9cfb5c2 100644 --- a/.gitlab/issue_templates/Geo Replicate a new Git repository type.md +++ b/.gitlab/issue_templates/Geo Replicate a new Git repository type.md @@ -313,6 +313,10 @@ That's all of the required database changes. ::Gitlab::GitAccessCoolWidget end + def self.no_repo_message + git_access_class.error_message(:no_repo) + end + # The feature flag follows the format `geo_#{replicable_name}_replication`, # so here it would be `geo_cool_widget_replication` def self.replication_enabled_by_default? @@ -351,6 +355,9 @@ That's all of the required database changes. ``` - [ ] Make sure a Geo secondary site can request and download Cool Widgets on the Geo primary site. You may need to make some changes to `Gitlab::GitAccessCoolWidget`. For example, see [this change for Group-level Wikis](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/54914/diffs?commit_id=0f2b36f66697b4addbc69bd377ee2818f648dd33). + +- [ ] Make sure a Geo secondary site can replicate Cool Widgets where repository does not exist on the Geo primary site. The only way to know about this is to parse the error text. You may need to make some changes to `Gitlab::CoolWidgetReplicator.no_repo_message` to return the proper error message. For example, see [this change for Group-level Wikis](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74133). + - [ ] Generate the feature flag definition file by running the feature flag command and following the command prompts: ```shell diff --git a/ee/app/replicators/geo/group_wiki_repository_replicator.rb b/ee/app/replicators/geo/group_wiki_repository_replicator.rb index 66a9cacf9776589954663b98bd88203e9957a0b9..7398024f169e490537c8002aaa050b1e490ad0fb 100644 --- a/ee/app/replicators/geo/group_wiki_repository_replicator.rb +++ b/ee/app/replicators/geo/group_wiki_repository_replicator.rb @@ -12,6 +12,10 @@ def self.git_access_class ::Gitlab::GitAccessWiki end + def self.no_repo_message + git_access_class.error_message(:no_group_repo) + end + def repository model_record.repository end diff --git a/ee/app/replicators/geo/snippet_repository_replicator.rb b/ee/app/replicators/geo/snippet_repository_replicator.rb index 891fda1e05457257999c37b749a658d753b1c6bc..8590d6b23e9d7e5410700743286d3cb58db94176 100644 --- a/ee/app/replicators/geo/snippet_repository_replicator.rb +++ b/ee/app/replicators/geo/snippet_repository_replicator.rb @@ -13,6 +13,10 @@ def self.git_access_class ::Gitlab::GitAccessSnippet end + def self.no_repo_message + git_access_class.error_message(:no_repo) + end + override :verification_feature_flag_enabled? def self.verification_feature_flag_enabled? true diff --git a/ee/app/services/geo/framework_repository_sync_service.rb b/ee/app/services/geo/framework_repository_sync_service.rb index 1bd8972ed1aa622ed11f4a6a5e6a76121ed730a4..95e2beda93287eadc3822a9318e3cb20ee900b9d 100644 --- a/ee/app/services/geo/framework_repository_sync_service.rb +++ b/ee/app/services/geo/framework_repository_sync_service.rb @@ -45,9 +45,11 @@ def sync_repository log_info('Expiring caches') repository.after_create rescue Gitlab::Shell::Error, Gitlab::Git::BaseError => e - # In some cases repository does not exist, the only way to know about this is to parse the error text. - # If it does not exist we should consider it as successfully downloaded. - if e.message.include?(replicator.class.git_access_class.error_message(:no_repo)) + # In some cases repository does not exist, the only way to know about this + # is to parse the error text. If the repository does not exist on the + # primary, then the state on this secondary matches the primary, and + # therefore the repository is successfully synced. + if e.message.include?(replicator.class.no_repo_message) log_info('Repository is not found, marking it as successfully synced') mark_sync_as_successful(missing_on_primary: true) else diff --git a/ee/spec/replicators/geo/group_wiki_repository_replicator_spec.rb b/ee/spec/replicators/geo/group_wiki_repository_replicator_spec.rb index 483135a695c0a3ba32306bccf1addcdcbe445801..c49d00dea3ed1612fb164a67d6eb8c0466781778 100644 --- a/ee/spec/replicators/geo/group_wiki_repository_replicator_spec.rb +++ b/ee/spec/replicators/geo/group_wiki_repository_replicator_spec.rb @@ -6,4 +6,10 @@ let(:model_record) { build(:group_wiki_repository, group: create(:group)) } include_examples 'a repository replicator' + + describe '.no_repo_message' do + it 'returns the proper error message for group-level wikis' do + expect(replicator.class.no_repo_message).to eq(::Gitlab::GitAccessWiki.error_message(:no_group_repo)) + end + end end diff --git a/ee/spec/replicators/geo/snippet_repository_replicator_spec.rb b/ee/spec/replicators/geo/snippet_repository_replicator_spec.rb index a10734713473271c4a5e49638554d8fe92ff124e..021a749da30a4e7376a519a147e1847ad289b970 100644 --- a/ee/spec/replicators/geo/snippet_repository_replicator_spec.rb +++ b/ee/spec/replicators/geo/snippet_repository_replicator_spec.rb @@ -8,4 +8,10 @@ include_examples 'a repository replicator' it_behaves_like 'a verifiable replicator' + + describe '.no_repo_message' do + it 'returns the proper error message for snippet repositories' do + expect(replicator.class.no_repo_message).to eq(::Gitlab::GitAccessSnippet.error_message(:no_repo)) + end + end end diff --git a/ee/spec/services/geo/framework_repository_sync_service_spec.rb b/ee/spec/services/geo/framework_repository_sync_service_spec.rb index 8494fbfdc8c541ef7e93a9c903c61bfabcaf6a5d..b778b0850b56b2c33896b05076cce2f9a9415779 100644 --- a/ee/spec/services/geo/framework_repository_sync_service_spec.rb +++ b/ee/spec/services/geo/framework_repository_sync_service_spec.rb @@ -120,6 +120,8 @@ .with(url_to_repo, forced: true, http_authorization_header: anything) .and_raise(Gitlab::Shell::Error.new(Gitlab::GitAccessSnippet::ERROR_MESSAGES[:no_repo])) + expect(replicator.class).to receive(:no_repo_message).once.and_call_original + subject.execute expect(registry).to have_attributes( diff --git a/ee/spec/support/shared_examples/models/concerns/repository_replicator_strategy_shared_examples.rb b/ee/spec/support/shared_examples/models/concerns/repository_replicator_strategy_shared_examples.rb index 842146911f6ec8ca62fefa82853a1c404a2c4863..1f0294ab2e35847bfbff06d03be874a5e78c623f 100644 --- a/ee/spec/support/shared_examples/models/concerns/repository_replicator_strategy_shared_examples.rb +++ b/ee/spec/support/shared_examples/models/concerns/repository_replicator_strategy_shared_examples.rb @@ -128,6 +128,12 @@ end end + describe '.no_repo_message' do + it 'is implemented' do + expect(replicator.class.no_repo_message).to be_a(String) + end + end + describe '#model' do let(:invoke_model) { replicator.class.model }