Add latest changes from gitlab-org/gitlab@master

12 files changed, 473 insertions, 1 deletions

diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index e576a3d2d40..abc9e728cb3 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -259,6 +259,17 @@ describe Projects::NotesController do
         end
       end
 
+      context 'the note does not have commands_only errors' do
+        context 'for empty note' do
+          let(:note_text) { '' }
+          let(:extra_request_params) { { format: :json } }
+
+          it "returns status 422 for json" do
+            expect(response).to have_gitlab_http_status(422)
+          end
+        end
+      end
+
       context 'the project is a private project' do
         let(:project_visibility) { Gitlab::VisibilityLevel::PRIVATE }
 
diff --git a/spec/factories/clusters/clusters.rb b/spec/factories/clusters/clusters.rb
index 609e7e20187..7121850e5ff 100644
--- a/spec/factories/clusters/clusters.rb
+++ b/spec/factories/clusters/clusters.rb
@@ -90,6 +90,28 @@ FactoryBot.define do
       domain { 'example.com' }
     end
 
+    trait :with_environments do
+      transient do
+        environments { %i(staging production) }
+      end
+
+      cluster_type { Clusters::Cluster.cluster_types[:project_type] }
+
+      before(:create) do |cluster, evaluator|
+        cluster_project = create(:cluster_project, cluster: cluster)
+
+        evaluator.environments.each do |env_name|
+          environment = create(:environment, name: env_name, project: cluster_project.project)
+
+          cluster.kubernetes_namespaces << create(:cluster_kubernetes_namespace,
+            cluster: cluster,
+            cluster_project: cluster_project,
+            project: cluster_project.project,
+            environment: environment)
+        end
+      end
+    end
+
     trait :not_managed do
       managed { false }
     end
diff --git a/spec/helpers/projects/error_tracking_helper_spec.rb b/spec/helpers/projects/error_tracking_helper_spec.rb
index 064b3ad21cb..5e7449e21b7 100644
--- a/spec/helpers/projects/error_tracking_helper_spec.rb
+++ b/spec/helpers/projects/error_tracking_helper_spec.rb
@@ -75,4 +75,21 @@ describe Projects::ErrorTrackingHelper do
       end
     end
   end
+
+  describe '#error_details_data' do
+    let(:issue_id) { 1234 }
+    let(:route_params) { [project.owner, project, issue_id, { format: :json }] }
+    let(:details_path) { details_namespace_project_error_tracking_index_path(*route_params) }
+    let(:stack_trace_path) { stack_trace_namespace_project_error_tracking_index_path(*route_params) }
+
+    let(:result) { helper.error_details_data(project, issue_id) }
+
+    it 'returns the correct details path' do
+      expect(result['issue-details-path']).to eq details_path
+    end
+
+    it 'returns the correct stack trace path' do
+      expect(result['issue-stack-trace-path']).to eq stack_trace_path
+    end
+  end
 end
diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb
index a163229e15a..049db4f7013 100644
--- a/spec/models/clusters/cluster_spec.rb
+++ b/spec/models/clusters/cluster_spec.rb
@@ -500,6 +500,48 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     end
   end
 
+  describe '.with_persisted_applications' do
+    let(:cluster) { create(:cluster) }
+    let!(:helm) { create(:clusters_applications_helm, :installed, cluster: cluster) }
+
+    it 'preloads persisted applications' do
+      query_rec = ActiveRecord::QueryRecorder.new do
+        described_class.with_persisted_applications.find_by_id(cluster.id).application_helm
+      end
+
+      expect(query_rec.count).to eq(1)
+    end
+  end
+
+  describe '#persisted_applications' do
+    let(:cluster) { create(:cluster) }
+
+    subject { cluster.persisted_applications }
+
+    context 'when all applications are created' do
+      let!(:helm) { create(:clusters_applications_helm, cluster: cluster) }
+      let!(:ingress) { create(:clusters_applications_ingress, cluster: cluster) }
+      let!(:cert_manager) { create(:clusters_applications_cert_manager, cluster: cluster) }
+      let!(:prometheus) { create(:clusters_applications_prometheus, cluster: cluster) }
+      let!(:runner) { create(:clusters_applications_runner, cluster: cluster) }
+      let!(:jupyter) { create(:clusters_applications_jupyter, cluster: cluster) }
+      let!(:knative) { create(:clusters_applications_knative, cluster: cluster) }
+
+      it 'returns a list of created applications' do
+        is_expected.to contain_exactly(helm, ingress, cert_manager, prometheus, runner, jupyter, knative)
+      end
+    end
+
+    context 'when not all were created' do
+      let!(:helm) { create(:clusters_applications_helm, cluster: cluster) }
+      let!(:ingress) { create(:clusters_applications_ingress, cluster: cluster) }
+
+      it 'returns a list of created applications' do
+        is_expected.to contain_exactly(helm, ingress)
+      end
+    end
+  end
+
   describe '#applications' do
     set(:cluster) { create(:cluster) }
 
