Merge branch 'feature/cluster-cleanup-state-machine' into 'master'

Adds Clusters::Cluster#cleanup_status state machine

See merge request gitlab-org/gitlab!18144

app/models/clusters/cluster.rb

@@ -6,6 +6,7 @@ module Clusters
     include Gitlab::Utils::StrongMemoize
     include FromUnion
     include ReactiveCaching
+    include AfterCommitQueue
 
     self.table_name = 'clusters'
 
@@ -126,7 +127,55 @@ module Clusters
       hierarchy_groups.flat_map(&:clusters) + Instance.new.clusters
     end
 
+    state_machine :cleanup_status, initial: :cleanup_not_started do
+      state :cleanup_not_started, value: 1
+      state :cleanup_uninstalling_applications, value: 2
+      state :cleanup_removing_project_namespaces, value: 3
+      state :cleanup_removing_service_account, value: 4
+      state :cleanup_errored, value: 5
+
+      event :start_cleanup do |cluster|
+        transition [:cleanup_not_started, :cleanup_errored] => :cleanup_uninstalling_applications
+      end
+
+      event :continue_cleanup do
+        transition(
+          cleanup_uninstalling_applications: :cleanup_removing_project_namespaces,
+          cleanup_removing_project_namespaces: :cleanup_removing_service_account)
+      end
+
+      event :make_cleanup_errored do
+        transition any => :cleanup_errored
+      end
+
+      before_transition any => [:cleanup_errored] do |cluster, transition|
+        status_reason = transition.args.first
+        cluster.cleanup_status_reason = status_reason if status_reason
+      end
+
+      after_transition [:cleanup_not_started, :cleanup_errored] => :cleanup_uninstalling_applications do |cluster|
+        cluster.run_after_commit do
+          Clusters::Cleanup::AppWorker.perform_async(cluster.id)
+        end
+      end
+
+      after_transition cleanup_uninstalling_applications: :cleanup_removing_project_namespaces do |cluster|
+        cluster.run_after_commit do
+          Clusters::Cleanup::ProjectNamespaceWorker.perform_async(cluster.id)
+        end
+      end
+
+      after_transition cleanup_removing_project_namespaces: :cleanup_removing_service_account do |cluster|
+        cluster.run_after_commit do
+          Clusters::Cleanup::ServiceAccountWorker.perform_async(cluster.id)
+        end
+      end
+    end
+
     def status_name
+      return cleanup_status_name if cleanup_errored?
+      return :cleanup_ongoing unless cleanup_not_started?
+
       provider&.status_name || connection_status.presence || :created
     end
 

app/workers/all_queues.yml

@@ -45,6 +45,9 @@
 - gcp_cluster:cluster_project_configure
 - gcp_cluster:clusters_applications_wait_for_uninstall_app
 - gcp_cluster:clusters_applications_uninstall
+- gcp_cluster:clusters_cleanup_app
+- gcp_cluster:clusters_cleanup_project_namespace
+- gcp_cluster:clusters_cleanup_service_account
 
 - github_import_advance_stage
 - github_importer:github_import_import_diff_note

app/workers/clusters/cleanup/app_worker.rb

+# frozen_string_literal: true
+
+module Clusters
+  module Cleanup
+    class AppWorker
+      include ApplicationWorker
+      include ClusterQueue
+      include ClusterApplications
+
+      # TODO: Merge with https://gitlab.com/gitlab-org/gitlab/merge_requests/16954
+      # We're splitting the above MR in smaller chunks to facilitate reviews
+      def perform
+      end
+    end
+  end
+end

app/workers/clusters/cleanup/project_namespace_worker.rb

+# frozen_string_literal: true
+
+module Clusters
+  module Cleanup
+    class ProjectNamespaceWorker
+      include ApplicationWorker
+      include ClusterQueue
+      include ClusterApplications
+
+      # TODO: Merge with https://gitlab.com/gitlab-org/gitlab/merge_requests/16954
+      # We're splitting the above MR in smaller chunks to facilitate reviews
+      def perform
+      end
+    end
+  end
+end

app/workers/clusters/cleanup/service_account_worker.rb

+# frozen_string_literal: true
+
+module Clusters
+  module Cleanup
+    class ServiceAccountWorker
+      include ApplicationWorker
+      include ClusterQueue
+      include ClusterApplications
+
+      # TODO: Merge with https://gitlab.com/gitlab-org/gitlab/merge_requests/16954
+      # We're splitting the above MR in smaller chunks to facilitate reviews
+      def perform
+      end
+    end
+  end
+end

changelogs/unreleased/feature-cluster-cleanup-state-machine.yml

+---
+title: Add cleanup status to clusters
+merge_request: 18144
+author:
+type: added

db/migrate/20190918104731_add_cleanup_status_to_cluster.rb

+# frozen_string_literal: true
+
+class AddCleanupStatusToCluster < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:clusters, :cleanup_status,
+      :smallint,
+      default: 1,
+      allow_null: false)
+  end
+
+  def down
+    remove_column(:clusters, :cleanup_status)
+  end
+end

db/migrate/20190918121135_add_cleanup_status_reason_to_cluster.rb

+# frozen_string_literal: true
+
+class AddCleanupStatusReasonToCluster < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    add_column :clusters, :cleanup_status_reason, :text
+  end
+end

db/schema.rb

