Merge branch '2024-schema-checks' into 'master'

Update schema checks to be aware of all migrations

Update the schema checks of the `wait-for-deps` pattern to be aware
of all migrations, by directly comparing the entries in the appropriate
directories in the application to the table within the database.

This replaces the previous implementation that looked at the latest dated
migration in the codebase versus the database. That check was naive, and
fast, but problematic for several reasons.

This should address multiple factors:
- support concerns of migrations being milestone ordered
- migrations dated in the past
- failed intermediate migrations, from previous upgrades

Changelog: fixed

Closes #2024

See merge request https://gitlab.com/gitlab-org/build/CNG/-/merge_requests/1859



Merged-by: Jason Plum <jplum@gitlab.com>
Approved-by: Krasimir Angelov <kangelov@gitlab.com>
Approved-by: João Alexandre Cunha <j.a.cunha@gmail.com>
Approved-by: Jason Plum <jplum@gitlab.com>
Reviewed-by: Jason Plum <jplum@gitlab.com>
Reviewed-by: João Alexandre Cunha <j.a.cunha@gmail.com>
Reviewed-by: Krasimir Angelov <kangelov@gitlab.com>
Co-authored-by: Jon Jenkins <jjenkins@gitlab.com>

gitlab-rails/scripts/lib/checks/postgresql.rb

@@ -8,7 +8,7 @@ module Checks
   # Usage: `Checks::PostgreSQL.run`
   module PostgreSQL
     @@config = nil
-    @@database_version = 0
+    @@database_versions = []
 
     def self.run
       counter = 1
@@ -33,6 +33,10 @@ module Checks
       ENV['CONFIG_DIRECTORY']
     end
 
+    def self.schema_versions_dir
+      ENV['SCHEMA_VERSIONS_DIR']
+    end
+
     def self.database_file
       ENV['DATABASE_FILE']
     end
@@ -49,13 +53,15 @@ module Checks
       def check_schema_version
         success = database_schema_version
 
-        puts "Database Schema - #{@shard_name} (#{ActiveRecord::Base.connection_db_config.database}) - current: #{@database_version}, codebase: #{codebase_schema_version}"
-        puts 'NOTICE: Database has not been initialized yet.' unless @database_version.to_i.positive?
+        puts "Database Schema - #{@shard_name} (#{ActiveRecord::Base.connection_db_config.database})"
+        puts 'NOTICE: Database has not been initialized yet.' if @database_versions.empty?
         puts "WARNING: schema version check bypassed by #{BYPASS_SCHEMA_VERSION_KEY}='#{ENV[BYPASS_SCHEMA_VERSION_KEY]}'" if bypass_schema_version_check
+        pending_migrations = (Set.new(codebase_schema_versions) - Set.new(@database_versions)).length
+        puts "NOTICE: There are #{pending_migrations} pending migrations." if pending_migrations > 0
 
         return true if (bypass_schema_version_check && success)
 
-        (success && @database_version.to_i >= codebase_schema_version)
+        success && !(pending_migrations > 0)
       rescue => e
         puts "Error checking #{@shard_name}: #{e.message}"
         false
@@ -79,31 +85,33 @@ module Checks
           schema_migrations_table_name = ActiveRecord::Base.schema_migrations_table_name
 
           # if connection is bad, we will get an error (rescue below)
-          # if table exists, fetch version.
-          if connection.table_exists?(schema_migrations_table_name)
-            @database_version =
-              connection.execute("SELECT MAX(version) AS version FROM #{schema_migrations_table_name}")
-                        .first
-                        .fetch('version')
-          # otherwise, set 0
-          else
-            @database_version = 0
+          # if table exists, fetch versions.
+          table_exists = connection.table_exists?(schema_migrations_table_name)
+          @database_versions = []
+
+          if table_exists
+            @database_versions =
+              connection.select_values("SELECT version FROM #{schema_migrations_table_name}").map(&:to_i)
           end
 
           puts "WARNING: Problem accessing #{@shard_name} database (#{ActiveRecord::Base.connection_db_config.database})."\
-               " Confirm username, password, and permissions." if @database_version.nil?
+               " Confirm username, password, and permissions." if @database_versions.empty?
 
           # Returning false prevents bailing when BYPASS_SCHEMA_VERSION set.
-          !@database_version.nil?
+          if table_exists
+            return !@database_versions.empty?
+          end
+          true
         rescue RuntimeError => e
           puts "Error fetching #{@shard_name} schema: #{e.message}"
           false
         end
       end
 
-      def codebase_schema_version
-        # TODO: This may be suspect if there is a separate schema per shard
-        ENV['SCHEMA_VERSION'].to_i
+      def codebase_schema_versions
+        Dir.glob("#{Checks::PostgreSQL::schema_versions_dir}/*").map do |file|
+          File.basename(file, '.rb').split('_').first.to_i
+        end
       end
     end
 

gitlab-rails/scripts/wait-for-deps

@@ -27,15 +27,13 @@ if [ "${DB_SCHEMA_TARGET,,}" == "geo" ]; then
   fi
 fi
 
-# fetch the schema desired version directly from the source code
-SCHEMA_VERSION=$(ls -r -1 "${SCHEMA_DIR}/${SCHEMA_MIGRATIONS_DIR}" | head -1)
-# db/migrate has TIMESTAMP_abc_xyz.rb, so trim everything we don't want
-SCHEMA_VERSION=${SCHEMA_VERSION%%_*}
+# set the desired directory to retrieve schema versions
+SCHEMA_VERSIONS_DIR="${SCHEMA_DIR}/${SCHEMA_MIGRATIONS_DIR}"
 
 # export variables for rails-dependencies
 export WAIT_FOR_TIMEOUT
 export SLEEP_DURATION
-export SCHEMA_VERSION
+export SCHEMA_VERSIONS_DIR
 export DB_SCHEMA_TARGET
 export DATABASE_FILE