Merge branch 'disallow-retry-of-old-builds' into 'master'
Disallow retry of old builds Closes #50939 See merge request gitlab-org/gitlab-ce!22538
This commit is contained in:
commit
487820ad50
|
@ -115,6 +115,7 @@ module ApplicationSettingsHelper
|
|||
:akismet_api_key,
|
||||
:akismet_enabled,
|
||||
:allow_local_requests_from_hooks_and_services,
|
||||
:archive_builds_in_human_readable,
|
||||
:authorized_keys_enabled,
|
||||
:auto_devops_enabled,
|
||||
:auto_devops_domain,
|
||||
|
|
|
@ -5,6 +5,7 @@ class ApplicationSetting < ActiveRecord::Base
|
|||
include CacheMarkdownField
|
||||
include TokenAuthenticatable
|
||||
include IgnorableColumn
|
||||
include ChronicDurationAttribute
|
||||
|
||||
add_authentication_token_field :runners_registration_token
|
||||
add_authentication_token_field :health_check_access_token
|
||||
|
@ -45,6 +46,8 @@ class ApplicationSetting < ActiveRecord::Base
|
|||
|
||||
default_value_for :id, 1
|
||||
|
||||
chronic_duration_attr_writer :archive_builds_in_human_readable, :archive_builds_in_seconds
|
||||
|
||||
validates :uuid, presence: true
|
||||
|
||||
validates :session_expire_delay,
|
||||
|
@ -184,6 +187,10 @@ class ApplicationSetting < ActiveRecord::Base
|
|||
|
||||
validates :user_default_internal_regex, js_regex: true, allow_nil: true
|
||||
|
||||
validates :archive_builds_in_seconds,
|
||||
allow_nil: true,
|
||||
numericality: { only_integer: true, greater_than_or_equal_to: 1.day.seconds }
|
||||
|
||||
SUPPORTED_KEY_TYPES.each do |type|
|
||||
validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
|
||||
end
|
||||
|
@ -441,6 +448,10 @@ class ApplicationSetting < ActiveRecord::Base
|
|||
latest_terms
|
||||
end
|
||||
|
||||
def archive_builds_older_than
|
||||
archive_builds_in_seconds.seconds.ago if archive_builds_in_seconds
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def ensure_uuid!
|
||||
|
|
|
@ -258,8 +258,24 @@ module Ci
|
|||
self.name == 'pages'
|
||||
end
|
||||
|
||||
# degenerated build is one that cannot be run by Runner
|
||||
def degenerated?
|
||||
self.options.nil?
|
||||
end
|
||||
|
||||
def degenerate!
|
||||
self.update!(options: nil, yaml_variables: nil, commands: nil)
|
||||
end
|
||||
|
||||
def archived?
|
||||
return true if degenerated?
|
||||
|
||||
archive_builds_older_than = Gitlab::CurrentSettings.current_application_settings.archive_builds_older_than
|
||||
archive_builds_older_than.present? && created_at < archive_builds_older_than
|
||||
end
|
||||
|
||||
def playable?
|
||||
action? && (manual? || scheduled? || retryable?)
|
||||
action? && !archived? && (manual? || scheduled? || retryable?)
|
||||
end
|
||||
|
||||
def schedulable?
|
||||
|
@ -287,7 +303,7 @@ module Ci
|
|||
end
|
||||
|
||||
def retryable?
|
||||
success? || failed? || canceled?
|
||||
!archived? && (success? || failed? || canceled?)
|
||||
end
|
||||
|
||||
def retries_count
|
||||
|
@ -295,7 +311,7 @@ module Ci
|
|||
end
|
||||
|
||||
def retries_max
|
||||
self.options.fetch(:retry, 0).to_i
|
||||
self.options.to_h.fetch(:retry, 0).to_i
|
||||
end
|
||||
|
||||
def latest?
|
||||
|
|
|
@ -51,7 +51,8 @@ class CommitStatus < ActiveRecord::Base
|
|||
missing_dependency_failure: 5,
|
||||
runner_unsupported: 6,
|
||||
stale_schedule: 7,
|
||||
job_execution_timeout: 8
|
||||
job_execution_timeout: 8,
|
||||
archived_failure: 9
|
||||
}
|
||||
|
||||
##
|
||||
|
@ -167,16 +168,18 @@ class CommitStatus < ActiveRecord::Base
|
|||
false
|
||||
end
|
||||
|
||||
# To be overridden when inherrited from
|
||||
def retryable?
|
||||
false
|
||||
end
|
||||
|
||||
# To be overridden when inherrited from
|
||||
def cancelable?
|
||||
false
|
||||
end
|
||||
|
||||
def archived?
|
||||
false
|
||||
end
|
||||
|
||||
def stuck?
|
||||
false
|
||||
end
|
||||
|
|
|
@ -20,12 +20,17 @@ module Ci
|
|||
@subject.project.branch_allows_collaboration?(@user, @subject.ref)
|
||||
end
|
||||
|
||||
condition(:archived, scope: :subject) do
|
||||
@subject.archived?
|
||||
end
|
||||
|
||||
condition(:terminal, scope: :subject) do
|
||||
@subject.has_terminal?
|
||||
end
|
||||
|
||||
rule { protected_ref }.policy do
|
||||
rule { protected_ref | archived }.policy do
|
||||
prevent :update_build
|
||||
prevent :update_commit_status
|
||||
prevent :erase_build
|
||||
end
|
||||
|
||||
|
|
|
@ -2,4 +2,13 @@
|
|||
|
||||
class DeploymentPolicy < BasePolicy
|
||||
delegate { @subject.project }
|
||||
|
||||
condition(:can_retry_deployable) do
|
||||
can?(:update_build, @subject.deployable)
|
||||
end
|
||||
|
||||
rule { ~can_retry_deployable }.policy do
|
||||
prevent :create_deployment
|
||||
prevent :update_deployment
|
||||
end
|
||||
end
|
||||
|
|
|
@ -10,7 +10,8 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
|
|||
missing_dependency_failure: 'There has been a missing dependency failure',
|
||||
runner_unsupported: 'Your runner is outdated, please upgrade your runner',
|
||||
stale_schedule: 'Delayed job could not be executed by some reason, please try again',
|
||||
job_execution_timeout: 'The script exceeded the maximum execution time set for the job'
|
||||
job_execution_timeout: 'The script exceeded the maximum execution time set for the job',
|
||||
archived_failure: 'The job is archived and cannot be run'
|
||||
}.freeze
|
||||
|
||||
private_constant :CALLOUT_FAILURE_MESSAGES
|
||||
|
@ -30,6 +31,6 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
|
|||
end
|
||||
|
||||
def unrecoverable?
|
||||
script_failure? || missing_dependency_failure?
|
||||
script_failure? || missing_dependency_failure? || archived_failure?
|
||||
end
|
||||
end
|
||||
|
|
|
@ -7,6 +7,7 @@ class JobEntity < Grape::Entity
|
|||
expose :name
|
||||
|
||||
expose :started?, as: :started
|
||||
expose :archived?, as: :archived
|
||||
|
||||
expose :build_path do |build|
|
||||
build_path(build)
|
||||
|
|
|
@ -82,6 +82,11 @@ module Ci
|
|||
return false
|
||||
end
|
||||
|
||||
if build.archived?
|
||||
build.drop!(:archived_failure)
|
||||
return false
|
||||
end
|
||||
|
||||
build.run!
|
||||
true
|
||||
end
|
||||
|
|
|
@ -41,5 +41,13 @@
|
|||
The default unit is in seconds, but you can define an alternative. For example:
|
||||
<code>4 mins 2 sec</code>, <code>2h42min</code>.
|
||||
= link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'default-artifacts-expiration')
|
||||
.form-group
|
||||
= f.label :archive_builds_in_human_readable, 'Archive builds in', class: 'label-bold'
|
||||
= f.text_field :archive_builds_in_human_readable, class: 'form-control', placeholder: 'never'
|
||||
.form-text.text-muted
|
||||
Set the duration when build gonna be considered old. Archived builds cannot be retried.
|
||||
Make it empty to never expire builds. It has to be larger than 1 day.
|
||||
The default unit is in seconds, but you can define an alternative. For example:
|
||||
<code>4 mins 2 sec</code>, <code>2h42min</code>.
|
||||
|
||||
= f.submit 'Save changes', class: "btn btn-success"
|
||||
|
|
|
@ -1,4 +1,4 @@
|
|||
- if can?(current_user, :create_deployment, deployment) && deployment.deployable
|
||||
- if can?(current_user, :create_deployment, deployment)
|
||||
- tooltip = deployment.last? ? s_('Environments|Re-deploy to environment') : s_('Environments|Rollback environment')
|
||||
= link_to [:retry, @project.namespace.becomes(Namespace), @project, deployment.deployable], method: :post, class: 'btn btn-build has-tooltip', title: tooltip do
|
||||
- if deployment.last?
|
||||
|
|
|
@ -61,12 +61,14 @@
|
|||
%td.responsive-table-cell.build-failure{ data: { column: _("Failure")} }
|
||||
= build.present.callout_failure_message
|
||||
%td.responsive-table-cell.build-actions
|
||||
= link_to retry_project_job_path(build.project, build, return_to: request.original_url), method: :post, title: _('Retry'), class: 'btn btn-build' do
|
||||
= icon('repeat')
|
||||
%tr.build-trace-row.responsive-table-border-end
|
||||
%td
|
||||
%td.responsive-table-cell.build-trace-container{ colspan: 4 }
|
||||
%pre.build-trace.build-trace-rounded
|
||||
%code.bash.js-build-output
|
||||
= build_summary(build)
|
||||
- if can?(current_user, :update_build, job)
|
||||
= link_to retry_project_job_path(build.project, build, return_to: request.original_url), method: :post, title: _('Retry'), class: 'btn btn-build' do
|
||||
= icon('repeat')
|
||||
- if can?(current_user, :read_build, job)
|
||||
%tr.build-trace-row.responsive-table-border-end
|
||||
%td
|
||||
%td.responsive-table-cell.build-trace-container{ colspan: 4 }
|
||||
%pre.build-trace.build-trace-rounded
|
||||
%code.bash.js-build-output
|
||||
= build_summary(build)
|
||||
= render_if_exists "projects/pipelines/tabs_content", pipeline: @pipeline, project: @project
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Soft-archive old jobs
|
||||
merge_request:
|
||||
author:
|
||||
type: added
|
|
@ -0,0 +1,11 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class AddArchiveBuildsDurationToApplicationSettings < ActiveRecord::Migration
|
||||
include Gitlab::Database::MigrationHelpers
|
||||
|
||||
DOWNTIME = false
|
||||
|
||||
def change
|
||||
add_column(:application_settings, :archive_builds_in_seconds, :integer, allow_null: true)
|
||||
end
|
||||
end
|
|
@ -165,6 +165,7 @@ ActiveRecord::Schema.define(version: 20181101144347) do
|
|||
t.integer "usage_stats_set_by_user_id"
|
||||
t.integer "receive_max_input_size"
|
||||
t.integer "diff_max_patch_bytes", default: 102400, null: false
|
||||
t.integer "archive_builds_in_seconds"
|
||||
end
|
||||
|
||||
create_table "audit_events", force: :cascade do |t|
|
||||
|
|
|
@ -14,7 +14,8 @@ module Gitlab
|
|||
missing_dependency_failure: 'missing dependency failure',
|
||||
runner_unsupported: 'unsupported runner',
|
||||
stale_schedule: 'stale schedule',
|
||||
job_execution_timeout: 'job execution timeout'
|
||||
job_execution_timeout: 'job execution timeout',
|
||||
archived_failure: 'archived failure'
|
||||
}.freeze
|
||||
|
||||
private_constant :REASONS
|
||||
|
|
|
@ -27,6 +27,12 @@ FactoryBot.define do
|
|||
|
||||
pipeline factory: :ci_pipeline
|
||||
|
||||
trait :degenerated do
|
||||
commands nil
|
||||
options nil
|
||||
yaml_variables nil
|
||||
end
|
||||
|
||||
trait :started do
|
||||
started_at 'Di 29. Okt 09:51:28 CET 2013'
|
||||
end
|
||||
|
|
|
@ -53,10 +53,21 @@ describe 'Environment' do
|
|||
|
||||
it 'does show build name' do
|
||||
expect(page).to have_link("#{build.name} (##{build.id})")
|
||||
expect(page).to have_link('Re-deploy')
|
||||
expect(page).not_to have_link('Re-deploy')
|
||||
expect(page).not_to have_terminal_button
|
||||
end
|
||||
|
||||
context 'when user has ability to re-deploy' do
|
||||
let(:permissions) do
|
||||
create(:protected_branch, :developers_can_merge,
|
||||
name: build.ref, project: project)
|
||||
end
|
||||
|
||||
it 'does show re-deploy' do
|
||||
expect(page).to have_link('Re-deploy')
|
||||
end
|
||||
end
|
||||
|
||||
context 'with manual action' do
|
||||
let(:action) do
|
||||
create(:ci_build, :manual, pipeline: pipeline,
|
||||
|
|
|
@ -388,54 +388,83 @@ describe 'Pipeline', :js do
|
|||
let(:pipeline_failures_page) { failures_project_pipeline_path(project, pipeline) }
|
||||
let!(:failed_build) { create(:ci_build, :failed, pipeline: pipeline) }
|
||||
|
||||
subject { visit pipeline_failures_page }
|
||||
|
||||
context 'with failed build' do
|
||||
before do
|
||||
failed_build.trace.set('4 examples, 1 failure')
|
||||
|
||||
visit pipeline_failures_page
|
||||
end
|
||||
|
||||
it 'shows jobs tab pane as active' do
|
||||
subject
|
||||
|
||||
expect(page).to have_content('Failed Jobs')
|
||||
expect(page).to have_css('#js-tab-failures.active')
|
||||
end
|
||||
|
||||
it 'lists failed builds' do
|
||||
subject
|
||||
|
||||
expect(page).to have_content(failed_build.name)
|
||||
expect(page).to have_content(failed_build.stage)
|
||||
end
|
||||
|
||||
it 'shows build failure logs' do
|
||||
subject
|
||||
|
||||
expect(page).to have_content('4 examples, 1 failure')
|
||||
end
|
||||
|
||||
it 'shows the failure reason' do
|
||||
subject
|
||||
|
||||
expect(page).to have_content('There is an unknown failure, please try again')
|
||||
end
|
||||
|
||||
it 'shows retry button for failed build' do
|
||||
page.within(find('.build-failures', match: :first)) do
|
||||
expect(page).to have_link('Retry')
|
||||
context 'when user does not have permission to retry build' do
|
||||
it 'shows retry button for failed build' do
|
||||
subject
|
||||
|
||||
page.within(find('.build-failures', match: :first)) do
|
||||
expect(page).not_to have_link('Retry')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when user does have permission to retry build' do
|
||||
before do
|
||||
create(:protected_branch, :developers_can_merge,
|
||||
name: pipeline.ref, project: project)
|
||||
end
|
||||
|
||||
it 'shows retry button for failed build' do
|
||||
subject
|
||||
|
||||
page.within(find('.build-failures', match: :first)) do
|
||||
expect(page).to have_link('Retry')
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when missing build logs' do
|
||||
before do
|
||||
visit pipeline_failures_page
|
||||
end
|
||||
|
||||
it 'shows jobs tab pane as active' do
|
||||
subject
|
||||
|
||||
expect(page).to have_content('Failed Jobs')
|
||||
expect(page).to have_css('#js-tab-failures.active')
|
||||
end
|
||||
|
||||
it 'lists failed builds' do
|
||||
subject
|
||||
|
||||
expect(page).to have_content(failed_build.name)
|
||||
expect(page).to have_content(failed_build.stage)
|
||||
end
|
||||
|
||||
it 'does not show trace' do
|
||||
subject
|
||||
|
||||
expect(page).to have_content('No job trace')
|
||||
end
|
||||
end
|
||||
|
@ -448,11 +477,9 @@ describe 'Pipeline', :js do
|
|||
end
|
||||
|
||||
context 'when accessing failed jobs page' do
|
||||
before do
|
||||
visit pipeline_failures_page
|
||||
end
|
||||
|
||||
it 'fails to access the page' do
|
||||
subject
|
||||
|
||||
expect(page).to have_title('Access Denied')
|
||||
end
|
||||
end
|
||||
|
@ -461,11 +488,11 @@ describe 'Pipeline', :js do
|
|||
context 'without failures' do
|
||||
before do
|
||||
failed_build.update!(status: :success)
|
||||
|
||||
visit pipeline_failures_page
|
||||
end
|
||||
|
||||
it 'displays the pipeline graph' do
|
||||
subject
|
||||
|
||||
expect(current_path).to eq(pipeline_path(pipeline))
|
||||
expect(page).not_to have_content('Failed Jobs')
|
||||
expect(page).to have_selector('.pipeline-visualization')
|
||||
|
|
|
@ -9,7 +9,8 @@
|
|||
"playable",
|
||||
"created_at",
|
||||
"updated_at",
|
||||
"status"
|
||||
"status",
|
||||
"archived"
|
||||
],
|
||||
"properties": {
|
||||
"id": { "type": "integer" },
|
||||
|
@ -27,7 +28,8 @@
|
|||
"updated_at": { "type": "string" },
|
||||
"status": { "$ref": "../status/ci_detailed_status.json" },
|
||||
"callout_message": { "type": "string" },
|
||||
"recoverable": { "type": "boolean" }
|
||||
"recoverable": { "type": "boolean" },
|
||||
"archived": { "type": "boolean" }
|
||||
},
|
||||
"additionalProperties": true
|
||||
}
|
||||
|
|
|
@ -594,4 +594,24 @@ describe ApplicationSetting do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#archive_builds_older_than' do
|
||||
subject { setting.archive_builds_older_than }
|
||||
|
||||
context 'when the archive_builds_in_seconds is set' do
|
||||
before do
|
||||
setting.archive_builds_in_seconds = 3600
|
||||
end
|
||||
|
||||
it { is_expected.to be_within(1.minute).of(1.hour.ago) }
|
||||
end
|
||||
|
||||
context 'when the archive_builds_in_seconds is set' do
|
||||
before do
|
||||
setting.archive_builds_in_seconds = nil
|
||||
end
|
||||
|
||||
it { is_expected.to be_nil }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1314,6 +1314,14 @@ describe Ci::Build do
|
|||
|
||||
it { is_expected.not_to be_retryable }
|
||||
end
|
||||
|
||||
context 'when build is degenerated' do
|
||||
before do
|
||||
build.degenerate!
|
||||
end
|
||||
|
||||
it { is_expected.not_to be_retryable }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -1396,6 +1404,14 @@ describe Ci::Build do
|
|||
expect(subject.retries_max).to eq 0
|
||||
end
|
||||
end
|
||||
|
||||
context 'when build is degenerated' do
|
||||
subject { create(:ci_build, :degenerated) }
|
||||
|
||||
it 'returns zero' do
|
||||
expect(subject.retries_max).to eq 0
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -1659,6 +1675,12 @@ describe Ci::Build do
|
|||
|
||||
it { is_expected.to be_playable }
|
||||
end
|
||||
|
||||
context 'when build is a manual and degenerated' do
|
||||
subject { build_stubbed(:ci_build, :manual, :degenerated, status: :manual) }
|
||||
|
||||
it { is_expected.not_to be_playable }
|
||||
end
|
||||
end
|
||||
|
||||
context 'when build is scheduled' do
|
||||
|
@ -3227,4 +3249,54 @@ describe Ci::Build do
|
|||
it { expect(build.deployment_status).to eq(:creating) }
|
||||
end
|
||||
end
|
||||
|
||||
describe '#degenerated?' do
|
||||
context 'when build is degenerated' do
|
||||
subject { create(:ci_build, :degenerated) }
|
||||
|
||||
it { is_expected.to be_degenerated }
|
||||
end
|
||||
|
||||
context 'when build is valid' do
|
||||
subject { create(:ci_build) }
|
||||
|
||||
it { is_expected.not_to be_degenerated }
|
||||
|
||||
context 'and becomes degenerated' do
|
||||
before do
|
||||
subject.degenerate!
|
||||
end
|
||||
|
||||
it { is_expected.to be_degenerated }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#archived?' do
|
||||
context 'when build is degenerated' do
|
||||
subject { create(:ci_build, :degenerated) }
|
||||
|
||||
it { is_expected.to be_archived }
|
||||
end
|
||||
|
||||
context 'for old build' do
|
||||
subject { create(:ci_build, created_at: 1.day.ago) }
|
||||
|
||||
context 'when archive_builds_in is set' do
|
||||
before do
|
||||
stub_application_setting(archive_builds_in_seconds: 3600)
|
||||
end
|
||||
|
||||
it { is_expected.to be_archived }
|
||||
end
|
||||
|
||||
context 'when archive_builds_in is not set' do
|
||||
before do
|
||||
stub_application_setting(archive_builds_in_seconds: nil)
|
||||
end
|
||||
|
||||
it { is_expected.not_to be_archived }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -267,7 +267,7 @@ describe Ci::BuildPresenter do
|
|||
let(:build) { create(:ci_build, :failed, :script_failure) }
|
||||
|
||||
context 'when is a script or missing dependency failure' do
|
||||
let(:failure_reasons) { %w(script_failure missing_dependency_failure) }
|
||||
let(:failure_reasons) { %w(script_failure missing_dependency_failure archived_failure) }
|
||||
|
||||
it 'should return false' do
|
||||
failure_reasons.each do |failure_reason|
|
||||
|
|
|
@ -478,6 +478,20 @@ module Ci
|
|||
it_behaves_like 'validation is not active'
|
||||
end
|
||||
end
|
||||
|
||||
context 'when build is degenerated' do
|
||||
let!(:pending_job) { create(:ci_build, :pending, :degenerated, pipeline: pipeline) }
|
||||
|
||||
subject { execute(specific_runner, {}) }
|
||||
|
||||
it 'does not pick the build and drops the build' do
|
||||
expect(subject).to be_nil
|
||||
|
||||
pending_job.reload
|
||||
expect(pending_job).to be_failed
|
||||
expect(pending_job).to be_archived_failure
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#register_success' do
|
||||
|
|
Loading…
Reference in New Issue