From f8234d9a086a43a95698da13d2734fe62ddb9ad7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Cunha?= <j.a.cunha@gmail.com>
Date: Mon, 18 Feb 2019 17:06:51 +0000
Subject: [PATCH] Creates Clusterss::ApplciationsController update endpoint

- Creates new route
- Creates new controller action
- Creates call stack:
  Clusterss::ApplciationsController calls -->
  Clusters::Applications::UpdateService calls -->
  Clusters::Applications::ScheduleUpdateService calls -->
  ClusterUpdateAppWorker calls -->
  Clusters::Applications::PatchService -->
  ClusterWaitForAppInstallationWorker

DRY req params

Adds gcp_cluster:cluster_update_app queue

Schedule_update_service is uneeded

Extract common logic to a parent class (UpdateService will need it)

Introduce new UpdateService

Fix rescue class namespace

Fix RuboCop offenses

Adds BaseService for create and update services

Remove request_handler code duplication

Fixes update command

Move update_command to ApplicationCore so all apps can use it

Adds tests for Knative update_command

Adds specs for PatchService

Raise error if update receives an unistalled app

Adds update_service spec

Fix RuboCop offense

Use subject in favor of go

Adds update endpoint specs for project namespace

Adds update endpoint specs for group namespace
---
 .../clusters/applications_controller.rb       |  29 +++-
 .../clusters/concerns/application_core.rb     |   6 +
 .../applications/base_helm_service.rb         |   4 +
 .../clusters/applications/base_service.rb     |  76 +++++++++++
 .../clusters/applications/create_service.rb   |  57 +-------
 .../clusters/applications/patch_service.rb    |  26 ++++
 .../clusters/applications/update_service.rb   |  34 +++++
 app/workers/all_queues.yml                    |   1 +
 app/workers/cluster_update_app_worker.rb      |  13 ++
 config/routes.rb                              |   1 +
 lib/gitlab/kubernetes/helm/install_command.rb |   3 +-
 .../clusters/applications_controller_spec.rb  | 100 +++++++++++---
 .../clusters/applications_controller_spec.rb  |  97 ++++++++++---
 .../clusters/applications/knative_spec.rb     |  26 +++-
 .../applications/patch_service_spec.rb        | 128 ++++++++++++++++++
 .../applications/update_service_spec.rb       |  72 ++++++++++
 16 files changed, 569 insertions(+), 104 deletions(-)
 create mode 100644 app/services/clusters/applications/base_service.rb
 create mode 100644 app/services/clusters/applications/patch_service.rb
 create mode 100644 app/services/clusters/applications/update_service.rb
 create mode 100644 app/workers/cluster_update_app_worker.rb
 create mode 100644 spec/services/clusters/applications/patch_service_spec.rb
 create mode 100644 spec/services/clusters/applications/update_service_spec.rb

diff --git a/app/controllers/clusters/applications_controller.rb b/app/controllers/clusters/applications_controller.rb
index c4e7fc950f9c3..73c744efeba5a 100644
--- a/app/controllers/clusters/applications_controller.rb
+++ b/app/controllers/clusters/applications_controller.rb
@@ -3,26 +3,41 @@
 class Clusters::ApplicationsController < Clusters::BaseController
   before_action :cluster
   before_action :authorize_create_cluster!, only: [:create]
+  before_action :authorize_update_cluster!, only: [:update]
 
   def create
-    Clusters::Applications::CreateService
-      .new(@cluster, current_user, create_cluster_application_params)
-      .execute(request)
+    request_handler do
+      Clusters::Applications::CreateService
+        .new(@cluster, current_user, cluster_application_params)
+        .execute(request)
+    end
+  end
+
+  def update
+    request_handler do
+      Clusters::Applications::UpdateService
+        .new(@cluster, current_user, cluster_application_params)
+        .execute(request)
+    end
+  end
+
+  private
+
+  def request_handler
+    yield
 
     head :no_content
-  rescue Clusters::Applications::CreateService::InvalidApplicationError
+  rescue Clusters::Applications::BaseService::InvalidApplicationError
     render_404
   rescue StandardError
     head :bad_request
   end
 
