Merge branch 'ak/use-cluster-in-service' into 'master'

Make logs services agnostic from environment

See merge request gitlab-org/gitlab!25252

ee/app/controllers/projects/logs_controller.rb

@@ -4,6 +4,7 @@ module Projects
   class LogsController < Projects::ApplicationController
     before_action :authorize_read_pod_logs!
     before_action :environment
+    before_action :ensure_deployments, only: %i(k8s elasticsearch)
 
     def index
       if environment.nil?
@@ -27,7 +28,7 @@ module Projects
       ::Gitlab::UsageCounters::PodLogs.increment(project.id)
       ::Gitlab::PollingInterval.set_header(response, interval: 3_000)
 
-      result = service.new(environment, params: permitted_params).execute
+      result = service.new(cluster, namespace, params: permitted_params).execute
 
       if result.nil?
         head :accepted
@@ -57,5 +58,22 @@ module Projects
                          project.default_environment
                        end
     end
+
+    def cluster
+      environment.deployment_platform&.cluster
+    end
+
+    def namespace
+      environment.deployment_namespace
+    end
+
+    def ensure_deployments
+      return if cluster && namespace.present?
+
+      render status: :bad_request, json: {
+        status: :error,
+        message: _('Environment does not have deployments')
+      }
+    end
   end
 end

ee/app/services/pod_logs/base_service.rb

@@ -5,7 +5,7 @@ module PodLogs
     include ReactiveCaching
     include Stepable
 
-    attr_reader :environment, :params
+    attr_reader :cluster, :namespace, :params
 
     CACHE_KEY_GET_POD_LOG = 'get_pod_log'
     K8S_NAME_MAX_LENGTH = 253
@@ -13,24 +13,26 @@ module PodLogs
     SUCCESS_RETURN_KEYS = %i(status logs pod_name container_name pods).freeze
 
     def id
-      environment.id
+      cluster.id
     end
 
-    def initialize(environment, params: {})
-      @environment = environment
+    def initialize(cluster, namespace, params: {})
+      @cluster = cluster
+      @namespace = namespace
       @params = filter_params(params.dup.stringify_keys).to_hash
     end
 
     def execute
       with_reactive_cache(
         CACHE_KEY_GET_POD_LOG,
+        namespace,
         params
       ) do |result|
         result
       end
     end
 
-    def calculate_reactive_cache(request, _opts)
+    def calculate_reactive_cache(request, _namespace, _params)
       case request
       when CACHE_KEY_GET_POD_LOG
         execute_steps
@@ -47,6 +49,13 @@ module PodLogs
       %w(pod_name container_name)
     end
 
+    def check_arguments(result)
+      return error(_('Cluster does not exist')) if cluster.nil?
+      return error(_('Namespace is empty')) if namespace.blank?
+
+      success(result)
+    end
+
     def check_param_lengths(_result)
       pod_name = params['pod_name'].presence
       container_name = params['container_name'].presence
@@ -62,17 +71,8 @@ module PodLogs
       success(pod_name: pod_name, container_name: container_name)
     end
 
-    def check_deployment_platform(result)
-      unless environment.deployment_platform
-        return error(_('No deployment platform available'))
-      end
-
-      success(result)
-    end
-
     def get_raw_pods(result)
-      namespace = environment.deployment_namespace
-      result[:raw_pods] = environment.deployment_platform.kubeclient.get_pods(namespace: namespace)
+      result[:raw_pods] = cluster.kubeclient.get_pods(namespace: namespace)
 
       success(result)
     end
@@ -85,7 +85,7 @@ module PodLogs
 
     def check_pod_name(result)
       # If pod_name is not received as parameter, get the pod logs of the first
-      # pod of this environment.
+      # pod of this namespace.
       result[:pod_name] ||= result[:pods].first
 
       unless result[:pod_name]

ee/app/services/pod_logs/elasticsearch_service.rb

@@ -2,8 +2,8 @@
 
 module PodLogs
   class ElasticsearchService < BaseService
