Merge branch 'mk/bandaid-gitlab-managed-object-storage-replication-verification-loop' into 'master'

Geo: Fix verification failures of remote stored files

See merge request gitlab-org/gitlab!79303

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

@@ -74,5 +74,9 @@ module Geo
     def in_replicables_for_current_secondary?
       self.class.replicables_for_current_secondary(self).exists?
     end
+
+    def in_available_verifiables?
+      self.class.available_verifiables.primary_key_in(self).exists?
+    end
   end
 end

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

@@ -69,9 +69,33 @@ module Geo
 
     override :after_synced
     def after_synced
+      # If this resource will never become checksummed on the primary (because
+      # e.g. it is a remote stored file), then as a bandaid, mark it as
+      # verification succeeded. This will stop the cycle of:
+      # Sync succeeded => Verification failed => Sync failed => Sync succeeded
+      #
+      # A better fix is proposed in
+      # https://gitlab.com/gitlab-org/gitlab/-/issues/299819
+      if will_never_be_checksummed_on_the_primary?
+        # To ensure we avoid transition errors
+        self.verification_started
+
+        # A checksum value is required by a state machine validation rule, so
+        # set it to zeroes
+        self.verification_checksum = '0000000000000000000000000000000000000000'
+        self.verification_succeeded!
+        return
+      end
+
       self.verification_pending!
     end
 
+    # For example, remote stored files are filtered from available_verifiables
+    # because we don't support verification of remote stored files.
+    def will_never_be_checksummed_on_the_primary?
+      !replicator.model_record.in_available_verifiables?
+    end
+
     override :before_verification_failed
     def before_verification_failed
       # Let verification failure fields get set as usual

ee/spec/models/concerns/geo/replicable_model_spec.rb

@@ -86,4 +86,12 @@ RSpec.describe Geo::ReplicableModel do
       subject.in_replicables_for_current_secondary?
     end
   end
+
+  describe '#in_available_verifiables?' do
+    it 'reuses available_verifiables' do
+      expect(DummyModel).to receive(:available_verifiables).once.and_call_original
+
+      subject.in_available_verifiables?
+    end
+  end
 end

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

@@ -9,13 +9,45 @@ RSpec.shared_examples 'a Geo verifiable registry' do
 
   context 'state machine' do
     context 'when transitioning to synced' do
+      let(:registry) { create(registry_class_factory, :started, :verification_succeeded) }
+
       it 'marks verification as pending' do
-        registry = create(registry_class_factory, :started, :verification_succeeded)
+        # It's likely we will remove will_never_be_checksummed_on_the_primary?
+        # so using `expect` will remind us to remove it here too.
+        expect(registry).to receive(:will_never_be_checksummed_on_the_primary?).and_return(false)
 
         registry.synced!
 
         expect(registry.reload).to be_verification_pending
       end
+
+      context 'band-aid for GitLab managed object storage replication verification loop' do
+        context 'when the model_record will never be checksummed on the primary' do
+          before do
+            allow(registry).to receive(:will_never_be_checksummed_on_the_primary?).and_return(true)
+          end
+
+          context 'when the registry is already verification_succeeded' do
+            let(:registry) { create(registry_class_factory, :started, :verification_succeeded) }
+
+            it 'leaves verification as succeeded' do
+              registry.synced!
+
+              expect(registry.reload).to be_verification_succeeded
+            end
+          end
+
+          context 'when the registry is verification_pending' do
+            let(:registry) { create(registry_class_factory, :started) }
+
+            it 'changes verification to succeeded' do
+              registry.synced!
+
+              expect(registry.reload).to be_verification_succeeded
+            end
+          end
+        end
+      end
     end
   end
 
@@ -253,6 +285,28 @@ RSpec.shared_examples 'a Geo verifiable registry' do
     end
   end
 
+  describe '#will_never_be_checksummed_on_the_primary?' do
+    context 'when the model record is not in available_verifiables' do
+      it 'returns true' do
+        model_record = double('model_record', in_available_verifiables?: false)
+        replicator = double('replicator', model_record: model_record)
+        allow(subject).to receive(:replicator).and_return(replicator)
+
+        expect(subject.will_never_be_checksummed_on_the_primary?).to be_truthy
+      end
+    end
+
+    context 'when the model record is in available_verifiables' do
+      it 'returns false' do
+        model_record = double('model_record', in_available_verifiables?: true)
+        replicator = double('replicator', model_record: model_record)
+        allow(subject).to receive(:replicator).and_return(replicator)
+
+        expect(subject.will_never_be_checksummed_on_the_primary?).to be_falsey
+      end
+    end
+  end
+
   def verification_state_value(key)
     described_class.verification_state_value(key)
   end