-  private
-
   def cluster
     @cluster ||= clusterable.clusters.find(params[:id]) || render_404
   end
 
-  def create_cluster_application_params
+  def cluster_application_params
     params.permit(:application, :hostname, :email)
   end
 end
diff --git a/app/models/clusters/concerns/application_core.rb b/app/models/clusters/concerns/application_core.rb
index 683b45331f620..cdb4211728191 100644
--- a/app/models/clusters/concerns/application_core.rb
+++ b/app/models/clusters/concerns/application_core.rb
@@ -30,6 +30,12 @@ def schedule_status_update
           # Override if you need extra data synchronized
           # from K8s after installation
         end
+
+        def update_command
+          command = install_command
+          command.version = version
+          command
+        end
       end
     end
   end
diff --git a/app/services/clusters/applications/base_helm_service.rb b/app/services/clusters/applications/base_helm_service.rb
index 8a71730d5ecae..c38b265626059 100644
--- a/app/services/clusters/applications/base_helm_service.rb
+++ b/app/services/clusters/applications/base_helm_service.rb
@@ -46,6 +46,10 @@ def install_command
         @install_command ||= app.install_command
       end
 
+      def update_command
+        @update_command ||= app.update_command
+      end
+
       def upgrade_command(new_values = "")
         app.upgrade_command(new_values)
       end
diff --git a/app/services/clusters/applications/base_service.rb b/app/services/clusters/applications/base_service.rb
new file mode 100644
index 0000000000000..cbd1cf03ae140
--- /dev/null
+++ b/app/services/clusters/applications/base_service.rb
@@ -0,0 +1,76 @@
+# frozen_string_literal: true
+
+module Clusters
+  module Applications
+    class BaseService
+      InvalidApplicationError = Class.new(StandardError)
+
+      attr_reader :cluster, :current_user, :params
+
+      def initialize(cluster, user, params = {})
+        @cluster = cluster
+        @current_user = user
+        @params = params.dup
+      end
+
+      def execute(request)
+        instantiate_application.tap do |application|
+          if application.has_attribute?(:hostname)
+            application.hostname = params[:hostname]
+          end
+
+          if application.has_attribute?(:email)
+            application.email = params[:email]
+          end
+
+          if application.respond_to?(:oauth_application)
+            application.oauth_application = create_oauth_application(application, request)
+          end
+
+          worker = worker_class(application)
+
+          application.make_scheduled!
+
+          worker.perform_async(application.name, application.id)
+        end
+      end
+
+      protected
+
+      def worker_class(application)
+        raise NotImplementedError
+      end
+
+      def builders
+        raise NotImplementedError
+      end
+
+      def project_builders
+        raise NotImplementedError
+      end
+
+      def instantiate_application
+        builder.call(@cluster) || raise(InvalidApplicationError, "invalid application: #{application_name}")
+      end
+
+      def builder
+        builders[application_name] || raise(InvalidApplicationError, "invalid application: #{application_name}")
+      end
+
+      def application_name
+        params[:application]
+      end
+
+      def create_oauth_application(application, request)
+        oauth_application_params = {
+          name: params[:application],
+          redirect_uri: application.callback_url,
+          scopes: 'api read_user openid',
+          owner: current_user
+        }
+
+        ::Applications::CreateService.new(current_user, oauth_application_params).execute(request)
+      end
+    end
+  end
+end
diff --git a/app/services/clusters/applications/create_service.rb b/app/services/clusters/applications/create_service.rb
index 12f8c849d41ff..bd7c31bb9819c 100644
--- a/app/services/clusters/applications/create_service.rb
+++ b/app/services/clusters/applications/create_service.rb
@@ -2,47 +2,11 @@
 
 module Clusters
   module Applications
-    class CreateService
-      InvalidApplicationError = Class.new(StandardError)
-
-      attr_reader :cluster, :current_user, :params
-
-      def initialize(cluster, user, params = {})
-        @cluster = cluster
-        @current_user = user
-        @params = params.dup
-      end
-
-      def execute(request)
-        create_application.tap do |application|
-          if application.has_attribute?(:hostname)
-            application.hostname = params[:hostname]
-          end
-
-          if application.has_attribute?(:email)
-            application.email = params[:email]
-          end
-
-          if application.respond_to?(:oauth_application)
-            application.oauth_application = create_oauth_application(application, request)
-          end
-
-          worker = application.updateable? ? ClusterUpgradeAppWorker : ClusterInstallAppWorker
-
-          application.make_scheduled!
-
-          worker.perform_async(application.name, application.id)
-        end
-      end
-
+    class CreateService < Clusters::Applications::BaseService
       private
 
-      def create_application
-        builder.call(@cluster)
-      end
-
-      def builder
-        builders[application_name] || raise(InvalidApplicationError, "invalid application: #{application_name}")
+      def worker_class(application)
+        application.updateable? ? ClusterUpgradeAppWorker : ClusterInstallAppWorker
       end
 
       def builders
@@ -65,21 +29,6 @@ def project_builders
           "knative" => -> (cluster) { cluster.application_knative || cluster.build_application_knative }
         }
       end
-
-      def application_name
-        params[:application]
-      end
-
-      def create_oauth_application(application, request)
-        oauth_application_params = {
-          name: params[:application],
-          redirect_uri: application.callback_url,
-          scopes: 'api read_user openid',
-          owner: current_user
-        }
-
-        ::Applications::CreateService.new(current_user, oauth_application_params).execute(request)
-      end
     end
   end
 end
diff --git a/app/services/clusters/applications/patch_service.rb b/app/services/clusters/applications/patch_service.rb
new file mode 100644
index 0000000000000..67fc8fd980027
--- /dev/null
+++ b/app/services/clusters/applications/patch_service.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module Clusters
+  module Applications
+    class PatchService < BaseHelmService
+      def execute
+        return unless app.scheduled?
+
+        begin
+          app.make_updating!
+
+          helm_api.update(update_command)
+
+          ClusterWaitForAppInstallationWorker.perform_in(
+            ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id)
+        rescue Kubeclient::HttpError => e
+          log_error(e)
+          app.make_update_errored!("Kubernetes error: #{e.error_code}")
+        rescue StandardError => e
+          log_error(e)
+          app.make_update_errored!("Can't start update process.")
+        end
+      end
+    end
+  end
+end
diff --git a/app/services/clusters/applications/update_service.rb b/app/services/clusters/applications/update_service.rb
new file mode 100644
index 0000000000000..979d870cc94b4
--- /dev/null
+++ b/app/services/clusters/applications/update_service.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module Clusters
+  module Applications
+    class UpdateService < Clusters::Applications::BaseService
+      private
+
+      def worker_class(application)
+        ClusterUpdateAppWorker
+      end
+
+      def builders
+        {
+          "helm" => -> (cluster) { cluster.application_helm },
+          "ingress" => -> (cluster) { cluster.application_ingress },
+          "cert_manager" => -> (cluster) { cluster.application_cert_manager }
+        }.tap do |hash|
+          hash.merge!(project_builders) if cluster.project_type?
+        end
+      end
+
+      # These applications will need extra configuration to enable them to work
+      # with groups of projects
+      def project_builders
+        {
+          "prometheus" => -> (cluster) { cluster.application_prometheus },
+          "runner" => -> (cluster) { cluster.application_runner },
+          "jupyter" => -> (cluster) { cluster.application_jupyter },
+          "knative" => -> (cluster) { cluster.application_knative }
+        }
+      end
+    end
+  end
+end
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index d86f654dd44a9..bc7db5669e1ff 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -23,6 +23,7 @@
 - cronjob:prune_web_hook_logs
 
 - gcp_cluster:cluster_install_app
+- gcp_cluster:cluster_update_app
 - gcp_cluster:cluster_upgrade_app
 - gcp_cluster:cluster_provision
 - gcp_cluster:cluster_wait_for_app_installation
diff --git a/app/workers/cluster_update_app_worker.rb b/app/workers/cluster_update_app_worker.rb
new file mode 100644
index 0000000000000..bec422c34a916
--- /dev/null
+++ b/app/workers/cluster_update_app_worker.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class ClusterUpdateAppWorker
+  include ApplicationWorker
+  include ClusterQueue
+  include ClusterApplications
+
+  def perform(app_name, app_id)
+    find_application(app_name, app_id) do |app|
+      Clusters::Applications::PatchService.new(app).execute
+    end
+  end
+end
diff --git a/config/routes.rb b/config/routes.rb
index 484e05114befe..53c6225eff14f 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -101,6 +101,7 @@
       member do
         scope :applications do
           post '/:application', to: 'clusters/applications#create', as: :install_applications
