From 43be4d54f3940633ad76e746a9a999c4a9a65870 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Mon, 8 Apr 2019 13:26:05 +1200
Subject: Define state transitions for uninstalling apps

Added :uninstalled state as wasn't sure if we should be destroying the
cluster apps
---
 app/models/clusters/concerns/application_status.rb | 18 ++++-
 spec/factories/clusters/applications/helm.rb       | 21 ++++--
 .../cluster_application_status_shared_examples.rb  | 76 +++++++++++++++++++---
 3 files changed, 98 insertions(+), 17 deletions(-)

diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb
index 1273ed83abe..16679a21e64 100644
--- a/app/models/clusters/concerns/application_status.rb
+++ b/app/models/clusters/concerns/application_status.rb
@@ -25,9 +25,12 @@ module Clusters
           state :updating, value: 4
           state :updated, value: 5
           state :update_errored, value: 6
+          state :uninstalling, value: 7
+          state :uninstall_errored, value: 8
+          state :uninstalled, value: 9
 
           event :make_scheduled do
-            transition [:installable, :errored, :installed, :updated, :update_errored] => :scheduled
+            transition [:installable, :errored, :installed, :updated, :update_errored, :uninstall_errored] => :scheduled
           end
 
           event :make_installing do
@@ -40,8 +43,9 @@ module Clusters
           end
 
           event :make_errored do
-            transition any - [:updating] => :errored
+            transition any - [:updating, :uninstalling] => :errored
             transition [:updating] => :update_errored
+            transition [:uninstalling] => :uninstall_errored
           end
 
           event :make_updating do
@@ -52,6 +56,14 @@ module Clusters
             transition any => :update_errored
           end
 
+          event :make_uninstalling do
+            transition [:scheduled] => :uninstalling
+          end
+
+          event :make_uninstalled do
+            transition [:uninstalling] => :uninstalled
+          end
+
           before_transition any => [:scheduled] do |app_status, _|
             app_status.status_reason = nil
           end
@@ -65,7 +77,7 @@ module Clusters
             app_status.status_reason = nil
           end
 
-          before_transition any => [:update_errored] do |app_status, transition|
+          before_transition any => [:update_errored, :uninstall_errored] do |app_status, transition|
             status_reason = transition.args.first
             app_status.status_reason = status_reason if status_reason
           end
diff --git a/spec/factories/clusters/applications/helm.rb b/spec/factories/clusters/applications/helm.rb
index fe56ac5b71d..ac230950fce 100644
--- a/spec/factories/clusters/applications/helm.rb
+++ b/spec/factories/clusters/applications/helm.rb
@@ -6,6 +6,11 @@ FactoryBot.define do
       status(-2)
     end
 
+    trait :errored do
+      status(-1)
+      status_reason 'something went wrong'
+    end
+
     trait :installable do
       status 0
     end
@@ -30,16 +35,24 @@ FactoryBot.define do
       status 5
     end
 
-    trait :errored do
-      status(-1)
+    trait :update_errored do
+      status(6)
       status_reason 'something went wrong'
     end
 
-    trait :update_errored do
-      status(6)
+    trait :uninstalling do
+      status 7
+    end
+
+    trait :uninstall_errored do
+      status(8)
       status_reason 'something went wrong'
     end
 
+    trait :uninstalled do
+      status 9
+    end
+
     trait :timeouted do
       installing
       updated_at { ClusterWaitForAppInstallationWorker::TIMEOUT.ago }
diff --git a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
index b8c19cab0c4..c56b148cb8c 100644
--- a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
+++ b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
@@ -114,6 +114,17 @@ shared_examples 'cluster application status specs' do |application_name|
           expect(subject.status_reason).to eq(reason)
         end
       end
+
+      context 'application is uninstalling' do
+        subject { create(application_name, :uninstalling) }
+
+        it 'is uninstall_errored' do
+          subject.make_errored(reason)
+
+          expect(subject).to be_uninstall_errored
+          expect(subject.status_reason).to eq(reason)
+        end
+      end
     end
 
     describe '#make_scheduled' do
@@ -125,6 +136,16 @@ shared_examples 'cluster application status specs' do |application_name|
         expect(subject).to be_scheduled
       end
 
+      describe 'when installed' do
+        subject { create(application_name, :installed) }
+
+        it 'is scheduled' do
+          subject.make_scheduled
+
+          expect(subject).to be_scheduled
+        end
+      end
+
       describe 'when was errored' do
         subject { create(application_name, :errored) }
 
@@ -148,6 +169,38 @@ shared_examples 'cluster application status specs' do |application_name|
           expect(subject.status_reason).to be_nil
         end
       end
+
+      describe 'when was uninstall_errored' do
+        subject { create(application_name, :uninstall_errored) }
+
+        it 'clears #status_reason' do
+          expect(subject.status_reason).not_to be_nil
+
+          subject.make_scheduled!
+
+          expect(subject.status_reason).to be_nil
+        end
+      end
+    end
+
+    describe '#make_uninstalling' do
+      subject { create(application_name, :scheduled) }
+
+      it 'is uninstalling' do
+        subject.make_uninstalling!
+
+        expect(subject).to be_uninstalling
+      end
+    end
+
+    describe '#make_uninstalled' do
+      subject { create(application_name, :uninstalling) }
+
+      it 'is uninstalled' do
+        subject.make_uninstalled!
+
+        expect(subject).to be_uninstalled
+      end
     end
   end
 
@@ -155,16 +208,19 @@ shared_examples 'cluster application status specs' do |application_name|
     using RSpec::Parameterized::TableSyntax
 
     where(:trait, :available) do
-      :not_installable  | false
-      :installable      | false
-      :scheduled        | false
-      :installing       | false
-      :installed        | true
-      :updating         | false
-      :updated          | true
-      :errored          | false
-      :update_errored   | false
-      :timeouted        | false
+      :not_installable   | false
+      :installable       | false
+      :scheduled         | false
+      :installing        | false
+      :installed         | true
+      :updating          | false
+      :updated           | true
+      :errored           | false
+      :update_errored    | false
+      :uninstalling      | false
+      :uninstall_errored | false
+      :uninstalled       | false
+      :timeouted         | false
     end
 
     with_them do
-- 
cgit v1.2.1


From 33a765c17a246e4a2376056b1c301707c78806d0 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Wed, 10 Apr 2019 10:57:23 +1200
Subject: Teach Helm::Api about #uninstall

---
 lib/gitlab/kubernetes/helm/api.rb           |  7 +++++++
 spec/lib/gitlab/kubernetes/helm/api_spec.rb | 22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/lib/gitlab/kubernetes/helm/api.rb b/lib/gitlab/kubernetes/helm/api.rb
index 7dfd9ed4f35..ff1dadf9247 100644
--- a/lib/gitlab/kubernetes/helm/api.rb
+++ b/lib/gitlab/kubernetes/helm/api.rb
@@ -22,6 +22,13 @@ module Gitlab
 
         alias_method :update, :install
 
+        def uninstall(command)
+          namespace.ensure_exists!
+
+          delete_pod!(command.pod_name)
+          kubeclient.create_pod(command.pod_resource)
+        end
+
         ##
         # Returns Pod phase
         #
diff --git a/spec/lib/gitlab/kubernetes/helm/api_spec.rb b/spec/lib/gitlab/kubernetes/helm/api_spec.rb
index 8433d40b2ea..24ce397ec3d 100644
--- a/spec/lib/gitlab/kubernetes/helm/api_spec.rb
+++ b/spec/lib/gitlab/kubernetes/helm/api_spec.rb
@@ -33,6 +33,28 @@ describe Gitlab::Kubernetes::Helm::Api do
     end
   end
 
+  describe '#uninstall' do
+    before do
+      allow(client).to receive(:create_pod).and_return(nil)
+      allow(client).to receive(:delete_pod).and_return(nil)
+      allow(namespace).to receive(:ensure_exists!).once
+    end
+
+    it 'ensures the namespace exists before creating the POD' do
+      expect(namespace).to receive(:ensure_exists!).once.ordered
+      expect(client).to receive(:create_pod).once.ordered
+
+      subject.uninstall(command)
+    end
+
+    it 'removes an existing pod before installing' do
+      expect(client).to receive(:delete_pod).with('install-app-name', 'gitlab-managed-apps').once.ordered
+      expect(client).to receive(:create_pod).once.ordered
+
+      subject.uninstall(command)
+    end
+  end
+
   describe '#install' do
     before do
       allow(client).to receive(:create_pod).and_return(nil)
-- 
cgit v1.2.1


From 938e90f47288901790a96c50a8c0dfa2b7eab137 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Wed, 10 Apr 2019 14:50:14 +1200
Subject: Services to uninstall cluster application

+ to monitor progress of uninstallation pod
---
 .../check_uninstall_progress_service.rb            |  62 ++++++++++
 .../clusters/applications/uninstall_service.rb     |  29 +++++
 app/workers/all_queues.yml                         |   1 +
 .../applications/wait_for_uninstall_app_worker.rb  |  20 ++++
 .../check_uninstall_progress_service_spec.rb       | 127 +++++++++++++++++++++
 .../applications/uninstall_service_spec.rb         |  77 +++++++++++++
 .../wait_for_uninstall_app_worker_spec.rb          |  32 ++++++
 7 files changed, 348 insertions(+)
 create mode 100644 app/services/clusters/applications/check_uninstall_progress_service.rb
 create mode 100644 app/services/clusters/applications/uninstall_service.rb
 create mode 100644 app/workers/clusters/applications/wait_for_uninstall_app_worker.rb
 create mode 100644 spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
 create mode 100644 spec/services/clusters/applications/uninstall_service_spec.rb
 create mode 100644 spec/workers/clusters/applications/wait_for_uninstall_app_worker_spec.rb

