Delete Elasticsearch migrations index in rake elastic:delete_index

The migrations Elasticsearch index was recently introduced but we didn't
add anything to our normal lifecycle rake tasks to delete it. This
should have been done initially and will be the expected user
experience.

This task is also used by `gitlab:elastic:index` which is the main way
for resetting your index back to scratch. It makes sense that these all
will clear out all index types including the migrations index. This will
help with any users trying to use this rake task to do a reset of
everything and will also help with local development.

This MR also makes 2 minor refactors to introduce methods
`#delete_migrations_index` and `#migrations_index_exists?` which should
simplify a lot of duplicated calls. Most of the changes in this MR is
refactoring to use these new helper methods.

ee/app/workers/elastic/migration_worker.rb

@@ -24,7 +24,7 @@ module Elastic
           break false
         end
 
-        unless helper.index_exists?(index_name: helper.migrations_index_name)
+        unless helper.migrations_index_exists?
           logger.info 'MigrationWorker: creating migrations index'
           helper.create_migrations_index
         end

ee/changelogs/unreleased/elastic-delete-rake-task-should-delete-migrations-index.yml

+---
+title: Delete Elasticsearch migrations index in rake gitlab:elastic:delete_index
+merge_request: 50817
+author:
+type: added

ee/lib/gitlab/elastic/helper.rb

@@ -141,6 +141,14 @@ module Gitlab
         end
       end
 
+      def delete_migrations_index
+        delete_index(index_name: migrations_index_name)
+      end
+
+      def migrations_index_exists?
+        index_exists?(index_name: migrations_index_name)
+      end
+
       def create_empty_index(with_alias: true, options: {})
         new_index_name = options[:index_name] || "#{target_name}-#{Time.now.strftime("%Y%m%d-%H%M")}"
 

ee/lib/tasks/gitlab/elastic.rake

@@ -78,7 +78,7 @@ namespace :gitlab do
           puts "Alias '#{alias_name}' -> '#{index_name}' has been created.".color(:green)
         end
 
-        helper.create_migrations_index unless helper.index_exists?(index_name: helper.migrations_index_name)
+        helper.create_migrations_index unless helper.migrations_index_exists?
         ::Elastic::DataMigrationService.mark_all_as_completed!
       end
 
@@ -104,6 +104,12 @@ namespace :gitlab do
           puts "Index '#{index_name}' with alias '#{alias_name}' was not found".color(:green)
         end
       end
+
+      if helper.delete_migrations_index
+        puts "Index/alias '#{helper.migrations_index_name}' has been deleted".color(:green)
+      else
+        puts "Index/alias '#{helper.migrations_index_name}' was not found".color(:green)
+      end
     end
 
     desc "GitLab | Elasticsearch | Recreate indexes"

ee/spec/lib/ee/gitlab/elastic/helper_spec.rb

@@ -59,12 +59,12 @@ RSpec.describe Gitlab::Elastic::Helper do
 
   describe '#create_migrations_index' do
     after do
-      helper.delete_index(index_name: helper.migrations_index_name)
+      helper.delete_migrations_index
     end
 
     it 'creates the index' do
       expect { helper.create_migrations_index }
-             .to change { helper.index_exists?(index_name: helper.migrations_index_name) }
+             .to change { helper.migrations_index_exists? }
              .from(false).to(true)
     end
   end
@@ -107,6 +107,18 @@ RSpec.describe Gitlab::Elastic::Helper do
     it_behaves_like 'deletes all standalone indices'
   end
 
+  describe '#delete_migrations_index' do
+    before do
+      helper.create_migrations_index
+    end
+
+    it 'deletes the migrations index' do
+      expect { helper.delete_migrations_index }
+             .to change { helper.migrations_index_exists? }
+             .from(true).to(false)
+    end
+  end
+
   describe '#create_empty_index' do
     context 'with an empty cluster' do
       context 'with alias and index' do
@@ -196,6 +208,26 @@ RSpec.describe Gitlab::Elastic::Helper do
     end
   end
 
