From 8dfec0cedd4d623226ee9d058323a13470631bce Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Fri, 12 Apr 2019 13:22:11 +0000 Subject: [PATCH] Do not rescue errors from state transitions As this are un-expected errors which we should hear about from Sentry. Still rescue StandardError when operating a Helm action as we can get non Kubeclient errors such as SSL certificate or network errors --- .../clusters/applications/install_service.rb | 9 ++++- .../clusters/applications/patch_service.rb | 10 ++++-- .../clusters/applications/upgrade_service.rb | 36 ++++++++++--------- .../applications/install_service_spec.rb | 6 ++-- .../applications/patch_service_spec.rb | 6 ++-- .../applications/upgrade_service_spec.rb | 6 ++-- 6 files changed, 42 insertions(+), 31 deletions(-) diff --git a/app/services/clusters/applications/install_service.rb b/app/services/clusters/applications/install_service.rb index 1f62b3eb4de..b37ac510ff4 100644 --- a/app/services/clusters/applications/install_service.rb +++ b/app/services/clusters/applications/install_service.rb @@ -7,6 +7,13 @@ module Clusters return unless app.scheduled? app.make_installing! + + install + end + + private + + def install log_event(:begin_install) helm_api.install(install_command) @@ -18,7 +25,7 @@ module Clusters app.make_errored!("Kubernetes error: #{e.error_code}") rescue StandardError => e log_error(e) - app.make_errored!("Can't start installation process.") + app.make_errored!('Failed to install.') end end end diff --git a/app/services/clusters/applications/patch_service.rb b/app/services/clusters/applications/patch_service.rb index c3d317e226b..977a5e91041 100644 --- a/app/services/clusters/applications/patch_service.rb +++ b/app/services/clusters/applications/patch_service.rb @@ -8,6 +8,12 @@ module Clusters app.make_updating! + patch + end + + private + + def patch log_event(:begin_patch) helm_api.update(update_command) @@ -16,10 +22,10 @@ module Clusters ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) rescue Kubeclient::HttpError => e log_error(e) - app.make_update_errored!("Kubernetes error: #{e.error_code}") + app.make_errored!("Kubernetes error: #{e.error_code}") rescue StandardError => e log_error(e) - app.make_update_errored!("Can't start update process.") + app.make_errored!('Failed to update.') end end end diff --git a/app/services/clusters/applications/upgrade_service.rb b/app/services/clusters/applications/upgrade_service.rb index c34391bc8ad..813a9c4d071 100644 --- a/app/services/clusters/applications/upgrade_service.rb +++ b/app/services/clusters/applications/upgrade_service.rb @@ -6,24 +6,28 @@ module Clusters def execute return unless app.scheduled? - begin - app.make_updating! + app.make_updating! - log_event(:begin_upgrade) - # install_command works with upgrades too - # as it basically does `helm upgrade --install` - helm_api.update(install_command) + upgrade + end - log_event(:schedule_wait_for_upgrade) - 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 upgrade process.") - end + private + + def upgrade + # install_command works with upgrades too + # as it basically does `helm upgrade --install` + log_event(:begin_upgrade) + helm_api.update(install_command) + + log_event(:schedule_wait_for_upgrade) + ClusterWaitForAppInstallationWorker.perform_in( + ClusterWaitForAppInstallationWorker::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 upgrade.') end end end diff --git a/spec/services/clusters/applications/install_service_spec.rb b/spec/services/clusters/applications/install_service_spec.rb index db0c33a95b2..14739ff9e7c 100644 --- a/spec/services/clusters/applications/install_service_spec.rb +++ b/spec/services/clusters/applications/install_service_spec.rb @@ -58,7 +58,7 @@ describe Clusters::Applications::InstallService do let(:error) { StandardError.new('something bad happened') } before do - expect(application).to receive(:make_installing!).once.and_raise(error) + expect(helm_client).to receive(:install).with(install_command).and_raise(error) end include_examples 'logs kubernetes errors' do @@ -68,12 +68,10 @@ describe Clusters::Applications::InstallService do end it 'make the application errored' do - expect(helm_client).not_to receive(:install) - service.execute expect(application).to be_errored - expect(application.status_reason).to eq("Can't start installation process.") + expect(application.status_reason).to eq('Failed to install.') end end end diff --git a/spec/services/clusters/applications/patch_service_spec.rb b/spec/services/clusters/applications/patch_service_spec.rb index 10b1379a127..3ebe0540837 100644 --- a/spec/services/clusters/applications/patch_service_spec.rb +++ b/spec/services/clusters/applications/patch_service_spec.rb @@ -66,16 +66,14 @@ describe Clusters::Applications::PatchService do end before do - expect(application).to receive(:make_updating!).once.and_raise(error) + expect(helm_client).to receive(:update).with(update_command).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.") + expect(application.status_reason).to eq('Failed to update.') end end end diff --git a/spec/services/clusters/applications/upgrade_service_spec.rb b/spec/services/clusters/applications/upgrade_service_spec.rb index dd2e6e94e4f..a80b1d9127c 100644 --- a/spec/services/clusters/applications/upgrade_service_spec.rb +++ b/spec/services/clusters/applications/upgrade_service_spec.rb @@ -60,7 +60,7 @@ describe Clusters::Applications::UpgradeService do let(:error) { StandardError.new('something bad happened') } before do - expect(application).to receive(:make_updating!).once.and_raise(error) + expect(helm_client).to receive(:update).with(install_command).and_raise(error) end include_examples 'logs kubernetes errors' do @@ -70,12 +70,10 @@ describe Clusters::Applications::UpgradeService do 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 upgrade process.") + expect(application.status_reason).to eq('Failed to upgrade.') end end end