Merge branch '11965-geo-prevent-using-non-empty-tracking-database' into 'master'

Geo - Warn when reusing an existing tracking database

Closes #11965

See merge request gitlab-org/gitlab-ee!14981

db/migrate/20190802195602_add_timestamps_columns_to_geo_nodes.rb

+# frozen_string_literal: true
+
+class AddTimestampsColumnsToGeoNodes < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column(:geo_nodes, :created_at, :datetime_with_timezone, null: true)
+    add_column(:geo_nodes, :updated_at, :datetime_with_timezone, null: true)
+  end
+end

db/schema.rb

@@ -1454,6 +1454,8 @@ ActiveRecord::Schema.define(version: 2019_08_02_235445) do
     t.string "internal_url"
     t.string "name", null: false
     t.integer "container_repositories_max_capacity", default: 10, null: false
+    t.datetime_with_timezone "created_at"
+    t.datetime_with_timezone "updated_at"
     t.index ["access_key"], name: "index_geo_nodes_on_access_key"
     t.index ["name"], name: "index_geo_nodes_on_name", unique: true
     t.index ["primary"], name: "index_geo_nodes_on_primary"

doc/administration/geo/replication/troubleshooting.md

@@ -230,7 +230,7 @@ sudo gitlab-ctl reconfigure
 This will increase the timeout to three hours (10800 seconds). Choose a time
 long enough to accommodate a full clone of your largest repositories.
 
-### Reseting Geo **secondary** node replication
+### Resetting Geo **secondary** node replication
 
 If you get a **secondary** node in a broken state and want to reset the replication state,
 to start again from scratch, there are a few steps that can help you:
@@ -524,6 +524,20 @@ If it doesn't exist or inadvertent changes have been made to it, run `sudo gitla
 
 If this path is mounted on a remote volume, please check your volume configuration and that it has correct permissions.
 
+### An existing tracking database cannot be reused
+
+Geo cannot reuse an existing tracking database.
+
+It is safest to use a fresh secondary, or reset the whole secondary by following
+[Resetting Geo secondary node replication](#resetting-geo-secondary-node-replication).
+
+If you are not concerned about possible orphaned directories and files, then you
+can simply reset the existing tracking database with:
+
+```sh
+sudo gitlab-rake geo:db:reset
+```
+
 ### Geo node has a database that is writable which is an indication it is not configured for replication with the primary node.
 
 This error refers to a problem with the database replica on a **secondary** node,

ee/changelogs/unreleased/11965-geo-prevent-using-non-empty-tracking-database.yml

+---
+title: Geo - Warn when reusing an existing tracking database
+merge_request: 14981
+author:
+type: fixed

ee/db/geo/migrate/20190802200655_add_created_at_to_event_log_states.rb

+# frozen_string_literal: true
+
+class AddCreatedAtToEventLogStates < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def up
+    add_column(:event_log_states, :created_at, :datetime, null: true)
+
+    # There should only be 1 record in the event_log_states table
+    execute('UPDATE event_log_states SET created_at = (
+      SELECT COALESCE(
+        (SELECT project_registry.created_at
+         FROM project_registry
+         ORDER BY project_registry.id ASC
+         LIMIT 1), NOW()
+      )
+    )')
+
+    change_column_null(:event_log_states, :created_at, false)
+  end
+
+  def down
+    remove_column(:event_log_states, :created_at)
+  end
+end

ee/db/geo/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_06_12_211021) do
+ActiveRecord::Schema.define(version: 2019_08_02_200655) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -29,6 +29,7 @@ ActiveRecord::Schema.define(version: 2019_06_12_211021) do
   end
 
   create_table "event_log_states", primary_key: "event_id", force: :cascade do |t|
+    t.datetime "created_at", null: false
   end
 
   create_table "file_registry", id: :serial, force: :cascade do |t|

ee/lib/gitlab/geo/health_check.rb

@@ -8,6 +8,7 @@ module Gitlab
       def perform_checks
         return '' unless Gitlab::Geo.secondary?
         return 'Geo database configuration file is missing.' unless Gitlab::Geo.geo_database_configured?
+        return 'An existing tracking database cannot be reused.' if reusing_existing_tracking_database?
         return 'Geo node has a database that is writable which is an indication it is not configured for replication with the primary node.' unless Gitlab::Database.db_read_only?
         return 'Geo node does not appear to be replicating the database from the primary node.' if replication_enabled? && !replication_working?
         return "Geo database version (#{database_version}) does not match latest migration (#{migration_version}).\nYou may have to run `gitlab-rake geo:db:migrate` as root on the secondary." unless database_migration_version_match?
@@ -42,6 +43,13 @@ module Gitlab
         some_replication_active?
       end
 
+      def reusing_existing_tracking_database?
+        return false unless ::Geo::EventLogState.exists?
+        return false if Gitlab::Geo.current_node.created_at.nil?
+
+        Gitlab::Geo.current_node.created_at.utc > ::Geo::EventLogState.last.created_at.utc
+      end
+
       private
 
       def db_replication_lag_seconds_query