+  describe '#migrations_index_exists?' do
+    subject { helper.migrations_index_exists? }
+
+    context 'without an existing migrations index' do
+      before do
+        helper.delete_migrations_index
+      end
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when it exists' do
+      before do
+        helper.create_migrations_index
+      end
+
+      it { is_expected.to be_truthy }
+    end
+  end
+
   describe '#alias_exists?' do
     subject { helper.alias_exists? }
 

ee/spec/lib/elastic/migration_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe Elastic::Migration, :elastic do
       def migrate
         log "number_of_nodes: #{client.cluster.health['number_of_nodes']}"
 
-        raise 'Index does not exist' unless helper.index_exists?(index_name: helper.migrations_index_name)
+        raise 'Index does not exist' unless helper.migrations_index_exists?
       end
     end
   end

ee/spec/models/elastic/migration_record_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe Elastic::MigrationRecord, :elastic do
 
   describe '#save!' do
     it 'creates an index if it is not found' do
-      es_helper.delete_index(index_name: es_helper.migrations_index_name)
+      es_helper.delete_migrations_index
 
       expect { record.save!(completed: true) }.to raise_error(/index is not found/)
     end
@@ -52,7 +52,7 @@ RSpec.describe Elastic::MigrationRecord, :elastic do
     let(:in_progress_migration) { described_class.new(version: 10, name: 10, filename: nil) }
 
     before do
-      es_helper.delete_index(index_name: es_helper.migrations_index_name)
+      es_helper.delete_migrations_index
       es_helper.create_migrations_index
       completed_versions.each { |migration| migration.save!(completed: true) }
       in_progress_migration.save!(completed: false)
@@ -66,7 +66,7 @@ RSpec.describe Elastic::MigrationRecord, :elastic do
     end
 
     it 'returns empty array if no index present' do
-      es_helper.delete_index(index_name: es_helper.migrations_index_name)
+      es_helper.delete_migrations_index
 
       expect(described_class.persisted_versions(completed: true)).to eq([])
       expect(described_class.persisted_versions(completed: false)).to eq([])

ee/spec/services/elastic/data_migration_service_spec.rb

@@ -70,7 +70,7 @@ RSpec.describe Elastic::DataMigrationService, :elastic do
     before do
       # Clear out the migrations index since it is setup initially with
       # everything finished migrating
-      es_helper.delete_index(index_name: es_helper.migrations_index_name)
+      es_helper.delete_migrations_index
       es_helper.create_migrations_index
     end
 

ee/spec/tasks/gitlab/elastic_rake_spec.rb

@@ -12,8 +12,8 @@ RSpec.describe 'gitlab:elastic namespace rake tasks', :elastic do
 
     before do
       es_helper.delete_index
-      es_helper.delete_index(index_name: es_helper.migrations_index_name)
       es_helper.delete_standalone_indices
+      es_helper.delete_migrations_index
     end
 
     it 'creates the default index' do
@@ -97,6 +97,17 @@ RSpec.describe 'gitlab:elastic namespace rake tasks', :elastic do
     it_behaves_like 'deletes all standalone indices' do
       let(:helper) { es_helper }
     end
+
+    it 'removes the migrations index' do
+      expect { subject }.to change { es_helper.migrations_index_exists? }.from(true).to(false)
+    end
+
+    context 'when the index does not exist' do
+      it 'does not error' do
+        run_rake_task('gitlab:elastic:delete_index')
+        run_rake_task('gitlab:elastic:delete_index')
+      end
+    end
   end
 
   context "with elasticsearch_indexing enabled" do

ee/spec/workers/elastic/migration_worker_spec.rb

@@ -27,9 +27,9 @@ RSpec.describe Elastic::MigrationWorker, :elastic do
       end
 
       it 'creates an index if it does not exist' do
-        Gitlab::Elastic::Helper.default.delete_index(index_name: es_helper.migrations_index_name)
+        Gitlab::Elastic::Helper.default.delete_migrations_index
 
-        expect { subject.perform }.to change { Gitlab::Elastic::Helper.default.index_exists?(index_name: es_helper.migrations_index_name) }.from(false).to(true)
+        expect { subject.perform }.to change { Gitlab::Elastic::Helper.default.migrations_index_exists? }.from(false).to(true)
       end
 
       context 'no unexecuted migrations' do