diff --git a/spec/services/clusters/cleanup/app_service_spec.rb b/spec/services/clusters/cleanup/app_service_spec.rb
new file mode 100644
index 00000000000..cc27f409086
--- /dev/null
+++ b/spec/services/clusters/cleanup/app_service_spec.rb
@@ -0,0 +1,117 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Cleanup::AppService do
+  describe '#execute' do
+    let!(:cluster) { create(:cluster, :project, :cleanup_uninstalling_applications, provider_type: :gcp) }
+    let(:service) { described_class.new(cluster) }
+    let(:logger) { service.send(:logger) }
+    let(:log_meta) do
+      {
+        service: described_class.name,
+        cluster_id: cluster.id,
+        execution_count: 0
+      }
+    end
+
+    subject { service.execute }
+
+    shared_examples 'does not reschedule itself' do
+      it 'does not reschedule itself' do
+        expect(Clusters::Cleanup::AppWorker).not_to receive(:perform_in)
+      end
+    end
+
+    context 'when cluster has no applications available or transitioning applications' do
+      it_behaves_like 'does not reschedule itself'
+
+      it 'transitions cluster to cleanup_removing_project_namespaces' do
+        expect { subject }
+          .to change { cluster.reload.cleanup_status_name }
+          .from(:cleanup_uninstalling_applications)
+          .to(:cleanup_removing_project_namespaces)
+      end
+
+      it 'schedules Clusters::Cleanup::ProjectNamespaceWorker' do
+        expect(Clusters::Cleanup::ProjectNamespaceWorker).to receive(:perform_async).with(cluster.id)
+        subject
+      end
+
+      it 'logs all events' do
+        expect(logger).to receive(:info)
+          .with(log_meta.merge(event: :schedule_remove_project_namespaces))
+
+        subject
+      end
+    end
+
+    context 'when cluster has uninstallable applications' do
+      shared_examples 'reschedules itself' do
+        it 'reschedules itself' do
+          expect(Clusters::Cleanup::AppWorker)
+            .to receive(:perform_in)
+            .with(1.minute, cluster.id, 1)
+
+          subject
+        end
+      end
+
+      context 'has applications with dependencies' do
+        let!(:helm) { create(:clusters_applications_helm, :installed, cluster: cluster) }
+        let!(:ingress) { create(:clusters_applications_ingress, :installed, cluster: cluster) }
+        let!(:cert_manager) { create(:clusters_applications_cert_manager, :installed, cluster: cluster) }
+        let!(:jupyter) { create(:clusters_applications_jupyter, :installed, cluster: cluster) }
+
+        it_behaves_like 'reschedules itself'
+
+        it 'only uninstalls apps that are not dependencies for other installed apps' do
+          expect(Clusters::Applications::UninstallWorker)
+            .not_to receive(:perform_async).with(helm.name, helm.id)
+
+          expect(Clusters::Applications::UninstallWorker)
+            .not_to receive(:perform_async).with(ingress.name, ingress.id)
+
+          expect(Clusters::Applications::UninstallWorker)
+            .to receive(:perform_async).with(cert_manager.name, cert_manager.id)
+            .and_call_original
+
+          expect(Clusters::Applications::UninstallWorker)
+            .to receive(:perform_async).with(jupyter.name, jupyter.id)
+            .and_call_original
+
+          subject
+        end
+
+        it 'logs application uninstalls and next execution' do
+          expect(logger).to receive(:info)
+            .with(log_meta.merge(event: :uninstalling_app, application: kind_of(String))).exactly(2).times
+          expect(logger).to receive(:info)
+            .with(log_meta.merge(event: :scheduling_execution, next_execution: 1))
+
+          subject
+        end
+
+        context 'cluster is not cleanup_uninstalling_applications' do
+          let!(:cluster) { create(:cluster, :project, provider_type: :gcp) }
+
+          it_behaves_like 'does not reschedule itself'
+        end
+      end
+
+      context 'when applications are still uninstalling/scheduled/depending on others' do
+        let!(:helm) { create(:clusters_applications_helm, :installed, cluster: cluster) }
+        let!(:ingress) { create(:clusters_applications_ingress, :scheduled, cluster: cluster) }
+        let!(:runner) { create(:clusters_applications_runner, :uninstalling, cluster: cluster) }
+
+        it_behaves_like 'reschedules itself'
+
+        it 'does not call the uninstallation service' do
+          expect(Clusters::Applications::UninstallWorker).not_to receive(:new)
+
+          subject
+        end
+      end
+    end
+  end
+end
diff --git a/spec/services/clusters/cleanup/project_namespace_service_spec.rb b/spec/services/clusters/cleanup/project_namespace_service_spec.rb
new file mode 100644
index 00000000000..22e29cc57d1
--- /dev/null
+++ b/spec/services/clusters/cleanup/project_namespace_service_spec.rb
@@ -0,0 +1,85 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Cleanup::ProjectNamespaceService do
+  describe '#execute' do
+    subject { service.execute }
+
+    let!(:service) { described_class.new(cluster) }
+    let!(:cluster) { create(:cluster, :with_environments, :cleanup_removing_project_namespaces) }
+    let!(:logger) { service.send(:logger) }
+    let(:log_meta) do
+      {
+        service: described_class.name,
+        cluster_id: cluster.id,
+        execution_count: 0
+      }
+    end
+    let(:kubeclient_instance_double) do
+      instance_double(Gitlab::Kubernetes::KubeClient, delete_namespace: nil, delete_service_account: nil)
+    end
+
+    before do
+      allow_any_instance_of(Clusters::Cluster).to receive(:kubeclient).and_return(kubeclient_instance_double)
+    end
+
+    context 'when cluster has namespaces to be deleted' do
+      it 'deletes namespaces from cluster' do
+        expect(kubeclient_instance_double).to receive(:delete_namespace)
+          .with cluster.kubernetes_namespaces[0].namespace
+        expect(kubeclient_instance_double).to receive(:delete_namespace)
+          .with(cluster.kubernetes_namespaces[1].namespace)
+
+        subject
+      end
+
+      it 'deletes namespaces from database' do
+        expect { subject }.to change { cluster.kubernetes_namespaces.exists? }.from(true).to(false)
+      end
+
+      it 'schedules ::ServiceAccountWorker' do
+        expect(Clusters::Cleanup::ServiceAccountWorker).to receive(:perform_async).with(cluster.id)
+        subject
+      end
+
+      it 'logs all events' do
+        expect(logger).to receive(:info)
+          .with(
+            log_meta.merge(
+              event: :deleting_project_namespace,
+              namespace: cluster.kubernetes_namespaces[0].namespace))
+        expect(logger).to receive(:info)
+          .with(
+            log_meta.merge(
+              event: :deleting_project_namespace,
+              namespace: cluster.kubernetes_namespaces[1].namespace))
+
+        subject
+      end
+    end
+
+    context 'when cluster has no namespaces' do
+      let!(:cluster) { create(:cluster, :cleanup_removing_project_namespaces) }
+
+      it 'schedules Clusters::Cleanup::ServiceAccountWorker' do
+        expect(Clusters::Cleanup::ServiceAccountWorker).to receive(:perform_async).with(cluster.id)
+
+        subject
+      end
+
+      it 'transitions to cleanup_removing_service_account' do
+        expect { subject }
+          .to change { cluster.reload.cleanup_status_name }
+          .from(:cleanup_removing_project_namespaces)
+          .to(:cleanup_removing_service_account)
+      end
+
+      it 'does not try to delete namespaces' do
+        expect(kubeclient_instance_double).not_to receive(:delete_namespace)
+
+        subject
+      end
+    end
+  end
+end
diff --git a/spec/services/clusters/cleanup/service_account_service_spec.rb b/spec/services/clusters/cleanup/service_account_service_spec.rb
new file mode 100644
index 00000000000..ecaf0da9fa3
--- /dev/null
+++ b/spec/services/clusters/cleanup/service_account_service_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Cleanup::ServiceAccountService do
+  describe '#execute' do
+    subject { service.execute }
+
+    let!(:service) { described_class.new(cluster) }
+    let!(:cluster) { create(:cluster, :cleanup_removing_service_account) }
+    let!(:logger) { service.send(:logger) }
+    let(:log_meta) do
+      {
+        service: described_class.name,
+        cluster_id: cluster.id,
+        execution_count: 0
+      }
+    end
+    let(:kubeclient_instance_double) do
+      instance_double(Gitlab::Kubernetes::KubeClient, delete_namespace: nil, delete_service_account: nil)
+    end
+
+    before do
+      allow_any_instance_of(Clusters::Cluster).to receive(:kubeclient).and_return(kubeclient_instance_double)
+    end
+
+    it 'deletes gitlab service account' do
+      expect(kubeclient_instance_double).to receive(:delete_service_account)
+        .with(
+          ::Clusters::Kubernetes::GITLAB_SERVICE_ACCOUNT_NAME,
+          ::Clusters::Kubernetes::GITLAB_SERVICE_ACCOUNT_NAMESPACE)
+
+      subject
+    end
+
+    it 'logs all events' do
+      expect(logger).to receive(:info).with(log_meta.merge(event: :deleting_gitlab_service_account))
+      expect(logger).to receive(:info).with(log_meta.merge(event: :destroying_cluster))
+
+      subject
+    end
+
+    it 'deletes cluster' do
+      expect { subject }.to change { Clusters::Cluster.where(id: cluster.id).exists? }.from(true).to(false)
+    end
+  end
+end
diff --git a/spec/services/clusters/destroy_service_spec.rb b/spec/services/clusters/destroy_service_spec.rb
index c0fcc971500..43ebf8f499e 100644
--- a/spec/services/clusters/destroy_service_spec.rb
+++ b/spec/services/clusters/destroy_service_spec.rb
@@ -45,7 +45,7 @@ describe Clusters::DestroyService do
           expect(Clusters::Cluster.where(id: cluster.id).exists?).not_to be_falsey
         end
 
