Merge branch 'mk/display-verification-progress' into 'master'

Geo Framework: Display verification progress in Admin UI and rake tasks

See merge request gitlab-org/gitlab!52299

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

@@ -19,17 +19,9 @@ module Geo
         enabled? && verification_feature_flag_enabled?
       end
 
-      # Overridden by PackageFileReplicator with its own feature flag so we can
-      # release verification for PackageFileReplicator alone, at first.
-      # This feature flag name is not dynamic like the replication feature flag,
-      # because Geo is proliferating too many permanent feature flags, and if
-      # there is a serious bug with verification that needs to be shut off
-      # immediately, then the replication feature flag can be disabled until it
-      # is fixed. This feature flag is intended to be removed after it is
-      # defaulted on.
-      # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46998 for more
+      # Override this to check a feature flag
       def verification_feature_flag_enabled?
-        Feature.enabled?(:geo_framework_verification)
+        false
       end
 
       # Called every minute by VerificationCronWorker
@@ -108,6 +100,22 @@ module Geo
 
         model.available_replicables.verification_failed.count
       end
+
+      def verified_count
+        # When verification is disabled, this returns nil.
+        # Bonus: This causes the progress bar to be hidden.
+        return unless verification_enabled?
+
+        registry_class.synced.verification_succeeded.count
+      end
+
+      def verification_failed_count
+        # When verification is disabled, this returns nil.
+        # Bonus: This causes the progress bar to be hidden.
+        return unless verification_enabled?
+
+        registry_class.synced.verification_failed.count
+      end
     end
 
     def after_verifiable_update

ee/app/models/geo_node_status.rb

@@ -38,7 +38,9 @@ class GeoNodeStatus < ApplicationRecord
       "#{replicable_class.replicable_name_plural}_checksum_failed_count".to_sym => "Number of #{replicable_class.replicable_title_plural} failed to checksum on primary",
       "#{replicable_class.replicable_name_plural}_synced_count".to_sym => "Number of #{replicable_class.replicable_title_plural} in the registry",
       "#{replicable_class.replicable_name_plural}_failed_count".to_sym => "Number of #{replicable_class.replicable_title_plural} synced on secondary",
-      "#{replicable_class.replicable_name_plural}_registry_count".to_sym => "Number of #{replicable_class.replicable_title_plural} failed to sync on secondary"
+      "#{replicable_class.replicable_name_plural}_registry_count".to_sym => "Number of #{replicable_class.replicable_title_plural} failed to sync on secondary",
+      "#{replicable_class.replicable_name_plural}_verified_count".to_sym => "Number of #{replicable_class.replicable_title_plural} verified on the secondary",
+      "#{replicable_class.replicable_name_plural}_verification_failed_count".to_sym => "Number of #{replicable_class.replicable_title_plural} failed to verify on secondary"
     }
   end
 
@@ -375,8 +377,8 @@ class GeoNodeStatus < ApplicationRecord
   def self.add_attr_in_percentage_for_replicable_classes
     Gitlab::Geo::REPLICATOR_CLASSES.each do |replicator|
       replicable = replicator.replicable_name_plural
-      attr_in_percentage "#{replicable}_checksummed",  "#{replicable}_checksummed_count",  "#{replicable}_count"
       attr_in_percentage "#{replicable}_synced",       "#{replicable}_synced_count",       "#{replicable}_registry_count"
+      attr_in_percentage "#{replicable}_verified",     "#{replicable}_verified_count",     "#{replicable}_registry_count"
     end
   end
 
@@ -400,8 +402,8 @@ class GeoNodeStatus < ApplicationRecord
     public_send("#{replicator_class.replicable_name_plural}_synced_in_percentage") # rubocop:disable GitlabSecurity/PublicSend
   end
 
