Commit 5c7a4307 authored by Reuben Pereira's avatar Reuben Pereira Committed by Thong Kuah

Add ETag caching for the pod logs json API

- ETag caching will help reduce the load from polling the API.
- Query parameters are not supported with ETag caching since the
middleware only looks at the request path.

Modify the frontend to work with path params

We've changed the pod logs API to use path params instead of
query params in order to support ETag caching.

Simpify api replacing patterns

- Use a single object as a parameter
- Remove some code
- Update specs

Separate out html and json logs endpoints

- Create separate APIs for the logs HTML page and the JSON endpoint
that returns k8s logs.
- Also correct the route for the JSON endpoint by adding the resource
names to the route.
parent 6e0ff297
......@@ -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
module ClassMethods
def match(path)
epic_route = ::Gitlab::EtagCaching::Router::Route.new(
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
return epic_route if epic_route.regexp.match(path)
super
module ClassMethods
def match(path)
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