Commit 969d098e authored by Thong Kuah's avatar Thong Kuah

Merge branch '12819-fix-pod-logs-bug' into 'master'

Fix error when displaying pod logs of pod that has multiple containers

See merge request gitlab-org/gitlab!16236
parents d68d7872 985aaedc
......@@ -6,6 +6,7 @@ module Clusters
include Gitlab::Kubernetes
include EnumWithNil
include AfterCommitQueue
include ReactiveCaching
RESERVED_NAMESPACES = %w(gitlab-managed-apps).freeze
......
......@@ -4,7 +4,7 @@ import { getParameterValues } from '~/lib/utils/url_utility';
import { isScrolledToBottom, scrollDown, toggleDisableButton } from '~/lib/utils/scroll_utils';
import LogOutputBehaviours from '~/lib/utils/logoutput_behaviours';
import createFlash from '~/flash';
import { __, s__ } from '~/locale';
import { sprintf, __, s__ } from '~/locale';
import _ from 'underscore';
export default class KubernetesPodLogs extends LogOutputBehaviours {
......@@ -69,7 +69,18 @@ export default class KubernetesPodLogs extends LogOutputBehaviours {
this.$buildRefreshAnimation.hide();
toggleDisableButton(this.$refreshLogBtn, false);
})
.catch(() => createFlash(__('Something went wrong on our end')));
.catch(err => {
let message = '';
if (err.response) {
message = sprintf(`Error: %{message}`, { message: err.response.data.message });
}
createFlash(
sprintf(__(`Something went wrong on our end. %{message}`), {
message,
}),
);
});
}
populateDropdown(pods) {
......
......@@ -18,13 +18,25 @@ module EE
::Gitlab::UsageCounters::PodLogs.increment(project.id)
::Gitlab::PollingInterval.set_header(response, interval: 3_000)
result = PodLogsService.new(environment, params: params.permit!).execute
if result.nil?
head :accepted
elsif result[:status] == :success
render json: {
logs: pod_logs.strip.split("\n").as_json,
pods: environment.pod_names
pods: environment.pod_names,
logs: result[:logs],
message: result[:message]
}
else
render status: :bad_request, json: {
pods: environment.pod_names,
message: result[:message]
}
end
end
end
end
private
......@@ -32,10 +44,6 @@ module EE
environment
end
def pod_logs
environment.deployment_platform.read_pod_logs(params[:pod_name], environment.deployment_namespace)
end
def authorize_create_environment_terminal!
return render_404 unless can?(current_user, :create_environment_terminal, environment)
end
......
......@@ -27,13 +27,70 @@ module EE
end
def read_pod_logs(pod_name, namespace, container: nil)
kubeclient.get_pod_log(pod_name, namespace, container: container, tail_lines: LOGS_LIMIT).as_json
rescue Kubeclient::ResourceNotFoundError
[]
if ::Feature.enabled?(:pod_logs_reactive_cache)
with_reactive_cache(
'get_pod_log',
'pod_name' => pod_name,
'namespace' => namespace,
'container' => container
) do |result|
result
end
else
pod_logs(pod_name, namespace, container: container)
end
end
def calculate_reactive_cache(request, opts)
case request
when 'get_pod_log'
handle_exceptions(_('Pod not found')) do
container = opts['container']
pod_name = opts['pod_name']
namespace = opts['namespace']
container ||= container_names_of(pod_name, namespace).first
pod_logs(pod_name, namespace, container: container)
end
end
end
private
def pod_logs(pod_name, namespace, container: nil)
handle_exceptions(_('Pod not found')) do
logs = kubeclient.get_pod_log(
pod_name, namespace, container: container, tail_lines: LOGS_LIMIT
).body
{ logs: logs, status: :success }
end
end
def handle_exceptions(resource_not_found_error_message, &block)
yield
rescue Kubeclient::ResourceNotFoundError
{ error: resource_not_found_error_message, status: :error }
rescue Kubeclient::HttpError => e
::Gitlab::Sentry.track_acceptable_exception(e)
{
error: _('Kubernetes API returned status code: %{error_code}') % {
error_code: e.error_code
},
status: :error
}
end
def container_names_of(pod_name, namespace)
return [] unless pod_name.present?
pod_details = kubeclient.get_pod(pod_name, namespace)
pod_details.spec.containers.collect(&:name)
end
def read_deployments(namespace)
kubeclient.get_deployments(namespace: namespace).as_json
rescue Kubeclient::ResourceNotFoundError
......
# frozen_string_literal: true
class PodLogsService < ::BaseService
attr_reader :environment
K8S_NAME_MAX_LENGTH = 253
PARAMS = %w(pod_name container_name).freeze
def initialize(environment, params: {})
@environment = environment
@params = filter_params(params.dup).to_hash
end
def execute
pod_name = params['pod_name'].presence
container_name = params['container_name'].presence
if pod_name&.length.to_i > K8S_NAME_MAX_LENGTH
return error(_('pod_name cannot be larger than %{max_length}'\
' chars' % { max_length: K8S_NAME_MAX_LENGTH }))
elsif container_name&.length.to_i > K8S_NAME_MAX_LENGTH
return error(_('container_name cannot be larger than'\
' %{max_length} chars' % { max_length: K8S_NAME_MAX_LENGTH }))
end
unless environment.deployment_platform
return error('No deployment platform')
end
# If pod_name is not received as parameter, get the pod logs of the first
# pod of this environment.
pod_name ||= environment.pod_names&.first
pod_logs(pod_name, container_name)
end
private
def pod_logs(pod_name, container_name)
result = environment.deployment_platform.read_pod_logs(
pod_name,
namespace,
container: container_name
)
return unless result
if result[:status] == :error
error(result[:error])
else
logs = split_by_newline(result[:logs])
success(logs: logs)
end
end
def filter_params(params)
params.slice(*PARAMS)
end
def split_by_newline(logs)
return unless logs
logs.strip.split("\n").as_json
end
def namespace
environment.deployment_namespace
end
end
......@@ -86,12 +86,22 @@ describe Projects::EnvironmentsController do
environment_scope: '*', projects: [project])
create(:deployment, :success, environment: environment)
allow_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
.with(pod_name, environment.deployment_namespace).and_return(kube_logs_body)
allow_any_instance_of(Gitlab::Kubernetes::RolloutStatus).to receive(:instances)
.and_return([{ pod_name: pod_name }])
end
shared_examples 'resource not found' do |message|
let(:service_result) { { status: :error, message: message } }
it 'returns 400' do
get :logs, params: environment_params(pod_name: pod_name, format: :json)
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])
end
end
context 'when unlicensed' do
before do
stub_licensed_features(pod_logs: false)
......@@ -114,12 +124,25 @@ describe Projects::EnvironmentsController do
end
context 'when using JSON format' do
let(:service_result) do
{
status: :success,
logs: ['Log 1', 'Log 2', 'Log 3'],
message: 'message'
}
end
before do
allow_any_instance_of(PodLogsService).to receive(:execute).and_return(service_result)
end
it 'returns the logs for a specific pod' do
get :logs, params: environment_params(pod_name: pod_name, format: :json)
expect(response).to be_ok
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])
end
it 'registers a usage of the endpoint' do
......@@ -127,6 +150,35 @@ describe Projects::EnvironmentsController do
get :logs, params: environment_params(pod_name: pod_name, format: :json)
end
context 'when kubernetes API returns error' do
let(:service_result) { { status: :error, message: 'Kubernetes API returned status code: 400' } }
it 'returns bad request' do
get :logs, 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')
end
end
context 'when pod does not exist' do
let(:service_result) { { status: :error, message: 'Pod not found' } }
it_behaves_like 'resource not found', 'Pod not found'
end
context 'when service returns nil' do
let(:service_result) { nil }
it 'renders accepted' do
get :logs, params: environment_params(pod_name: pod_name, format: :json)
expect(response).to have_gitlab_http_status(:accepted)
end
end
end
end
......
......@@ -11,15 +11,20 @@ describe 'Environment > Pod Logs', :js do
let(:pod_name) { pod_names.first }
let(:project) { create(:project, :repository) }
let(:environment) { create(:environment, project: project) }
let(:service) { create(:cluster_platform_kubernetes, :configured) }
before do
stub_licensed_features(pod_logs: true)
# We're setting this feature flag to false since the FE does not support it
# as yet.
stub_feature_flags(pod_logs_reactive_cache: false)
create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project])
create(:deployment, :success, environment: environment)
allow_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
.with(pod_name, environment.deployment_namespace).and_return(kube_logs_body)
stub_kubeclient_logs(pod_name, environment.deployment_namespace, container: nil)
allow_any_instance_of(EE::Environment).to receive(:pod_names).and_return(pod_names)
sign_in(project.owner)
......@@ -41,7 +46,7 @@ describe 'Environment > Pod Logs', :js do
expect(item.text).to eq(pod_names[i])
end
end
expect(page).to have_content("Log 1 Log 2 Log 3")
expect(page).to have_content("Log 1\\nLog 2\\nLog 3")
end
end
......@@ -79,11 +84,11 @@ describe 'Environment > Pod Logs', :js do
end
def perf_bar_height
page.evaluate_script("$('#js-peek').height()")
page.evaluate_script("$('#js-peek').height()").to_i
end
def navbar_height
page.evaluate_script("$('.js-navbar').height()")
page.evaluate_script("$('.js-navbar').height()").to_i
end
def log_header_top
......
......@@ -4,6 +4,25 @@ require 'spec_helper'
describe Clusters::Platforms::Kubernetes do
include KubernetesHelpers
include ReactiveCachingHelpers
shared_examples 'resource not found error' do |message|
it 'raises error' do
result = subject
expect(result[:error]).to eq(message)
expect(result[:status]).to eq(:error)
end
end
shared_examples 'kubernetes API error' do |error_code|
it 'raises error' do
result = subject
expect(result[:error]).to eq("Kubernetes API returned status code: #{error_code}")
expect(result[:status]).to eq(:error)
end
end
describe '#rollout_status' do
let(:deployments) { [] }
......@@ -120,16 +139,21 @@ describe Clusters::Platforms::Kubernetes do
let(:service) { create(:cluster_platform_kubernetes, :configured) }
let(:pod_name) { 'pod-1' }
let(:namespace) { 'app' }
let(:container) { 'some-container' }
subject { service.read_pod_logs(pod_name, namespace) }
subject { service.read_pod_logs(pod_name, namespace, container: container) }
context 'when kubernetes responds with valid logs' do
before do
stub_kubeclient_logs(pod_name, namespace)
shared_examples 'successful log request' do
it do
expect(subject[:logs]).to eq("\"Log 1\\nLog 2\\nLog 3\"")
expect(subject[:status]).to eq(:success)
end
end
shared_examples 'successful log request' do
it { expect(subject.body).to eq("\"Log 1\\nLog 2\\nLog 3\"") }
shared_examples 'k8s responses' do
context 'when kubernetes responds with valid logs' do
before do
stub_kubeclient_logs(pod_name, namespace, container: container)
end
context 'on a project level cluster' do
......@@ -153,18 +177,117 @@ describe Clusters::Platforms::Kubernetes do
context 'when kubernetes responds with 500s' do
before do
stub_kubeclient_logs(pod_name, namespace, status: 500)
stub_kubeclient_logs(pod_name, namespace, container: 'some-container', status: 500)
end
it { expect { subject }.to raise_error(::Kubeclient::HttpError) }
it_behaves_like 'kubernetes API error', 500
end
context 'when container does not exist' do
before do
container = 'some-container'
stub_kubeclient_logs(pod_name, namespace, container: container,
status: 400, message: "container #{container} is not valid for pod #{pod_name}")
end
it_behaves_like 'kubernetes API error', 400
end
context 'when kubernetes responds with 404s' do
before do
stub_kubeclient_logs(pod_name, namespace, status: 404)
stub_kubeclient_logs(pod_name, namespace, container: 'some-container', status: 404)
end
it_behaves_like 'resource not found error', 'Pod not found'
end
end
context 'without pod_logs_reactive_cache feature flag' do
before do
stub_feature_flags(pod_logs_reactive_cache: false)
end
it_behaves_like 'k8s responses'
context 'when container name is not specified' do
subject { service.read_pod_logs(pod_name, namespace) }
before do
stub_kubeclient_logs(pod_name, namespace, container: nil)
end
include_examples 'successful log request'
end
end
it { is_expected.to be_empty }
context 'with pod_logs_reactive_cache feature flag' do
before do
stub_feature_flags(pod_logs_reactive_cache: true)
synchronous_reactive_cache(service)
end
it_behaves_like 'k8s responses'
context 'when container name is not specified' do
subject { service.read_pod_logs(pod_name, namespace) }
before do
stub_kubeclient_pod_details(pod_name, namespace)
stub_kubeclient_logs(pod_name, namespace, container: 'container-0')
end
include_examples 'successful log request'
end
end
context 'with caching', :use_clean_rails_memory_store_caching do
let(:opts) do
['get_pod_log', { 'pod_name' => pod_name, 'namespace' => namespace, 'container' => container }]
end
before do
stub_feature_flags(pod_logs_reactive_cache: true)
end
context 'result is cacheable' do
before do
stub_kubeclient_logs(pod_name, namespace, container: container)
end
it do
result = subject
expect { stub_reactive_cache(service, result, opts) }.not_to raise_error
end
end
context 'when value present in cache' do
let(:return_value) { { 'status' => :success, 'logs' => 'logs' } }
before do
stub_reactive_cache(service, return_value, opts)
end
it 'returns cached value' do
result = subject
expect(result).to eq(return_value)
end
end
context 'when value not present in cache' do
it 'returns nil' do
expect(ReactiveCachingWorker)
.to receive(:perform_async)
.with(service.class, service.id, *opts)
result = subject
expect(result).to eq(nil)
end
end
end
end
......
# frozen_string_literal: true
require 'spec_helper'
describe PodLogsService do
include KubernetesHelpers
include ReactiveCachingHelpers
describe '#execute' do
let(:environment) { create(:environment, name: 'production') }
let(:project) { environment.project }
let(:pod_name) { 'pod-1' }
let(:container_name) { 'container-1' }
let(:logs) { ['Log 1', 'Log 2', 'Log 3'] }
let(:result) { subject.execute }
let(:params) do
ActionController::Parameters.new(
{
'pod_name' => pod_name,
'container_name' => container_name
}
).permit!
end
subject { described_class.new(environment, params: params) }
shared_examples 'success' do |message|
it do
expect(result[:status]).to eq(:success)
expect(result[:logs]).to eq(logs)
end
end
shared_examples 'error' do |message|
it do
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq(message)
end
end
shared_context 'return error' do |message|
before do
allow_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
.with(pod_name, environment.deployment_namespace, container: container_name)
.and_return({ status: :error, error: message })
end
end
shared_context 'return success' do
before do
allow_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
.with(pod_name, environment.deployment_namespace, container: container_name)
.and_return({ status: :success, logs: "Log 1\nLog 2\nLog 3" })
end
end
shared_context 'deployment platform' do
before do
create(:cluster, :provided_by_gcp,
environment_scope: '*', projects: [project])
allow_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
.with(pod_name, environment.deployment_namespace, container: container_name)
.and_return(kube_logs_body)
end
end
context 'when pod name is too large' do
let(:pod_name) { '1' * 254 }
it_behaves_like 'error', 'pod_name cannot be larger than 253 chars'
end
context 'when container name is too large' do
let(:container_name) { '1' * 254 }
it_behaves_like 'error', 'container_name cannot be larger than 253 chars'
end
context 'without deployment platform' do
it_behaves_like 'error', 'No deployment platform'
end
context 'with deployment platform' do
include_context 'deployment platform'
context 'when pod does not exist' do
include_context 'return error', 'Pod not found'
it_behaves_like 'error', 'Pod not found'
end
context 'when container_name is specified' do
include_context 'return success'
it_behaves_like 'success'
end
context 'when container_name is not specified' do
let(:container_name) { nil }
let(:params) do
ActionController::Parameters.new(
{
'pod_name' => pod_name,
'container_name' => nil
}
).permit!
end
include_context 'return success'
it_behaves_like 'success'
end
context 'when pod_name is not specified' do
let(:pod_name) { '' }
let(:container_name) { nil }
let(:first_pod_name) { 'some-pod' }
before do
create(:deployment, :success, environment: environment)
allow_any_instance_of(Gitlab::Kubernetes::RolloutStatus).to receive(:instances)
.and_return([{ pod_name: first_pod_name }])
allow_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
.with(first_pod_name, environment.deployment_namespace, container: nil)
.and_return({ status: :success, logs: "Log 1\nLog 2\nLog 3" })
end
it_behaves_like 'success'
it 'returns logs of first pod' do
expect_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
.with(first_pod_name, environment.deployment_namespace, container: nil)
subject.execute
end
end
context 'when error is returned' do
include_context 'return error', 'Kubernetes API returned status code: 400'
it_behaves_like 'error', 'Kubernetes API returned status code: 400'
end
context 'when nil is returned' do
before do
allow_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
.with(pod_name, environment.deployment_namespace, container: container_name)
.and_return(nil)
end
it 'returns nil' do
expect(result).to eq(nil)
end
end
end
end
end
......@@ -8937,6 +8937,9 @@ msgstr ""
msgid "Kubernetes"
msgstr ""
msgid "Kubernetes API returned status code: %{error_code}"
msgstr ""
msgid "Kubernetes Cluster"
msgstr ""
......@@ -11517,6 +11520,9 @@ msgstr ""
msgid "Please wait while we import the repository for you. Refresh at will."
msgstr ""
msgid "Pod not found"
msgstr ""
msgid "Pods in use"
msgstr ""
......@@ -14582,6 +14588,9 @@ msgstr ""
msgid "Something went wrong on our end."
msgstr ""
msgid "Something went wrong on our end. %{message}"
msgstr ""
msgid "Something went wrong on our end. Please try again!"
msgstr ""
......@@ -18861,6 +18870,9 @@ msgstr ""
msgid "connecting"
msgstr ""
msgid "container_name cannot be larger than %{max_length} chars"
msgstr ""
msgid "could not read private key, is the passphrase correct?"
msgstr ""
......@@ -19431,6 +19443,9 @@ msgstr ""
msgid "pipeline"
msgstr ""
msgid "pod_name cannot be larger than %{max_length} chars"
msgstr ""
msgid "point"
msgid_plural "points"
msgstr[0] ""
......
......@@ -11,6 +11,10 @@ module KubernetesHelpers
kube_response(kube_pods_body)
end
def kube_pod_response
kube_response(kube_pod)
end
def kube_logs_response
kube_response(kube_logs_body)
end
......@@ -63,11 +67,30 @@ module KubernetesHelpers
WebMock.stub_request(:get, pods_url).to_return(response || kube_pods_response)
end
def stub_kubeclient_logs(pod_name, namespace, status: nil)
def stub_kubeclient_pod_details(pod, namespace, status: nil)
stub_kubeclient_discover(service.api_url)
logs_url = service.api_url + "/api/v1/namespaces/#{namespace}/pods/#{pod_name}/log?tailLines=#{Clusters::Platforms::Kubernetes::LOGS_LIMIT}"
pod_url = service.api_url + "/api/v1/namespaces/#{namespace}/pods/#{pod}"
response = { status: status } if status
WebMock.stub_request(:get, pod_url).to_return(response || kube_pod_response)
end
def stub_kubeclient_logs(pod_name, namespace, container: nil, status: nil, message: nil)
stub_kubeclient_discover(service.api_url)
if container
container_query_param = "container=#{container}&"
end
logs_url = service.api_url + "/api/v1/namespaces/#{namespace}/pods/#{pod_name}" \
"/log?#{container_query_param}tailLines=#{Clusters::Platforms::Kubernetes::LOGS_LIMIT}"
if status
response = { status: status }
response[:body] = { message: message }.to_json if message
end
WebMock.stub_request(:get, logs_url).to_return(response || kube_logs_response)
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment