diff options
author | João Cunha <j.a.cunha@gmail.com> | 2019-02-18 20:06:51 +0300 |
---|---|---|
committer | jerasmus <jerasmus@gitlab.com> | 2019-03-05 09:32:51 +0300 |
commit | f8234d9a086a43a95698da13d2734fe62ddb9ad7 (patch) | |
tree | 14ac13feff76a5e622e26e58393863761a9eaa19 | |
parent | cf1b85dd726c1947f9ff2af8d89aa240747f462d (diff) |
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
-rw-r--r-- | app/controllers/clusters/applications_controller.rb | 29 | ||||
-rw-r--r-- | app/models/clusters/concerns/application_core.rb | 6 | ||||
-rw-r--r-- | app/services/clusters/applications/base_helm_service.rb | 4 | ||||
-rw-r--r-- | app/services/clusters/applications/base_service.rb | 76 | ||||
-rw-r--r-- | app/services/clusters/applications/create_service.rb | 57 | ||||
-rw-r--r-- | app/services/clusters/applications/patch_service.rb | 26 | ||||
-rw-r--r-- | app/services/clusters/applications/update_service.rb | 34 | ||||
-rw-r--r-- | app/workers/all_queues.yml | 1 | ||||
-rw-r--r-- | app/workers/cluster_update_app_worker.rb | 13 | ||||
-rw-r--r-- | config/routes.rb | 1 | ||||
-rw-r--r-- | lib/gitlab/kubernetes/helm/install_command.rb | 3 | ||||
-rw-r--r-- | spec/controllers/groups/clusters/applications_controller_spec.rb | 100 | ||||
-rw-r--r-- | spec/controllers/projects/clusters/applications_controller_spec.rb | 97 | ||||
-rw-r--r-- | spec/models/clusters/applications/knative_spec.rb | 26 | ||||
-rw-r--r-- | spec/services/clusters/applications/patch_service_spec.rb | 128 | ||||
-rw-r--r-- | spec/services/clusters/applications/update_service_spec.rb | 72 |
16 files changed, 569 insertions, 104 deletions
diff --git a/app/controllers/clusters/applications_controller.rb b/app/controllers/clusters/applications_controller.rb index c4e7fc950f9..73c744efeba 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 683b45331f6..cdb42117281 100644 --- a/app/models/clusters/concerns/application_core.rb +++ b/app/models/clusters/concerns/application_core.rb @@ -30,6 +30,12 @@ module Clusters # 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 8a71730d5ec..c38b2656260 100644 --- a/app/services/clusters/applications/base_helm_service.rb +++ b/app/services/clusters/applications/base_helm_service.rb @@ -46,6 +46,10 @@ module Clusters @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 00000000000..cbd1cf03ae1 --- /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 12f8c849d41..bd7c31bb981 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 @@ module Clusters "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 00000000000..67fc8fd9800 --- /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 00000000000..979d870cc94 --- /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 d86f654dd44..bc7db5669e1 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 00000000000..bec422c34a9 --- /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 484e05114be..53c6225eff1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -101,6 +101,7 @@ Rails.application.routes.draw do 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 f931248b747..e33ba9305ce 100644 --- a/lib/gitlab/kubernetes/helm/install_command.rb +++ b/lib/gitlab/kubernetes/helm/install_command.rb @@ -7,7 +7,8 @@ module Gitlab 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 dd5263b077c..84da43fe2ef 100644 --- a/spec/controllers/groups/clusters/applications_controller_spec.rb +++ b/spec/controllers/groups/clusters/applications_controller_spec.rb @@ -9,9 +9,25 @@ describe Groups::Clusters::ApplicationsController do 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 @@ describe Groups::Clusters::ApplicationsController do 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 @@ describe Groups::Clusters::ApplicationsController do 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 @@ describe Groups::Clusters::ApplicationsController do 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 @@ describe Groups::Clusters::ApplicationsController do 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 @@ describe Groups::Clusters::ApplicationsController do 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 cb558259225..247adf3f8c7 100644 --- a/spec/controllers/projects/clusters/applications_controller_spec.rb +++ b/spec/controllers/projects/clusters/applications_controller_spec.rb @@ -9,7 +9,22 @@ describe Projects::Clusters::ApplicationsController do 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 @@ describe Projects::Clusters::ApplicationsController do 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 @@ describe Projects::Clusters::ApplicationsController do 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 @@ describe Projects::Clusters::ApplicationsController do 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 @@ describe Projects::Clusters::ApplicationsController do 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 @@ describe Projects::Clusters::ApplicationsController do 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 006b922ab27..4884a5927fb 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -66,9 +66,7 @@ describe Clusters::Applications::Knative do 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 @@ describe Clusters::Applications::Knative do 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 @@ describe Clusters::Applications::Knative do 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 00000000000..d4ee3243b84 --- /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 00000000000..22ad698f77d --- /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 |