@@ -1041,6 +1041,8 @@ ActiveRecord::Schema.define(version: 2019_10_26_041447) do
     t.boolean "managed", default: true, null: false
     t.boolean "namespace_per_environment", default: true, null: false
     t.integer "management_project_id"
+    t.integer "cleanup_status", limit: 2, default: 1, null: false
+    t.text "cleanup_status_reason"
     t.index ["enabled"], name: "index_clusters_on_enabled"
     t.index ["management_project_id"], name: "index_clusters_on_management_project_id", where: "(management_project_id IS NOT NULL)"
     t.index ["user_id"], name: "index_clusters_on_user_id"

spec/factories/clusters/clusters.rb

@@ -93,5 +93,25 @@ FactoryBot.define do
     trait :not_managed do
       managed { false }
     end
+
+    trait :cleanup_not_started do
+      cleanup_status { 1 }
+    end
+
+    trait :cleanup_uninstalling_applications do
+      cleanup_status { 2 }
+    end
+
+    trait :cleanup_removing_project_namespaces do
+      cleanup_status { 3 }
+    end
+
+    trait :cleanup_removing_service_account do
+      cleanup_status { 4 }
+    end
+
+    trait :cleanup_errored do
+      cleanup_status { 5 }
+    end
   end
 end

spec/models/clusters/cluster_spec.rb

@@ -686,12 +686,36 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
 
     context 'the cluster has a provider' do
       let(:cluster) { create(:cluster, :provided_by_gcp) }
+      let(:provider_status) { :errored }
 
       before do
         cluster.provider.make_errored!
       end
 
-      it { is_expected.to eq :errored }
+      it { is_expected.to eq provider_status }
+
+      context 'when cluster cleanup is ongoing' do
+        using RSpec::Parameterized::TableSyntax
+
+        where(:status_name, :cleanup_status) do
+          provider_status  | :cleanup_not_started
+          :cleanup_ongoing | :cleanup_uninstalling_applications
+          :cleanup_ongoing | :cleanup_removing_project_namespaces
+          :cleanup_ongoing | :cleanup_removing_service_account
+          :cleanup_errored | :cleanup_errored
+        end
+
+        with_them do
+          it 'returns cleanup_ongoing when uninstalling applications' do
+            cluster.cleanup_status = described_class
+              .state_machines[:cleanup_status]
+              .states[cleanup_status]
+              .value
+
+            is_expected.to eq status_name
+          end
+        end
+      end
     end
 
     context 'there is a cached connection status' do
@@ -715,6 +739,83 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     end
   end
 
+  describe 'cleanup_status state_machine' do
+    shared_examples 'cleanup_status transition' do
+      let(:cluster) { create(:cluster, from_state) }
+
+      it 'transitions cleanup_status correctly' do
+        expect { subject }.to change { cluster.cleanup_status_name }
+          .from(from_state).to(to_state)
+      end
+
+      it 'schedules a Clusters::Cleanup::*Worker' do
+        expect(expected_worker_class).to receive(:perform_async).with(cluster.id)
+        subject
+      end
+    end
+
+    describe '#start_cleanup!' do
+      let(:expected_worker_class) { Clusters::Cleanup::AppWorker }
+      let(:to_state) { :cleanup_uninstalling_applications }
+
+      subject { cluster.start_cleanup! }
+
+      context 'when cleanup_status is cleanup_not_started' do
+        let(:from_state) { :cleanup_not_started }
+
+        it_behaves_like 'cleanup_status transition'
+      end
+
+      context 'when cleanup_status is errored' do
+        let(:from_state) { :cleanup_errored }
+
+        it_behaves_like 'cleanup_status transition'
+      end
+    end
+
+    describe '#make_cleanup_errored!' do
+      NON_ERRORED_STATES = Clusters::Cluster.state_machines[:cleanup_status].states.keys - [:cleanup_errored]
+
+      NON_ERRORED_STATES.each do |state|
+        it "transitions cleanup_status from #{state} to cleanup_errored" do
+          cluster = create(:cluster, state)
+
+          expect { cluster.make_cleanup_errored! }.to change { cluster.cleanup_status_name }
+            .from(state).to(:cleanup_errored)
+        end
+
+        it "sets error message" do
+          cluster = create(:cluster, state)
+
+          expect { cluster.make_cleanup_errored!("Error Message") }.to change { cluster.cleanup_status_reason }
+            .from(nil).to("Error Message")
+        end
+      end
+    end
+
+    describe '#continue_cleanup!' do
+      context 'when cleanup_status is cleanup_uninstalling_applications' do
+        let(:expected_worker_class) { Clusters::Cleanup::ProjectNamespaceWorker }
+        let(:from_state) { :cleanup_uninstalling_applications }
+        let(:to_state) { :cleanup_removing_project_namespaces }
+
+        subject { cluster.continue_cleanup! }
+
+        it_behaves_like 'cleanup_status transition'
+      end
+
+      context 'when cleanup_status is cleanup_removing_project_namespaces' do
+        let(:expected_worker_class) { Clusters::Cleanup::ServiceAccountWorker }
+        let(:from_state) { :cleanup_removing_project_namespaces }
+        let(:to_state) { :cleanup_removing_service_account }
+
+        subject { cluster.continue_cleanup! }
+
+        it_behaves_like 'cleanup_status transition'
+      end
+    end
+  end
+
   describe '#connection_status' do
     let(:cluster) { create(:cluster) }
     let(:status) { :connected }