diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb index 7bb68d75224..c5711fd0b58 100644 --- a/app/models/clusters/concerns/application_status.rb +++ b/app/models/clusters/concerns/application_status.rb @@ -24,7 +24,7 @@ module Clusters end event :make_scheduled do - transition any - [:scheduled] => :scheduled + transition %i(installable errored) => :scheduled end before_transition any => [:scheduled] do |app_status, _| diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb index 81306a2ff5b..1bd5dae0584 100644 --- a/app/services/clusters/applications/check_installation_progress_service.rb +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -6,7 +6,7 @@ module Clusters case installation_phase when Gitlab::Kubernetes::Pod::SUCCEEDED - on_succeeded + finalize_installation when Gitlab::Kubernetes::Pod::FAILED on_failed else @@ -18,14 +18,6 @@ module Clusters private - def on_succeeded - if app.make_installed - finalize_installation - else - app.make_errored!("Failed to update app record; #{app.errors}") - end - end - def on_failed app.make_errored!(installation_errors || 'Installation silently failed') finalize_installation @@ -34,6 +26,7 @@ module Clusters def check_timeout if Time.now.utc - app.updated_at.to_time.utc > ClusterWaitForAppInstallationWorker::TIMEOUT app.make_errored!('Installation timeouted') + finalize_installation else ClusterWaitForAppInstallationWorker.perform_in( ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) diff --git a/app/services/clusters/applications/finalize_installation_service.rb b/app/services/clusters/applications/finalize_installation_service.rb index 339d671c091..292c789b67b 100644 --- a/app/services/clusters/applications/finalize_installation_service.rb +++ b/app/services/clusters/applications/finalize_installation_service.rb @@ -4,13 +4,7 @@ module Clusters def execute helm_api.delete_installation_pod!(app) - app.make_errored!('Installation aborted') if aborted? - end - - private - - def aborted? - app.installing? || app.scheduled? + app.make_installed! if app.installing? end end end diff --git a/app/services/clusters/applications/install_service.rb b/app/services/clusters/applications/install_service.rb index 5ed0968a98a..4eba19a474e 100644 --- a/app/services/clusters/applications/install_service.rb +++ b/app/services/clusters/applications/install_service.rb @@ -5,14 +5,11 @@ module Clusters return unless app.scheduled? begin + app.make_installing! helm_api.install(app) - if app.make_installing - ClusterWaitForAppInstallationWorker.perform_in( - ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) - else - app.make_errored!("Failed to update app record; #{app.errors}") - end + ClusterWaitForAppInstallationWorker.perform_in( + ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) rescue KubeException => ke app.make_errored!("Kubernetes error: #{ke.message}") rescue StandardError diff --git a/app/services/clusters/applications/schedule_installation_service.rb b/app/services/clusters/applications/schedule_installation_service.rb index 17b3a09948d..eb8caa68ef7 100644 --- a/app/services/clusters/applications/schedule_installation_service.rb +++ b/app/services/clusters/applications/schedule_installation_service.rb @@ -2,15 +2,10 @@ module Clusters module Applications class ScheduleInstallationService < ::BaseService def execute - application = application_class.find_or_create_by!(cluster: cluster) - - application.make_scheduled! - ClusterInstallAppWorker.perform_async(application.name, application.id) - true - rescue ActiveRecord::RecordInvalid - false - rescue StateMachines::InvalidTransition - false + application_class.find_or_create_by!(cluster: cluster).try do |application| + application.make_scheduled! + ClusterInstallAppWorker.perform_async(application.name, application.id) + end end private 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 c64c3a0c94c..fe04fac9613 100644 --- a/spec/services/clusters/applications/check_installation_progress_service_spec.rb +++ b/spec/services/clusters/applications/check_installation_progress_service_spec.rb @@ -3,20 +3,27 @@ require 'spec_helper' describe Clusters::Applications::CheckInstallationProgressService do RESCHEDULE_PHASES = Gitlab::Kubernetes::Pod::PHASES - [Gitlab::Kubernetes::Pod::SUCCEEDED, Gitlab::Kubernetes::Pod::FAILED].freeze - def mock_helm_api(phase, errors: nil) - expect(service).to receive(:installation_phase).once.and_return(phase) - expect(service).to receive(:installation_errors).once.and_return(errors) if errors.present? + let(:application) { create(:applications_helm, :installing) } + let(:service) { described_class.new(application) } + let(:phase) { Gitlab::Kubernetes::Pod::UNKNOWN } + let(:errors) { nil } + + shared_examples 'a terminated installation' do + it 'finalize the installation' do + expect(service).to receive(:finalize_installation).once + + service.execute + end end - shared_examples 'not yet completed phase' do |phase| - context "when the installation POD phase is #{phase}" do - before do - mock_helm_api(phase) - end + shared_examples 'a not yet terminated installation' do |a_phase| + let(:phase) { a_phase } + context "when phase is #{a_phase}" do context 'when not timeouted' do it 'reschedule a new check' do expect(ClusterWaitForAppInstallationWorker).to receive(:perform_in).once + expect(service).not_to receive(:finalize_installation) service.execute @@ -28,6 +35,8 @@ describe Clusters::Applications::CheckInstallationProgressService do context 'when timeouted' do let(:application) { create(:applications_helm, :timeouted) } + it_behaves_like 'a terminated installation' + it 'make the application errored' do expect(ClusterWaitForAppInstallationWorker).not_to receive(:perform_in) @@ -40,36 +49,34 @@ describe Clusters::Applications::CheckInstallationProgressService do end end + before do + expect(service).to receive(:installation_phase).once.and_return(phase) + + allow(service).to receive(:installation_errors).and_return(errors) + allow(service).to receive(:finalize_installation).and_return(nil) + end + describe '#execute' do - let(:application) { create(:applications_helm, :installing) } - let(:service) { described_class.new(application) } - context 'when installation POD succeeded' do - it 'make the application installed' do - mock_helm_api(Gitlab::Kubernetes::Pod::SUCCEEDED) - expect(service).to receive(:finalize_installation).once + let(:phase) { Gitlab::Kubernetes::Pod::SUCCEEDED } - service.execute - - expect(application).to be_installed - expect(application.status_reason).to be_nil - end + it_behaves_like 'a terminated installation' end context 'when installation POD failed' do - let(:error_message) { 'test installation failed' } + let(:phase) { Gitlab::Kubernetes::Pod::FAILED } + let(:errors) { 'test installation failed' } + + it_behaves_like 'a terminated installation' it 'make the application errored' do - mock_helm_api(Gitlab::Kubernetes::Pod::FAILED, errors: error_message) - expect(service).to receive(:finalize_installation).once - service.execute expect(application).to be_errored - expect(application.status_reason).to eq(error_message) + expect(application.status_reason).to eq(errors) end end - RESCHEDULE_PHASES.each { |phase| it_behaves_like 'not yet completed phase', phase } + RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated installation', phase } end end diff --git a/spec/services/clusters/applications/finalize_installation_service_spec.rb b/spec/services/clusters/applications/finalize_installation_service_spec.rb new file mode 100644 index 00000000000..08b7a80dfcd --- /dev/null +++ b/spec/services/clusters/applications/finalize_installation_service_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Clusters::Applications::FinalizeInstallationService do + describe '#execute' do + let(:application) { create(:applications_helm, :installing) } + let(:service) { described_class.new(application) } + + before do + expect_any_instance_of(Gitlab::Kubernetes::Helm).to receive(:delete_installation_pod!).with(application) + end + + context 'when installation POD succeeded' do + it 'make the application installed' do + service.execute + + expect(application).to be_installed + expect(application.status_reason).to be_nil + end + end + + context 'when installation POD failed' do + let(:application) { create(:applications_helm, :errored) } + + it 'make the application errored' do + service.execute + + expect(application).to be_errored + expect(application.status_reason).not_to be_nil + end + end + end +end diff --git a/spec/services/clusters/applications/install_service_spec.rb b/spec/services/clusters/applications/install_service_spec.rb new file mode 100644 index 00000000000..a646dac1cae --- /dev/null +++ b/spec/services/clusters/applications/install_service_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe Clusters::Applications::InstallService do + describe '#execute' do + let(:application) { create(:applications_helm, :scheduled) } + let(:service) { described_class.new(application) } + + context 'when there are no errors' do + before do + expect_any_instance_of(Gitlab::Kubernetes::Helm).to receive(:install).with(application) + allow(ClusterWaitForAppInstallationWorker).to receive(:perform_in).and_return(nil) + end + + it 'make the application installing' do + service.execute + + expect(application).to be_installing + end + + it 'schedule async installation status check' do + expect(ClusterWaitForAppInstallationWorker).to receive(:perform_in).once + + service.execute + end + end + + context 'when k8s cluster communication fails' do + before do + error = KubeException.new(500, 'system failure', nil) + expect_any_instance_of(Gitlab::Kubernetes::Helm).to receive(:install).with(application).and_raise(error) + end + + it 'make the application errored' do + service.execute + + expect(application).to be_errored + expect(application.status_reason).to match(/kubernetes error:/i) + end + end + + context 'when application cannot be persisted' do + let(:application) { build(:applications_helm, :scheduled) } + + it 'make the application errored' do + expect(application).to receive(:make_installing!).once.and_raise(ActiveRecord::RecordInvalid) + expect_any_instance_of(Gitlab::Kubernetes::Helm).not_to receive(:install) + + service.execute + + expect(application).to be_errored + end + end + end +end diff --git a/spec/services/clusters/applications/schedule_installation_service_spec.rb b/spec/services/clusters/applications/schedule_installation_service_spec.rb new file mode 100644 index 00000000000..6ba587a41db --- /dev/null +++ b/spec/services/clusters/applications/schedule_installation_service_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Clusters::Applications::ScheduleInstallationService do + def count_scheduled + application_class&.with_status(:scheduled)&.count || 0 + end + + shared_examples 'a failing service' do + it 'raise an exception' do + expect(ClusterInstallAppWorker).not_to receive(:perform_async) + count_before = count_scheduled + + expect { service.execute }.to raise_error(StandardError) + expect(count_scheduled).to eq(count_before) + end + end + + describe '#execute' do + let(:application_class) { Clusters::Applications::Helm } + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.project } + let(:service) { described_class.new(project, nil, cluster: cluster, application_class: application_class) } + + it 'creates a new application' do + expect { service.execute }.to change { application_class.count }.by(1) + end + + it 'make the application scheduled' do + expect(ClusterInstallAppWorker).to receive(:perform_async).with(application_class.application_name, kind_of(Numeric)).once + + expect { service.execute }.to change { application_class.with_status(:scheduled).count }.by(1) + end + + context 'when installation is already in progress' do + let(:application) { create(:applications_helm, :installing) } + let(:cluster) { application.cluster } + + it_behaves_like 'a failing service' + end + + context 'when application_class is nil' do + let(:application_class) { nil } + + it_behaves_like 'a failing service' + end + + context 'when application cannot be persisted' do + before do + expect_any_instance_of(application_class).to receive(:make_scheduled!).once.and_raise(ActiveRecord::RecordInvalid) + end + + it_behaves_like 'a failing service' + end + end +end