Commit d864efd9 authored by Thong Kuah's avatar Thong Kuah

Merge branch 'pod-logs-add-etag-caching' into 'master'

Add ETag caching for the pod logs json API

See merge request gitlab-org/gitlab!18950
parents 6e0ff297 5c7a4307
......@@ -433,6 +433,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
Gitlab.ee do
get :logs
get '/pods/(:pod_name)/containers/(:container_name)/logs', to: 'environments#k8s_pod_logs', as: :k8s_pod_logs
end
end
......
......@@ -10,6 +10,11 @@ export default {
groupEpicsPath:
'/api/:version/groups/:id/epics?include_ancestor_groups=:includeAncestorGroups&include_descendant_groups=:includeDescendantGroups',
epicIssuePath: '/api/:version/groups/:id/epics/:epic_iid/issues/:issue_id',
podLogsPath: '/:project_full_path/environments/:environment_id/pods/containers/logs.json',
podLogsPathWithPod:
'/:project_full_path/environments/:environment_id/pods/:pod_name/containers/logs.json',
podLogsPathWithPodContainer:
'/:project_full_path/environments/:environment_id/pods/:pod_name/containers/:container_name/logs.json',
userSubscription(namespaceId) {
const url = Api.buildUrl(this.subscriptionPath).replace(':id', encodeURIComponent(namespaceId));
......@@ -70,4 +75,25 @@ export default {
return axios.delete(url);
},
getPodLogs({ projectFullPath, environmentId, podName, containerName }) {
let logPath = this.podLogsPath;
if (podName && containerName) {
logPath = this.podLogsPathWithPodContainer;
} else if (podName) {
logPath = this.podLogsPathWithPod;
}
let url = this.buildUrl(logPath)
.replace(':project_full_path', projectFullPath)
.replace(':environment_id', environmentId);
if (podName) {
url = url.replace(':pod_name', podName);
}
if (containerName) {
url = url.replace(':container_name', containerName);
}
return axios.get(url);
},
};
......@@ -8,13 +8,13 @@ import flash from '~/flash';
import { __, s__, sprintf } from '~/locale';
import _ from 'underscore';
import { backOff } from '~/lib/utils/common_utils';
import Api from 'ee/api';
const requestWithBackoff = (url, params) =>
const TWO_MINUTES = 120000;
const requestWithBackoff = (projectFullPath, environmentId, podName, containerName) =>
backOff((next, stop) => {
axios
.get(url, {
params,
})
Api.getPodLogs({ projectFullPath, environmentId, podName, containerName })
.then(res => {
if (!res.data) {
next();
......@@ -25,17 +25,23 @@ const requestWithBackoff = (url, params) =>
.catch(err => {
stop(err);
});
});
}, TWO_MINUTES);
export default class KubernetesPodLogs extends LogOutputBehaviours {
constructor(container) {
super();
this.options = $(container).data();
const { currentEnvironmentName, environmentsPath, logsEndpoint } = this.options;
const {
currentEnvironmentName,
environmentsPath,
projectFullPath,
environmentId,
} = this.options;
this.environmentName = currentEnvironmentName;
this.environmentsPath = environmentsPath;
this.logsEndpoint = logsEndpoint;
this.projectFullPath = projectFullPath;
this.environmentId = environmentId;
[this.podName] = getParameterValues('pod_name');
if (this.podName) {
......@@ -94,7 +100,7 @@ export default class KubernetesPodLogs extends LogOutputBehaviours {
}
getLogs() {
return requestWithBackoff(this.logsEndpoint, { pod_name: this.podName })
return requestWithBackoff(this.projectFullPath, this.environmentId, this.podName)
.then(res => {
const { logs, pods } = res.data;
this.setupPodsDropdown(pods);
......
......@@ -6,17 +6,16 @@ module EE
extend ActiveSupport::Concern
prepended do
before_action :authorize_read_pod_logs!, only: [:logs]
before_action :environment_ee, only: [:logs]
before_action :authorize_read_pod_logs!, only: [:k8s_pod_logs, :logs]
before_action :environment_ee, only: [:k8s_pod_logs, :logs]
before_action :authorize_create_environment_terminal!, only: [:terminal]
before_action do
push_frontend_feature_flag(:environment_logs_use_vue_ui)
end
end
def logs
def k8s_pod_logs
respond_to do |format|
format.html
format.json do
::Gitlab::UsageCounters::PodLogs.increment(project.id)
::Gitlab::PollingInterval.set_header(response, interval: 3_000)
......@@ -34,6 +33,9 @@ module EE
end
end
def logs
end
private
def environment_ee
......
......@@ -34,7 +34,8 @@ module EE
{
"current-environment-name": environment.name,
"environments-path": project_environments_path(project, format: :json),
"logs-endpoint": logs_project_environment_path(project, environment, format: :json)
"project-full-path": project.full_path,
"environment-id": environment.id
}
end
......
......@@ -6,6 +6,8 @@ module EE
module Kubernetes
extend ActiveSupport::Concern
CACHE_KEY_GET_POD_LOG = 'get_pod_log'
LOGS_LIMIT = 500.freeze
def calculate_reactive_cache_for(environment)
......@@ -26,9 +28,12 @@ module EE
::Gitlab::Kubernetes::RolloutStatus.from_deployments(*deployments, pods: pods, legacy_deployments: legacy_deployments)
end
def read_pod_logs(pod_name, namespace, container: nil)
def read_pod_logs(environment_id, pod_name, namespace, container: nil)
# environment_id is required for use in reactive_cache_updated(),
# to invalidate the ETag cache.
with_reactive_cache(
'get_pod_log',
CACHE_KEY_GET_POD_LOG,
'environment_id' => environment_id,
'pod_name' => pod_name,
'namespace' => namespace,
'container' => container
......@@ -39,7 +44,7 @@ module EE
def calculate_reactive_cache(request, opts)
case request
when 'get_pod_log'
when CACHE_KEY_GET_POD_LOG
container = opts['container']
pod_name = opts['pod_name']
namespace = opts['namespace']
......@@ -52,6 +57,28 @@ module EE
end
end
def reactive_cache_updated(request, opts)
super
case request
when CACHE_KEY_GET_POD_LOG
environment = ::Environment.find_by(id: opts['environment_id'])
return unless environment
::Gitlab::EtagCaching::Store.new.tap do |store|
store.touch(
::Gitlab::Routing.url_helpers.k8s_pod_logs_project_environment_path(
environment.project,
environment,
opts['pod_name'],
opts['container_name'],
format: :json
)
)
end
end
end
private
def pod_logs(pod_name, namespace, container: nil)
......
......@@ -75,6 +75,7 @@ class PodLogsService < ::BaseService
def pod_logs(result)
response = environment.deployment_platform.read_pod_logs(
environment.id,
result[:pod_name],
namespace,
container: result[:container_name]
......
......@@ -4,16 +4,20 @@ module EE
module Gitlab
module EtagCaching
module Router
EE_ROUTES = [
::Gitlab::EtagCaching::Router::Route.new(
%r(^/groups/#{::Gitlab::PathRegex.full_namespace_route_regex}/-/epics/\d+/notes\z),
'epic_notes'
),
::Gitlab::EtagCaching::Router::Route.new(
%r(#{::Gitlab::EtagCaching::Router::RESERVED_WORDS_PREFIX}/environments/\d+/pods/(\S+/)?containers/(\S+/)?logs\.json\z),
'k8s_pod_logs'
)
].freeze
module ClassMethods
def match(path)
epic_route = ::Gitlab::EtagCaching::Router::Route.new(
%r(^/groups/#{::Gitlab::PathRegex.full_namespace_route_regex}/-/epics/\d+/notes\z),
'epic_notes'
)
return epic_route if epic_route.regexp.match(path)
super
EE_ROUTES.find { |route| route.regexp.match(path) } || super
end
end
......
......@@ -92,7 +92,7 @@ describe Projects::EnvironmentsController do
shared_examples 'resource not found' do |message|
it 'returns 400' do
get :logs, params: environment_params(pod_name: pod_name, format: :json)
get :k8s_pod_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)
......@@ -114,7 +114,7 @@ describe Projects::EnvironmentsController do
end
end
context 'when using HTML format' do
context 'when licensed' do
it 'renders logs template' do
get :logs, params: environment_params(pod_name: pod_name)
......@@ -142,7 +142,7 @@ describe Projects::EnvironmentsController do
end
it 'returns the logs for a specific pod' do
get :logs, params: environment_params(pod_name: pod_name, format: :json)
get :k8s_pod_logs, 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"])
......@@ -155,7 +155,7 @@ describe Projects::EnvironmentsController do
it 'registers a usage of the endpoint' do
expect(::Gitlab::UsageCounters::PodLogs).to receive(:increment).with(project.id)
get :logs, params: environment_params(pod_name: pod_name, format: :json)
get :k8s_pod_logs, params: environment_params(pod_name: pod_name, format: :json)
end
context 'when kubernetes API returns error' do
......@@ -170,7 +170,7 @@ describe Projects::EnvironmentsController do
end
it 'returns bad request' do
get :logs, params: environment_params(pod_name: pod_name, format: :json)
get :k8s_pod_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)
......@@ -204,7 +204,7 @@ describe Projects::EnvironmentsController do
end
it 'returns the error without pods, pod_name and container_name' do
get :logs, params: environment_params(pod_name: pod_name, format: :json)
get :k8s_pod_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('No deployment platform')
......@@ -216,7 +216,7 @@ describe Projects::EnvironmentsController do
let(:service_result) { { status: :processing } }
it 'renders accepted' do
get :logs, params: environment_params(pod_name: pod_name, format: :json)
get :k8s_pod_logs, params: environment_params(pod_name: pod_name, format: :json)
expect(response).to have_gitlab_http_status(:accepted)
end
......
......@@ -162,4 +162,54 @@ describe('Api', () => {
.catch(done.fail);
});
});
describe('getPodLogs', () => {
const projectFullPath = 'root/test-project';
const environmentId = 2;
const podName = 'pod';
const containerName = 'container';
const lastUrl = () => mock.history.get[0].url;
beforeEach(() => {
mock.onAny().reply(200);
});
afterEach(() => {
mock.reset();
});
it('calls `axios.get` with pod_name and container_name', done => {
const expectedUrl = `${dummyUrlRoot}/${projectFullPath}/environments/${environmentId}/pods/${podName}/containers/${containerName}/logs.json`;
Api.getPodLogs({ projectFullPath, environmentId, podName, containerName })
.then(() => {
expect(expectedUrl).toBe(lastUrl());
})
.then(done)
.catch(done.fail);
});
it('calls `axios.get` without pod_name and container_name', done => {
const expectedUrl = `${dummyUrlRoot}/${projectFullPath}/environments/${environmentId}/pods/containers/logs.json`;
Api.getPodLogs({ projectFullPath, environmentId })
.then(() => {
expect(expectedUrl).toBe(lastUrl());
})
.then(done)
.catch(done.fail);
});
it('calls `axios.get` with pod_name', done => {
const expectedUrl = `${dummyUrlRoot}/${projectFullPath}/environments/${environmentId}/pods/${podName}/containers/logs.json`;
Api.getPodLogs({ projectFullPath, environmentId, podName })
.then(() => {
expect(expectedUrl).toBe(lastUrl());
})
.then(done)
.catch(done.fail);
});
});
});
......@@ -41,9 +41,10 @@ describe EnvironmentsHelper do
)
end
it 'returns logs parameters data' do
it 'returns parameters for forming the pod logs API URL' do
expect(subject).to include(
"logs-endpoint": logs_project_environment_path(project, environment, format: :json)
"project-full-path": project.full_path,
"environment-id": environment.id
)
end
end
......
......@@ -11,6 +11,7 @@ describe('Kubernetes Logs', () => {
let kubernetesLog;
let mock;
let mockFlash;
let podLogsAPIPath;
preloadFixtures(fixtureTemplate);
......@@ -24,9 +25,11 @@ describe('Kubernetes Logs', () => {
mockDataset = kubernetesLogContainer.dataset;
podLogsAPIPath = `/${mockDataset.projectFullPath}/environments/${mockDataset.environmentId}/pods/containers/logs.json`;
mock = new MockAdapter(axios);
mock.onGet(mockDataset.environmentsPath).reply(200, { environments: mockEnvironmentData });
mock.onGet(mockDataset.logsEndpoint).reply(200, { logs: logMockData, pods: podMockData });
mock.onGet(podLogsAPIPath).reply(200, { logs: logMockData, pods: podMockData });
});
afterEach(() => {
......@@ -158,7 +161,7 @@ describe('Kubernetes Logs', () => {
describe('shows an alert', () => {
it('with an error', done => {
mock.onGet(mockDataset.logsEndpoint).reply(400);
mock.onGet(podLogsAPIPath).reply(400);
kubernetesLog = new KubernetesLogs(kubernetesLogContainer);
kubernetesLog
......@@ -173,7 +176,7 @@ describe('Kubernetes Logs', () => {
it('with some explicit error', done => {
const errorMsg = 'Some k8s error';
mock.onGet(mockDataset.logsEndpoint).reply(400, {
mock.onGet(podLogsAPIPath).reply(400, {
message: errorMsg,
});
......@@ -198,7 +201,7 @@ describe('Kubernetes Logs', () => {
kubernetesLogContainer = document.querySelector('.js-kubernetes-logs');
mock = new MockAdapter(axios);
mock.onGet(mockDataset.logsEndpoint).reply(200, { logs: logMockData, pods: [hackyPodName] });
mock.onGet(podLogsAPIPath).reply(200, { logs: logMockData, pods: [hackyPodName] });
});
afterEach(() => {
......@@ -226,13 +229,15 @@ describe('Kubernetes Logs', () => {
mockDataset = kubernetesLogContainer.dataset;
podLogsAPIPath = `/${mockDataset.projectFullPath}/environments/${
mockDataset.environmentId
}/pods/${podMockData[1]}/containers/logs.json`;
mock = new MockAdapter(axios);
mock.onGet(mockDataset.environmentsPath).reply(200, { environments: mockEnvironmentData });
// Simulate reactive cache, 2 tries needed
mock.onGet(mockDataset.logsEndpoint, { pod_name: podMockData[1] }).replyOnce(202);
mock
.onGet(mockDataset.logsEndpoint, { pod_name: podMockData[1] })
.reply(200, { logs: logMockData, pods: podMockData });
mock.onGet(podLogsAPIPath).replyOnce(202);
mock.onGet(podLogsAPIPath).reply(200, { logs: logMockData, pods: podMockData });
});
it('queries the pod log data polling for reactive cache', done => {
......@@ -243,12 +248,10 @@ describe('Kubernetes Logs', () => {
kubernetesLog
.getData()
.then(() => {
const calls = mock.history.get.filter(r => r.url === mockDataset.logsEndpoint);
const calls = mock.history.get.filter(r => r.url === podLogsAPIPath);
// expect 2 tries
expect(calls.length).toEqual(2);
expect(calls[0].params).toEqual({ pod_name: podMockData[1] });
expect(calls[1].params).toEqual({ pod_name: podMockData[1] });
expect(document.querySelector('.js-build-output').textContent).toContain(
logMockData[0].trim(),
......@@ -270,6 +273,10 @@ describe('Kubernetes Logs', () => {
spyOnDependency(KubernetesLogs, 'getParameterValues').and.callFake(() => [podMockData[2]]);
kubernetesLogContainer = document.querySelector('.js-kubernetes-logs');
podLogsAPIPath = `/${mockDataset.projectFullPath}/environments/${
mockDataset.environmentId
}/pods/${podMockData[2]}/containers/logs.json`;
mock = new MockAdapter(axios);
});
......@@ -278,10 +285,9 @@ describe('Kubernetes Logs', () => {
kubernetesLog
.getData()
.then(() => {
const logsCall = mock.history.get.filter(call => call.url === mockDataset.logsEndpoint);
const logsCall = mock.history.get.filter(call => call.url === podLogsAPIPath);
expect(logsCall.length).toBe(1);
expect(logsCall[0].params.pod_name).toEqual(podMockData[2]);
done();
})
.catch(done.fail);
......
......@@ -19,4 +19,41 @@ describe Gitlab::EtagCaching::Router do
expect(result).to be_blank
end
context 'k8s pod logs' do
it 'matches with pod_name and container_name' do
result = described_class.match(
'/environments/7/pods/pod_name/containers/container_name/logs.json'
)
expect(result).to be_present
expect(result.name).to eq 'k8s_pod_logs'
end
it 'matches with pod_name' do
result = described_class.match(
'/environments/7/pods/pod_name/containers/logs.json'
)
expect(result).to be_present
expect(result.name).to eq 'k8s_pod_logs'
end
it 'matches without pod_name and container_name' do
result = described_class.match(
'/environments/7/pods/containers/logs.json'
)
expect(result).to be_present
expect(result.name).to eq 'k8s_pod_logs'
end
it 'does not match non json format' do
result = described_class.match(
'/environments/7/logs'
)
expect(result).not_to be_present
end
end
end
......@@ -135,13 +135,14 @@ describe Clusters::Platforms::Kubernetes do
end
describe '#read_pod_logs' do
let(:environment) { create(:environment) }
let(:cluster) { create(:cluster, :project, platform_kubernetes: service) }
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, container: container) }
subject { service.read_pod_logs(environment.id, pod_name, namespace, container: container) }
shared_examples 'successful log request' do
it do
......@@ -224,7 +225,7 @@ describe Clusters::Platforms::Kubernetes do
context 'when container name is not specified' do
let(:container) { 'container-0' }
subject { service.read_pod_logs(pod_name, namespace) }
subject { service.read_pod_logs(environment.id, pod_name, namespace) }
before do
stub_kubeclient_pod_details(pod_name, namespace)
......@@ -237,7 +238,15 @@ describe Clusters::Platforms::Kubernetes do
context 'with caching', :use_clean_rails_memory_store_caching do
let(:opts) do
['get_pod_log', { 'pod_name' => pod_name, 'namespace' => namespace, 'container' => container }]
[
'get_pod_log',
{
'environment_id' => environment.id,
'pod_name' => pod_name,
'namespace' => namespace,
'container' => container
}
]
end
context 'result is cacheable' do
......@@ -278,6 +287,37 @@ describe Clusters::Platforms::Kubernetes do
end
end
end
context '#reactive_cache_updated' do
let(:opts) do
{
'environment_id' => environment.id,
'pod_name' => pod_name,
'namespace' => namespace,
'container' => container
}
end
subject { service.reactive_cache_updated('get_pod_log', opts) }
it 'expires k8s_pod_logs etag cache' do
expect_next_instance_of(Gitlab::EtagCaching::Store) do |store|
expect(store).to receive(:touch)
.with(
::Gitlab::Routing.url_helpers.k8s_pod_logs_project_environment_path(
environment.project,
environment,
opts['pod_name'],
opts['container_name'],
format: :json
)
)
.and_call_original
end
subject
end
end
end
describe '#calculate_reactive_cache_for' do
......
......@@ -54,7 +54,7 @@ describe PodLogsService do
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)
.with(environment.id, pod_name, environment.deployment_namespace, container: container_name)
.and_return({
status: :error,
error: message,
......@@ -67,7 +67,7 @@ describe PodLogsService do
shared_context 'return success' do
before do
allow_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
.with(response_pod_name, environment.deployment_namespace, container: container_name)
.with(environment.id, response_pod_name, environment.deployment_namespace, container: container_name)
.and_return({
status: :success,
logs: "Log 1\nLog 2\nLog 3",
......@@ -151,7 +151,7 @@ describe PodLogsService do
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)
.with(environment.id, first_pod_name, environment.deployment_namespace, container: nil)
subject.execute
end
......@@ -188,7 +188,7 @@ describe PodLogsService do
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)
.with(environment.id, pod_name, environment.deployment_namespace, container: container_name)
.and_return(nil)
end
......
......@@ -3,8 +3,6 @@
module Gitlab
module EtagCaching
class Router
prepend_if_ee('EE::Gitlab::EtagCaching::Router') # rubocop: disable Cop/InjectEnterpriseEditionModule
Route = Struct.new(:regexp, :name)
# We enable an ETag for every request matching the regex.
# To match a regex the path needs to match the following:
......@@ -80,3 +78,5 @@ module Gitlab
end
end
end
Gitlab::EtagCaching::Router.prepend_if_ee('EE::Gitlab::EtagCaching::Router')
......@@ -2,7 +2,8 @@
class="js-kubernetes-logs"
data-current-environment-name="production"
data-environments-path="/root/my-project/environments.json"
data-logs-endpoint="/root/my-project/environments/1/logs.json"
data-project-full-path="root/my-project"
data-environment-id=1
>
<div class="build-page-pod-logs">
<div class="build-trace-container prepend-top-default">
......
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