-    steps :check_param_lengths,
-          :check_deployment_platform,
+    steps :check_arguments,
+          :check_param_lengths,
           :get_raw_pods,
           :get_pod_names,
           :check_pod_name,
@@ -13,7 +13,7 @@ module PodLogs
           :pod_logs,
           :filter_return_keys
 
-    self.reactive_cache_worker_finder = ->(id, _cache_key, params) { new(Environment.find(id), params: params) }
+    self.reactive_cache_worker_finder = ->(id, _cache_key, namespace, params) { new(::Clusters::Cluster.find(id), namespace, params: params) }
 
     private
 
@@ -37,8 +37,7 @@ module PodLogs
     end
 
     def pod_logs(result)
-      namespace = environment.deployment_namespace
-      client = environment.deployment_platform.cluster&.application_elastic_stack&.elasticsearch_client
+      client = cluster&.application_elastic_stack&.elasticsearch_client
       return error(_('Unable to connect to Elasticsearch')) unless client
 
       result[:logs] = ::Gitlab::Elasticsearch::Logs.new(client).pod_logs(

ee/app/services/pod_logs/kubernetes_service.rb

@@ -4,8 +4,8 @@ module PodLogs
   class KubernetesService < BaseService
     LOGS_LIMIT = 500.freeze
 
-    steps :check_param_lengths,
-          :check_deployment_platform,
+    steps :check_arguments,
+          :check_param_lengths,
           :get_raw_pods,
           :get_pod_names,
           :check_pod_name,
@@ -13,14 +13,14 @@ module PodLogs
           :pod_logs,
           :filter_return_keys
 
-    self.reactive_cache_worker_finder = ->(id, _cache_key, params) { new(Environment.find(id), params: params) }
+    self.reactive_cache_worker_finder = ->(id, _cache_key, namespace, params) { new(::Clusters::Cluster.find(id), namespace, params: params) }
 
     private
 
     def pod_logs(result)
-      logs = environment.deployment_platform.kubeclient.get_pod_log(
+      logs = cluster.kubeclient.get_pod_log(
         result[:pod_name],
-        environment.deployment_namespace,
+        namespace,
         container: result[:container_name],
         tail_lines: LOGS_LIMIT,
         timestamps: true

ee/spec/controllers/projects/logs_controller_spec.rb

@@ -58,120 +58,111 @@ describe Projects::LogsController do
     end
   end
 
-  describe "GET #k8s" do
+  shared_examples 'pod logs service' do |endpoint, service|
     let(:service_result) do
       {
         status: :success,
         logs: ['Log 1', 'Log 2', 'Log 3'],
-        message: 'message',
         pods: [pod_name],
         pod_name: pod_name,
         container_name: container
       }
     end
+    let(:service_result_json) { JSON.parse(service_result.to_json) }
+
+    let_it_be(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) }
 
     before do
       stub_licensed_features(pod_logs: true)
 
-      allow_next_instance_of(::PodLogs::KubernetesService) do |instance|
+      allow_next_instance_of(service) do |instance|
         allow(instance).to receive(:execute).and_return(service_result)
       end
     end
 
-    shared_examples 'resource not found' do |message|
-      it 'returns 400', :aggregate_failures do
-        get :k8s, params: environment_params(pod_name: pod_name, format: :json)
+    it 'returns 404 when unlicensed' do
+      stub_licensed_features(pod_logs: false)
 
-        expect(response).to have_gitlab_http_status(:bad_request)
-        expect(json_response['message']).to eq(message)
-        expect(json_response['pods']).to match_array([pod_name])
-        expect(json_response['pod_name']).to eq(pod_name)
-        expect(json_response['container_name']).to eq(container)
-      end
+      get endpoint, params: environment_params(pod_name: pod_name, format: :json)
+
+      expect(response).to have_gitlab_http_status(:not_found)
     end
 
-    it 'returns the logs for a specific pod', :aggregate_failures do
-      get :k8s, params: environment_params(pod_name: pod_name, format: :json)
+    it 'returns the service result' do
+      get endpoint, params: environment_params(pod_name: pod_name, format: :json)
 
       expect(response).to have_gitlab_http_status(:success)
-      expect(json_response["logs"]).to match_array(["Log 1", "Log 2", "Log 3"])
-      expect(json_response["pods"]).to match_array([pod_name])
-      expect(json_response['message']).to eq(service_result[:message])
-      expect(json_response['pod_name']).to eq(pod_name)
-      expect(json_response['container_name']).to eq(container)
+      expect(json_response).to eq(service_result_json)
     end
 
     it 'registers a usage of the endpoint' do
       expect(::Gitlab::UsageCounters::PodLogs).to receive(:increment).with(project.id)
 
-      get :k8s, params: environment_params(pod_name: pod_name, format: :json)
-    end
+      get endpoint, params: environment_params(pod_name: pod_name, format: :json)
 
-    context 'when kubernetes API returns error' do
-      let(:service_result) do
-        {
-          status: :error,
-          message: 'Kubernetes API returned status code: 400',
-          pods: [pod_name],
-          pod_name: pod_name,
-          container_name: container
-        }
-      end
+      expect(response).to have_gitlab_http_status(:success)
+    end
 
-      it 'returns bad request' do
-        get :k8s, params: environment_params(pod_name: pod_name, format: :json)
+    it 'sets the polling header' do
+      get endpoint, params: environment_params(pod_name: pod_name, format: :json)
 
-        expect(response).to have_gitlab_http_status(:bad_request)
-        expect(json_response["logs"]).to eq(nil)
-        expect(json_response["pods"]).to match_array([pod_name])
-        expect(json_response["message"]).to eq('Kubernetes API returned status code: 400')
-        expect(json_response['pod_name']).to eq(pod_name)
-        expect(json_response['container_name']).to eq(container)
-      end
+      expect(response).to have_gitlab_http_status(:success)
+      expect(response.headers['Poll-Interval']).to eq('3000')
     end
 
-    context 'when pod does not exist' do
-      let(:service_result) do
-        {
-          status: :error,
-          message: 'Pod not found',
-          pods: [pod_name],
-          pod_name: pod_name,
-          container_name: container
-        }
-      end
+    context 'when service is processing' do
+      let(:service_result) { nil }
 
-      it_behaves_like 'resource not found', 'Pod not found'
+      it 'returns a 202' do
+        get endpoint, params: environment_params(pod_name: pod_name, format: :json)
+
+        expect(response).to have_gitlab_http_status(:accepted)
+      end
     end
 
-    context 'when service returns error without pods, pod_name, container_name' do
+    shared_examples 'unsuccessful execution response' do |message|
       let(:service_result) do
         {
           status: :error,
-          message: 'No deployment platform'
+          message: message
         }
       end
 
-      it 'returns the error without pods, pod_name and container_name' do
-        get :k8s, params: environment_params(pod_name: pod_name, format: :json)
+      it 'returns the error' do
+        get endpoint, params: environment_params(pod_name: pod_name, format: :json)
 
         expect(response).to have_gitlab_http_status(:bad_request)
-        expect(json_response['message']).to eq('No deployment platform')
-        expect(json_response.keys).to contain_exactly('message', 'status')
+        expect(json_response).to eq(service_result_json)
       end
     end
 
-    context 'when service is processing (returns nil)' do
-      let(:service_result) { nil }
+    context 'when service is failing' do
+      it_behaves_like 'unsuccessful execution response', 'some error'
+    end
 
-      it 'renders accepted' do
-        get :k8s, params: environment_params(pod_name: pod_name, format: :json)
+    context 'when cluster is nil' do
+      let!(:cluster) { nil }
 
-        expect(response).to have_gitlab_http_status(:accepted)
+      it_behaves_like 'unsuccessful execution response', 'Environment does not have deployments'
+    end
+
+    context 'when namespace is empty' do
+      before do
+        allow(environment).to receive(:deployment_namespace).and_return('')
       end
+
+      it_behaves_like 'unsuccessful execution response', 'Environment does not have deployments'
     end
   end
 
+  describe 'GET #k8s' do
+    it_behaves_like 'pod logs service', :k8s, PodLogs::KubernetesService
+  end
+
+  describe 'GET #elasticsearch' do
+    it_behaves_like 'pod logs service', :elasticsearch, PodLogs::ElasticsearchService
+  end
+
   def environment_params(opts = {})
     opts.reverse_merge(namespace_id: project.namespace,
                        project_id: project,

ee/spec/services/pod_logs/base_service_spec.rb

@@ -5,7 +5,8 @@ require 'spec_helper'
 describe ::PodLogs::BaseService do
   include KubernetesHelpers
 
-  let_it_be(:environment, refind: true) { create(:environment) }
+  let_it_be(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*') }
+  let(:namespace) { 'autodevops-deploy-9-production' }
 
   let(:pod_name) { 'pod-1' }
   let(:container_name) { 'container-0' }
@@ -16,7 +17,7 @@ describe ::PodLogs::BaseService do
     ].to_json, object_class: OpenStruct)
   end
 
-  subject { described_class.new(environment, params: params) }
+  subject { described_class.new(cluster, namespace, params: params) }
 
   describe '#initialize' do
     let(:params) do
@@ -27,7 +28,8 @@ describe ::PodLogs::BaseService do
     end
 
     it 'filters the parameters' do
-      expect(subject.environment).to eq(environment)
+      expect(subject.cluster).to eq(cluster)
+      expect(subject.namespace).to eq(namespace)
       expect(subject.params).to eq({
         'container_name' => container_name
       })
@@ -35,6 +37,49 @@ describe ::PodLogs::BaseService do
     end
   end
 
+  describe '#check_arguments' do
+    context 'when cluster and namespace are provided' do
+      it 'returns success' do
+        result = subject.send(:check_arguments, {})
+
+        expect(result[:status]).to eq(:success)
+      end
+    end
+
+    context 'when cluster is nil' do
+      let(:cluster) { nil }
+
+      it 'returns an error' do
+        result = subject.send(:check_arguments, {})
+
+        expect(result[:status]).to eq(:error)
+        expect(result[:message]).to eq('Cluster does not exist')
+      end
+    end
+
+    context 'when namespace is nil' do
+      let(:namespace) { nil }
+
+      it 'returns an error' do
+        result = subject.send(:check_arguments, {})
+
+        expect(result[:status]).to eq(:error)
+        expect(result[:message]).to eq('Namespace is empty')
+      end
+    end
+
+    context 'when namespace is empty' do
+      let(:namespace) { '' }
+
+      it 'returns an error' do
+        result = subject.send(:check_arguments, {})
+
+        expect(result[:status]).to eq(:error)
+        expect(result[:message]).to eq('Namespace is empty')
+      end
+    end
+  end
+
   describe '#check_param_lengths' do
     context 'when pod_name and container_name are provided' do
       let(:params) do
@@ -84,31 +129,11 @@ describe ::PodLogs::BaseService do
     end
   end
 
-  describe '#check_deployment_platform' do
-    it 'returns success when deployment platform exist' do
-      create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [environment.project])
-      create(:deployment, :success, environment: environment)
-
-      result = subject.send(:check_deployment_platform, {})
-
-      expect(result[:status]).to eq(:success)
-    end
-
-    it 'returns error when deployment platform does not exist' do
-      result = subject.send(:check_deployment_platform, {})
-
-      expect(result[:status]).to eq(:error)
-      expect(result[:message]).to eq('No deployment platform available')
-    end
-  end
-
   describe '#get_raw_pods' do
     let(:service) { create(:cluster_platform_kubernetes, :configured) }
 
     it 'returns success with passthrough k8s response' do
-      create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [environment.project])
-      create(:deployment, :success, environment: environment)
-      stub_kubeclient_pods(environment.deployment_namespace)
+      stub_kubeclient_pods(namespace)
 
       result = subject.send(:get_raw_pods, {})
 