-        it 'transition cluster#cleanup_status from cleanup_not_started to uninstalling_applications' do
+        it 'transition cluster#cleanup_status from cleanup_not_started to cleanup_uninstalling_applications' do
           expect { subject }.to change { cluster.cleanup_status_name }
             .from(:cleanup_not_started)
             .to(:cleanup_uninstalling_applications)
diff --git a/spec/support/shared_examples/models/cluster_cleanup_worker_base_shared_examples.rb b/spec/support/shared_examples/models/cluster_cleanup_worker_base_shared_examples.rb
new file mode 100644
index 00000000000..66bbd908ea8
--- /dev/null
+++ b/spec/support/shared_examples/models/cluster_cleanup_worker_base_shared_examples.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+shared_examples 'cluster cleanup worker base specs' do
+  it 'transitions to errored if sidekiq retries exhausted' do
+    job = { 'args' => [cluster.id, 0], 'jid' => '123' }
+
+    described_class.sidekiq_retries_exhausted_block.call(job)
+
+    expect(cluster.reload.cleanup_status_name).to eq(:cleanup_errored)
+  end
+end
diff --git a/spec/workers/clusters/cleanup/app_worker_spec.rb b/spec/workers/clusters/cleanup/app_worker_spec.rb
new file mode 100644
index 00000000000..29c00db8079
--- /dev/null
+++ b/spec/workers/clusters/cleanup/app_worker_spec.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Cleanup::AppWorker do
+  describe '#perform' do
+    subject { worker_instance.perform(cluster.id) }
+
+    let!(:worker_instance) { described_class.new }
+    let!(:cluster) { create(:cluster, :project, :cleanup_uninstalling_applications, provider_type: :gcp) }
+    let!(:logger) { worker_instance.send(:logger) }
+
+    it_behaves_like 'cluster cleanup worker base specs'
+
+    context 'when exceeded the execution limit' do
+      subject { worker_instance.perform(cluster.id, worker_instance.send(:execution_limit)) }
+
+      let(:worker_instance) { described_class.new }
+      let(:logger) { worker_instance.send(:logger) }
+      let!(:helm) { create(:clusters_applications_helm, :installed, cluster: cluster) }
+      let!(:ingress) { create(:clusters_applications_ingress, :scheduled, cluster: cluster) }
+
+      it 'logs the error' do
+        expect(logger).to receive(:error)
+        .with(
+          hash_including(
+            exception: 'ClusterCleanupMethods::ExceededExecutionLimitError',
+            cluster_id: kind_of(Integer),
+            class_name: described_class.name,
+            applications: "helm:installed,ingress:scheduled",
+            cleanup_status: cluster.cleanup_status_name,
+            event: :failed_to_remove_cluster_and_resources,
+            message: "exceeded execution limit of 10 tries"
+          )
+        )
+
+        subject
+      end
+    end
+  end
+end
diff --git a/spec/workers/clusters/cleanup/project_namespace_worker_spec.rb b/spec/workers/clusters/cleanup/project_namespace_worker_spec.rb
new file mode 100644
index 00000000000..8b6f22e9a61
--- /dev/null
+++ b/spec/workers/clusters/cleanup/project_namespace_worker_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Cleanup::ProjectNamespaceWorker do
+  describe '#perform' do
+    context 'when cluster.cleanup_status is cleanup_removing_project_namespaces' do
+      let!(:cluster) { create(:cluster, :with_environments, :cleanup_removing_project_namespaces) }
+      let!(:worker_instance) { described_class.new }
+      let!(:logger) { worker_instance.send(:logger) }
+
+      it_behaves_like 'cluster cleanup worker base specs'
+
+      it 'calls Clusters::Cleanup::ProjectNamespaceService' do
+        expect_any_instance_of(Clusters::Cleanup::ProjectNamespaceService).to receive(:execute).once
+
+        subject.perform(cluster.id)
+      end
+
+      context 'when exceeded the execution limit' do
+        subject { worker_instance.perform(cluster.id, worker_instance.send(:execution_limit))}
+
+        it 'logs the error' do
+          expect(logger).to receive(:error)
+            .with(
+              hash_including(
+                exception: 'ClusterCleanupMethods::ExceededExecutionLimitError',
+                cluster_id: kind_of(Integer),
+                class_name: described_class.name,
+                applications: "",
+                cleanup_status: cluster.cleanup_status_name,
+                event: :failed_to_remove_cluster_and_resources,
+                message: "exceeded execution limit of 10 tries"
+              )
+            )
+
+          subject
+        end
+      end
+    end
+
+    context 'when cluster.cleanup_status is not cleanup_removing_project_namespaces' do
+      let!(:cluster) { create(:cluster, :with_environments) }
+
+      it 'does not call Clusters::Cleanup::ProjectNamespaceService' do
+        expect(Clusters::Cleanup::ProjectNamespaceService).not_to receive(:new)
+
+        subject.perform(cluster.id)
+      end
+    end
+  end
+end
diff --git a/spec/workers/clusters/cleanup/service_account_worker_spec.rb b/spec/workers/clusters/cleanup/service_account_worker_spec.rb
new file mode 100644
index 00000000000..9af53dd63c1
--- /dev/null
+++ b/spec/workers/clusters/cleanup/service_account_worker_spec.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Cleanup::ServiceAccountWorker do
+  describe '#perform' do
+    let!(:cluster) { create(:cluster, :cleanup_removing_service_account) }
+
+    context 'when cluster.cleanup_status is cleanup_removing_service_account' do
+      it 'calls Clusters::Cleanup::ServiceAccountService' do
+        expect_any_instance_of(Clusters::Cleanup::ServiceAccountService).to receive(:execute).once
+
+        subject.perform(cluster.id)
+      end
+    end
+
+    context 'when cluster.cleanup_status is not cleanup_removing_service_account' do
+      let!(:cluster) { create(:cluster, :with_environments) }
+
+      it 'does not call Clusters::Cleanup::ServiceAccountService' do
+        expect(Clusters::Cleanup::ServiceAccountService).not_to receive(:new)
+
+        subject.perform(cluster.id)
+      end
+    end
+  end
+end