+          patch '/:application', to: 'clusters/applications#update', as: :update_applications
         end
 
         get :cluster_status, format: :json
diff --git a/lib/gitlab/kubernetes/helm/install_command.rb b/lib/gitlab/kubernetes/helm/install_command.rb
index f931248b747e9..e33ba9305ce19 100644
--- a/lib/gitlab/kubernetes/helm/install_command.rb
+++ b/lib/gitlab/kubernetes/helm/install_command.rb
@@ -7,7 +7,8 @@ class InstallCommand
         include BaseCommand
         include ClientCommand
 
-        attr_reader :name, :files, :chart, :version, :repository, :preinstall, :postinstall
+        attr_reader :name, :files, :chart, :repository, :preinstall, :postinstall
+        attr_accessor :version
 
         def initialize(name:, chart:, files:, rbac:, version: nil, repository: nil, preinstall: nil, postinstall: nil)
           @name = name
diff --git a/spec/controllers/groups/clusters/applications_controller_spec.rb b/spec/controllers/groups/clusters/applications_controller_spec.rb
index dd5263b077c6f..84da43fe2ef18 100644
--- a/spec/controllers/groups/clusters/applications_controller_spec.rb
+++ b/spec/controllers/groups/clusters/applications_controller_spec.rb
@@ -9,9 +9,25 @@ def current_application
     Clusters::Cluster::APPLICATIONS[application]
   end
 
+  shared_examples 'a secure endpoint' do
+    it { expect { subject }.to be_allowed_for(:admin) }
+    it { expect { subject }.to be_allowed_for(:owner).of(group) }
+    it { expect { subject }.to be_allowed_for(:maintainer).of(group) }
+    it { expect { subject }.to be_denied_for(:developer).of(group) }
+    it { expect { subject }.to be_denied_for(:reporter).of(group) }
+    it { expect { subject }.to be_denied_for(:guest).of(group) }
+    it { expect { subject }.to be_denied_for(:user) }
+    it { expect { subject }.to be_denied_for(:external) }
+  end
+
+  let(:cluster) { create(:cluster, :group, :provided_by_gcp) }
+  let(:group) { cluster.group }
+
   describe 'POST create' do
-    let(:cluster) { create(:cluster, :group, :provided_by_gcp) }
-    let(:group) { cluster.group }
+    subject do
+      post :create, params: params.merge(group_id: group)
+    end
+
     let(:application) { 'helm' }
     let(:params) { { application: application, id: cluster.id } }
 
@@ -26,7 +42,7 @@ def current_application
       it 'schedule an application installation' do
         expect(ClusterInstallAppWorker).to receive(:perform_async).with(application, anything).once
 
-        expect { go }.to change { current_application.count }
+        expect { subject }.to change { current_application.count }
         expect(response).to have_http_status(:no_content)
         expect(cluster.application_helm).to be_scheduled
       end
@@ -37,7 +53,7 @@ def current_application
         end
 
         it 'return 404' do
-          expect { go }.not_to change { current_application.count }
+          expect { subject }.not_to change { current_application.count }
           expect(response).to have_http_status(:not_found)
         end
       end
@@ -46,9 +62,7 @@ def current_application
         let(:application) { 'unkwnown-app' }
 
         it 'return 404' do
-          go
-
-          expect(response).to have_http_status(:not_found)
+          is_expected.to have_http_status(:not_found)
         end
       end
 
@@ -58,9 +72,7 @@ def current_application
         end
 
         it 'returns 400' do
-          go
-
-          expect(response).to have_http_status(:bad_request)
+          is_expected.to have_http_status(:bad_request)
         end
       end
     end
@@ -70,18 +82,66 @@ def current_application
         allow(ClusterInstallAppWorker).to receive(:perform_async)
       end
 
-      it { expect { go }.to be_allowed_for(:admin) }
-      it { expect { go }.to be_allowed_for(:owner).of(group) }
-      it { expect { go }.to be_allowed_for(:maintainer).of(group) }
-      it { expect { go }.to be_denied_for(:developer).of(group) }
-      it { expect { go }.to be_denied_for(:reporter).of(group) }
-      it { expect { go }.to be_denied_for(:guest).of(group) }
-      it { expect { go }.to be_denied_for(:user) }
-      it { expect { go }.to be_denied_for(:external) }
+      it_behaves_like 'a secure endpoint'
     end
+  end
 
-    def go
-      post :create, params: params.merge(group_id: group)
+  describe 'PATCH update' do
+    subject do
+      patch :update, 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, email: "new-email@example.com" } }
+
+    describe 'functionality' do
+      let(:user) { create(:user) }
+
+      before do
+        group.add_maintainer(user)
+        sign_in(user)
+      end
+
+      context "when cluster and app exists" do
+        it "schedules an application update" do
+          expect(ClusterUpdateAppWorker).to receive(:perform_async).with(application.name, anything).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
+
+        it { is_expected.to have_http_status(:bad_request) }
+      end
+    end
+
+    describe 'security' do
+      before do
+        allow(ClusterUpdateAppWorker).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 cb5582592259f..247adf3f8c7e1 100644
--- a/spec/controllers/projects/clusters/applications_controller_spec.rb
+++ b/spec/controllers/projects/clusters/applications_controller_spec.rb
@@ -9,7 +9,22 @@ def current_application
     Clusters::Cluster::APPLICATIONS[application]
   end
 
+  shared_examples 'a secure endpoint' do
+    it { expect { subject }.to be_allowed_for(:admin) }
+    it { expect { subject }.to be_allowed_for(:owner).of(project) }
+    it { expect { subject }.to be_allowed_for(:maintainer).of(project) }
+    it { expect { subject }.to be_denied_for(:developer).of(project) }
+    it { expect { subject }.to be_denied_for(:reporter).of(project) }
+    it { expect { subject }.to be_denied_for(:guest).of(project) }
+    it { expect { subject }.to be_denied_for(:user) }
+    it { expect { subject }.to be_denied_for(:external) }
+  end
+
   describe 'POST create' do
+    subject do
+      post :create, 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) { 'helm' }
@@ -26,7 +41,7 @@ def current_application
       it 'schedule an application installation' do
         expect(ClusterInstallAppWorker).to receive(:perform_async).with(application, anything).once
 
-        expect { go }.to change { current_application.count }
+        expect { subject }.to change { current_application.count }
         expect(response).to have_http_status(:no_content)
         expect(cluster.application_helm).to be_scheduled
       end
@@ -37,7 +52,7 @@ def current_application
         end
 
         it 'return 404' do
-          expect { go }.not_to change { current_application.count }
+          expect { subject }.not_to change { current_application.count }
           expect(response).to have_http_status(:not_found)
         end
       end
@@ -46,9 +61,7 @@ def current_application
         let(:application) { 'unkwnown-app' }
 
         it 'return 404' do
-          go
-
-          expect(response).to have_http_status(:not_found)
+          is_expected.to have_http_status(:not_found)
         end
       end
 
@@ -58,9 +71,7 @@ def current_application
         end
 
         it 'returns 400' do
-          go
-
-          expect(response).to have_http_status(:bad_request)
+          is_expected.to have_http_status(:bad_request)
         end
       end
     end
@@ -70,18 +81,68 @@ def current_application
         allow(ClusterInstallAppWorker).to receive(:perform_async)
       end
 
-      it { expect { go }.to be_allowed_for(:admin) }
-      it { expect { go }.to be_allowed_for(:owner).of(project) }
-      it { expect { go }.to be_allowed_for(:maintainer).of(project) }
-      it { expect { go }.to be_denied_for(:developer).of(project) }
-      it { expect { go }.to be_denied_for(:reporter).of(project) }
-      it { expect { go }.to be_denied_for(:guest).of(project) }
-      it { expect { go }.to be_denied_for(:user) }
-      it { expect { go }.to be_denied_for(:external) }
+      it_behaves_like 'a secure endpoint'
     end