ee/spec/services/pod_logs/elasticsearch_service_spec.rb

@@ -3,7 +3,8 @@
 require 'spec_helper'
 
 describe ::PodLogs::ElasticsearchService do
-  let_it_be(:environment, refind: true) { create(:environment) }
+  let_it_be(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*') }
+  let(:namespace) { 'autodevops-deploy-9-production' }
 
   let(:pod_name) { 'pod-1' }
   let(:container_name) { 'container-1' }
@@ -19,7 +20,7 @@ describe ::PodLogs::ElasticsearchService do
     ]
   end
 
-  subject { described_class.new(environment, params: params) }
+  subject { described_class.new(cluster, namespace, params: params) }
 
   describe '#check_times' do
     context 'with start and end provided and valid' do
@@ -116,7 +117,6 @@ describe ::PodLogs::ElasticsearchService do
   end
 
   describe '#pod_logs' do
-    let(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [environment.project]) }
     let(:result_arg) do
       {
         pod_name: pod_name,
@@ -128,7 +128,6 @@ describe ::PodLogs::ElasticsearchService do
     end
 
     before do
-      create(:deployment, :success, environment: environment)
       create(:clusters_applications_elastic_stack, :installed, cluster: cluster)
     end
 
@@ -138,7 +137,7 @@ describe ::PodLogs::ElasticsearchService do
         .and_return(Elasticsearch::Transport::Client.new)
       allow_any_instance_of(::Gitlab::Elasticsearch::Logs)
         .to receive(:pod_logs)
-        .with(environment.deployment_namespace, pod_name, container_name, search, start_time, end_time)
+        .with(namespace, pod_name, container_name, search, start_time, end_time)
         .and_return(expected_logs)
 
       result = subject.send(:pod_logs, result_arg)

ee/spec/services/pod_logs/kubernetes_service_spec.rb

@@ -5,7 +5,8 @@ require 'spec_helper'
 describe ::PodLogs::KubernetesService do
   include KubernetesHelpers
 
-  let_it_be(:environment, refind: true) { create(:environment) }
+  let_it_be(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*') }
+  let(:namespace) { 'autodevops-deploy-9-production' }
 
   let(:pod_name) { 'pod-1' }
   let(:container_name) { 'container-1' }
@@ -18,7 +19,7 @@ describe ::PodLogs::KubernetesService do
     ]
   end
 
-  subject { described_class.new(environment, params: params) }
+  subject { described_class.new(cluster, namespace, params: params) }
 
   describe '#pod_logs' do
     let(:result_arg) do
@@ -29,13 +30,8 @@ describe ::PodLogs::KubernetesService do
     end
     let(:service) { create(:cluster_platform_kubernetes, :configured) }
 
-    before do
-      create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [environment.project])
-      create(:deployment, :success, environment: environment)
-    end
-
     it 'returns the logs' do
