Add latest changes from gitlab-org/gitlab@master
This commit is contained in:
parent
6ede90f5dd
commit
c3ad57034c
|
@ -51,6 +51,8 @@ class Projects::JobsController < Projects::ApplicationController
|
|||
build.trace.read do |stream|
|
||||
respond_to do |format|
|
||||
format.json do
|
||||
build.trace.being_watched!
|
||||
|
||||
# TODO: when the feature flag is removed we should not pass
|
||||
# content_format to serialize method.
|
||||
content_format = Feature.enabled?(:job_log_json, @project, default_enabled: true) ? :json : :html
|
||||
|
|
|
@ -29,4 +29,14 @@ class Projects::UploadsController < Projects::ApplicationController
|
|||
|
||||
Project.find_by_full_path("#{namespace}/#{id}")
|
||||
end
|
||||
|
||||
# Overrides ApplicationController#build_canonical_path since there are
|
||||
# multiple routes that match project uploads:
|
||||
# https://gitlab.com/gitlab-org/gitlab/issues/196396
|
||||
def build_canonical_path(project)
|
||||
return super unless action_name == 'show'
|
||||
return super unless params[:secret] && params[:filename]
|
||||
|
||||
show_namespace_project_uploads_url(project.namespace.to_param, project.to_param, params[:secret], params[:filename])
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,10 +1,12 @@
|
|||
-# We currently only support `alert`, `notice`, `success`, 'toast'
|
||||
- icons = {'alert' => 'error', 'notice' => 'information-o', 'success' => 'check-circle'};
|
||||
.flash-container.flash-container-page.sticky
|
||||
- flash.each do |key, value|
|
||||
- if key == 'toast' && value
|
||||
.js-toast-message{ data: { message: value } }
|
||||
- elsif value
|
||||
%div{ class: "flash-#{key} mb-2" }
|
||||
= sprite_icon(icons[key], size: 16, css_class: 'align-middle mr-1') unless icons[key].nil?
|
||||
%span= value
|
||||
%div{ class: "close-icon-wrapper js-close-icon" }
|
||||
= sprite_icon('close', size: 16, css_class: 'close-icon')
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Request less frequent updates from Runner when job log is not being watched
|
||||
merge_request: 20841
|
||||
author:
|
||||
type: performance
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Fix aligment for icons on alerts
|
||||
merge_request: 22760
|
||||
author: Rajendra Kadam
|
||||
type: added
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Fix upload redirections when project has moved
|
||||
merge_request: 22822
|
||||
author:
|
||||
type: fixed
|
|
@ -68,7 +68,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
|
|||
|
||||
resources :uploads, only: [:create] do
|
||||
collection do
|
||||
get ":secret/:filename", action: :show, as: :show, constraints: { filename: %r{[^/]+} }
|
||||
get ":secret/:filename", action: :show, as: :show, constraints: { filename: %r{[^/]+} }, format: false, defaults: { format: nil }
|
||||
post :authorize
|
||||
end
|
||||
end
|
||||
|
|
|
@ -452,7 +452,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
|
|||
|
||||
resources :uploads, only: [:create] do
|
||||
collection do
|
||||
get ":secret/:filename", action: :show, as: :show, constraints: { filename: %r{[^/]+} }
|
||||
get ":secret/:filename", action: :show, as: :show, constraints: { filename: %r{[^/]+} }, format: false, defaults: { format: nil }
|
||||
post :authorize
|
||||
end
|
||||
end
|
||||
|
|
|
@ -21,9 +21,11 @@ scope path: :uploads do
|
|||
as: 'appearance_upload'
|
||||
|
||||
# Project markdown uploads
|
||||
# DEPRECATED: Remove this in GitLab 13.0 because this is redundant to show_namespace_project_uploads
|
||||
# https://gitlab.com/gitlab-org/gitlab/issues/196396
|
||||
get ":namespace_id/:project_id/:secret/:filename",
|
||||
to: "projects/uploads#show",
|
||||
constraints: { namespace_id: /[a-zA-Z.0-9_\-]+/, project_id: /[a-zA-Z.0-9_\-]+/, filename: %r{[^/]+} }
|
||||
to: redirect("%{namespace_id}/%{project_id}/uploads/%{secret}/%{filename}"),
|
||||
constraints: { namespace_id: /[a-zA-Z.0-9_\-]+/, project_id: /[a-zA-Z.0-9_\-]+/, filename: %r{[^/]+} }, format: false, defaults: { format: nil }
|
||||
|
||||
# create uploads for models, snippets (notes) available for now
|
||||
post ':model',
|
||||
|
|
|
@ -200,6 +200,10 @@ module API
|
|||
status 202
|
||||
header 'Job-Status', job.status
|
||||
header 'Range', "0-#{stream_size}"
|
||||
|
||||
if Feature.enabled?(:runner_job_trace_update_interval_header, default_enabled: true)
|
||||
header 'X-GitLab-Trace-Update-Interval', job.trace.update_interval.to_s
|
||||
end
|
||||
end
|
||||
|
||||
desc 'Authorize artifacts uploading for job' do
|
||||
|
|
|
@ -9,6 +9,10 @@ module Gitlab
|
|||
LOCK_TTL = 10.minutes
|
||||
LOCK_RETRIES = 2
|
||||
LOCK_SLEEP = 0.001.seconds
|
||||
WATCH_FLAG_TTL = 10.seconds
|
||||
|
||||
UPDATE_FREQUENCY_DEFAULT = 30.seconds
|
||||
UPDATE_FREQUENCY_WHEN_BEING_WATCHED = 3.seconds
|
||||
|
||||
ArchiveError = Class.new(StandardError)
|
||||
AlreadyArchivedError = Class.new(StandardError)
|
||||
|
@ -119,6 +123,22 @@ module Gitlab
|
|||
end
|
||||
end
|
||||
|
||||
def update_interval
|
||||
being_watched? ? UPDATE_FREQUENCY_WHEN_BEING_WATCHED : UPDATE_FREQUENCY_DEFAULT
|
||||
end
|
||||
|
||||
def being_watched!
|
||||
Gitlab::Redis::SharedState.with do |redis|
|
||||
redis.set(being_watched_cache_key, true, ex: WATCH_FLAG_TTL)
|
||||
end
|
||||
end
|
||||
|
||||
def being_watched?
|
||||
Gitlab::Redis::SharedState.with do |redis|
|
||||
redis.exists(being_watched_cache_key)
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def unsafe_write!(mode, &blk)
|
||||
|
@ -236,6 +256,10 @@ module Gitlab
|
|||
def trace_artifact
|
||||
job.job_artifacts_trace
|
||||
end
|
||||
|
||||
def being_watched_cache_key
|
||||
"gitlab:ci:trace:#{job.id}:watched"
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -19,6 +19,22 @@ describe Groups::UploadsController do
|
|||
let(:uploader_class) { NamespaceFileUploader }
|
||||
end
|
||||
|
||||
context 'with a moved group' do
|
||||
let!(:upload) { create(:upload, :issuable_upload, :with_file, model: model) }
|
||||
let(:group) { model }
|
||||
let(:old_path) { group.to_param + 'old' }
|
||||
let!(:redirect_route) { model.redirect_routes.create(path: old_path) }
|
||||
let(:upload_path) { File.basename(upload.path) }
|
||||
|
||||
it 'redirects to a file with the proper extension' do
|
||||
get :show, params: { group_id: old_path, filename: upload_path, secret: upload.secret }
|
||||
|
||||
expect(response.location).to eq(show_group_uploads_url(group, upload.secret, upload_path))
|
||||
expect(response.location).to end_with(upload.path)
|
||||
expect(response).to have_gitlab_http_status(:redirect)
|
||||
end
|
||||
end
|
||||
|
||||
def post_authorize(verified: true)
|
||||
request.headers.merge!(workhorse_internal_api_request_header) if verified
|
||||
|
||||
|
|
|
@ -556,6 +556,12 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
|
|||
expect(json_response['status']).to eq job.status
|
||||
expect(json_response['lines']).to eq [{ 'content' => [{ 'text' => 'BUILD TRACE' }], 'offset' => 0 }]
|
||||
end
|
||||
|
||||
it 'sets being-watched flag for the job' do
|
||||
expect(response).to have_gitlab_http_status(:ok)
|
||||
|
||||
expect(job.trace.being_watched?).to be(true)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when job has no traces' do
|
||||
|
|
|
@ -25,6 +25,21 @@ describe Projects::UploadsController do
|
|||
end
|
||||
end
|
||||
|
||||
context 'with a moved project' do
|
||||
let!(:upload) { create(:upload, :issuable_upload, :with_file, model: model) }
|
||||
let(:project) { model }
|
||||
let(:upload_path) { File.basename(upload.path) }
|
||||
let!(:redirect_route) { project.redirect_routes.create(path: project.full_path + 'old') }
|
||||
|
||||
it 'redirects to a file with the proper extension' do
|
||||
get :show, params: { namespace_id: project.namespace, project_id: project.to_param + 'old', filename: File.basename(upload.path), secret: upload.secret }
|
||||
|
||||
expect(response.location).to eq(show_project_uploads_url(project, upload.secret, upload_path))
|
||||
expect(response.location).to end_with(upload.path)
|
||||
expect(response).to have_gitlab_http_status(:redirect)
|
||||
end
|
||||
end
|
||||
|
||||
context "when exception occurs" do
|
||||
before do
|
||||
allow(FileUploader).to receive(:workhorse_authorize).and_raise(SocketError.new)
|
||||
|
|
|
@ -26,4 +26,66 @@ describe Gitlab::Ci::Trace, :clean_gitlab_redis_shared_state do
|
|||
|
||||
it_behaves_like 'trace with enabled live trace feature'
|
||||
end
|
||||
|
||||
describe '#update_interval' do
|
||||
context 'it is not being watched' do
|
||||
it 'returns 30 seconds' do
|
||||
expect(trace.update_interval).to eq(30.seconds)
|
||||
end
|
||||
end
|
||||
|
||||
context 'it is being watched' do
|
||||
before do
|
||||
trace.being_watched!
|
||||
end
|
||||
|
||||
it 'returns 3 seconds' do
|
||||
expect(trace.update_interval).to eq(3.seconds)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#being_watched!' do
|
||||
let(:cache_key) { "gitlab:ci:trace:#{build.id}:watched" }
|
||||
|
||||
it 'sets gitlab:ci:trace:<job.id>:watched in redis' do
|
||||
trace.being_watched!
|
||||
|
||||
result = Gitlab::Redis::SharedState.with do |redis|
|
||||
redis.exists(cache_key)
|
||||
end
|
||||
|
||||
expect(result).to eq(true)
|
||||
end
|
||||
|
||||
it 'updates the expiry of gitlab:ci:trace:<job.id>:watched in redis', :clean_gitlab_redis_shared_state do
|
||||
Gitlab::Redis::SharedState.with do |redis|
|
||||
redis.set(cache_key, true, ex: 4.seconds)
|
||||
end
|
||||
|
||||
expect do
|
||||
trace.being_watched!
|
||||
end.to change { Gitlab::Redis::SharedState.with { |redis| redis.pttl(cache_key) } }
|
||||
end
|
||||
end
|
||||
|
||||
describe '#being_watched?' do
|
||||
context 'gitlab:ci:trace:<job.id>:watched in redis is set', :clean_gitlab_redis_shared_state do
|
||||
before do
|
||||
Gitlab::Redis::SharedState.with do |redis|
|
||||
redis.set("gitlab:ci:trace:#{build.id}:watched", true)
|
||||
end
|
||||
end
|
||||
|
||||
it 'returns true' do
|
||||
expect(trace.being_watched?).to be(true)
|
||||
end
|
||||
end
|
||||
|
||||
context 'gitlab:ci:trace:<job.id>:watched in redis is not set' do
|
||||
it 'returns false' do
|
||||
expect(trace.being_watched?).to be(false)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1154,6 +1154,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
|
|||
expect(job.reload.trace.raw).to eq 'BUILD TRACE appended'
|
||||
expect(response.header).to have_key 'Range'
|
||||
expect(response.header).to have_key 'Job-Status'
|
||||
expect(response.header).to have_key 'X-GitLab-Trace-Update-Interval'
|
||||
end
|
||||
|
||||
context 'when job has been updated recently' do
|
||||
|
@ -1291,6 +1292,41 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
|
|||
expect(response.header['Job-Status']).to eq 'canceled'
|
||||
end
|
||||
end
|
||||
|
||||
context 'when build trace is being watched' do
|
||||
before do
|
||||
job.trace.being_watched!
|
||||
end
|
||||
|
||||
it 'returns X-GitLab-Trace-Update-Interval as 3' do
|
||||
patch_the_trace
|
||||
|
||||
expect(response.status).to eq 202
|
||||
expect(response.header['X-GitLab-Trace-Update-Interval']).to eq('3')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when build trace is not being watched' do
|
||||
it 'returns X-GitLab-Trace-Update-Interval as 30' do
|
||||
patch_the_trace
|
||||
|
||||
expect(response.status).to eq 202
|
||||
expect(response.header['X-GitLab-Trace-Update-Interval']).to eq('30')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when feature flag runner_job_trace_update_interval_header is disabled' do
|
||||
before do
|
||||
stub_feature_flags(runner_job_trace_update_interval_header: { enabled: false })
|
||||
end
|
||||
|
||||
it 'does not return X-GitLab-Trace-Update-Interval header' do
|
||||
patch_the_trace
|
||||
|
||||
expect(response.status).to eq 202
|
||||
expect(response.header).not_to have_key 'X-GitLab-Trace-Update-Interval'
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when Runner makes a force-patch' do
|
||||
|
|
|
@ -28,4 +28,12 @@ describe 'Uploads', 'routing' do
|
|||
expect(post("/uploads/#{model}?id=1")).not_to be_routable
|
||||
end
|
||||
end
|
||||
|
||||
describe 'legacy paths' do
|
||||
include RSpec::Rails::RequestExampleGroup
|
||||
|
||||
it 'redirects project uploads to canonical path under project namespace' do
|
||||
expect(get('/uploads/namespace/project/12345/test.png')).to redirect_to('/namespace/project/uploads/12345/test.png')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue