Merge branch '301247-geo-successful-sync-should-cause-verification' into 'master'

Geo - Successful sync should cause verification

See merge request gitlab-org/gitlab!63104

.gitlab/issue_templates/Geo Replicate a new Git repository type.md

@@ -513,6 +513,12 @@ That's all of the required database changes.
         last_synced_at { 1.day.ago }
         retry_count { 0 }
       end
+
+      trait :verification_succeeded do
+        verification_checksum { 'e079a831cab27bcda7d81cd9b48296d0c3dd92ef' }
+        verification_state { Geo::CoolWidgetRegistry.verification_state_value(:verification_succeeded) }
+        verified_at { 5.days.ago }
+      end
     end
   end
   ```

.gitlab/issue_templates/Geo Replicate a new blob type.md

@@ -479,6 +479,12 @@ That's all of the required database changes.
         last_synced_at { 1.day.ago }
         retry_count { 0 }
       end
+
+      trait :verification_succeeded do
+        verification_checksum { 'e079a831cab27bcda7d81cd9b48296d0c3dd92ef' }
+        verification_state { Geo::CoolWidgetRegistry.verification_state_value(:verification_succeeded) }
+        verified_at { 5.days.ago }
+      end
     end
   end
   ```

ee/app/models/concerns/geo/replicable_registry.rb

@@ -44,6 +44,11 @@ module Geo::ReplicableRegistry
     end
   end
 
+  # Overridden by Geo::VerifiableRegistry
+  def after_synced
+    # No-op
+  end
+
   def replicator_class
     Gitlab::Geo::Replicator.for_class_name(self)
   end
@@ -86,6 +91,10 @@ module Geo::ReplicableRegistry
         registry.retry_at = nil
       end
 
+      after_transition any => :synced do |registry, _|
+        registry.after_synced
+      end
+
       event :start do
         transition [:pending, :synced, :failed] => :started
       end

ee/app/models/concerns/geo/verifiable_registry.rb

@@ -3,6 +3,7 @@
 module Geo
   module VerifiableRegistry
     extend ActiveSupport::Concern
+    extend ::Gitlab::Utils::Override
     include ::Gitlab::Geo::VerificationState
 
     class_methods do
@@ -38,6 +39,7 @@ module Geo
       extend ::Gitlab::Utils::Override
 
       sha_attribute :verification_checksum_mismatched
+
       scope :available_verifiables, -> { all }
 
       override :clear_verification_failure_fields!
@@ -79,5 +81,10 @@ module Geo
         verification_succeeded_with_checksum!(checksum, calculation_started_at)
       end
     end
+
+    override :after_synced
+    def after_synced
+      self.verification_pending!
+    end
   end
 end

ee/lib/gitlab/geo/verification_state.rb

@@ -22,6 +22,7 @@ module Gitlab
         verification_succeeded: 2,
         verification_failed: 3
       }.freeze
+
       VERIFICATION_TIMEOUT = 8.hours
 
       included do
@@ -54,9 +55,14 @@ module Gitlab
             instance.verification_started_at = Time.current
           end
 
-          before_transition any => :verification_pending do |instance, _|
-            instance.verification_retry_count = 0
-            instance.verification_retry_at = nil
+          before_transition [:verification_pending, :verification_started, :verification_succeeded] => :verification_pending do |instance, _|
+            instance.clear_verification_failure_fields!
+          end
+
+          before_transition verification_failed: :verification_pending do |instance, _|
+            # If transitioning from verification_failed, then don't clear
+            # verification_retry_count and verification_retry_at to ensure
+            # progressive backoff of syncs-due-to-verification-failures
             instance.verification_failure = nil
           end
 
@@ -270,7 +276,7 @@ module Gitlab
         end
       end
 
-      # Overridden by ReplicableRegistry
+      # Overridden by Geo::VerifiableRegistry
       def clear_verification_failure_fields!
         self.verification_retry_count = 0
         self.verification_retry_at = nil

ee/spec/factories/geo/package_file_registry.rb

@@ -22,5 +22,11 @@ FactoryBot.define do
       last_synced_at { 1.day.ago }
       retry_count { 0 }
     end
+
+    trait :verification_succeeded do
+      verification_checksum { 'e079a831cab27bcda7d81cd9b48296d0c3dd92ef' }
+      verification_state { Geo::PackageFileRegistry.verification_state_value(:verification_succeeded) }
+      verified_at { 5.days.ago }
+    end
   end
 end

ee/spec/factories/geo/pipeline_artifact_registry.rb

@@ -22,5 +22,11 @@ FactoryBot.define do
       last_synced_at { 1.day.ago }
       retry_count { 0 }
     end
+
+    trait :verification_succeeded do
+      verification_checksum { 'e079a831cab27bcda7d81cd9b48296d0c3dd92ef' }
+      verification_state { Geo::PipelineArtifactRegistry.verification_state_value(:verification_succeeded) }
+      verified_at { 5.days.ago }
+    end
   end
 end

ee/spec/factories/geo/snippet_repository_registry.rb

@@ -22,5 +22,11 @@ FactoryBot.define do
       last_synced_at { 1.day.ago }
       retry_count { 0 }
     end
+
+    trait :verification_succeeded do
+      verification_checksum { 'e079a831cab27bcda7d81cd9b48296d0c3dd92ef' }
+      verification_state { Geo::SnippetRepositoryRegistry.verification_state_value(:verification_succeeded) }
+      verified_at { 5.days.ago }
+    end
   end
 end

ee/spec/factories/geo/terraform_state_version_registry.rb

@@ -22,5 +22,11 @@ FactoryBot.define do
       last_synced_at { 1.day.ago }
       retry_count { 0 }
     end
+
+    trait :verification_succeeded do
+      verification_checksum { 'e079a831cab27bcda7d81cd9b48296d0c3dd92ef' }
+      verification_state { Geo::TerraformStateVersionRegistry.verification_state_value(:verification_succeeded) }
+      verified_at { 5.days.ago }
+    end
   end
 end

ee/spec/lib/gitlab/geo/verification_state_spec.rb

@@ -23,6 +23,33 @@ RSpec.describe Gitlab::Geo::VerificationState do
 
   subject { DummyModel.new }
 
+  context 'state machine' do
+    context 'when failed' do
+      before do
+        subject.verification_started
+        subject.verification_failed_with_message!('foo')
+      end
+
+      context 'and transitioning to pending' do
+        it 'marks verification as pending' do
+          subject.verification_pending!
+
+          expect(subject.reload.verification_pending?).to be_truthy
+        end
+
+        it 'does not clear retry attributes' do
+          subject.verification_pending!
+
+          expect(subject.reload).to have_attributes(
+            verification_state: DummyModel.verification_state_value(:verification_pending),
+            verification_retry_count: 1,
+            verification_retry_at: be_present
+          )
+        end
+      end
+    end
+  end
+
   describe '.verification_pending_batch' do
     # Insert 2 records for a total of 3 with subject
     let!(:other_pending_records) do

ee/spec/support/shared_examples/models/geo_verifiable_registry_shared_examples.rb

@@ -7,6 +7,18 @@ RSpec.shared_examples 'a Geo verifiable registry' do
 
   subject(:registry_record) { create(registry_class_factory, :synced) }
 
+  context 'state machine' do
+    context 'when transitioning to synced' do
+      it 'marks verification as pending' do
+        registry = create(registry_class_factory, :started, :verification_succeeded)
+
+        registry.synced!
+
+        expect(registry.reload).to be_verification_pending
+      end
+    end
+  end
+
   describe '.verification_pending_batch' do
     before do
       subject.save!