-      stub_kubeclient_logs(pod_name, environment.deployment_namespace, container: container_name)
+      stub_kubeclient_logs(pod_name, namespace, container: container_name)
 
       result = subject.send(:pod_logs, result_arg)
 

locale/gitlab.pot

@@ -3935,6 +3935,9 @@ msgstr ""
 msgid "Cluster cache cleared."
 msgstr ""
 
+msgid "Cluster does not exist"
+msgstr ""
+
 msgid "ClusterIntegration| %{custom_domain_start}More information%{custom_domain_end}."
 msgstr ""
 
@@ -7412,6 +7415,9 @@ msgstr ""
 msgid "Environment"
 msgstr ""
 
+msgid "Environment does not have deployments"
+msgstr ""
+
 msgid "Environment variables are applied to environments via the runner. They can be protected by only exposing them to protected branches or tags. Additionally, they can be masked so they are hidden in job logs, though they must match certain regexp requirements to do so. You can use environment variables for passwords, secret keys, or whatever you want."
 msgstr ""
 
@@ -12659,6 +12665,9 @@ msgstr ""
 msgid "Name:"
 msgstr ""
 
+msgid "Namespace is empty"
+msgstr ""
+
 msgid "Namespace: %{namespace}"
 msgstr ""
 
@@ -12916,9 +12925,6 @@ msgstr ""
 msgid "No data to display"
 msgstr ""
 
-msgid "No deployment platform available"
-msgstr ""
-
 msgid "No deployments found"
 msgstr ""