-  def checksummed_in_percentage_for(replicator_class)
-    public_send("#{replicator_class.replicable_name_plural}_checksummed_in_percentage") # rubocop:disable GitlabSecurity/PublicSend
+  def verified_in_percentage_for(replicator_class)
+    public_send("#{replicator_class.replicable_name_plural}_verified_in_percentage") # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def count_for(replicator_class)
@@ -578,6 +580,11 @@ class GeoNodeStatus < ApplicationRecord
     self.wikis_checksum_mismatch_count = Geo::ProjectRegistry.mismatch(:wiki).count
     self.repositories_retrying_verification_count = Geo::ProjectRegistry.retrying_verification(:repository).count
     self.wikis_retrying_verification_count = Geo::ProjectRegistry.retrying_verification(:wiki).count
+
+    ::Gitlab::Geo.verification_enabled_replicator_classes.each do |replicator|
+      public_send("#{replicator.replicable_name_plural}_verified_count=", replicator.verified_count) # rubocop:disable GitlabSecurity/PublicSend
+      public_send("#{replicator.replicable_name_plural}_verification_failed_count=", replicator.verification_failed_count) # rubocop:disable GitlabSecurity/PublicSend
+    end
   end
 
   def primary_storage_digest

ee/app/replicators/geo/package_file_replicator.rb

@@ -3,13 +3,13 @@
 module Geo
   class PackageFileReplicator < Gitlab::Geo::Replicator
     include ::Geo::BlobReplicatorStrategy
+    extend ::Gitlab::Utils::Override
 
     def self.model
       ::Packages::PackageFile
     end
 
-    # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46998 for
-    # reasoning about this override.
+    override :verification_feature_flag_enabled?
     def self.verification_feature_flag_enabled?
       Feature.enabled?(:geo_package_file_verification)
     end

ee/config/feature_flags/development/geo_framework_verification.yml

----
-name: geo_framework_verification
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46998
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/277400
-milestone: '13.6'
-type: development
-group: group::geo
-default_enabled: false

ee/lib/gitlab/geo/geo_node_status_check.rb

@@ -35,7 +35,7 @@ module Gitlab
         print_design_repositories_status
         print_replicators_status
         print_repositories_checked_status
-        print_replicators_checked_status
+        print_replicators_verification_status
 
         print_sync_settings
         print_db_replication_lag
@@ -57,7 +57,7 @@ module Gitlab
         print_design_repositories_status
         print_replicators_status
         print_repositories_checked_status
-        print_replicators_checked_status
+        print_replicators_verification_status
       end
 
       def replication_verification_complete?
@@ -104,7 +104,7 @@ module Gitlab
             status.push current_node_status.synced_in_percentage_for(replicator_class)
 
             if replicator_class.verification_enabled?
-              status.push current_node_status.checksummed_in_percentage_for(replicator_class)
+              status.push current_node_status.verified_in_percentage_for(replicator_class)
             end
           end
         end
@@ -301,14 +301,14 @@ module Gitlab
         end
       end
 
-      def print_replicators_checked_status
-        verifiable_replicator_classes = Gitlab::Geo.enabled_replicator_classes.select(&:verification_enabled?)
+      def print_replicators_verification_status
+        verifiable_replicator_classes = Gitlab::Geo.verification_enabled_replicator_classes
 
         verifiable_replicator_classes.each do |replicator_class|
-          print "#{replicator_class.replicable_title_plural} Checked: ".rjust(GEO_STATUS_COLUMN_WIDTH)
-          show_failed_value(replicator_class.checksum_failed_count)
-          print "#{replicator_class.checksummed_count}/#{replicator_class.registry_count} "
-          puts using_percentage(current_node_status.checksummed_in_percentage_for(replicator_class))
+          print "#{replicator_class.replicable_title_plural} Verified: ".rjust(GEO_STATUS_COLUMN_WIDTH)
+          show_failed_value(replicator_class.verification_failed_count)
+          print "#{replicator_class.verified_count}/#{replicator_class.registry_count} "
+          puts using_percentage(current_node_status.verified_in_percentage_for(replicator_class))
         end
       end
 

ee/spec/fixtures/api/schemas/public_api/v4/geo_node_status.json

