From 1cec47ec1a9ebba716a5396b560bf7efc3319020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cunha?= Date: Thu, 29 Aug 2019 17:21:35 +0000 Subject: [PATCH] DRY check progress services Extract duplicated code from two similar classes into a parent one. --- .../check_installation_progress_service.rb | 35 +------------ .../applications/check_progress_service.rb | 50 +++++++++++++++++++ .../check_uninstall_progress_service.rb | 35 ++----------- ...heck_installation_progress_service_spec.rb | 16 +++--- .../check_uninstall_progress_service_spec.rb | 10 ++-- 5 files changed, 69 insertions(+), 77 deletions(-) create mode 100644 app/services/clusters/applications/check_progress_service.rb diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb index 3c6803d24e6..65d08966802 100644 --- a/app/services/clusters/applications/check_installation_progress_service.rb +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -2,24 +2,7 @@ module Clusters module Applications - class CheckInstallationProgressService < BaseHelmService - def execute - return unless operation_in_progress? - - 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 - + class CheckInstallationProgressService < CheckProgressService private def operation_in_progress? @@ -32,10 +15,6 @@ module Clusters 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 timed_out? begin @@ -54,18 +33,6 @@ module Clusters def timed_out? Time.now.utc - app.updated_at.utc > ClusterWaitForAppInstallationWorker::TIMEOUT end - - def remove_installation_pod - helm_api.delete_pod!(pod_name) - end - - def installation_phase - helm_api.status(pod_name) - end - - def installation_errors - helm_api.log(pod_name) - end end end end diff --git a/app/services/clusters/applications/check_progress_service.rb b/app/services/clusters/applications/check_progress_service.rb new file mode 100644 index 00000000000..4a07b955f8e --- /dev/null +++ b/app/services/clusters/applications/check_progress_service.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Clusters + module Applications + class CheckProgressService < BaseHelmService + def execute + return unless operation_in_progress? + + case pod_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: %{error_code}') % { error_code: e.error_code }) + end + + private + + def operation_in_progress? + raise NotImplementedError + end + + def on_success + raise NotImplementedError + end + + def pod_name + raise NotImplementedError + end + + def on_failed + app.make_errored!(_('Operation failed. Check pod logs for %{pod_name} for more details.') % { pod_name: pod_name }) + end + + def timed_out? + raise NotImplementedError + end + + def pod_phase + helm_api.status(pod_name) + end + end + end +end diff --git a/app/services/clusters/applications/check_uninstall_progress_service.rb b/app/services/clusters/applications/check_uninstall_progress_service.rb index e51d84ef052..6a618d61c4f 100644 --- a/app/services/clusters/applications/check_uninstall_progress_service.rb +++ b/app/services/clusters/applications/check_uninstall_progress_service.rb @@ -2,26 +2,13 @@ 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: %{error_code}') % { error_code: e.error_code }) - end - + class CheckUninstallProgressService < CheckProgressService private + def operation_in_progress? + app.uninstalling? + end + def on_success app.post_uninstall app.destroy! @@ -31,10 +18,6 @@ module Clusters remove_installation_pod end - def on_failed - 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.') % { pod_name: pod_name }) @@ -50,14 +33,6 @@ module Clusters def timed_out? Time.now.utc - app.updated_at.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/spec/services/clusters/applications/check_installation_progress_service_spec.rb b/spec/services/clusters/applications/check_installation_progress_service_spec.rb index a54bd85a11a..464a67649ff 100644 --- a/spec/services/clusters/applications/check_installation_progress_service_spec.rb +++ b/spec/services/clusters/applications/check_installation_progress_service_spec.rb @@ -14,7 +14,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do let(:phase) { a_phase } before do - expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:pod_phase).once.and_return(phase) end context "when phase is #{a_phase}" do @@ -44,7 +44,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do before do application.update!(cluster: cluster) - expect(service).to receive(:installation_phase).and_raise(error) + expect(service).to receive(:pod_phase).and_raise(error) end include_examples 'logs kubernetes errors' do @@ -77,7 +77,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do context 'when installation POD succeeded' do let(:phase) { Gitlab::Kubernetes::Pod::SUCCEEDED } before do - expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:pod_phase).once.and_return(phase) end it 'removes the installation POD' do @@ -101,7 +101,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do let(:errors) { 'test installation failed' } before do - expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:pod_phase).once.and_return(phase) end it 'make the application errored' do @@ -116,7 +116,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do let(:application) { create(:clusters_applications_helm, :timed_out, :updating) } before do - expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:pod_phase).once.and_return(phase) end it 'make the application errored' do @@ -138,7 +138,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do context 'when installation POD succeeded' do let(:phase) { Gitlab::Kubernetes::Pod::SUCCEEDED } before do - expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:pod_phase).once.and_return(phase) end it 'removes the installation POD' do @@ -162,7 +162,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do let(:errors) { 'test installation failed' } before do - expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:pod_phase).once.and_return(phase) end it 'make the application errored' do @@ -177,7 +177,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do let(:application) { create(:clusters_applications_helm, :timed_out) } before do - expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:pod_phase).once.and_return(phase) end it 'make the application errored' do 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 a948b442441..1a9f7089c3d 100644 --- a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb +++ b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb @@ -20,7 +20,7 @@ describe Clusters::Applications::CheckUninstallProgressService do let(:phase) { a_phase } before do - expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:pod_phase).once.and_return(phase) end context "when phase is #{a_phase}" do @@ -47,7 +47,7 @@ describe Clusters::Applications::CheckUninstallProgressService do context 'when installation POD succeeded' do let(:phase) { Gitlab::Kubernetes::Pod::SUCCEEDED } before do - expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:pod_phase).once.and_return(phase) end it 'removes the installation POD' do @@ -95,7 +95,7 @@ describe Clusters::Applications::CheckUninstallProgressService do let(:errors) { 'test installation failed' } before do - expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:pod_phase).once.and_return(phase) end it 'make the application errored' do @@ -110,7 +110,7 @@ describe Clusters::Applications::CheckUninstallProgressService do let(:application) { create(:clusters_applications_prometheus, :timed_out, :uninstalling) } before do - expect(service).to receive(:installation_phase).once.and_return(phase) + expect(service).to receive(:pod_phase).once.and_return(phase) end it 'make the application errored' do @@ -131,7 +131,7 @@ describe Clusters::Applications::CheckUninstallProgressService do before do application.update!(cluster: cluster) - expect(service).to receive(:installation_phase).and_raise(error) + expect(service).to receive(:pod_phase).and_raise(error) end include_examples 'logs kubernetes errors' do