Merge branch '222951-send-connection-status' into 'master'

Send kubectl connection errors to frontend

See merge request gitlab-org/gitlab!36995

app/models/clusters/cluster.rb

@@ -218,6 +218,24 @@ module Clusters
       provider&.status_name || connection_status.presence || :created
     end
 
+    def connection_error
+      with_reactive_cache do |data|
+        data[:connection_error]
+      end
+    end
+
+    def node_connection_error
+      with_reactive_cache do |data|
+        data[:node_connection_error]
+      end
+    end
+
+    def metrics_connection_error
+      with_reactive_cache do |data|
+        data[:metrics_connection_error]
+      end
+    end
+
     def connection_status
       with_reactive_cache do |data|
         data[:connection_status]
@@ -233,9 +251,7 @@ module Clusters
     def calculate_reactive_cache
       return unless enabled?
 
-      gitlab_kubernetes_nodes = Gitlab::Kubernetes::Node.new(self)
-
-      { connection_status: retrieve_connection_status, nodes: gitlab_kubernetes_nodes.all.presence }
+      connection_data.merge(Gitlab::Kubernetes::Node.new(self).all)
     end
 
     def persisted_applications
@@ -395,9 +411,10 @@ module Clusters
       @instance_domain ||= Gitlab::CurrentSettings.auto_devops_domain
     end
 
-    def retrieve_connection_status
+    def connection_data
       result = ::Gitlab::Kubernetes::KubeClient.graceful_request(id) { kubeclient.core_client.discover }
-      result[:status]
+
+      { connection_status: result[:status], connection_error: result[:connection_error] }.compact
     end
 
     # To keep backward compatibility with AUTO_DEVOPS_DOMAIN

app/serializers/cluster_entity.rb

@@ -20,4 +20,8 @@ class ClusterEntity < Grape::Entity
   expose :gitlab_managed_apps_logs_path do |cluster|
     Clusters::ClusterPresenter.new(cluster, current_user: request.current_user).gitlab_managed_apps_logs_path # rubocop: disable CodeReuse/Presenter
   end
+
+  expose :kubernetes_errors do |cluster|
+    ClusterErrorEntity.new(cluster)
+  end
 end

app/serializers/cluster_error_entity.rb

+# frozen_string_literal: true
+
+class ClusterErrorEntity < Grape::Entity
+  expose :connection_error
+  expose :metrics_connection_error
+  expose :node_connection_error
+end

app/serializers/cluster_serializer.rb

@@ -11,6 +11,7 @@ class ClusterSerializer < BaseSerializer
         :enabled,
         :environment_scope,
         :gitlab_managed_apps_logs_path,
+        :kubernetes_errors,
         :name,
         :nodes,
         :path,

lib/gitlab/kubernetes/kube_client.rb

@@ -116,15 +116,15 @@ module Gitlab
       def self.graceful_request(cluster_id)
         { status: :connected, response: yield }
       rescue *Gitlab::Kubernetes::Errors::CONNECTION
-        { status: :unreachable }
+        { status: :unreachable, connection_error: :connection_error }
       rescue *Gitlab::Kubernetes::Errors::AUTHENTICATION
-        { status: :authentication_failure }
+        { status: :authentication_failure, connection_error: :authentication_error }
       rescue Kubeclient::HttpError => e
-        { status: kubeclient_error_status(e.message) }
+        { status: kubeclient_error_status(e.message), connection_error: :http_error }
       rescue => e
         Gitlab::ErrorTracking.track_exception(e, cluster_id: cluster_id)
 
-        { status: :unknown_failure }
+        { status: :unknown_failure, connection_error: :unknown_error }
       end
 
       # KubeClient uses the same error class

lib/gitlab/kubernetes/node.rb

@@ -8,22 +8,29 @@ module Gitlab
       end
 
       def all
-        nodes.map do |node|
-          attributes = node(node)
-          attributes.merge(node_metrics(node))
-        end
+        {
+          nodes: metadata.presence,
+          node_connection_error: nodes_from_cluster[:connection_error],
+          metrics_connection_error: nodes_metrics_from_cluster[:connection_error]
+        }.compact
       end
 
       private
 
       attr_reader :cluster
 