@@ -48,36 +48,44 @@
     "wikis_checksummed_in_percentage",
     "merge_request_diffs_count",
     "merge_request_diffs_checksum_failed_count",
-    "merge_request_diffs_checksummed_in_percentage",
     "merge_request_diffs_checksummed_count",
     "merge_request_diffs_registry_count",
     "merge_request_diffs_failed_count",
     "merge_request_diffs_synced_count",
     "merge_request_diffs_synced_in_percentage",
+    "merge_request_diffs_verification_failed_count",
+    "merge_request_diffs_verified_count",
+    "merge_request_diffs_verified_in_percentage",
     "package_files_count",
     "package_files_checksum_failed_count",
-    "package_files_checksummed_in_percentage",
     "package_files_checksummed_count",
     "package_files_registry_count",
     "package_files_failed_count",
     "package_files_synced_count",
     "package_files_synced_in_percentage",
+    "package_files_verification_failed_count",
+    "package_files_verified_count",
+    "package_files_verified_in_percentage",
     "terraform_state_versions_count",
     "terraform_state_versions_checksum_failed_count",
-    "terraform_state_versions_checksummed_in_percentage",
     "terraform_state_versions_checksummed_count",
     "terraform_state_versions_registry_count",
     "terraform_state_versions_failed_count",
     "terraform_state_versions_synced_count",
     "terraform_state_versions_synced_in_percentage",
+    "terraform_state_versions_verification_failed_count",
+    "terraform_state_versions_verified_count",
+    "terraform_state_versions_verified_in_percentage",
     "snippet_repositories_count",
     "snippet_repositories_checksum_failed_count",
-    "snippet_repositories_checksummed_in_percentage",
     "snippet_repositories_checksummed_count",
     "snippet_repositories_registry_count",
     "snippet_repositories_failed_count",
     "snippet_repositories_synced_count",
     "snippet_repositories_synced_in_percentage",
+    "snippet_repositories_verification_failed_count",
+    "snippet_repositories_verified_count",
+    "snippet_repositories_verified_in_percentage",
     "repositories_verified_count",
     "repositories_verification_failed_count",
     "repositories_verified_in_percentage",
@@ -166,7 +174,9 @@
     "merge_request_diffs_failed_count": { "type": ["integer", "null"] },
     "merge_request_diffs_synced_count": { "type": ["integer", "null"] },
     "merge_request_diffs_synced_in_percentage": { "type": "string" },
-    "merge_request_diffs_checksummed_in_percentage": { "type": "string" },
+    "merge_request_diffs_verification_failed_count": { "type": ["integer", "null"] },
+    "merge_request_diffs_verified_count": { "type": ["integer", "null"] },
+    "merge_request_diffs_verified_in_percentage": { "type": "string" },
     "package_files_count": { "type": ["integer", "null"] },
     "package_files_checksummed_count": { "type": ["integer", "null"] },
     "package_files_checksum_failed_count": { "type": ["integer", "null"] },
@@ -174,7 +184,9 @@
     "package_files_failed_count": { "type": ["integer", "null"] },
     "package_files_synced_count": { "type": ["integer", "null"] },
     "package_files_synced_in_percentage": { "type": "string" },
-    "package_files_checksummed_in_percentage": { "type": "string" },
+    "package_files_verification_failed_count": { "type": ["integer", "null"] },
+    "package_files_verified_count": { "type": ["integer", "null"] },
+    "package_files_verified_in_percentage": { "type": "string" },
     "terraform_state_versions_count": { "type": ["integer", "null"] },
     "terraform_state_versions_checksummed_count": { "type": ["integer", "null"] },
     "terraform_state_versions_checksum_failed_count": { "type": ["integer", "null"] },
@@ -182,7 +194,9 @@
     "terraform_state_versions_failed_count": { "type": ["integer", "null"] },
     "terraform_state_versions_synced_count": { "type": ["integer", "null"] },
     "terraform_state_versions_synced_in_percentage": { "type": "string" },