+  end
 
-    def go
-      post :create, params: params.merge(namespace_id: project.namespace, project_id: project)
+  describe 'PATCH update' do
+    subject do
+      patch :update, 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_knative, :installed, cluster: cluster) }
+    let(:application_name) { application.name }
+    let(:params) { { application: application_name, id: cluster.id, hostname: "new.example.com" } }
+
+    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(ClusterUpdateAppWorker).to receive(:perform_async).with(application.name, anything).once
+
+          is_expected.to have_http_status(:no_content)
+
+          expect(cluster.application_knative).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(ClusterUpdateAppWorker).to receive(:perform_async)
+      end
+
+      it_behaves_like 'a secure endpoint'
     end
   end
 end
diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb
index 006b922ab2771..4884a5927fb1e 100644
--- a/spec/models/clusters/applications/knative_spec.rb
+++ b/spec/models/clusters/applications/knative_spec.rb
@@ -66,9 +66,7 @@
     end
   end
 
-  describe '#install_command' do
-    subject { knative.install_command }
-
+  shared_examples 'a command' do
     it 'should be an instance of Helm::InstallCommand' do
       expect(subject).to be_an_instance_of(Gitlab::Kubernetes::Helm::InstallCommand)
     end
@@ -76,7 +74,6 @@
     it 'should be initialized with knative arguments' do
       expect(subject.name).to eq('knative')
       expect(subject.chart).to eq('knative/knative')
-      expect(subject.version).to eq('0.2.2')
       expect(subject.files).to eq(knative.files)
     end
 
@@ -98,6 +95,27 @@
     end
   end
 
+  describe '#install_command' do
+    subject { knative.install_command }
+
+    it 'should be initialized with latest version' do
+      expect(subject.version).to eq('0.2.2')
+    end
+
+    it_behaves_like 'a command'
+  end
+
+  describe '#update_command' do
+    let!(:current_installed_version) { knative.version = '0.1.0' }
+    subject { knative.update_command }
+
+    it 'should be initialized with current version' do
+      expect(subject.version).to eq(current_installed_version)
+    end
+
+    it_behaves_like 'a command'
+  end
+
   describe '#files' do
     let(:application) { knative }
     let(:values) { subject[:'values.yaml'] }