+      def metadata
+        nodes.map do |node|
+          base_data(node).merge(node_metrics(node))
+        end
+      end
+
       def nodes_from_cluster
-        graceful_request { cluster.kubeclient.get_nodes }
+        @nodes_from_cluster ||= graceful_request { cluster.kubeclient.get_nodes }
       end
 
       def nodes_metrics_from_cluster
-        graceful_request { cluster.kubeclient.metrics_client.get_nodes }
+        @nodes_metrics_from_cluster ||= graceful_request { cluster.kubeclient.metrics_client.get_nodes }
       end
 
       def nodes
@@ -44,7 +51,7 @@ module Gitlab
         ::Gitlab::Kubernetes::KubeClient.graceful_request(cluster.id, &block)
       end
 
-      def node(node)
+      def base_data(node)
         {
           'metadata' => {
             'name' => node.metadata.name

spec/lib/gitlab/kubernetes/kube_client_spec.rb

@@ -80,13 +80,13 @@ RSpec.describe Gitlab::Kubernetes::KubeClient do
     context 'errored' do
       using RSpec::Parameterized::TableSyntax
 
-      where(:error, :error_status) do
-        SocketError                     | :unreachable
-        OpenSSL::X509::CertificateError | :authentication_failure
-        StandardError                   | :unknown_failure
-        Kubeclient::HttpError.new(408, "timed out", nil) | :unreachable
-        Kubeclient::HttpError.new(408, "timeout", nil) | :unreachable
-        Kubeclient::HttpError.new(408, "", nil) | :authentication_failure
+      where(:error, :connection_status, :error_status) do
+        SocketError                                      | :unreachable            | :connection_error
+        OpenSSL::X509::CertificateError                  | :authentication_failure | :authentication_error
+        StandardError                                    | :unknown_failure        | :unknown_error
+        Kubeclient::HttpError.new(408, "timed out", nil) | :unreachable            | :http_error
+        Kubeclient::HttpError.new(408, "timeout", nil)   | :unreachable            | :http_error
+        Kubeclient::HttpError.new(408, "", nil)          | :authentication_failure | :http_error
       end
 
       with_them do
@@ -97,7 +97,7 @@ RSpec.describe Gitlab::Kubernetes::KubeClient do
         it 'returns error status' do
           result = described_class.graceful_request(1) { client.foo }
 
-          expect(result).to eq({ status: error_status })
+          expect(result).to eq({ status: connection_status, connection_error: error_status })
         end
       end
     end

spec/lib/gitlab/kubernetes/node_spec.rb

@@ -7,45 +7,51 @@ RSpec.describe Gitlab::Kubernetes::Node do
 
   describe '#all' do
     let(:cluster) { create(:cluster, :provided_by_user, :group) }
-    let(:expected_nodes) { [] }
+    let(:expected_nodes) { nil }
+    let(:nodes) { [kube_node.merge(kube_node_metrics)] }
+
+    subject { described_class.new(cluster).all }
 
     before do
       stub_kubeclient_nodes_and_nodes_metrics(cluster.platform.api_url)
     end
 
-    subject { described_class.new(cluster).all }
-
     context 'when connection to the cluster is successful' do
-      let(:expected_nodes) { [kube_node.merge(kube_node_metrics)] }
+      let(:expected_nodes) { { nodes: nodes } }
 
       it { is_expected.to eq(expected_nodes) }
     end
 
-    context 'when cluster cannot be reached' do
-      before do
-        allow(cluster.kubeclient.core_client).to receive(:discover)
-          .and_raise(SocketError)
+    context 'when there is a connection error' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:error, :error_status) do
+        SocketError                             | :kubernetes_connection_error
+        OpenSSL::X509::CertificateError         | :kubernetes_authentication_error
+        StandardError                           | :unknown_error
+        Kubeclient::HttpError.new(408, "", nil) | :kubeclient_http_error
       end
 
-      it { is_expected.to eq(expected_nodes) }
-    end
+      context 'when there is an error while querying nodes' do
+        with_them do
+          before do
+            allow(cluster.kubeclient).to receive(:get_nodes).and_raise(error)
+          end
 
-    context 'when cluster cannot be authenticated to' do
-      before do
-        allow(cluster.kubeclient.core_client).to receive(:discover)
-          .and_raise(OpenSSL::X509::CertificateError.new('Certificate error'))
+          it { is_expected.to eq({ node_connection_error: error_status }) }
+        end
       end
 
-      it { is_expected.to eq(expected_nodes) }
-    end
+      context 'when there is an error while querying metrics' do
+        with_them do
+          before do
+            allow(cluster.kubeclient).to receive(:get_nodes).and_return({ response: nodes })
+            allow(cluster.kubeclient).to receive(:metrics_client).and_raise(error)
+          end
 
-    context 'when Kubeclient::HttpError is raised' do
-      before do
-        allow(cluster.kubeclient.core_client).to receive(:discover)
-          .and_raise(Kubeclient::HttpError.new(403, 'Forbidden', nil))
+          it { is_expected.to eq({ nodes: nodes, metrics_connection_error: error_status }) }
+        end
       end
-
-      it { is_expected.to eq(expected_nodes) }
     end
 
     context 'when an uncategorised error is raised' do
@@ -54,7 +60,7 @@ RSpec.describe Gitlab::Kubernetes::Node do
           .and_raise(StandardError)
       end
 
-      it { is_expected.to eq(expected_nodes) }
+      it { is_expected.to eq({ node_connection_error: :unknown_error }) }
 
       it 'notifies Sentry' do
         expect(Gitlab::ErrorTracking).to receive(:track_exception)

spec/models/clusters/cluster_spec.rb

@@ -1153,6 +1153,57 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     end
   end
 
+  describe '#connection_error' do
+    let(:cluster) { create(:cluster) }
+    let(:error) { :unknown_error }
+
+    subject { cluster.connection_error }
+
+    it { is_expected.to be_nil }
+
+    context 'with a cached status' do
+      before do
+        stub_reactive_cache(cluster, connection_error: error)
+      end
+
+      it { is_expected.to eq(error) }
+    end
+  end
+
+  describe '#node_connection_error' do
+    let(:cluster) { create(:cluster) }
+    let(:error) { :unknown_error }
+
+    subject { cluster.node_connection_error }
+
+    it { is_expected.to be_nil }
+
+    context 'with a cached status' do
+      before do
+        stub_reactive_cache(cluster, node_connection_error: error)
+      end
+
+      it { is_expected.to eq(error) }
+    end
+  end
+
+  describe '#metrics_connection_error' do
+    let(:cluster) { create(:cluster) }
+    let(:error) { :unknown_error }
+
+    subject { cluster.metrics_connection_error }
+
+    it { is_expected.to be_nil }
+
+    context 'with a cached status' do
+      before do
+        stub_reactive_cache(cluster, metrics_connection_error: error)
+      end
+
+      it { is_expected.to eq(error) }
+    end
+  end
+
   describe '#nodes' do
     let(:cluster) { create(:cluster) }
 
@@ -1186,43 +1237,49 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     context 'cluster is enabled' do
       let(:cluster) { create(:cluster, :provided_by_user, :group) }
       let(:gl_k8s_node_double) { double(Gitlab::Kubernetes::Node) }
-      let(:expected_nodes) { nil }
+      let(:expected_nodes) { {} }
 
       before do
         stub_kubeclient_discover(cluster.platform.api_url)
         allow(Gitlab::Kubernetes::Node).to receive(:new).with(cluster).and_return(gl_k8s_node_double)
-        allow(gl_k8s_node_double).to receive(:all).and_return([])
+        allow(gl_k8s_node_double).to receive(:all).and_return(expected_nodes)
       end
 
       context 'connection to the cluster is successful' do
+        let(:expected_nodes) { { nodes: [kube_node.merge(kube_node_metrics)] } }
+        let(:connection_status) { { connection_status: :connected } }
+
         before do
           allow(gl_k8s_node_double).to receive(:all).and_return(expected_nodes)
         end
 
-        let(:expected_nodes) { [kube_node.merge(kube_node_metrics)] }
-
-        it { is_expected.to eq(connection_status: :connected, nodes: expected_nodes) }
+        it { is_expected.to eq(**connection_status, **expected_nodes) }
       end
 
       context 'cluster cannot be reached' do
+        let(:connection_status) { { connection_status: :unreachable, connection_error: :connection_error } }
+
         before do
           allow(cluster.kubeclient.core_client).to receive(:discover)
             .and_raise(SocketError)
         end
 
-        it { is_expected.to eq(connection_status: :unreachable, nodes: expected_nodes) }
+        it { is_expected.to eq(**connection_status, **expected_nodes) }
       end
 
       context 'cluster cannot be authenticated to' do
+        let(:connection_status) { { connection_status: :authentication_failure, connection_error: :authentication_error } }
+
         before do
           allow(cluster.kubeclient.core_client).to receive(:discover)
             .and_raise(OpenSSL::X509::CertificateError.new("Certificate error"))
         end
 
-        it { is_expected.to eq(connection_status: :authentication_failure, nodes: expected_nodes) }
+        it { is_expected.to eq(**connection_status, **expected_nodes) }
       end
 
       describe 'Kubeclient::HttpError' do
+        let(:connection_status) { { connection_status: :authentication_failure, connection_error: :http_error } }
         let(:error_code) { 403 }
         let(:error_message) { "Forbidden" }
 
@@ -1231,28 +1288,32 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
             .and_raise(Kubeclient::HttpError.new(error_code, error_message, nil))
         end
 
-        it { is_expected.to eq(connection_status: :authentication_failure, nodes: expected_nodes) }
+        it { is_expected.to eq(**connection_status, **expected_nodes) }
 
         context 'generic timeout' do
+          let(:connection_status) { { connection_status: :unreachable, connection_error: :http_error } }
           let(:error_message) { 'Timed out connecting to server'}
 
-          it { is_expected.to eq(connection_status: :unreachable, nodes: expected_nodes) }
+          it { is_expected.to eq(**connection_status, **expected_nodes) }
         end
 
         context 'gateway timeout' do
+          let(:connection_status) { { connection_status: :unreachable, connection_error: :http_error } }
           let(:error_message) { '504 Gateway Timeout for GET https://kubernetes.example.com/api/v1'}
 
-          it { is_expected.to eq(connection_status: :unreachable, nodes: expected_nodes) }
+          it { is_expected.to eq(**connection_status, **expected_nodes) }
         end
       end
 
       context 'an uncategorised error is raised' do
+        let(:connection_status) { { connection_status: :unknown_failure, connection_error: :unknown_error } }
+
         before do
           allow(cluster.kubeclient.core_client).to receive(:discover)
             .and_raise(StandardError)
         end
 
-        it { is_expected.to eq(connection_status: :unknown_failure, nodes: expected_nodes) }
+        it { is_expected.to eq(**connection_status, **expected_nodes) }
 
         it 'notifies Sentry' do
           expect(Gitlab::ErrorTracking).to receive(:track_exception)

spec/serializers/cluster_error_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ClusterErrorEntity do
+  describe '#as_json' do
+    let(:cluster) { create(:cluster, :provided_by_user, :group) }
+
+    subject { described_class.new(cluster).as_json }
+
+    context 'when connection_error is present' do
+      before do
+        allow(cluster).to receive(:connection_error).and_return(:connection_error)
+      end
+
+      it { is_expected.to eq({ connection_error: :connection_error, metrics_connection_error: nil, node_connection_error: nil }) }
+    end
+
+    context 'when metrics_connection_error is present' do
+      before do
+        allow(cluster).to receive(:metrics_connection_error).and_return(:http_error)
+      end
+
+      it { is_expected.to eq({ connection_error: nil, metrics_connection_error: :http_error, node_connection_error: nil }) }
+    end
+
+    context 'when node_connection_error is present' do
+      before do
+        allow(cluster).to receive(:node_connection_error).and_return(:unknown_error)
+      end
+
+      it { is_expected.to eq({ connection_error: nil, metrics_connection_error: nil, node_connection_error: :unknown_error }) }
+    end
+  end
+end

spec/serializers/cluster_serializer_spec.rb

@@ -14,6 +14,7 @@ RSpec.describe ClusterSerializer do
         :enabled,
         :environment_scope,
         :gitlab_managed_apps_logs_path,
+        :kubernetes_errors,
         :name,
         :nodes,
         :path,