-    "terraform_state_versions_checksummed_in_percentage": { "type": "string" },
+    "terraform_state_versions_verification_failed_count": { "type": ["integer", "null"] },
+    "terraform_state_versions_verified_count": { "type": ["integer", "null"] },
+    "terraform_state_versions_verified_in_percentage": { "type": "string" },
     "snippet_repositories_count": { "type": ["integer", "null"] },
     "snippet_repositories_checksummed_count": { "type": ["integer", "null"] },
     "snippet_repositories_checksum_failed_count": { "type": ["integer", "null"] },
@@ -190,7 +204,9 @@
     "snippet_repositories_failed_count": { "type": ["integer", "null"] },
     "snippet_repositories_synced_count": { "type": ["integer", "null"] },
     "snippet_repositories_synced_in_percentage": { "type": "string" },
-    "snippet_repositories_checksummed_in_percentage": { "type": "string" },
+    "snippet_repositories_verification_failed_count": { "type": ["integer", "null"] },
+    "snippet_repositories_verified_count": { "type": ["integer", "null"] },
+    "snippet_repositories_verified_in_percentage": { "type": "string" },
     "repositories_verified_count": { "type": ["integer", "null"] },
     "repositories_verification_failed_count": { "type": ["integer", "null"] },
     "repositories_verified_in_percentage": { "type": "string" },

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

@@ -36,9 +36,8 @@ RSpec.describe Gitlab::Geo::GeoNodeStatusCheck do
     end
 
     context 'replicators' do
-      let(:replicators) { Gitlab::Geo.enabled_replicator_classes }
-
       context 'replication' do