ee/lib/system_check/geo/geo_database_configured_check.rb

@@ -6,9 +6,12 @@ module SystemCheck
       set_name 'GitLab Geo secondary database is correctly configured'
       set_skip_reason 'not a secondary node'
 
+      DATABASE_DOCS = 'doc/gitlab-geo/database.md'.freeze
+      TROUBLESHOOTING_DOCS = 'doc/gitlab-geo/troubleshooting.md'.freeze
       WRONG_CONFIGURATION_MESSAGE = 'Check if you enabled the `geo_secondary_role` or `geo_postgresql` in the gitlab.rb config file.'.freeze
       UNHEALTHY_CONNECTION_MESSAGE = 'Check the tracking database configuration as the connection could not be established'.freeze
       NO_TABLES_MESSAGE = 'Run the tracking database migrations: gitlab-rake geo:db:migrate'.freeze
+      REUSING_EXISTING_DATABASE_MESSAGE = 'If you are reusing an existing tracking database, make sure you have reset it.'.freeze
 
       def skip?
         !Gitlab::Geo.secondary?
@@ -17,30 +20,32 @@ module SystemCheck
       def multi_check
         unless Gitlab::Geo.geo_database_configured?
           $stdout.puts 'no'.color(:red)
-
           try_fixing_it(WRONG_CONFIGURATION_MESSAGE)
-
-          for_more_information('doc/gitlab-geo/database.md')
+          for_more_information(DATABASE_DOCS)
 
           return false
         end
 
         unless connection_healthy?
           $stdout.puts 'no'.color(:red)
-
           try_fixing_it(UNHEALTHY_CONNECTION_MESSAGE)
-
-          for_more_information('doc/gitlab-geo/database.md')
+          for_more_information(DATABASE_DOCS)
 
           return false
         end
 
         unless tables_present?
           $stdout.puts 'no'.color(:red)
-
           try_fixing_it(NO_TABLES_MESSAGE)
+          for_more_information(DATABASE_DOCS)
 
-          for_more_information('doc/gitlab-geo/database.md')
+          return false
+        end
+
+        unless fresh_database?
+          $stdout.puts 'no'.color(:red)
+          try_fixing_it(REUSING_EXISTING_DATABASE_MESSAGE)
+          for_more_information(TROUBLESHOOTING_DOCS)
 
           return false
         end
@@ -58,6 +63,14 @@ module SystemCheck
       def tables_present?
         Gitlab::Geo::DatabaseTasks.with_geo_db { !ActiveRecord::Base.connection.migration_context.needs_migration? }
       end
+
+      def geo_health_check
+        @geo_health_check ||= Gitlab::Geo::HealthCheck.new
+      end
+
+      def fresh_database?
+        !geo_health_check.reusing_existing_tracking_database?
+      end
     end
   end
 end

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

@@ -46,6 +46,14 @@ describe Gitlab::Geo::HealthCheck, :geo do
           end
         end
 
+        context 'when reusing an existing tracking database' do
+          it 'returns an error when event_log_state is older than current node created_at' do
+            create(:geo_event_log_state, created_at: 3.months.ago)
+
+            expect(subject.perform_checks).to include('An existing tracking database cannot be reused.')
+          end
+        end
+
         context 'when the database is writable' do
           let(:db_read_only) { false }
 

ee/spec/lib/system_check/geo/geo_database_configured_check_spec.rb

@@ -35,10 +35,22 @@ describe SystemCheck::Geo::GeoDatabaseConfiguredCheck do
       expect(subject.multi_check).to be_falsey
     end
 
+    it "checks if existing database is being reused" do
+      stub_configuration_check(true)
+      stub_connection_state(true)
+      stub_tables_existence(true)
+      stub_fresh_database(false)
+
+      expect(subject).to receive(:try_fixing_it).with(described_class::REUSING_EXISTING_DATABASE_MESSAGE)
+
+      expect(subject.multi_check).to be_falsey
+    end
+
     it "returns true when all checks passed" do
       stub_configuration_check(true)
       stub_connection_state(true)
       stub_tables_existence(true)
+      stub_fresh_database(true)
 
       expect(subject).not_to receive(:try_fixing_it)
 
@@ -59,4 +71,10 @@ describe SystemCheck::Geo::GeoDatabaseConfiguredCheck do
   def stub_tables_existence(state)
     expect_any_instance_of(ActiveRecord::MigrationContext).to receive(:needs_migration?).and_return(!state)
   end
+
+  def stub_fresh_database(state)
+    expect_next_instance_of(Gitlab::Geo::HealthCheck) do |geo_health_check|
+      expect(geo_health_check).to receive(:reusing_existing_tracking_database?).and_return(!state)
+    end
+  end
 end