diff --git a/spec/services/clusters/applications/patch_service_spec.rb b/spec/services/clusters/applications/patch_service_spec.rb
new file mode 100644
index 0000000000000..d4ee3243b8401
--- /dev/null
+++ b/spec/services/clusters/applications/patch_service_spec.rb
@@ -0,0 +1,128 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Applications::PatchService do
+  describe '#execute' do
+    let(:application) { create(:clusters_applications_knative, :scheduled) }
+    let!(:update_command) { application.update_command }
+    let(:service) { described_class.new(application) }
+    let(:helm_client) { instance_double(Gitlab::Kubernetes::Helm::Api) }
+
+    before do
+      allow(service).to receive(:update_command).and_return(update_command)
+      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(:update).with(update_command)
+        allow(ClusterWaitForAppInstallationWorker).to receive(:perform_in).and_return(nil)
+      end
+
+      it 'make the application updating' do
+        expect(application.cluster).not_to be_nil
+        service.execute
+
+        expect(application).to be_updating
+      end
+
+      it 'schedule async installation status check' do
+        expect(ClusterWaitForAppInstallationWorker).to receive(:perform_in).once
+
+        service.execute
+      end
+    end
+
+    context 'when kubernetes cluster communication fails' do
+      let(:error) { Kubeclient::HttpError.new(500, 'system failure', nil) }
+
+      before do
+        expect(helm_client).to receive(:update).with(update_command).and_raise(error)
+      end
+
+      it 'make the application errored' do
+        service.execute
+
+        expect(application).to be_update_errored
+        expect(application.status_reason).to match('Kubernetes error: 500')
+      end
+
+      it 'logs errors' do
+        expect(service.send(:logger)).to receive(:error).with(
+          {
+            exception: 'Kubeclient::HttpError',
+            message: 'system failure',
+            service: 'Clusters::Applications::PatchService',
+            app_id: application.id,
+            project_ids: application.cluster.project_ids,
+            group_ids: [],
+            error_code: 500
+          }
+        )
+
+        expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
+          error,
+          extra: {
+            exception: 'Kubeclient::HttpError',
+            message: 'system failure',
+            service: 'Clusters::Applications::PatchService',
+            app_id: application.id,
+            project_ids: application.cluster.project_ids,
+            group_ids: [],
+            error_code: 500
+          }
+        )
+
+        service.execute
+      end
+    end
+
+    context 'a non kubernetes error happens' do
+      let(:application) { create(:clusters_applications_knative, :scheduled) }
+      let(:error) { StandardError.new('something bad happened') }
+
+      before do
+        expect(application).to receive(:make_updating!).once.and_raise(error)
+      end
+
+      it 'make the application errored' do
+        expect(helm_client).not_to receive(:update)
+
+        service.execute
+
+        expect(application).to be_update_errored
+        expect(application.status_reason).to eq("Can't start update process.")
+      end
+
+      it 'logs errors' do
+        expect(service.send(:logger)).to receive(:error).with(
+          {
+            exception: 'StandardError',
+            error_code: nil,
+            message: 'something bad happened',
+            service: 'Clusters::Applications::PatchService',
+            app_id: application.id,
+            project_ids: application.cluster.projects.pluck(:id),
+            group_ids: []
+          }
+        )
+
+        expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
+          error,
+          extra: {
+            exception: 'StandardError',
+            error_code: nil,
+            message: 'something bad happened',
+            service: 'Clusters::Applications::PatchService',
+            app_id: application.id,
+            project_ids: application.cluster.projects.pluck(:id),
+            group_ids: []
+          }
+        )
+
+        service.execute
+      end
+    end
+  end
+end
diff --git a/spec/services/clusters/applications/update_service_spec.rb b/spec/services/clusters/applications/update_service_spec.rb
new file mode 100644
index 0000000000000..22ad698f77d26
--- /dev/null
+++ b/spec/services/clusters/applications/update_service_spec.rb
@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Applications::UpdateService do
+  include TestRequestHelpers
+
+  let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
+  let(:user) { create(:user) }
+  let(:params) { { application: 'knative', hostname: 'udpate.example.com' } }
+  let(:service) { described_class.new(cluster, user, params) }
+
+  subject { service.execute(test_request) }
+
+  describe '#execute' do
+    before do
+      allow(ClusterUpdateAppWorker).to receive(:perform_async)
+    end
+
+    context 'application is not installed' do
+      it 'raises Clusters::Applications::BaseService::InvalidApplicationError' do
+        expect(ClusterUpdateAppWorker).not_to receive(:perform_async)
+
+        expect { subject }
+          .to raise_exception { Clusters::Applications::BaseService::InvalidApplicationError }
+          .and not_change { Clusters::Applications::Knative.count }
+          .and not_change { Clusters::Applications::Knative.with_status(:scheduled).count }
+      end
+    end
+
+    context 'application is installed' do
+      context 'application is schedulable' do
+        let!(:application) do
+          create(:clusters_applications_knative, status: 3, cluster: cluster)
+        end
+
+        it 'updates the application data' do
+          expect do
+            subject
+          end.to change { application.reload.hostname }.to(params[:hostname])
+        end
+
+        it 'makes application scheduled!' do
+          subject
+
+          expect(application.reload).to be_scheduled
+        end
+
+        it 'schedules ClusterUpdateAppWorker' do
+          expect(ClusterUpdateAppWorker).to receive(:perform_async)
+
+          subject
+        end
+      end
+
+      context 'application is not schedulable' do
+        let!(:application) do
+          create(:clusters_applications_knative, status: 4, cluster: cluster)
+        end
+
+        it 'raises StateMachines::InvalidTransition' do
+          expect(ClusterUpdateAppWorker).not_to receive(:perform_async)
+
+          expect { subject }
+            .to raise_exception { StateMachines::InvalidTransition }
+            .and not_change { application.reload.hostname }
+            .and not_change { Clusters::Applications::Knative.with_status(:scheduled).count }
+        end
+      end
+    end
+  end
+end
-- 
GitLab