+        let(:replicators) { Gitlab::Geo.enabled_replicator_classes }
         let(:checks) do
           replicators.map { |k| /#{k.replicable_title_plural}:/ }
         end
@@ -51,8 +50,9 @@ RSpec.describe Gitlab::Geo::GeoNodeStatusCheck do
       end
 
       context 'verification' do
+        let(:replicators) { Gitlab::Geo.verification_enabled_replicator_classes }
         let(:checks) do
-          replicators.map { |k| /#{k.replicable_title_plural} Checked:/ }
+          replicators.map { |k| /#{k.replicable_title_plural} Verified:/ }
         end
 
         context 'when verification is enabled' do

ee/spec/lib/gitlab/geo_spec.rb

@@ -407,7 +407,6 @@ RSpec.describe Gitlab::Geo, :geo, :request_store do
     context 'when there are no Replicator classes with verification enabled' do
       it 'returns the total capacity' do
         stub_feature_flags(geo_package_file_verification: false)
-        stub_feature_flags(geo_framework_verification: false)
 
         expect(described_class.verification_max_capacity_per_replicator_class).to eq(verification_max_capacity)
       end
@@ -415,8 +414,6 @@ RSpec.describe Gitlab::Geo, :geo, :request_store do
 
     context 'when there is 1 Replicator class with verification enabled' do
       it 'returns half capacity' do
-        stub_feature_flags(geo_framework_verification: false)
-
         expect(described_class.verification_max_capacity_per_replicator_class).to eq(verification_max_capacity / 2)
       end
     end

ee/spec/models/geo_node_status_spec.rb

@@ -1200,86 +1200,159 @@ RSpec.describe GeoNodeStatus, :geo do
       end
 
       context 'verification' do
-        let(:checksummed_count_method) { "#{replicable_name}_checksummed_count" }
-        let(:checksum_failed_count_method) { "#{replicable_name}_checksum_failed_count" }
-        let(:checksummed_in_percentage_method) { "#{replicable_name}_checksummed_in_percentage" }
+        context 'on the primary' do
+          let(:checksummed_count_method) { "#{replicable_name}_checksummed_count" }
+          let(:checksum_failed_count_method) { "#{replicable_name}_checksum_failed_count" }
 
-        context 'when verification is enabled' do
           before do
-            skip "#{replicator.model} does not include the VerificationState concern yet" unless replicator.model.respond_to?(:verification_state)
-
             stub_current_geo_node(primary)
-            allow(replicator).to receive(:verification_enabled?).and_return(true)
           end
 
-          context 'when there are replicables' do
+          context 'when verification is enabled' do
             before do
-              create(model_factory, :verification_succeeded)
-              create(model_factory, :verification_succeeded)
-              create(model_factory, :verification_failed)
+              skip "#{replicator.model} does not include the VerificationState concern yet" unless replicator.model.respond_to?(:verification_state)
+
+              allow(replicator).to receive(:verification_enabled?).and_return(true)
             end
 
-            describe '#<replicable_name>_checksummed_count' do
-              it 'returns the right number of checksummed replicables' do
-                expect(subject.send(checksummed_count_method)).to eq(2)
+            context 'when there are replicables' do
+              before do
+                create(model_factory, :verification_succeeded)
+                create(model_factory, :verification_succeeded)
+                create(model_factory, :verification_failed)
               end
-            end
 
-            describe '#<replicable_name>_checksum_failed_count' do
-              it 'returns the right number of failed replicables' do
-                expect(subject.send(checksum_failed_count_method)).to eq(1)
+              describe '#<replicable_name>_checksummed_count' do
+                it 'returns the right number of checksummed replicables' do
+                  expect(subject.send(checksummed_count_method)).to eq(2)
+                end
+              end
+
+              describe '#<replicable_name>_checksum_failed_count' do
+                it 'returns the right number of failed replicables' do
+                  expect(subject.send(checksum_failed_count_method)).to eq(1)
+                end
               end
             end
 
-            describe '#<replicable_name>_checksummed_in_percentage' do
-              it 'returns the right percentage' do
-                expect(subject.send(checksummed_in_percentage_method)).to be_within(0.01).of(66.67)
+            context 'when there are no replicables' do
+              describe '#<replicable_name>_checksummed_count' do
+                it 'returns 0' do
+                  expect(subject.send(checksummed_count_method)).to eq(0)
+                end
+              end
+
+              describe '#<replicable_name>_checksum_failed_count' do
+                it 'returns 0' do
+                  expect(subject.send(checksum_failed_count_method)).to eq(0)
+                end
               end
             end
           end
 
-          context 'when there are no replicables' do
-            describe '#<replicable_name>_checksummed_count' do
-              it 'returns 0' do
-                expect(subject.send(checksummed_count_method)).to eq(0)
-              end
+          context 'when verification is disabled' do
+            before do
+              allow(replicator).to receive(:verification_enabled?).and_return(false)
             end
 
-            describe '#<replicable_name>_checksum_failed_count' do
-              it 'returns 0' do
-                expect(subject.send(checksum_failed_count_method)).to eq(0)
+            describe '#<replicable_name>_checksummed_count' do
+              it 'returns nil' do
+                expect(subject.send(checksummed_count_method)).to be_nil
               end
             end
 
-            describe '#<replicable_name>_checksummed_in_percentage' do
-              it 'returns 0' do
-                expect(subject.send(checksummed_in_percentage_method)).to eq(0)
+            describe '#<replicable_name>_checksum_failed_count' do
+              it 'returns nil' do
+                expect(subject.send(checksum_failed_count_method)).to be_nil
               end
             end
           end
         end
 
-        context 'when verification is disabled' do
+        context 'on the secondary' do
+          let(:verified_count_method) { "#{replicable_name}_verified_count" }
+          let(:verification_failed_count_method) { "#{replicable_name}_verification_failed_count" }
+          let(:verified_in_percentage_method) { "#{replicable_name}_verified_in_percentage" }
+
           before do
-            stub_current_geo_node(primary)
-            allow(replicator).to receive(:verification_enabled?).and_return(false)
+            stub_current_geo_node(secondary)
           end
 
-          describe '#<replicable_name>_checksummed_count' do
-            it 'returns nil' do
-              expect(subject.send(checksummed_count_method)).to be_nil
+          context 'when verification is enabled' do
+            before do
+              skip "#{replicator.registry_class} does not include the VerificationState concern yet" unless replicator.registry_class.respond_to?(:verification_state)
+
+              allow(replicator).to receive(:verification_enabled?).and_return(true)
             end
-          end
 
-          describe '#<replicable_name>_checksum_failed_count' do
-            it 'returns nil' do
-              expect(subject.send(checksum_failed_count_method)).to be_nil
+            context 'when there are replicables' do
+              before do
+                create(model_factory, :verification_succeeded)
+                create(model_factory, :verification_succeeded)
+                create(model_factory, :verification_failed)
+              end
+
+              describe '#<replicable_name>_verified_count' do
+                it 'returns the right number of checksummed replicables' do
+                  expect(subject.send(verified_count_method)).to eq(2)
+                end
+              end
+
+              describe '#<replicable_name>_verification_failed_count' do
+                it 'returns the right number of failed replicables' do
+                  expect(subject.send(verification_failed_count_method)).to eq(1)
+                end
+              end
+
+              describe '#<replicable_name>_verified_in_percentage' do
+                it 'returns the right percentage' do
+                  expect(subject.send(verified_in_percentage_method)).to be_within(0.01).of(66.67)
+                end
+              end
+            end
+
+            context 'when there are no replicables' do
+              describe '#<replicable_name>_verified_count' do
+                it 'returns 0' do
+                  expect(subject.send(verified_count_method)).to eq(0)
+                end
+              end
+
+              describe '#<replicable_name>_verification_failed_count' do
+                it 'returns 0' do
+                  expect(subject.send(verification_failed_count_method)).to eq(0)
+                end
+              end
+
+              describe '#<replicable_name>_verified_in_percentage' do
+                it 'returns 0' do
+                  expect(subject.send(verified_in_percentage_method)).to eq(0)
+                end
+              end
             end
           end
 
-          describe '#<replicable_name>_checksummed_in_percentage' do
-            it 'returns 0' do
-              expect(subject.send(checksummed_in_percentage_method)).to eq(0)
+          context 'when verification is disabled' do
+            before do
+              allow(replicator).to receive(:verification_enabled?).and_return(false)
+            end
+
+            describe '#<replicable_name>_verified_count' do
+              it 'returns nil' do
+                expect(subject.send(verified_count_method)).to be_nil
+              end
+            end
+
+            describe '#<replicable_name>_verification_failed_count' do
+              it 'returns nil' do
+                expect(subject.send(verification_failed_count_method)).to be_nil
+              end
+            end
+
+            describe '#<replicable_name>_verified_in_percentage' do
+              it 'returns 0' do
+                expect(subject.send(verified_in_percentage_method)).to eq(0)
+              end
             end
           end
         end

ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb

@@ -18,7 +18,7 @@ RSpec.shared_examples 'a verifiable replicator' do
         expect(described_class).to receive(:enabled?).and_return(true)
       end
 
-      context 'when the verification feature flag is enabled' do
+      context 'when verification_feature_flag_enabled? returns true' do
         it 'returns true' do
           allow(described_class).to receive(:verification_feature_flag_enabled?).and_return(true)
 
@@ -26,7 +26,7 @@ RSpec.shared_examples 'a verifiable replicator' do
         end
       end
 
-      context 'when geo_framework_verification feature flag is disabled' do
+      context 'when verification_feature_flag_enabled? returns false' do
         it 'returns false' do
           allow(described_class).to receive(:verification_feature_flag_enabled?).and_return(false)
 

ee/spec/tasks/geo_rake_spec.rb

@@ -364,7 +364,7 @@ RSpec.describe 'geo rake tasks', :geo do
             /Repositories Checked: /,
             /Last event ID seen from primary: /,
             /Last status report was: /
-          ] + Gitlab::Geo.enabled_replicator_classes.map { |k| /#{k.replicable_title_plural} Checked:/ } +
+          ] + Gitlab::Geo.verification_enabled_replicator_classes.map { |k| /#{k.replicable_title_plural} Verified:/ } +
               Gitlab::Geo.enabled_replicator_classes.map { |k| /#{k.replicable_title_plural}:/ }
 
           checks.each do |text|