Merge branch 'ag-bug-fix-verification-counrt' into 'master'

Check if all replication and verification succeeded

See merge request gitlab-org/gitlab!46785

ee/changelogs/unreleased/ag-bug-fix-verification-counrt.yml

+---
+title: Fix bug where primary was promoted even when replication/verification was not
+  complete
+merge_request: 46785
+author:
+type: fixed

ee/lib/gitlab/geo/geo_node_status_check.rb

@@ -61,11 +61,45 @@ module Gitlab
       end
 
       def replication_verification_complete?
-        replication_complete? && verification_complete?
+        success_status = [
+          current_node_status.repositories_synced_in_percentage,
+          current_node_status.repositories_checksummed_in_percentage,
+          current_node_status.wikis_synced_in_percentage,
+          current_node_status.wikis_checksummed_in_percentage,
+          current_node_status.lfs_objects_synced_in_percentage,
+          current_node_status.job_artifacts_synced_in_percentage,
+          current_node_status.attachments_synced_in_percentage,
+          current_node_status.replication_slots_used_in_percentage,
+          current_node_status.design_repositories_synced_in_percentage
+        ] + conditional_checks_status
+
+        success_status.all? { |percentage| percentage == 100 }
       end
 
       private
 
+      def conditional_checks_status
+        [].tap do |status|
+          Gitlab::Geo.enabled_replicator_classes.each do |replicator_class|
+            status.push current_node_status.synced_in_percentage_for(replicator_class)
+            status.push current_node_status.checksummed_in_percentage_for(replicator_class)
+          end
+
+          if Gitlab::Geo.repository_verification_enabled?
+            status.push current_node_status.repositories_verified_in_percentage
+            status.push current_node_status.wikis_verified_in_percentage
+          end
+
+          if ::Geo::ContainerRepositoryRegistry.replication_enabled?
+            status.push current_node_status.container_repositories_synced_in_percentage
+          end
+
+          if Gitlab::CurrentSettings.repository_checks_enabled
+            status.push current_node_status.repositories_checked_in_percentage
+          end
+        end
+      end
+
       def print_current_node_info
         puts
         puts "Name: #{GeoNode.current_node_name}"
@@ -254,52 +288,6 @@ module Gitlab
         end
       end
 
-      def replication_complete?
-        replicables.all? { |failed_count| failed_count == 0 }
-      end
-
-      def verification_complete?
-        verifiables.all? { |failed_count| failed_count == 0 }
-      end
-
-      def replicables
-        [
-          current_node_status.repositories_failed_count,
-          current_node_status.wikis_failed_count,
-          current_node_status.lfs_objects_failed_count,
-          current_node_status.attachments_failed_count,
-          current_node_status.job_artifacts_failed_count,
-          current_node_status.design_repositories_failed_count
-        ].tap do |r|
-          if ::Geo::ContainerRepositoryRegistry.replication_enabled?
-            r.push current_node_status.container_repositories_failed_count
-          end
-
-          Gitlab::Geo.enabled_replicator_classes.each do |replicator_class|
-            r.push replicator_class.failed_count
-          end
-        end
-      end
-
-      def verifiables
-        [].tap do |v|
-          if Gitlab::Geo.repository_verification_enabled?
-            v.push(
-              current_node_status.repositories_verification_failed_count,
-              current_node_status.wikis_verification_failed_count
-            )
-          end
-
-          if Gitlab::CurrentSettings.repository_checks_enabled
-            v.push current_node_status.repositories_checked_failed_count
-          end
-
-          Gitlab::Geo.enabled_replicator_classes.each do |replicator_class|
-            v.push replicator_class.checksum_failed_count
-          end
-        end
-      end
-
       def show_failed_value(value)
         print "#{value}".color(:red) + '/' if value > 0
       end

ee/spec/factories/geo_node_statuses.rb

@@ -77,6 +77,33 @@ FactoryBot.define do
       repositories_verification_failed_count { 0 }
       wikis_verification_failed_count { 0 }
       repositories_checked_failed_count { 0 }
+
+      repositories_synced_count { 10 }
+      repositories_checksummed_count { 10 }
+      repositories_verified_count { 10 }
+      repositories_checked_count { 10 }
+      wikis_synced_count { 10 }
+      wikis_checksummed_count { 10 }
+      wikis_verified_count { 10 }
+      lfs_objects_synced_count { 10 }
+      job_artifacts_synced_count { 10 }
+      attachments_synced_count { 10 }
+      replication_slots_used_count { 10 }
+      container_repositories_synced_count { 10 }
+      design_repositories_synced_count { 10 }
+
+      repositories_count { 10 }
+      wikis_count { 10 }
+      lfs_objects_count { 10 }
+      job_artifacts_count { 10 }
+      attachments_count { 10 }
+      replication_slots_count { 10 }
+      container_repositories_count { 10 }
+      design_repositories_count { 10 }
+
+      GeoNodeStatus.replicator_class_status_fields.each do |field|
+        send(field) { 10 }
+      end
     end
 
     trait :unhealthy do

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

@@ -42,14 +42,8 @@ RSpec.describe Gitlab::Geo::GeoNodeStatusCheck do
     end
 
     context 'when replication is not up-to-date' do
-      it 'returns false when repositories_checked_failed_count is positive' do
-        allow(geo_node_status).to receive(:repositories_checked_failed_count).and_return(1)
-
-        expect(subject.replication_verification_complete?).to be_falsy
-      end
-
-      it 'returns false when there are package files failed to sync' do
-        allow(::Geo::PackageFileReplicator).to receive(:failed_count).and_return(1)
+      it 'returns false when not all replicables were synced' do
+        geo_node_status.update!(repositories_synced_count: 5)
 
         expect(subject.replication_verification_complete?).to be_falsy
       end