diff --git a/app/services/clusters/applications/check_uninstall_progress_service.rb b/app/services/clusters/applications/check_uninstall_progress_service.rb
new file mode 100644
index 00000000000..2a2594a30c8
--- /dev/null
+++ b/app/services/clusters/applications/check_uninstall_progress_service.rb
@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+module Clusters
+  module Applications
+    class CheckUninstallProgressService < BaseHelmService
+      def execute
+        return unless app.uninstalling?
+
+        case installation_phase
+        when Gitlab::Kubernetes::Pod::SUCCEEDED
+          on_success
+        when Gitlab::Kubernetes::Pod::FAILED
+          on_failed
+        else
+          check_timeout
+        end
+      rescue Kubeclient::HttpError => e
+        log_error(e)
+
+        app.make_errored!("Kubernetes error: #{e.error_code}")
+      end
+
+      private
+
+      def on_success
+        app.make_uninstalled!
+      ensure
+        remove_installation_pod
+      end
+
+      def on_failed
+        app.make_errored!("Operation failed. Check pod logs for #{pod_name} for more details.")
+      end
+
+      def check_timeout
+        if timeouted?
+          begin
+            app.make_errored!("Operation timed out. Check pod logs for #{pod_name} for more details.")
+          end
+        else
+          WaitForUninstallAppWorker.perform_in(WaitForUninstallAppWorker::INTERVAL, app.name, app.id)
+        end
+      end
+
+      def pod_name
+        app.uninstall_command.pod_name
+      end
+
+      def timeouted?
+        Time.now.utc - app.updated_at.to_time.utc > WaitForUninstallAppWorker::TIMEOUT
+      end
+
+      def remove_installation_pod
+        helm_api.delete_pod!(pod_name)
+      end
+
+      def installation_phase
+        helm_api.status(pod_name)
+      end
+    end
+  end
+end
diff --git a/app/services/clusters/applications/uninstall_service.rb b/app/services/clusters/applications/uninstall_service.rb
new file mode 100644
index 00000000000..50c8d806c14
--- /dev/null
+++ b/app/services/clusters/applications/uninstall_service.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module Clusters
+  module Applications
+    class UninstallService < BaseHelmService
+      def execute
+        return unless app.scheduled?
+
+        app.make_uninstalling!
+        uninstall
+      end
+
+      private
+
+      def uninstall
+        helm_api.uninstall(app.uninstall_command)
+
+        Clusters::Applications::WaitForUninstallAppWorker.perform_in(
+          Clusters::Applications::WaitForUninstallAppWorker::INTERVAL, app.name, app.id)
+      rescue Kubeclient::HttpError => e
+        log_error(e)
+        app.make_errored!("Kubernetes error: #{e.error_code}")
+      rescue StandardError => e
+        log_error(e)
+        app.make_errored!('Failed to uninstall.')
+      end
+    end
+  end
+end
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index d01bbbf269e..0d14d313d21 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -32,6 +32,7 @@
 - gcp_cluster:cluster_wait_for_ingress_ip_address
 - gcp_cluster:cluster_configure
 - gcp_cluster:cluster_project_configure
+- gcp_cluster:clusters_applications_wait_for_uninstall_app
 
 - github_import_advance_stage
 - github_importer:github_import_import_diff_note
diff --git a/app/workers/clusters/applications/wait_for_uninstall_app_worker.rb b/app/workers/clusters/applications/wait_for_uninstall_app_worker.rb
new file mode 100644
index 00000000000..163c99d3c3c
--- /dev/null
+++ b/app/workers/clusters/applications/wait_for_uninstall_app_worker.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module Clusters
+  module Applications
+    class WaitForUninstallAppWorker
+      include ApplicationWorker
+      include ClusterQueue
+      include ClusterApplications
+
+      INTERVAL = 10.seconds
+      TIMEOUT = 20.minutes
+
+      def perform(app_name, app_id)
+        find_application(app_name, app_id) do |app|
+          Clusters::Applications::CheckUninstallProgressService.new(app).execute
+        end
+      end
+    end
+  end
+end
diff --git a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
new file mode 100644
index 00000000000..ccae7fd133f
--- /dev/null
+++ b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
@@ -0,0 +1,127 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Applications::CheckUninstallProgressService do
+  RESCHEDULE_PHASES = Gitlab::Kubernetes::Pod::PHASES - [Gitlab::Kubernetes::Pod::SUCCEEDED, Gitlab::Kubernetes::Pod::FAILED].freeze
+
+  let(:application) { create(:clusters_applications_helm, :uninstalling) }
+  let(:service) { described_class.new(application) }
+  let(:phase) { Gitlab::Kubernetes::Pod::UNKNOWN }
+  let(:errors) { nil }
+  let(:worker_class) { Clusters::Applications::WaitForUninstallAppWorker }
+
+  shared_examples 'a not yet terminated installation' do |a_phase|
+    let(:phase) { a_phase }
+
+    before do
+      expect(service).to receive(:installation_phase).once.and_return(phase)
+    end
+
+    context "when phase is #{a_phase}" do
+      context 'when not timeouted' do
+        it 'reschedule a new check' do
+          expect(worker_class).to receive(:perform_in).once
+          expect(service).not_to receive(:remove_installation_pod)
+
+          expect do
+            service.execute
+
+            application.reload
+          end.not_to change(application, :status)
+
+          expect(application.status_reason).to be_nil
+        end
+      end
+    end
+  end
+
+  before do
+    allow(service).to receive(:installation_errors).and_return(errors)
+    allow(service).to receive(:remove_installation_pod).and_return(nil)
+  end
+
+  context 'when application is installing' do
+    RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated installation', phase }
+
+    context 'when installation POD succeeded' do
+      let(:phase) { Gitlab::Kubernetes::Pod::SUCCEEDED }
+      before do
+        expect(service).to receive(:installation_phase).once.and_return(phase)
+      end
+
+      it 'removes the installation POD' do
+        expect(service).to receive(:remove_installation_pod).once
+
+        service.execute
+      end
+
+      it 'make the application installed' do
+        expect(worker_class).not_to receive(:perform_in)
+
+        service.execute
+
+        expect(application).to be_uninstalled
+        expect(application.status_reason).to be_nil
+      end
+    end
+
+    context 'when installation POD failed' do
+      let(:phase) { Gitlab::Kubernetes::Pod::FAILED }
+      let(:errors) { 'test installation failed' }
+
+      before do
+        expect(service).to receive(:installation_phase).once.and_return(phase)
+      end
+
+      it 'make the application errored' do
+        service.execute
+
+        expect(application).to be_uninstall_errored
+        expect(application.status_reason).to eq('Operation failed. Check pod logs for uninstall-helm for more details.')
+      end
+    end
+
+    context 'when timed out' do
+      let(:application) { create(:clusters_applications_helm, :timeouted, :uninstalling) }
+
+      before do
+        expect(service).to receive(:installation_phase).once.and_return(phase)
+      end
+
+      it 'make the application errored' do
+        expect(worker_class).not_to receive(:perform_in)
+
+        service.execute
+
+        expect(application).to be_uninstall_errored
+        expect(application.status_reason).to eq('Operation timed out. Check pod logs for uninstall-helm for more details.')
+      end
+    end
+
+    context 'when installation raises a Kubeclient::HttpError' do
+      let(:cluster) { create(:cluster, :provided_by_user, :project) }
+      let(:logger) { service.send(:logger) }
+      let(:error) { Kubeclient::HttpError.new(401, 'Unauthorized', nil) }
+
+      before do
+        application.update!(cluster: cluster)
+
+        expect(service).to receive(:installation_phase).and_raise(error)
+      end
+
+      include_examples 'logs kubernetes errors' do
+        let(:error_name) { 'Kubeclient::HttpError' }
+        let(:error_message) { 'Unauthorized' }
+        let(:error_code) { 401 }
+      end
+
+      it 'shows the response code from the error' do
+        service.execute
+
+        expect(application).to be_uninstall_errored
+        expect(application.status_reason).to eq('Kubernetes error: 401')
+      end
+    end
+  end
+end
diff --git a/spec/services/clusters/applications/uninstall_service_spec.rb b/spec/services/clusters/applications/uninstall_service_spec.rb
new file mode 100644
index 00000000000..d1d0e923e18
--- /dev/null
+++ b/spec/services/clusters/applications/uninstall_service_spec.rb
@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Applications::UninstallService, '#execute' do
+  let(:application) { create(:clusters_applications_helm, :scheduled) }
+  let(:service) { described_class.new(application) }
+  let(:helm_client) { instance_double(Gitlab::Kubernetes::Helm::Api) }
+  let(:worker_class) { Clusters::Applications::WaitForUninstallAppWorker }
+
+  before do
+    allow(service).to receive(:helm_api).and_return(helm_client)
+  end
+
+  context 'when there are no errors' do
+    before do
+      expect(helm_client).to receive(:uninstall).with(kind_of(Gitlab::Kubernetes::Helm::ResetCommand))
+      allow(worker_class).to receive(:perform_in).and_return(nil)
+    end
+
+    it 'make the application to be uninstalling' do
+      expect(application.cluster).not_to be_nil
+      service.execute
+
+      expect(application).to be_uninstalling
+    end
+
+    it 'schedule async installation status check' do
+      expect(worker_class).to receive(:perform_in).once
+
+      service.execute
+    end
+  end
+
+  context 'when k8s cluster communication fails' do
+    let(:error) { Kubeclient::HttpError.new(500, 'system failure', nil) }
+
+    before do
+      expect(helm_client).to receive(:uninstall).with(kind_of(Gitlab::Kubernetes::Helm::ResetCommand)).and_raise(error)
+    end
+
+    include_examples 'logs kubernetes errors' do
+      let(:error_name) { 'Kubeclient::HttpError' }
+      let(:error_message) { 'system failure' }
+      let(:error_code) { 500 }
+    end
+
+    it 'make the application errored' do
+      service.execute
+
+      expect(application).to be_uninstall_errored
+      expect(application.status_reason).to match('Kubernetes error: 500')
+    end
+  end
+
+  context 'a non kubernetes error happens' do
+    let(:application) { create(:clusters_applications_helm, :scheduled) }
+    let(:error) { StandardError.new('something bad happened') }
+
+    before do
+      expect(helm_client).to receive(:uninstall).with(kind_of(Gitlab::Kubernetes::Helm::ResetCommand)).and_raise(error)
+    end
+
+    include_examples 'logs kubernetes errors' do
+      let(:error_name) { 'StandardError' }
+      let(:error_message) { 'something bad happened' }
+      let(:error_code) { nil }
+    end
+
+    it 'make the application errored' do
+      service.execute
+
+      expect(application).to be_uninstall_errored
+      expect(application.status_reason).to eq('Failed to uninstall.')
+    end
+  end
+end
diff --git a/spec/workers/clusters/applications/wait_for_uninstall_app_worker_spec.rb b/spec/workers/clusters/applications/wait_for_uninstall_app_worker_spec.rb
new file mode 100644
index 00000000000..aaf5c9defc4
--- /dev/null
+++ b/spec/workers/clusters/applications/wait_for_uninstall_app_worker_spec.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Applications::WaitForUninstallAppWorker, '#perform' do
+  let(:app) { create(:clusters_applications_helm) }
+  let(:app_name) { app.name }
+  let(:app_id) { app.id }
+
+  subject { described_class.new.perform(app_name, app_id) }
+
+  context 'app exists' do
+    let(:service) { instance_double(Clusters::Applications::CheckUninstallProgressService) }
+
+    it 'calls the check service' do
+      expect(Clusters::Applications::CheckUninstallProgressService).to receive(:new).with(app).and_return(service)
+      expect(service).to receive(:execute).once
+
+      subject
+    end
+  end
+
+  context 'app does not exist' do
+    let(:app_id) { 0 }
+
+    it 'does not call the check service' do
+      expect(Clusters::Applications::CheckUninstallProgressService).not_to receive(:new)
+
+      expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
+end
-- 
cgit v1.2.1


