Commit b2a7faa5 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'use-chronic-duration-attribute-for-project-build-timeout' into 'master'

Use chronic duration attribute for project build timeout

See merge request gitlab-org/gitlab-ce!17386
parents 63a1a570 0f66a1b4
...@@ -4,41 +4,4 @@ class Projects::PipelinesSettingsController < Projects::ApplicationController ...@@ -4,41 +4,4 @@ class Projects::PipelinesSettingsController < Projects::ApplicationController
def show def show
redirect_to project_settings_ci_cd_path(@project, params: params) redirect_to project_settings_ci_cd_path(@project, params: params)
end end
def update
Projects::UpdateService.new(project, current_user, update_params).tap do |service|
if service.execute
flash[:notice] = "Pipelines settings for '#{@project.name}' were successfully updated."
run_autodevops_pipeline(service)
redirect_to project_settings_ci_cd_path(@project)
else
render 'show'
end
end
end
private
def run_autodevops_pipeline(service)
return unless service.run_auto_devops_pipeline?
if @project.empty_repo?
flash[:warning] = "This repository is currently empty. A new Auto DevOps pipeline will be created after a new file has been pushed to a branch."
return
end
CreatePipelineWorker.perform_async(project.id, current_user.id, project.default_branch, :web, ignore_skip_ci: true, save_on_errors: false)
flash[:success] = "A new Auto DevOps pipeline has been created, go to <a href=\"#{project_pipelines_path(@project)}\">Pipelines page</a> for details".html_safe
end
def update_params
params.require(:project).permit(
:runners_token, :builds_enabled, :build_allow_git_fetch,
:build_timeout_in_minutes, :build_coverage_regex, :public_builds,
:auto_cancel_pending_pipelines, :ci_config_path,
auto_devops_attributes: [:id, :domain, :enabled]
)
end
end end
...@@ -25,7 +25,7 @@ class Projects::RefsController < Projects::ApplicationController ...@@ -25,7 +25,7 @@ class Projects::RefsController < Projects::ApplicationController
when "graphs_commits" when "graphs_commits"
commits_project_graph_path(@project, @id) commits_project_graph_path(@project, @id)
when "badges" when "badges"
project_pipelines_settings_path(@project, ref: @id) project_settings_ci_cd_path(@project, ref: @id)
else else
project_commits_path(@project, @id) project_commits_path(@project, @id)
end end
......
...@@ -2,13 +2,24 @@ module Projects ...@@ -2,13 +2,24 @@ module Projects
module Settings module Settings
class CiCdController < Projects::ApplicationController class CiCdController < Projects::ApplicationController
before_action :authorize_admin_pipeline! before_action :authorize_admin_pipeline!
before_action :define_variables
def show def show
define_runners_variables end
define_secret_variables
define_triggers_variables def update
define_badges_variables Projects::UpdateService.new(project, current_user, update_params).tap do |service|
define_auto_devops_variables result = service.execute
if result[:status] == :success
flash[:notice] = "Pipelines settings for '#{@project.name}' were successfully updated."
run_autodevops_pipeline(service)
redirect_to project_settings_ci_cd_path(@project)
else
render 'show'
end
end
end end
def reset_cache def reset_cache
...@@ -25,6 +36,35 @@ module Projects ...@@ -25,6 +36,35 @@ module Projects
private private
def update_params
params.require(:project).permit(
:runners_token, :builds_enabled, :build_allow_git_fetch,
:build_timeout_human_readable, :build_coverage_regex, :public_builds,
:auto_cancel_pending_pipelines, :ci_config_path,
auto_devops_attributes: [:id, :domain, :enabled]
)
end
def run_autodevops_pipeline(service)
return unless service.run_auto_devops_pipeline?
if @project.empty_repo?
flash[:warning] = "This repository is currently empty. A new Auto DevOps pipeline will be created after a new file has been pushed to a branch."
return
end
CreatePipelineWorker.perform_async(project.id, current_user.id, project.default_branch, :web, ignore_skip_ci: true, save_on_errors: false)
flash[:success] = "A new Auto DevOps pipeline has been created, go to <a href=\"#{project_pipelines_path(@project)}\">Pipelines page</a> for details".html_safe
end
def define_variables
define_runners_variables
define_secret_variables
define_triggers_variables
define_badges_variables
define_auto_devops_variables
end
def define_runners_variables def define_runners_variables
@project_runners = @project.runners.ordered @project_runners = @project.runners.ordered
@assignable_runners = current_user.ci_authorized_runners @assignable_runners = current_user.ci_authorized_runners
......
...@@ -324,7 +324,7 @@ class ProjectsController < Projects::ApplicationController ...@@ -324,7 +324,7 @@ class ProjectsController < Projects::ApplicationController
:avatar, :avatar,
:build_allow_git_fetch, :build_allow_git_fetch,
:build_coverage_regex, :build_coverage_regex,
:build_timeout_in_minutes, :build_timeout_human_readable,
:resolve_outdated_diff_discussions, :resolve_outdated_diff_discussions,
:container_registry_enabled, :container_registry_enabled,
:default_branch, :default_branch,
......
...@@ -8,14 +8,14 @@ module ChronicDurationAttribute ...@@ -8,14 +8,14 @@ module ChronicDurationAttribute
end end
end end
def chronic_duration_attr_writer(virtual_attribute, source_attribute) def chronic_duration_attr_writer(virtual_attribute, source_attribute, parameters = {})
chronic_duration_attr_reader(virtual_attribute, source_attribute) chronic_duration_attr_reader(virtual_attribute, source_attribute)
define_method("#{virtual_attribute}=") do |value| define_method("#{virtual_attribute}=") do |value|
chronic_duration_attributes[virtual_attribute] = value.presence || '' chronic_duration_attributes[virtual_attribute] = value.presence || parameters[:default].presence.to_s
begin begin
new_value = ChronicDuration.parse(value).to_i if value.present? new_value = value.present? ? ChronicDuration.parse(value).to_i : parameters[:default].presence
assign_attributes(source_attribute => new_value) assign_attributes(source_attribute => new_value)
rescue ChronicDuration::DurationParseError rescue ChronicDuration::DurationParseError
# ignore error as it will be caught by validation # ignore error as it will be caught by validation
......
...@@ -21,6 +21,7 @@ class Project < ActiveRecord::Base ...@@ -21,6 +21,7 @@ class Project < ActiveRecord::Base
include Gitlab::SQL::Pattern include Gitlab::SQL::Pattern
include DeploymentPlatform include DeploymentPlatform
include ::Gitlab::Utils::StrongMemoize include ::Gitlab::Utils::StrongMemoize
include ChronicDurationAttribute
extend Gitlab::ConfigHelper extend Gitlab::ConfigHelper
...@@ -325,6 +326,12 @@ class Project < ActiveRecord::Base ...@@ -325,6 +326,12 @@ class Project < ActiveRecord::Base
enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 } enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 }
chronic_duration_attr :build_timeout_human_readable, :build_timeout, default: 3600
validates :build_timeout, allow_nil: true,
numericality: { greater_than_or_equal_to: 600,
message: 'needs to be at least 10 minutes' }
# Returns a collection of projects that is either public or visible to the # Returns a collection of projects that is either public or visible to the
# logged in user. # logged in user.
def self.public_or_visible_to_user(user = nil) def self.public_or_visible_to_user(user = nil)
...@@ -1309,14 +1316,6 @@ class Project < ActiveRecord::Base ...@@ -1309,14 +1316,6 @@ class Project < ActiveRecord::Base
self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token) self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token)
end end
def build_timeout_in_minutes
build_timeout / 60
end
def build_timeout_in_minutes=(value)
self.build_timeout = value.to_i * 60
end
def open_issues_count def open_issues_count
Projects::OpenIssuesCountService.new(self).count Projects::OpenIssuesCountService.new(self).count
end end
......
.row.prepend-top-default .row.prepend-top-default
.col-lg-12 .col-lg-12
= form_for @project, url: project_pipelines_settings_path(@project) do |f| = form_for @project, url: project_settings_ci_cd_path(@project) do |f|
= form_errors(@project)
%fieldset.builds-feature %fieldset.builds-feature
.form-group .form-group
%h5 Auto DevOps (Beta) %h5 Auto DevOps (Beta)
...@@ -73,10 +74,10 @@ ...@@ -73,10 +74,10 @@
%hr %hr
.form-group .form-group
= f.label :build_timeout_in_minutes, 'Timeout', class: 'label-light' = f.label :build_timeout_human_readable, 'Timeout', class: 'label-light'
= f.number_field :build_timeout_in_minutes, class: 'form-control', min: '0' = f.text_field :build_timeout_human_readable, class: 'form-control'
%p.help-block %p.help-block
Per job in minutes. If a job passes this threshold, it will be marked as failed Per job. If a job passes this threshold, it will be marked as failed
= link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'timeout'), target: '_blank' = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'timeout'), target: '_blank'
%hr %hr
...@@ -160,4 +161,4 @@ ...@@ -160,4 +161,4 @@
%hr %hr
.row.prepend-top-default .row.prepend-top-default
= render partial: 'projects/pipelines_settings/badge', collection: @badges = render partial: 'badge', collection: @badges
...@@ -3,8 +3,9 @@ ...@@ -3,8 +3,9 @@
- page_title "CI / CD" - page_title "CI / CD"
- expanded = Rails.env.test? - expanded = Rails.env.test?
- general_expanded = @project.errors.empty? ? expanded : true
%section.settings#js-general-pipeline-settings.no-animate{ class: ('expanded' if expanded) } %section.settings#js-general-pipeline-settings.no-animate{ class: ('expanded' if general_expanded) }
.settings-header .settings-header
%h4 %h4
General pipelines settings General pipelines settings
...@@ -13,7 +14,7 @@ ...@@ -13,7 +14,7 @@
%p %p
Update your CI/CD configuration, like job timeout or Auto DevOps. Update your CI/CD configuration, like job timeout or Auto DevOps.
.settings-content .settings-content
= render 'projects/pipelines_settings/show' = render 'form'
%section.settings.no-animate{ class: ('expanded' if expanded) } %section.settings.no-animate{ class: ('expanded' if expanded) }
.settings-header .settings-header
......
---
title: Use human readable value build_timeout in Project
merge_request: 17386
author:
type: changed
...@@ -420,7 +420,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do ...@@ -420,7 +420,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end end
namespace :settings do namespace :settings do
get :members, to: redirect("%{namespace_id}/%{project_id}/project_members") get :members, to: redirect("%{namespace_id}/%{project_id}/project_members")
resource :ci_cd, only: [:show], controller: 'ci_cd' do resource :ci_cd, only: [:show, :update], controller: 'ci_cd' do
post :reset_cache post :reset_cache
end end
resource :integrations, only: [:show] resource :integrations, only: [:show]
......
...@@ -11,82 +11,11 @@ describe Projects::PipelinesSettingsController do ...@@ -11,82 +11,11 @@ describe Projects::PipelinesSettingsController do
sign_in(user) sign_in(user)
end end
describe 'PATCH update' do describe 'GET show' do
subject do it 'redirects with 302 status code' do
patch :update, get :show, namespace_id: project.namespace, project_id: project
namespace_id: project.namespace.to_param,
project_id: project,
project: {
auto_devops_attributes: params
}
end
context 'when updating the auto_devops settings' do
let(:params) { { enabled: '', domain: 'mepmep.md' } }
it 'redirects to the settings page' do
subject
expect(response).to have_gitlab_http_status(302)
expect(flash[:notice]).to eq("Pipelines settings for '#{project.name}' were successfully updated.")
end
context 'following the instance default' do
let(:params) { { enabled: '' } }
it 'allows enabled to be set to nil' do
subject
project_auto_devops.reload
expect(project_auto_devops.enabled).to be_nil
end
end
context 'when run_auto_devops_pipeline is true' do
before do
expect_any_instance_of(Projects::UpdateService).to receive(:run_auto_devops_pipeline?).and_return(true)
end
context 'when the project repository is empty' do
it 'sets a warning flash' do
expect(subject).to set_flash[:warning]
end
it 'does not queue a CreatePipelineWorker' do
expect(CreatePipelineWorker).not_to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
subject
end
end
context 'when the project repository is not empty' do
let(:project) { create(:project, :repository) }
it 'sets a success flash' do
allow(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
expect(subject).to set_flash[:success]
end
it 'queues a CreatePipelineWorker' do
expect(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
subject
end
end
end
context 'when run_auto_devops_pipeline is not true' do
before do
expect_any_instance_of(Projects::UpdateService).to receive(:run_auto_devops_pipeline?).and_return(false)
end
it 'does not queue a CreatePipelineWorker' do
expect(CreatePipelineWorker).not_to receive(:perform_async).with(project.id, user.id, :web, any_args)
subject expect(response).to have_gitlab_http_status(302)
end
end
end end
end end
end end
require('spec_helper') require('spec_helper')
describe Projects::Settings::CiCdController do describe Projects::Settings::CiCdController do
let(:project) { create(:project, :public, :access_requestable) } set(:user) { create(:user) }
let(:user) { create(:user) } set(:project_auto_devops) { create(:project_auto_devops) }
let(:project) { project_auto_devops.project }
before do before do
project.add_master(user) project.add_master(user)
...@@ -55,4 +56,107 @@ describe Projects::Settings::CiCdController do ...@@ -55,4 +56,107 @@ describe Projects::Settings::CiCdController do
end end
end end
end end
describe 'PATCH update' do
let(:params) { { ci_config_path: '' } }
subject do
patch :update,
namespace_id: project.namespace.to_param,
project_id: project,
project: params
end
it 'redirects to the settings page' do
subject
expect(response).to have_gitlab_http_status(302)
expect(flash[:notice]).to eq("Pipelines settings for '#{project.name}' were successfully updated.")
end
context 'when updating the auto_devops settings' do
let(:params) { { auto_devops_attributes: { enabled: '', domain: 'mepmep.md' } } }
context 'following the instance default' do
let(:params) { { auto_devops_attributes: { enabled: '' } } }
it 'allows enabled to be set to nil' do
subject
project_auto_devops.reload
expect(project_auto_devops.enabled).to be_nil
end
end
context 'when run_auto_devops_pipeline is true' do
before do
expect_any_instance_of(Projects::UpdateService).to receive(:run_auto_devops_pipeline?).and_return(true)
end
context 'when the project repository is empty' do
it 'sets a warning flash' do
expect(subject).to set_flash[:warning]
end
it 'does not queue a CreatePipelineWorker' do
expect(CreatePipelineWorker).not_to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
subject
end
end
context 'when the project repository is not empty' do
let(:project) { create(:project, :repository) }
it 'sets a success flash' do
allow(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
expect(subject).to set_flash[:success]
end
it 'queues a CreatePipelineWorker' do
expect(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
subject
end
end
end
context 'when run_auto_devops_pipeline is not true' do
before do
expect_any_instance_of(Projects::UpdateService).to receive(:run_auto_devops_pipeline?).and_return(false)
end
it 'does not queue a CreatePipelineWorker' do
expect(CreatePipelineWorker).not_to receive(:perform_async).with(project.id, user.id, :web, any_args)
subject
end
end
end
context 'when updating general settings' do
context 'when build_timeout_human_readable is not specified' do
let(:params) { { build_timeout_human_readable: '' } }
it 'set default timeout' do
subject
project.reload
expect(project.build_timeout).to eq(3600)
end
end
context 'when build_timeout_human_readable is specified' do
let(:params) { { build_timeout_human_readable: '1h 30m' } }
it 'set specified timeout' do
subject
project.reload
expect(project.build_timeout).to eq(5400)
end
end
end
end
end end
...@@ -6,7 +6,7 @@ feature 'list of badges' do ...@@ -6,7 +6,7 @@ feature 'list of badges' do
project = create(:project, :repository) project = create(:project, :repository)
project.add_master(user) project.add_master(user)
sign_in(user) sign_in(user)
visit project_pipelines_settings_path(project) visit project_settings_ci_cd_path(project)
end end
scenario 'user wants to see build status badge' do scenario 'user wants to see build status badge' do
......
...@@ -63,8 +63,8 @@ shared_examples 'ChronicDurationAttribute writer' do ...@@ -63,8 +63,8 @@ shared_examples 'ChronicDurationAttribute writer' do
subject.send("#{virtual_field}=", '') subject.send("#{virtual_field}=", '')
end end
it 'writes nil' do it 'writes default value' do
expect(subject.send(source_field)).to be_nil expect(subject.send(source_field)).to eq(default_value)
end end
it 'passes validation' do it 'passes validation' do
...@@ -77,8 +77,8 @@ shared_examples 'ChronicDurationAttribute writer' do ...@@ -77,8 +77,8 @@ shared_examples 'ChronicDurationAttribute writer' do
subject.send("#{virtual_field}=", nil) subject.send("#{virtual_field}=", nil)
end end
it 'writes nil' do it 'writes default value' do
expect(subject.send(source_field)).to be_nil expect(subject.send(source_field)).to eq(default_value)
end end
it 'passes validation' do it 'passes validation' do
...@@ -92,20 +92,34 @@ shared_examples 'ChronicDurationAttribute writer' do ...@@ -92,20 +92,34 @@ shared_examples 'ChronicDurationAttribute writer' do
end end
describe 'ChronicDurationAttribute' do describe 'ChronicDurationAttribute' do
let(:source_field) {:maximum_timeout} context 'when default value is not set' do
let(:virtual_field) {:maximum_timeout_human_readable} let(:source_field) {:maximum_timeout}
let(:virtual_field) {:maximum_timeout_human_readable}
let(:default_value) { nil }
subject { Ci::Runner.new } subject { create(:ci_runner) }
it_behaves_like 'ChronicDurationAttribute reader' it_behaves_like 'ChronicDurationAttribute reader'
it_behaves_like 'ChronicDurationAttribute writer' it_behaves_like 'ChronicDurationAttribute writer'
end
context 'when default value is set' do
let(:source_field) {:build_timeout}
let(:virtual_field) {:build_timeout_human_readable}
let(:default_value) { 3600 }
subject { create(:project) }
it_behaves_like 'ChronicDurationAttribute reader'
it_behaves_like 'ChronicDurationAttribute writer'
end
end end
describe 'ChronicDurationAttribute - reader' do describe 'ChronicDurationAttribute - reader' do
let(:source_field) {:timeout} let(:source_field) {:timeout}
let(:virtual_field) {:timeout_human_readable} let(:virtual_field) {:timeout_human_readable}
subject {Ci::BuildMetadata.new} subject { create(:ci_build).ensure_metadata }
it "doesn't contain dynamically created writer method" do it "doesn't contain dynamically created writer method" do
expect(subject.class).not_to be_public_method_defined("#{virtual_field}=") expect(subject.class).not_to be_public_method_defined("#{virtual_field}=")
......
require 'spec_helper' require 'spec_helper'
describe 'projects/pipelines_settings/_show' do describe 'projects/settings/ci_cd/_form' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
before do before do
......
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