From b991661080ced37f895542e721198dba7940c1fb Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 8 Nov 2018 17:17:15 +0000 Subject: Remove special handling for 404 response from K8s There is no reason to specially handle 404 here. We only handle 404 specifically if we are fetching something and want to return `nil` if 404. This does not apply here. --- .../clusters/gcp/kubernetes/create_or_update_namespace_service.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb b/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb index a888fab2789..2b607681082 100644 --- a/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb +++ b/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb @@ -16,8 +16,6 @@ module Clusters configure_kubernetes_token kubernetes_namespace.save! - rescue ::Kubeclient::HttpError => err - raise err unless err.error_code = 404 end private -- cgit v1.2.1 From f852b23537d34c1e6a6106413fba17dae2d75784 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 8 Nov 2018 17:36:21 +0000 Subject: Fix error handling in cluster_platform_configure_worker I have also extracted an issue in https://gitlab.com/gitlab-org/gitlab-ce/issues/53738 to surface these errors to users but now I am just fixing the bug where kubernetes_namespace is undefined while handling this error --- app/workers/cluster_platform_configure_worker.rb | 2 +- spec/workers/cluster_platform_configure_worker_spec.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/workers/cluster_platform_configure_worker.rb b/app/workers/cluster_platform_configure_worker.rb index 68e8335a09d..8f3689f0166 100644 --- a/app/workers/cluster_platform_configure_worker.rb +++ b/app/workers/cluster_platform_configure_worker.rb @@ -17,6 +17,6 @@ class ClusterPlatformConfigureWorker end rescue ::Kubeclient::HttpError => err - Rails.logger.error "Failed to create/update Kubernetes Namespace. id: #{kubernetes_namespace.id} message: #{err.message}" + Rails.logger.error "Failed to create/update Kubernetes namespace for cluster_id: #{cluster_id} with error: #{err.message}" end end diff --git a/spec/workers/cluster_platform_configure_worker_spec.rb b/spec/workers/cluster_platform_configure_worker_spec.rb index 1a7ad8923f6..b51f6e07c6a 100644 --- a/spec/workers/cluster_platform_configure_worker_spec.rb +++ b/spec/workers/cluster_platform_configure_worker_spec.rb @@ -30,4 +30,18 @@ describe ClusterPlatformConfigureWorker, '#execute' do described_class.new.perform(123) end end + + context 'when kubeclient raises error' do + let(:cluster) { create(:cluster, :project) } + + it 'rescues and logs the error' do + allow_any_instance_of(Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService).to receive(:execute).and_raise(::Kubeclient::HttpError.new(500, 'something baaaad happened', '')) + + expect(Rails.logger) + .to receive(:error) + .with("Failed to create/update Kubernetes namespace for cluster_id: #{cluster.id} with error: something baaaad happened") + + described_class.new.perform(cluster.id) + end + end end -- cgit v1.2.1 From 269b0e10319569cdfed1282e0af6a9e1c013f2fc Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Fri, 9 Nov 2018 10:10:47 +1300 Subject: Add changelog for fixing k8s integration error handling --- .../unreleased/fix-error-handling-bugs-in-kubernetes-integration.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/fix-error-handling-bugs-in-kubernetes-integration.yml diff --git a/changelogs/unreleased/fix-error-handling-bugs-in-kubernetes-integration.yml b/changelogs/unreleased/fix-error-handling-bugs-in-kubernetes-integration.yml new file mode 100644 index 00000000000..f2a117fe63f --- /dev/null +++ b/changelogs/unreleased/fix-error-handling-bugs-in-kubernetes-integration.yml @@ -0,0 +1,5 @@ +--- +title: Fix error handling bugs in kubernetes integration +merge_request: 22922 +author: +type: fixed -- cgit v1.2.1