From 3c8df0c944f0b23f9ee8b6b08a0a355b00456dd9 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Fri, 12 Apr 2019 17:28:06 +1200
Subject: Destroy app on successful uninstallation

Rescue and put into :uninstall_errored if something goes wrong while
destroying, which can happen. I think it is safe to expose the full
error message from the destroy error.

Remove the :uninstalled state as no longer used.
---
 app/models/clusters/concerns/application_status.rb |  5 -----
 .../check_uninstall_progress_service.rb            |  4 +++-
 spec/factories/clusters/applications/helm.rb       |  4 ----
 .../check_uninstall_progress_service_spec.rb       | 23 +++++++++++++++++++---
 .../cluster_application_status_shared_examples.rb  | 11 -----------
 5 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb
index 16679a21e64..54a3dda6d75 100644
--- a/app/models/clusters/concerns/application_status.rb
+++ b/app/models/clusters/concerns/application_status.rb
@@ -27,7 +27,6 @@ module Clusters
           state :update_errored, value: 6
           state :uninstalling, value: 7
           state :uninstall_errored, value: 8
-          state :uninstalled, value: 9
 
           event :make_scheduled do
             transition [:installable, :errored, :installed, :updated, :update_errored, :uninstall_errored] => :scheduled
@@ -60,10 +59,6 @@ module Clusters
             transition [:scheduled] => :uninstalling
           end
 
-          event :make_uninstalled do
-            transition [:uninstalling] => :uninstalled
-          end
-
           before_transition any => [:scheduled] do |app_status, _|
             app_status.status_reason = nil
           end
diff --git a/app/services/clusters/applications/check_uninstall_progress_service.rb b/app/services/clusters/applications/check_uninstall_progress_service.rb
index 2a2594a30c8..efb2bb1b67d 100644
--- a/app/services/clusters/applications/check_uninstall_progress_service.rb
+++ b/app/services/clusters/applications/check_uninstall_progress_service.rb
@@ -23,7 +23,9 @@ module Clusters
       private
 
       def on_success
-        app.make_uninstalled!
+        app.destroy!
+      rescue StandardError => e
+        app.make_errored!("Application uninstalled but failed to destroy: #{e.message}")
       ensure
         remove_installation_pod
       end
diff --git a/spec/factories/clusters/applications/helm.rb b/spec/factories/clusters/applications/helm.rb
index ac230950fce..22a0888947e 100644
--- a/spec/factories/clusters/applications/helm.rb
+++ b/spec/factories/clusters/applications/helm.rb
@@ -49,10 +49,6 @@ FactoryBot.define do
       status_reason 'something went wrong'
     end
 
-    trait :uninstalled do
-      status 9
-    end
-
     trait :timeouted do
       installing
       updated_at { ClusterWaitForAppInstallationWorker::TIMEOUT.ago }
diff --git a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
index ccae7fd133f..084f29d9d2d 100644
--- a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
+++ b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
@@ -56,13 +56,30 @@ describe Clusters::Applications::CheckUninstallProgressService do
         service.execute
       end
 
-      it 'make the application installed' do
+      it 'destroys the application' do
         expect(worker_class).not_to receive(:perform_in)
 
         service.execute
+        expect(application).to be_destroyed
+      end
+
+      context 'an error occurs while destroying' do
+        before do
+          expect(application).to receive(:destroy!).once.and_raise("destroy failed")
+        end
+
+        it 'still removes the installation POD' do
+          expect(service).to receive(:remove_installation_pod).once
 
-        expect(application).to be_uninstalled
-        expect(application.status_reason).to be_nil
+          service.execute
+        end
+
+        it 'makes the application uninstall_errored' do
+          service.execute
+
+          expect(application).to be_uninstall_errored
+          expect(application.status_reason).to eq('Application uninstalled but failed to destroy: destroy failed')
+        end
       end
     end
 
diff --git a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
index c56b148cb8c..233b9db8f7b 100644
--- a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
+++ b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
@@ -192,16 +192,6 @@ shared_examples 'cluster application status specs' do |application_name|
         expect(subject).to be_uninstalling
       end
     end
-
-    describe '#make_uninstalled' do
-      subject { create(application_name, :uninstalling) }
-
-      it 'is uninstalled' do
-        subject.make_uninstalled!
-
-        expect(subject).to be_uninstalled
-      end
-    end
   end
 
   describe '#available?' do
@@ -219,7 +209,6 @@ shared_examples 'cluster application status specs' do |application_name|
       :update_errored    | false
       :uninstalling      | false
       :uninstall_errored | false
-      :uninstalled       | false
       :timeouted         | false
     end
 
-- 
cgit v1.2.1


From 44eec56834b7f524a2bf99d0f5e1571b52576d72 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Fri, 12 Apr 2019 17:42:48 +1200
Subject: Expose can_uninstall in cluster_status.json

Only prometheus can be uninstalled atm, the rest will be dealt with
later.

Presumption is that new application types will have uninstallation
implmemented at the same time.
---
 app/models/clusters/applications/cert_manager.rb               |  6 ++++++
 app/models/clusters/applications/helm.rb                       |  7 +++++++
 app/models/clusters/applications/ingress.rb                    |  7 +++++++
 app/models/clusters/applications/jupyter.rb                    |  6 ++++++
 app/models/clusters/applications/knative.rb                    |  6 ++++++
 app/models/clusters/applications/runner.rb                     |  7 +++++++
 app/models/clusters/concerns/application_core.rb               | 10 ++++++++++
 app/serializers/cluster_application_entity.rb                  |  1 +
 spec/fixtures/api/schemas/cluster_status.json                  |  3 ++-
 spec/models/clusters/applications/cert_manager_spec.rb         |  6 ++++++
 spec/models/clusters/applications/helm_spec.rb                 |  8 ++++++++
 spec/models/clusters/applications/ingress_spec.rb              |  6 ++++++
 spec/models/clusters/applications/jupyter_spec.rb              |  9 +++++++++
 spec/models/clusters/applications/knative_spec.rb              |  6 ++++++
 spec/models/clusters/applications/prometheus_spec.rb           |  8 ++++++++
 spec/models/clusters/applications/runner_spec.rb               |  8 ++++++++
 spec/serializers/cluster_application_entity_spec.rb            |  4 ++++
 .../models/cluster_application_core_shared_examples.rb         |  8 ++++++++
 18 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/app/models/clusters/applications/cert_manager.rb b/app/models/clusters/applications/cert_manager.rb
index ac0e7eb03bc..d6a7d1d2bdd 100644
--- a/app/models/clusters/applications/cert_manager.rb
+++ b/app/models/clusters/applications/cert_manager.rb
@@ -24,6 +24,12 @@ module Clusters
         'stable/cert-manager'
       end
 
+      # We will implement this in future MRs.
+      # Need to reverse postinstall step
+      def allowed_to_uninstall?
+        false
+      end
+
       def install_command
         Gitlab::Kubernetes::Helm::InstallCommand.new(
           name: 'certmanager',
diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb
index 71aff00077d..a83d06c4b00 100644
--- a/app/models/clusters/applications/helm.rb
+++ b/app/models/clusters/applications/helm.rb
@@ -29,6 +29,13 @@ module Clusters
         self.status = 'installable' if cluster&.platform_kubernetes_active?
       end
 
+      # We will implement this in future MRs.
+      # Basically we need to check all other applications are not installed
+      # first.
+      def allowed_to_uninstall?
+        false
+      end
+
       def install_command
         Gitlab::Kubernetes::Helm::InitCommand.new(
           name: name,
diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb
index 376d54aab2c..a1023f44049 100644
--- a/app/models/clusters/applications/ingress.rb
+++ b/app/models/clusters/applications/ingress.rb
@@ -35,6 +35,13 @@ module Clusters
         'stable/nginx-ingress'
       end
 
+      # We will implement this in future MRs.
+      # Basically we need to check all dependent applications are not installed
+      # first.
+      def allowed_to_uninstall?
+        false
+      end
+
       def install_command
         Gitlab::Kubernetes::Helm::InstallCommand.new(
           name: name,
diff --git a/app/models/clusters/applications/jupyter.rb b/app/models/clusters/applications/jupyter.rb
index f86ff3551a1..987c057ad6d 100644
--- a/app/models/clusters/applications/jupyter.rb
+++ b/app/models/clusters/applications/jupyter.rb
@@ -38,6 +38,12 @@ module Clusters
         content_values.to_yaml
       end
 
+      # Will be addressed in future MRs
+      # We need to investigate and document what will be permenantly deleted.
+      def allowed_to_uninstall?
+        false
+      end
+
       def install_command
         Gitlab::Kubernetes::Helm::InstallCommand.new(
           name: name,
diff --git a/app/models/clusters/applications/knative.rb b/app/models/clusters/applications/knative.rb
index 38cbc9ce8eb..9fbf5d8af04 100644
--- a/app/models/clusters/applications/knative.rb
+++ b/app/models/clusters/applications/knative.rb
@@ -51,6 +51,12 @@ module Clusters
         { "domain" => hostname }.to_yaml
       end
 
+      # Handled in a new issue:
+      # https://gitlab.com/gitlab-org/gitlab-ce/issues/59369
+      def allowed_to_uninstall?
+        false
+      end
+
       def install_command
         Gitlab::Kubernetes::Helm::InstallCommand.new(
           name: name,
diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb
index 3ebf7a5cfba..af648db3708 100644
--- a/app/models/clusters/applications/runner.rb
+++ b/app/models/clusters/applications/runner.rb
@@ -29,6 +29,13 @@ module Clusters
         content_values.to_yaml
       end
 
+      # Need to investigate if pipelines run by this runner will stop upon the
+      # executor pod stopping
+      # I.e.run a pipeline, and uninstall runner while pipeline is running
+      def allowed_to_uninstall?
+        false
+      end
+
       def install_command
         Gitlab::Kubernetes::Helm::InstallCommand.new(
           name: name,
diff --git a/app/models/clusters/concerns/application_core.rb b/app/models/clusters/concerns/application_core.rb
index ee964fb7c93..4514498b84b 100644
--- a/app/models/clusters/concerns/application_core.rb
+++ b/app/models/clusters/concerns/application_core.rb
@@ -18,6 +18,16 @@ module Clusters
           self.status = 'installable' if cluster&.application_helm_available?
         end
 
+        def can_uninstall?
+          allowed_to_uninstall?
+        end
+
+        # All new applications should uninstall by default
+        # Override if there's dependencies that needs to be uninstalled first
+        def allowed_to_uninstall?
+          true
+        end
+
         def self.application_name
           self.to_s.demodulize.underscore
         end
diff --git a/app/serializers/cluster_application_entity.rb b/app/serializers/cluster_application_entity.rb
index a4a2c015c4e..2a916b13f52 100644
--- a/app/serializers/cluster_application_entity.rb
+++ b/app/serializers/cluster_application_entity.rb
@@ -10,4 +10,5 @@ class ClusterApplicationEntity < Grape::Entity
   expose :hostname, if: -> (e, _) { e.respond_to?(:hostname) }
   expose :email, if: -> (e, _) { e.respond_to?(:email) }
   expose :update_available?, as: :update_available, if: -> (e, _) { e.respond_to?(:update_available?) }
+  expose :can_uninstall?, as: :can_uninstall
 end
diff --git a/spec/fixtures/api/schemas/cluster_status.json b/spec/fixtures/api/schemas/cluster_status.json
index 9da07a0b253..695175689b9 100644
--- a/spec/fixtures/api/schemas/cluster_status.json
+++ b/spec/fixtures/api/schemas/cluster_status.json
@@ -36,7 +36,8 @@
         "external_hostname": { "type": ["string", "null"] },
         "hostname": { "type": ["string", "null"] },
         "email": { "type": ["string", "null"] },
-        "update_available": { "type": ["boolean", "null"] }
+        "update_available": { "type": ["boolean", "null"] },
+        "can_uninstall": { "type": "boolean" }
       },
       "required" : [ "name", "status" ]
     }
diff --git a/spec/models/clusters/applications/cert_manager_spec.rb b/spec/models/clusters/applications/cert_manager_spec.rb
index 5cd80edb3a1..8d853a04e33 100644
--- a/spec/models/clusters/applications/cert_manager_spec.rb
+++ b/spec/models/clusters/applications/cert_manager_spec.rb
@@ -10,6 +10,12 @@ describe Clusters::Applications::CertManager do
   include_examples 'cluster application version specs', :clusters_applications_cert_managers
   include_examples 'cluster application initial status specs'
 
+  describe '#can_uninstall?' do
+    subject { cert_manager.can_uninstall? }
+
+    it { is_expected.to be_falsey }
+  end
+
   describe '#install_command' do
     let(:cert_email) { 'admin@example.com' }
 
diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb
index f177d493a2e..6ea6c110d62 100644
--- a/spec/models/clusters/applications/helm_spec.rb
+++ b/spec/models/clusters/applications/helm_spec.rb
@@ -18,6 +18,14 @@ describe Clusters::Applications::Helm do
     it { is_expected.to contain_exactly(installed_cluster, updated_cluster) }
   end
 
+  describe '#can_uninstall?' do
+    let(:helm) { create(:clusters_applications_helm) }
+
+    subject { helm.can_uninstall? }
+
+    it { is_expected.to be_falsey }
+  end
+
   describe '#issue_client_cert' do
     let(:application) { create(:clusters_applications_helm) }
     subject { application.issue_client_cert }
diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb
index 113d29b5551..292ddabd2d8 100644
--- a/spec/models/clusters/applications/ingress_spec.rb
+++ b/spec/models/clusters/applications/ingress_spec.rb
@@ -18,6 +18,12 @@ describe Clusters::Applications::Ingress do
     allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async)
   end
 
+  describe '#can_uninstall?' do
+    subject { ingress.can_uninstall? }
+
+    it { is_expected.to be_falsey }
+  end
+
   describe '#make_installed!' do
     before do
       application.make_installed!
diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb
index 1a7363b64f9..fc9ebed863e 100644
--- a/spec/models/clusters/applications/jupyter_spec.rb
+++ b/spec/models/clusters/applications/jupyter_spec.rb
@@ -10,6 +10,15 @@ describe Clusters::Applications::Jupyter do
 
   it { is_expected.to belong_to(:oauth_application) }
 
+  describe '#can_uninstall?' do
+    let(:ingress) { create(:clusters_applications_ingress, :installed, external_hostname: 'localhost.localdomain') }
+    let(:jupyter) { create(:clusters_applications_jupyter, cluster: ingress.cluster) }
+
+    subject { jupyter.can_uninstall? }
+
+    it { is_expected.to be_falsey }
+  end
+
   describe '#set_initial_status' do
     before do
       jupyter.set_initial_status
diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb
index 405b5ad691c..d5974f47190 100644
--- a/spec/models/clusters/applications/knative_spec.rb
+++ b/spec/models/clusters/applications/knative_spec.rb
@@ -39,6 +39,12 @@ describe Clusters::Applications::Knative do
     end
   end
 
+  describe '#can_uninstall?' do
+    subject { knative.can_uninstall? }
+
+    it { is_expected.to be_falsey }
+  end
+
   describe '#schedule_status_update with external_ip' do
     let(:application) { create(:clusters_applications_knative, :installed) }
 
diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb
index e8ba9737c23..f390afe9b1f 100644
--- a/spec/models/clusters/applications/prometheus_spec.rb
+++ b/spec/models/clusters/applications/prometheus_spec.rb
@@ -29,6 +29,14 @@ describe Clusters::Applications::Prometheus do
     end
   end
 
+  describe '#can_uninstall?' do
+    let(:prometheus) { create(:clusters_applications_prometheus) }
+
+    subject { prometheus.can_uninstall? }
+
+    it { is_expected.to be_truthy }
+  end
+
   describe '#prometheus_client' do
     context 'cluster is nil' do
       it 'returns nil' do
diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb
index b66acf13135..bdc0cb8ed86 100644
--- a/spec/models/clusters/applications/runner_spec.rb
+++ b/spec/models/clusters/applications/runner_spec.rb
@@ -13,6 +13,14 @@ describe Clusters::Applications::Runner do
 
   it { is_expected.to belong_to(:runner) }
 
+  describe '#can_uninstall?' do
+    let(:gitlab_runner) { create(:clusters_applications_runner, runner: ci_runner) }
+
+    subject { gitlab_runner.can_uninstall? }
+
+    it { is_expected.to be_falsey }
+  end
+
   describe '#install_command' do
     let(:kubeclient) { double('kubernetes client') }
     let(:gitlab_runner) { create(:clusters_applications_runner, runner: ci_runner) }
diff --git a/spec/serializers/cluster_application_entity_spec.rb b/spec/serializers/cluster_application_entity_spec.rb
index 7e151c3744e..f38a18fcf59 100644
--- a/spec/serializers/cluster_application_entity_spec.rb
+++ b/spec/serializers/cluster_application_entity_spec.rb
@@ -21,6 +21,10 @@ describe ClusterApplicationEntity do
       expect(subject[:status_reason]).to be_nil
     end
 
+    it 'has can_uninstall' do
+      expect(subject[:can_uninstall]).to be_falsey
+    end
+
     context 'non-helm application' do
       let(:application) { build(:clusters_applications_runner, version: '0.0.0') }
 
diff --git a/spec/support/shared_examples/models/cluster_application_core_shared_examples.rb b/spec/support/shared_examples/models/cluster_application_core_shared_examples.rb
index 1f76b981292..d6490a808ce 100644
--- a/spec/support/shared_examples/models/cluster_application_core_shared_examples.rb
+++ b/spec/support/shared_examples/models/cluster_application_core_shared_examples.rb
@@ -2,6 +2,14 @@ shared_examples 'cluster application core specs' do |application_name|
   it { is_expected.to belong_to(:cluster) }
   it { is_expected.to validate_presence_of(:cluster) }
 
+  describe '#can_uninstall?' do
+    it 'calls allowed_to_uninstall?' do
+      expect(subject).to receive(:allowed_to_uninstall?).and_return(true)
+
+      expect(subject.can_uninstall?).to be_truthy
+    end
+  end
+
   describe '#name' do
     it 'is .application_name' do
       expect(subject.name).to eq(described_class.application_name)
-- 
cgit v1.2.1


From 43c284b711ddd4db55908de0590f946de5227db6 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Fri, 12 Apr 2019 23:15:50 +1200
Subject: Teach Prometheus about #uninstall_command

Add specs
---
 app/models/clusters/applications/prometheus.rb     |  8 +++++++
 .../clusters/applications/prometheus_spec.rb       | 28 ++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb
index 954c29da196..1a8543f378e 100644
--- a/app/models/clusters/applications/prometheus.rb
+++ b/app/models/clusters/applications/prometheus.rb
@@ -47,6 +47,14 @@ module Clusters
         )
       end
 
+      def uninstall_command
+        Gitlab::Kubernetes::Helm::DeleteCommand.new(
+          name: name,
+          rbac: cluster.platform_kubernetes_rbac?,
+          files: files
+        )
+      end
+
       def upgrade_command(values)
         ::Gitlab::Kubernetes::Helm::InstallCommand.new(
           name: name,
diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb
index f390afe9b1f..4022e01195d 100644
--- a/spec/models/clusters/applications/prometheus_spec.rb
+++ b/spec/models/clusters/applications/prometheus_spec.rb
@@ -142,6 +142,34 @@ describe Clusters::Applications::Prometheus do
     end
   end
 
+  describe '#uninstall_command' do
+    let(:prometheus) { create(:clusters_applications_prometheus) }
+
+    subject { prometheus.uninstall_command }
+
+    it { is_expected.to be_an_instance_of(Gitlab::Kubernetes::Helm::DeleteCommand) }
+
+    it 'has the application name' do
+      expect(subject.name).to eq('prometheus')
+    end
+
+    it 'has files' do
+      expect(subject.files).to eq(prometheus.files)
+    end
+
+    it 'is rbac' do
+      expect(subject).to be_rbac
+    end
+
+    context 'on a non rbac enabled cluster' do
+      before do
+        prometheus.cluster.platform_kubernetes.abac!
+      end
+
+      it { is_expected.not_to be_rbac }
+    end
+  end
+
   describe '#upgrade_command' do
     let(:prometheus) { build(:clusters_applications_prometheus) }
     let(:values) { prometheus.values }
-- 
cgit v1.2.1


From abb530a61958518f6e0c739406f34c558c504206 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Sat, 13 Apr 2019 00:19:28 +1200
Subject: DELETE clusters/:id/:application endpoint

Add endpoint to delete/uninstall a cluster application
---
 .../clusters/applications_controller.rb            | 13 +++++
 .../clusters/applications/destroy_service.rb       | 23 ++++++++
 app/workers/all_queues.yml                         |  1 +
 .../clusters/applications/uninstall_worker.rb      | 17 ++++++
 config/routes.rb                                   |  1 +
 .../clusters/applications_controller_spec.rb       | 60 +++++++++++++++++++++
 .../clusters/applications_controller_spec.rb       | 62 +++++++++++++++++++++
 .../clusters/applications/destroy_service_spec.rb  | 63 ++++++++++++++++++++++
 8 files changed, 240 insertions(+)
 create mode 100644 app/services/clusters/applications/destroy_service.rb
 create mode 100644 app/workers/clusters/applications/uninstall_worker.rb
 create mode 100644 spec/services/clusters/applications/destroy_service_spec.rb

diff --git a/app/controllers/clusters/applications_controller.rb b/app/controllers/clusters/applications_controller.rb
index 73c744efeba..16c2365f85d 100644
--- a/app/controllers/clusters/applications_controller.rb
+++ b/app/controllers/clusters/applications_controller.rb
@@ -4,6 +4,7 @@ class Clusters::ApplicationsController < Clusters::BaseController
   before_action :cluster
   before_action :authorize_create_cluster!, only: [:create]
   before_action :authorize_update_cluster!, only: [:update]
+  before_action :authorize_admin_cluster!, only: [:destroy]
 
   def create
     request_handler do
@@ -21,6 +22,14 @@ class Clusters::ApplicationsController < Clusters::BaseController
     end
   end
 
+  def destroy
+    request_handler do
+      Clusters::Applications::DestroyService
+        .new(@cluster, current_user, cluster_application_destroy_params)
+        .execute(request)
+    end
+  end
+
   private
 
   def request_handler
@@ -40,4 +49,8 @@ class Clusters::ApplicationsController < Clusters::BaseController
   def cluster_application_params
     params.permit(:application, :hostname, :email)
   end
+
+  def cluster_application_destroy_params
+    params.permit(:application)
+  end
 end
diff --git a/app/services/clusters/applications/destroy_service.rb b/app/services/clusters/applications/destroy_service.rb
new file mode 100644
index 00000000000..fc74e1300a9
--- /dev/null
+++ b/app/services/clusters/applications/destroy_service.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module Clusters
+  module Applications
+    class DestroyService < ::Clusters::Applications::BaseService
+      def execute(_request)
+        instantiate_application.tap do |application|
+          break unless application.can_uninstall?
+
+          application.make_scheduled!
+
+          Clusters::Applications::UninstallWorker.perform_async(application.name, application.id)
+        end
+      end
+
+      private
+
+      def builder
+        cluster.method("application_#{application_name}").call
+      end
+    end
+  end
+end
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 0d14d313d21..7cf2e7100d5 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -33,6 +33,7 @@
 - gcp_cluster:cluster_configure
 - gcp_cluster:cluster_project_configure
 - gcp_cluster:clusters_applications_wait_for_uninstall_app
+- gcp_cluster:clusters_applications_uninstall
 
 - github_import_advance_stage
 - github_importer:github_import_import_diff_note
diff --git a/app/workers/clusters/applications/uninstall_worker.rb b/app/workers/clusters/applications/uninstall_worker.rb
new file mode 100644
index 00000000000..85e8ecc4ad5
--- /dev/null
+++ b/app/workers/clusters/applications/uninstall_worker.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module Clusters
+  module Applications
+    class UninstallWorker
+      include ApplicationWorker
+      include ClusterQueue
+      include ClusterApplications
+
+      def perform(app_name, app_id)
+        find_application(app_name, app_id) do |app|
+          Clusters::Applications::UninstallService.new(app).execute
+        end
+      end
+    end
+  end
+end
diff --git a/config/routes.rb b/config/routes.rb
index bbf00208545..f5957f43655 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -103,6 +103,7 @@ Rails.application.routes.draw do
         scope :applications do
           post '/:application', to: 'clusters/applications#create', as: :install_applications
           patch '/:application', to: 'clusters/applications#update', as: :update_applications
+          delete '/:application', to: 'clusters/applications#destroy', as: :uninstall_applications
         end
 
         get :cluster_status, format: :json
diff --git a/spec/controllers/groups/clusters/applications_controller_spec.rb b/spec/controllers/groups/clusters/applications_controller_spec.rb
index 16a63536ea6..acb9405d1a6 100644
--- a/spec/controllers/groups/clusters/applications_controller_spec.rb
+++ b/spec/controllers/groups/clusters/applications_controller_spec.rb
@@ -144,4 +144,64 @@ describe Groups::Clusters::ApplicationsController do
       it_behaves_like 'a secure endpoint'
     end
   end
+
+  describe 'DELETE destroy' do
+    subject do
+      delete :destroy, params: params.merge(group_id: group)
+    end
+
+    let!(:application) { create(:clusters_applications_cert_managers, :installed, cluster: cluster) }
+    let(:application_name) { application.name }
+    let(:params) { { application: application_name, id: cluster.id } }
+    let(:worker_class) { Clusters::Applications::UninstallWorker }
+
+    describe 'functionality' do
+      let(:user) { create(:user) }
+
+      before do
+        group.add_maintainer(user)
+        sign_in(user)
+      end
+
+      context "when cluster and app exists" do
+        xit "schedules an application update" do
+          expect(worker_class).to receive(:perform_async).with(application.name, application.id).once
+
+          is_expected.to have_http_status(:no_content)
+
+          expect(cluster.application_cert_manager).to be_scheduled
+        end
+      end
+
+      context 'when cluster do not exists' do
+        before do
+          cluster.destroy!
+        end
+
+        it { is_expected.to have_http_status(:not_found) }
+      end
+
+      context 'when application is unknown' do
+        let(:application_name) { 'unkwnown-app' }
+
+        it { is_expected.to have_http_status(:not_found) }
+      end
+
+      context 'when application is already scheduled' do
+        before do
+          application.make_scheduled!
+        end
+
+        xit { is_expected.to have_http_status(:bad_request) }
+      end
+    end
+
+    describe 'security' do
+      before do
+        allow(worker_class).to receive(:perform_async)
+      end
+
+      it_behaves_like 'a secure endpoint'
+    end
+  end
 end
diff --git a/spec/controllers/projects/clusters/applications_controller_spec.rb b/spec/controllers/projects/clusters/applications_controller_spec.rb
index cd1a01f8acc..70b34f071c8 100644
--- a/spec/controllers/projects/clusters/applications_controller_spec.rb
+++ b/spec/controllers/projects/clusters/applications_controller_spec.rb
@@ -145,4 +145,66 @@ describe Projects::Clusters::ApplicationsController do
       it_behaves_like 'a secure endpoint'
     end
   end
+
+  describe 'DELETE destroy' do
+    subject do
+      delete :destroy, params: params.merge(namespace_id: project.namespace, project_id: project)
+    end
+
+    let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
+    let(:project) { cluster.project }
+    let!(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
+    let(:application_name) { application.name }
+    let(:params) { { application: application_name, id: cluster.id } }
+    let(:worker_class) { Clusters::Applications::UninstallWorker }
+
+    describe 'functionality' do
+      let(:user) { create(:user) }
+
+      before do
+        project.add_maintainer(user)
+        sign_in(user)
+      end
+
+      context "when cluster and app exists" do
+        it "schedules an application update" do
+          expect(worker_class).to receive(:perform_async).with(application.name, application.id).once
+
+          is_expected.to have_http_status(:no_content)
+
+          expect(cluster.application_prometheus).to be_scheduled
+        end
+      end
+
+      context 'when cluster do not exists' do
+        before do
+          cluster.destroy!
+        end
+
+        it { is_expected.to have_http_status(:not_found) }
+      end
+
+      context 'when application is unknown' do
+        let(:application_name) { 'unkwnown-app' }
+
+        it { is_expected.to have_http_status(:not_found) }
+      end
+
+      context 'when application is already scheduled' do
+        before do
+          application.make_scheduled!
+        end
+
+        it { is_expected.to have_http_status(:bad_request) }
+      end
+    end
+
+    describe 'security' do
+      before do
+        allow(worker_class).to receive(:perform_async)
+      end
+
+      it_behaves_like 'a secure endpoint'
+    end
+  end
 end
diff --git a/spec/services/clusters/applications/destroy_service_spec.rb b/spec/services/clusters/applications/destroy_service_spec.rb
new file mode 100644
index 00000000000..8d9dc6a0f11
--- /dev/null
+++ b/spec/services/clusters/applications/destroy_service_spec.rb
@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Applications::DestroyService, '#execute' do
+  let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
+  let(:user) { create(:user) }
+  let(:params) { { application: 'prometheus' } }
+  let(:service) { described_class.new(cluster, user, params) }
+  let(:test_request) { double }
+  let(:worker_class) { Clusters::Applications::UninstallWorker }
+
+  subject { service.execute(test_request) }
+
+  before do
+    allow(worker_class).to receive(:perform_async)
+  end
+
+  context 'application is not installed' do
+    it 'raises Clusters::Applications::BaseService::InvalidApplicationError' do
+      expect(worker_class).not_to receive(:perform_async)
+
+      expect { subject }
+        .to raise_exception { Clusters::Applications::BaseService::InvalidApplicationError }
+        .and not_change { Clusters::Applications::Prometheus.count }
+        .and not_change { Clusters::Applications::Prometheus.with_status(:scheduled).count }
+    end
+  end
+
+  context 'application is installed' do
+    context 'application is schedulable' do
+      let!(:application) do
+        create(:clusters_applications_prometheus, :installed, cluster: cluster)
+      end
+
+      it 'makes application scheduled!' do
+        subject
+
+        expect(application.reload).to be_scheduled
+      end
+
+      it 'schedules UninstallWorker' do
+        expect(worker_class).to receive(:perform_async).with(application.name, application.id)
+
+        subject
+      end
+    end
+
+    context 'application is not schedulable' do
+      let!(:application) do
+        create(:clusters_applications_prometheus, :updating, cluster: cluster)
+      end
+
+      it 'raises StateMachines::InvalidTransition' do
+        expect(worker_class).not_to receive(:perform_async)
+
+        expect { subject }
+          .to raise_exception { StateMachines::InvalidTransition }
+          .and not_change { Clusters::Applications::Prometheus.with_status(:scheduled).count }
+      end
+    end
+  end
+end
-- 
cgit v1.2.1


From eae0fc2bcd6f7e2e183a922321ace3380c329adc Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Mon, 15 Apr 2019 10:10:59 +1000
Subject: Remove xit test for uninstall group cluster app

---
 .../clusters/applications_controller_spec.rb       | 60 ----------------------
 1 file changed, 60 deletions(-)

diff --git a/spec/controllers/groups/clusters/applications_controller_spec.rb b/spec/controllers/groups/clusters/applications_controller_spec.rb
index acb9405d1a6..16a63536ea6 100644
--- a/spec/controllers/groups/clusters/applications_controller_spec.rb
+++ b/spec/controllers/groups/clusters/applications_controller_spec.rb
@@ -144,64 +144,4 @@ describe Groups::Clusters::ApplicationsController do
       it_behaves_like 'a secure endpoint'
     end
   end
-
-  describe 'DELETE destroy' do
-    subject do
-      delete :destroy, params: params.merge(group_id: group)
-    end
-
-    let!(:application) { create(:clusters_applications_cert_managers, :installed, cluster: cluster) }
-    let(:application_name) { application.name }
-    let(:params) { { application: application_name, id: cluster.id } }
-    let(:worker_class) { Clusters::Applications::UninstallWorker }
-
-    describe 'functionality' do
-      let(:user) { create(:user) }
-
-      before do
-        group.add_maintainer(user)
-        sign_in(user)
-      end
-
-      context "when cluster and app exists" do
-        xit "schedules an application update" do
-          expect(worker_class).to receive(:perform_async).with(application.name, application.id).once
-
-          is_expected.to have_http_status(:no_content)
-
-          expect(cluster.application_cert_manager).to be_scheduled
-        end
-      end
-
-      context 'when cluster do not exists' do
-        before do
-          cluster.destroy!
-        end
-
-        it { is_expected.to have_http_status(:not_found) }
-      end
-
-      context 'when application is unknown' do
-        let(:application_name) { 'unkwnown-app' }
-
-        it { is_expected.to have_http_status(:not_found) }
-      end
-
-      context 'when application is already scheduled' do
-        before do
-          application.make_scheduled!
-        end
-
-        xit { is_expected.to have_http_status(:bad_request) }
-      end
-    end
-
-    describe 'security' do
-      before do
-        allow(worker_class).to receive(:perform_async)
-      end
-
-      it_behaves_like 'a secure endpoint'
-    end
-  end
 end
-- 
cgit v1.2.1


From 024ddcab17517befafc0c5163d66cdcaae1b69e6 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Mon, 15 Apr 2019 16:02:52 +1200
Subject: Deactivate any prometheus_service upon destroy

Basically does the reverse of after_transition to :installed.
---
 app/models/clusters/applications/prometheus.rb       |  8 ++++++++
 spec/models/clusters/applications/prometheus_spec.rb | 17 +++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb
index 1a8543f378e..1a715830953 100644
--- a/app/models/clusters/applications/prometheus.rb
+++ b/app/models/clusters/applications/prometheus.rb
@@ -16,6 +16,8 @@ module Clusters
 
       default_value_for :version, VERSION
 
+      after_destroy :disable_prometheus_integration
+
       state_machine :status do
         after_transition any => [:installed] do |application|
           application.cluster.projects.each do |project|
@@ -90,6 +92,12 @@ module Clusters
 
       private
 
+      def disable_prometheus_integration
+        cluster.projects.each do |project|
+          project.prometheus_service&.update(active: false)
+        end
+      end
+
       def kube_client
         cluster&.kubeclient&.core_client
       end
diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb
index 4022e01195d..76d2c4a9d1c 100644
--- a/spec/models/clusters/applications/prometheus_spec.rb
+++ b/spec/models/clusters/applications/prometheus_spec.rb
@@ -11,6 +11,23 @@ describe Clusters::Applications::Prometheus do
   include_examples 'cluster application helm specs', :clusters_applications_prometheus
   include_examples 'cluster application initial status specs'
 
+  describe 'after_destroy' do
+    let(:project) { create(:project) }
+    let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) }
+    let!(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
+    let!(:prometheus_service) { project.create_prometheus_service(active: true) }
+
+    it 'deactivates prometheus_service after destroy' do
+      expect do
+        application.destroy
+
+        prometheus_service.reload
+      end.to change(prometheus_service, :active)
+
+      expect(prometheus_service).not_to be_active
+    end
+  end
+
   describe 'transition to installed' do
     let(:project) { create(:project) }
     let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) }
-- 
cgit v1.2.1


From 3cac5b05b2fd8377883d42be5ee5e0fb2370db04 Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Mon, 15 Apr 2019 16:42:43 +1000
Subject: Fix uninstall specs: helm not uninstallable

---
 .../applications/check_uninstall_progress_service_spec.rb      |  8 ++++----
 spec/services/clusters/applications/uninstall_service_spec.rb  | 10 +++++-----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
index 084f29d9d2d..fedca51a1b4 100644
--- a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
+++ b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
 describe Clusters::Applications::CheckUninstallProgressService do
   RESCHEDULE_PHASES = Gitlab::Kubernetes::Pod::PHASES - [Gitlab::Kubernetes::Pod::SUCCEEDED, Gitlab::Kubernetes::Pod::FAILED].freeze
 
-  let(:application) { create(:clusters_applications_helm, :uninstalling) }
+  let(:application) { create(:clusters_applications_prometheus, :uninstalling) }
   let(:service) { described_class.new(application) }
   let(:phase) { Gitlab::Kubernetes::Pod::UNKNOWN }
   let(:errors) { nil }
@@ -95,12 +95,12 @@ describe Clusters::Applications::CheckUninstallProgressService do
         service.execute
 
         expect(application).to be_uninstall_errored
-        expect(application.status_reason).to eq('Operation failed. Check pod logs for uninstall-helm for more details.')
+        expect(application.status_reason).to eq('Operation failed. Check pod logs for uninstall-prometheus for more details.')
       end
     end
 
     context 'when timed out' do
-      let(:application) { create(:clusters_applications_helm, :timeouted, :uninstalling) }
+      let(:application) { create(:clusters_applications_prometheus, :timeouted, :uninstalling) }
 
       before do
         expect(service).to receive(:installation_phase).once.and_return(phase)
@@ -112,7 +112,7 @@ describe Clusters::Applications::CheckUninstallProgressService do
         service.execute
 
         expect(application).to be_uninstall_errored
-        expect(application.status_reason).to eq('Operation timed out. Check pod logs for uninstall-helm for more details.')
+        expect(application.status_reason).to eq('Operation timed out. Check pod logs for uninstall-prometheus for more details.')
       end
     end
 
diff --git a/spec/services/clusters/applications/uninstall_service_spec.rb b/spec/services/clusters/applications/uninstall_service_spec.rb
index d1d0e923e18..16497d752b2 100644
--- a/spec/services/clusters/applications/uninstall_service_spec.rb
+++ b/spec/services/clusters/applications/uninstall_service_spec.rb
@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe Clusters::Applications::UninstallService, '#execute' do
-  let(:application) { create(:clusters_applications_helm, :scheduled) }
+  let(:application) { create(:clusters_applications_prometheus, :scheduled) }
   let(:service) { described_class.new(application) }
   let(:helm_client) { instance_double(Gitlab::Kubernetes::Helm::Api) }
   let(:worker_class) { Clusters::Applications::WaitForUninstallAppWorker }
@@ -14,7 +14,7 @@ describe Clusters::Applications::UninstallService, '#execute' do
 
   context 'when there are no errors' do
     before do
-      expect(helm_client).to receive(:uninstall).with(kind_of(Gitlab::Kubernetes::Helm::ResetCommand))
+      expect(helm_client).to receive(:uninstall).with(kind_of(Gitlab::Kubernetes::Helm::DeleteCommand))
       allow(worker_class).to receive(:perform_in).and_return(nil)
     end
 
@@ -36,7 +36,7 @@ describe Clusters::Applications::UninstallService, '#execute' do
     let(:error) { Kubeclient::HttpError.new(500, 'system failure', nil) }
 
     before do
-      expect(helm_client).to receive(:uninstall).with(kind_of(Gitlab::Kubernetes::Helm::ResetCommand)).and_raise(error)
+      expect(helm_client).to receive(:uninstall).with(kind_of(Gitlab::Kubernetes::Helm::DeleteCommand)).and_raise(error)
     end
 
     include_examples 'logs kubernetes errors' do
@@ -54,11 +54,11 @@ describe Clusters::Applications::UninstallService, '#execute' do
   end
 
   context 'a non kubernetes error happens' do
-    let(:application) { create(:clusters_applications_helm, :scheduled) }
+    let(:application) { create(:clusters_applications_prometheus, :scheduled) }
     let(:error) { StandardError.new('something bad happened') }
 
     before do
-      expect(helm_client).to receive(:uninstall).with(kind_of(Gitlab::Kubernetes::Helm::ResetCommand)).and_raise(error)
+      expect(helm_client).to receive(:uninstall).with(kind_of(Gitlab::Kubernetes::Helm::DeleteCommand)).and_raise(error)
     end
 
     include_examples 'logs kubernetes errors' do
-- 
cgit v1.2.1


From 50fa7847bb89254ab0dbe163b5e71bb43b6d6b14 Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Fri, 26 Apr 2019 13:28:12 +1000
Subject: CheckUninstallProgressService remove unnecessary begin

---
 .../clusters/applications/check_uninstall_progress_service.rb         | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/app/services/clusters/applications/check_uninstall_progress_service.rb b/app/services/clusters/applications/check_uninstall_progress_service.rb
index efb2bb1b67d..663e4b1daef 100644
--- a/app/services/clusters/applications/check_uninstall_progress_service.rb
+++ b/app/services/clusters/applications/check_uninstall_progress_service.rb
@@ -36,9 +36,7 @@ module Clusters
 
       def check_timeout
         if timeouted?
-          begin
-            app.make_errored!("Operation timed out. Check pod logs for #{pod_name} for more details.")
-          end
+          app.make_errored!("Operation timed out. Check pod logs for #{pod_name} for more details.")
         else
           WaitForUninstallAppWorker.perform_in(WaitForUninstallAppWorker::INTERVAL, app.name, app.id)
         end
-- 
cgit v1.2.1


From 0a4817dd77eb371f171b634e4df180eafa115721 Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Fri, 26 Apr 2019 13:28:49 +1000
Subject: In Prometheus use update! instead of update

In order to not miss any errors since we are not checking the return
value of update
---
 app/models/clusters/applications/prometheus.rb       | 4 ++--
 spec/models/clusters/applications/prometheus_spec.rb | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb
index 1a715830953..a6b7617b830 100644
--- a/app/models/clusters/applications/prometheus.rb
+++ b/app/models/clusters/applications/prometheus.rb
@@ -21,7 +21,7 @@ module Clusters
       state_machine :status do
         after_transition any => [:installed] do |application|
           application.cluster.projects.each do |project|
-            project.find_or_initialize_service('prometheus').update(active: true)
+            project.find_or_initialize_service('prometheus').update!(active: true)
           end
         end
       end
@@ -94,7 +94,7 @@ module Clusters
 
       def disable_prometheus_integration
         cluster.projects.each do |project|
-          project.prometheus_service&.update(active: false)
+          project.prometheus_service&.update!(active: false)
         end
       end
 
diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb
index 76d2c4a9d1c..8649d3e2710 100644
--- a/spec/models/clusters/applications/prometheus_spec.rb
+++ b/spec/models/clusters/applications/prometheus_spec.rb
@@ -40,7 +40,7 @@ describe Clusters::Applications::Prometheus do
     end
 
     it 'ensures Prometheus service is activated' do
-      expect(prometheus_service).to receive(:update).with(active: true)
+      expect(prometheus_service).to receive(:update!).with(active: true)
 
       subject.make_installed
     end
-- 
cgit v1.2.1


From 416f3971e66762246f0af3cda97c4b55e101f61b Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Fri, 26 Apr 2019 13:30:08 +1000
Subject: Minor refactoring in check_uninstall_progress_service_spec

---
 .../applications/check_uninstall_progress_service_spec.rb      | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
index fedca51a1b4..8ff92eb2d09 100644
--- a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
+++ b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
@@ -11,6 +11,11 @@ describe Clusters::Applications::CheckUninstallProgressService do
   let(:errors) { nil }
   let(:worker_class) { Clusters::Applications::WaitForUninstallAppWorker }
 
+  before do
+    allow(service).to receive(:installation_errors).and_return(errors)
+    allow(service).to receive(:remove_installation_pod)
+  end
+
   shared_examples 'a not yet terminated installation' do |a_phase|
     let(:phase) { a_phase }
 
@@ -36,11 +41,6 @@ describe Clusters::Applications::CheckUninstallProgressService do
     end
   end
 
-  before do
-    allow(service).to receive(:installation_errors).and_return(errors)
-    allow(service).to receive(:remove_installation_pod).and_return(nil)
-  end
-
   context 'when application is installing' do
     RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated installation', phase }
 
-- 
cgit v1.2.1


From dbb284de5a18361eae937926052abd1ea3c261b5 Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Fri, 26 Apr 2019 14:25:51 +1000
Subject: Minor refactor of prometheus_spec

---
 spec/models/clusters/applications/prometheus_spec.rb | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb
index 8649d3e2710..26267c64112 100644
--- a/spec/models/clusters/applications/prometheus_spec.rb
+++ b/spec/models/clusters/applications/prometheus_spec.rb
@@ -19,12 +19,10 @@ describe Clusters::Applications::Prometheus do
 
     it 'deactivates prometheus_service after destroy' do
       expect do
-        application.destroy
+        application.destroy!
 
         prometheus_service.reload
-      end.to change(prometheus_service, :active)
-
-      expect(prometheus_service).not_to be_active
+      end.to change(prometheus_service, :active).from(true).to(false)
     end
   end
 
-- 
cgit v1.2.1


From a1c216e5e4f566b762e185ad36bf566f14268cba Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Fri, 26 Apr 2019 13:27:06 +1000
Subject: Use #public_send instead #method.call

These builder methods are using user provided input inside a public_send
but this is safe to do in this instance because before they are called
we check before calling them that they match an expected application
name.
---
 app/services/clusters/applications/create_service.rb  | 4 ++--
 app/services/clusters/applications/destroy_service.rb | 2 +-
 app/services/clusters/applications/update_service.rb  | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/services/clusters/applications/create_service.rb b/app/services/clusters/applications/create_service.rb
index ae36da7b3dd..f723c42c049 100644
--- a/app/services/clusters/applications/create_service.rb
+++ b/app/services/clusters/applications/create_service.rb
@@ -10,8 +10,8 @@ module Clusters
       end
 
       def builder
-        cluster.method("application_#{application_name}").call ||
-          cluster.method("build_application_#{application_name}").call
+        cluster.public_send(:"application_#{application_name}") || # rubocop:disable GitlabSecurity/PublicSend
+          cluster.public_send(:"build_application_#{application_name}") # rubocop:disable GitlabSecurity/PublicSend
       end
     end
   end
diff --git a/app/services/clusters/applications/destroy_service.rb b/app/services/clusters/applications/destroy_service.rb
index fc74e1300a9..f3a4c4f754a 100644
--- a/app/services/clusters/applications/destroy_service.rb
+++ b/app/services/clusters/applications/destroy_service.rb
@@ -16,7 +16,7 @@ module Clusters
       private
 
       def builder
-        cluster.method("application_#{application_name}").call
+        cluster.public_send(:"application_#{application_name}") # rubocop:disable GitlabSecurity/PublicSend
       end
     end
   end
diff --git a/app/services/clusters/applications/update_service.rb b/app/services/clusters/applications/update_service.rb
index 5071c31839c..0fa937da865 100644
--- a/app/services/clusters/applications/update_service.rb
+++ b/app/services/clusters/applications/update_service.rb
@@ -10,7 +10,7 @@ module Clusters
       end
 
       def builder
-        cluster.method("application_#{application_name}").call
+        cluster.public_send(:"application_#{application_name}") # rubocop:disable GitlabSecurity/PublicSend
       end
     end
   end
-- 
cgit v1.2.1


From f84fae1386eec1b22c1405a8b87beb30d8f10519 Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Mon, 29 Apr 2019 15:57:12 +1000
Subject: Rename #timeouted -> #timed_out

---
 .../clusters/applications/check_installation_progress_service.rb    | 4 ++--
 .../clusters/applications/check_uninstall_progress_service.rb       | 4 ++--
 spec/factories/clusters/applications/helm.rb                        | 2 +-
 .../applications/check_installation_progress_service_spec.rb        | 6 +++---
 .../clusters/applications/check_uninstall_progress_service_spec.rb  | 4 ++--
 .../models/cluster_application_status_shared_examples.rb            | 2 +-
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb
index c592d608b89..ae4b86a0614 100644
--- a/app/services/clusters/applications/check_installation_progress_service.rb
+++ b/app/services/clusters/applications/check_installation_progress_service.rb
@@ -37,7 +37,7 @@ module Clusters
       end
 
       def check_timeout
-        if timeouted?
+        if timed_out?
           begin
             app.make_errored!("Operation timed out. Check pod logs for #{pod_name} for more details.")
           end
@@ -51,7 +51,7 @@ module Clusters
         install_command.pod_name
       end
 
-      def timeouted?
+      def timed_out?
         Time.now.utc - app.updated_at.to_time.utc > ClusterWaitForAppInstallationWorker::TIMEOUT
       end
 
diff --git a/app/services/clusters/applications/check_uninstall_progress_service.rb b/app/services/clusters/applications/check_uninstall_progress_service.rb
index 663e4b1daef..022cc754af9 100644
--- a/app/services/clusters/applications/check_uninstall_progress_service.rb
+++ b/app/services/clusters/applications/check_uninstall_progress_service.rb
@@ -35,7 +35,7 @@ module Clusters
       end
 
       def check_timeout
-        if timeouted?
+        if timed_out?
           app.make_errored!("Operation timed out. Check pod logs for #{pod_name} for more details.")
         else
           WaitForUninstallAppWorker.perform_in(WaitForUninstallAppWorker::INTERVAL, app.name, app.id)
@@ -46,7 +46,7 @@ module Clusters
         app.uninstall_command.pod_name
       end
 
-      def timeouted?
+      def timed_out?
         Time.now.utc - app.updated_at.to_time.utc > WaitForUninstallAppWorker::TIMEOUT
       end
 
diff --git a/spec/factories/clusters/applications/helm.rb b/spec/factories/clusters/applications/helm.rb
index 22a0888947e..d78f01828d7 100644
--- a/spec/factories/clusters/applications/helm.rb
+++ b/spec/factories/clusters/applications/helm.rb
@@ -49,7 +49,7 @@ FactoryBot.define do
       status_reason 'something went wrong'
     end
 
-    trait :timeouted do
+    trait :timed_out do
       installing
       updated_at { ClusterWaitForAppInstallationWorker::TIMEOUT.ago }
     end
diff --git a/spec/services/clusters/applications/check_installation_progress_service_spec.rb b/spec/services/clusters/applications/check_installation_progress_service_spec.rb
index 8ad90aaf720..a54bd85a11a 100644
--- a/spec/services/clusters/applications/check_installation_progress_service_spec.rb
+++ b/spec/services/clusters/applications/check_installation_progress_service_spec.rb
@@ -18,7 +18,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do
     end
 
     context "when phase is #{a_phase}" do
-      context 'when not timeouted' do
+      context 'when not timed_out' do
         it 'reschedule a new check' do
           expect(ClusterWaitForAppInstallationWorker).to receive(:perform_in).once
           expect(service).not_to receive(:remove_installation_pod)
@@ -113,7 +113,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do
     end
 
     context 'when timed out' do
-      let(:application) { create(:clusters_applications_helm, :timeouted, :updating) }
+      let(:application) { create(:clusters_applications_helm, :timed_out, :updating) }
 
       before do
         expect(service).to receive(:installation_phase).once.and_return(phase)
@@ -174,7 +174,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do
     end
 
     context 'when timed out' do
-      let(:application) { create(:clusters_applications_helm, :timeouted) }
+      let(:application) { create(:clusters_applications_helm, :timed_out) }
 
       before do
         expect(service).to receive(:installation_phase).once.and_return(phase)
diff --git a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
index 8ff92eb2d09..d0730399268 100644
--- a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
+++ b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
@@ -24,7 +24,7 @@ describe Clusters::Applications::CheckUninstallProgressService do
     end
 
     context "when phase is #{a_phase}" do
-      context 'when not timeouted' do
+      context 'when not timed_out' do
         it 'reschedule a new check' do
           expect(worker_class).to receive(:perform_in).once
           expect(service).not_to receive(:remove_installation_pod)
@@ -100,7 +100,7 @@ describe Clusters::Applications::CheckUninstallProgressService do
     end
 
     context 'when timed out' do
-      let(:application) { create(:clusters_applications_prometheus, :timeouted, :uninstalling) }
+      let(:application) { create(:clusters_applications_prometheus, :timed_out, :uninstalling) }
 
       before do
         expect(service).to receive(:installation_phase).once.and_return(phase)
diff --git a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
index 233b9db8f7b..4525c03837f 100644
--- a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
+++ b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
@@ -209,7 +209,7 @@ shared_examples 'cluster application status specs' do |application_name|
       :update_errored    | false
       :uninstalling      | false
       :uninstall_errored | false
-      :timeouted         | false
+      :timed_out         | false
     end
 
     with_them do
-- 
cgit v1.2.1


From d252b99a0eb739e277b7453c99ac78c592ed2e03 Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Tue, 30 Apr 2019 14:49:26 +1000
Subject: Internationalize errors CheckUninstallProgressService

---
 .../clusters/applications/check_uninstall_progress_service.rb    | 8 ++++----
 locale/gitlab.pot                                                | 9 +++++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/app/services/clusters/applications/check_uninstall_progress_service.rb b/app/services/clusters/applications/check_uninstall_progress_service.rb
index 022cc754af9..12245ec90c3 100644
--- a/app/services/clusters/applications/check_uninstall_progress_service.rb
+++ b/app/services/clusters/applications/check_uninstall_progress_service.rb
@@ -17,7 +17,7 @@ module Clusters
       rescue Kubeclient::HttpError => e
         log_error(e)
 
-        app.make_errored!("Kubernetes error: #{e.error_code}")
+        app.make_errored!(_('Kubernetes error: %{error_code}') % { error_code: e.error_code })
       end
 
       private
@@ -25,18 +25,18 @@ module Clusters
       def on_success
         app.destroy!
       rescue StandardError => e
-        app.make_errored!("Application uninstalled but failed to destroy: #{e.message}")
+        app.make_errored!(_('Application uninstalled but failed to destroy: %{error_message}') % { error_message: e.message })
       ensure
         remove_installation_pod
       end
 
       def on_failed
-        app.make_errored!("Operation failed. Check pod logs for #{pod_name} for more details.")
+        app.make_errored!(_('Operation failed. Check pod logs for %{pod_name} for more details.') % { pod_name: pod_name })
       end
 
       def check_timeout
         if timed_out?
-          app.make_errored!("Operation timed out. Check pod logs for #{pod_name} for more details.")
+          app.make_errored!(_('Operation timed out. Check pod logs for %{pod_name} for more details.') % { pod_name: pod_name })
         else
           WaitForUninstallAppWorker.perform_in(WaitForUninstallAppWorker::INTERVAL, app.name, app.id)
         end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index c8b583575c8..3d56efa9834 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -942,6 +942,9 @@ msgstr ""
 msgid "Application settings saved successfully"
 msgstr ""
 
+msgid "Application uninstalled but failed to destroy: %{error_message}"
+msgstr ""
+
 msgid "Application was successfully destroyed."
 msgstr ""
 
@@ -6274,6 +6277,12 @@ msgstr ""
 msgid "Opens in a new window"
 msgstr ""
 
+msgid "Operation failed. Check pod logs for %{pod_name} for more details."
+msgstr ""
+
+msgid "Operation timed out. Check pod logs for %{pod_name} for more details."
+msgstr ""
+
 msgid "Operations"
 msgstr ""
 
-- 
cgit v1.2.1


From 3d94ab3222323b2ba93b736b45a47519365dfd9e Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Tue, 30 Apr 2019 14:59:48 +1000
Subject: Remove unncessary `to_time` in cluster services

---
 .../clusters/applications/check_installation_progress_service.rb        | 2 +-
 app/services/clusters/applications/check_uninstall_progress_service.rb  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb
index ae4b86a0614..3c6803d24e6 100644
--- a/app/services/clusters/applications/check_installation_progress_service.rb
+++ b/app/services/clusters/applications/check_installation_progress_service.rb
@@ -52,7 +52,7 @@ module Clusters
       end
 
       def timed_out?
-        Time.now.utc - app.updated_at.to_time.utc > ClusterWaitForAppInstallationWorker::TIMEOUT
+        Time.now.utc - app.updated_at.utc > ClusterWaitForAppInstallationWorker::TIMEOUT
       end
 
       def remove_installation_pod
diff --git a/app/services/clusters/applications/check_uninstall_progress_service.rb b/app/services/clusters/applications/check_uninstall_progress_service.rb
index 12245ec90c3..8786d295d6a 100644
--- a/app/services/clusters/applications/check_uninstall_progress_service.rb
+++ b/app/services/clusters/applications/check_uninstall_progress_service.rb
@@ -47,7 +47,7 @@ module Clusters
       end
 
       def timed_out?
-        Time.now.utc - app.updated_at.to_time.utc > WaitForUninstallAppWorker::TIMEOUT
+        Time.now.utc - app.updated_at.utc > WaitForUninstallAppWorker::TIMEOUT
       end
 
       def remove_installation_pod
-- 
cgit v1.2.1


From 3c52ff8c78b4e9174214d5efe5a8664d8cc608ca Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Mon, 29 Apr 2019 22:54:17 -0700
Subject: Add a newline in spec for readability

---
 .../clusters/applications/check_uninstall_progress_service_spec.rb       | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
index d0730399268..9ab83d913f5 100644
--- a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
+++ b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb
@@ -60,6 +60,7 @@ describe Clusters::Applications::CheckUninstallProgressService do
         expect(worker_class).not_to receive(:perform_in)
 
         service.execute
+
         expect(application).to be_destroyed
       end
 
-- 
cgit v1.2.1