From 7029aa1aeb56ff15f4ad1b1dfd75b521e1e032d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Wed, 11 Apr 2018 09:19:47 +0000 Subject: Merge branch 'fix/gb/fix-variables-collection-item-file' into 'master' Fix file-specific variables collection item option Closes gitlab-ee#5444 See merge request gitlab-org/gitlab-ce!18307 --- lib/gitlab/ci/variables/collection/item.rb | 7 ++--- .../gitlab/ci/variables/collection/item_spec.rb | 35 ++++++++++++++++++---- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/lib/gitlab/ci/variables/collection/item.rb b/lib/gitlab/ci/variables/collection/item.rb index 23ed71db8b0..d00e5b07f95 100644 --- a/lib/gitlab/ci/variables/collection/item.rb +++ b/lib/gitlab/ci/variables/collection/item.rb @@ -3,12 +3,9 @@ module Gitlab module Variables class Collection class Item - def initialize(**options) + def initialize(key:, value:, public: true, file: false) @variable = { - key: options.fetch(:key), - value: options.fetch(:value), - public: options.fetch(:public, true), - file: options.fetch(:files, false) + key: key, value: value, public: public, file: file } end diff --git a/spec/lib/gitlab/ci/variables/collection/item_spec.rb b/spec/lib/gitlab/ci/variables/collection/item_spec.rb index bf9208f1ff4..e79f0a7f257 100644 --- a/spec/lib/gitlab/ci/variables/collection/item_spec.rb +++ b/spec/lib/gitlab/ci/variables/collection/item_spec.rb @@ -5,6 +5,18 @@ describe Gitlab::Ci::Variables::Collection::Item do { key: 'VAR', value: 'something', public: true } end + describe '.new' do + it 'raises error if unknown key i specified' do + expect { described_class.new(key: 'VAR', value: 'abc', files: true) } + .to raise_error ArgumentError, 'unknown keyword: files' + end + + it 'raises error when required keywords are not specified' do + expect { described_class.new(key: 'VAR') } + .to raise_error ArgumentError, 'missing keyword: value' + end + end + describe '.fabricate' do it 'supports using a hash' do resource = described_class.fabricate(variable) @@ -47,12 +59,25 @@ describe Gitlab::Ci::Variables::Collection::Item do end describe '#to_runner_variable' do - it 'returns a runner-compatible hash representation' do - runner_variable = described_class - .new(**variable) - .to_runner_variable + context 'when variable is not a file-related' do + it 'returns a runner-compatible hash representation' do + runner_variable = described_class + .new(**variable) + .to_runner_variable + + expect(runner_variable).to eq variable + end + end + + context 'when variable is file-related' do + it 'appends file description component' do + runner_variable = described_class + .new(key: 'VAR', value: 'value', file: true) + .to_runner_variable - expect(runner_variable).to eq variable + expect(runner_variable) + .to eq(key: 'VAR', value: 'value', public: true, file: true) + end end end end -- cgit v1.2.1 From 6df03ea9877315bc4c25a152eb673e8535b5a3f9 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 10 Apr 2018 11:15:10 +0000 Subject: Merge branch 'revert-e9e800f5' into 'master' Revert "Merge branch 'improve-jobs-queuing-time-metric' into 'master'" See merge request gitlab-org/gitlab-ce!18276 --- app/services/ci/register_job_service.rb | 19 +---- .../improve-jobs-queuing-time-metric.yml | 5 -- spec/factories/ci/builds.rb | 1 - spec/services/ci/register_job_service_spec.rb | 83 +--------------------- 4 files changed, 4 insertions(+), 104 deletions(-) delete mode 100644 changelogs/unreleased/improve-jobs-queuing-time-metric.yml diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index d46dcff34a1..e09b445636f 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -4,9 +4,6 @@ module Ci class RegisterJobService attr_reader :runner - JOB_QUEUE_DURATION_SECONDS_BUCKETS = [1, 3, 10, 30].freeze - JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET = 5.freeze - Result = Struct.new(:build, :valid?) def initialize(runner) @@ -107,22 +104,10 @@ module Ci end def register_success(job) - labels = { shared_runner: runner.shared?, - jobs_running_for_project: jobs_running_for_project(job) } - - job_queue_duration_seconds.observe(labels, Time.now - job.queued_at) + job_queue_duration_seconds.observe({ shared_runner: @runner.shared? }, Time.now - job.created_at) attempt_counter.increment end - def jobs_running_for_project(job) - return '+Inf' unless runner.shared? - - # excluding currently started job - running_jobs_count = job.project.builds.running.where(runner: Ci::Runner.shared) - .limit(JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET + 1).count - 1 - running_jobs_count < JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET ? running_jobs_count : "#{JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET}+" - end - def failed_attempt_counter @failed_attempt_counter ||= Gitlab::Metrics.counter(:job_register_attempts_failed_total, "Counts the times a runner tries to register a job") end @@ -132,7 +117,7 @@ module Ci end def job_queue_duration_seconds - @job_queue_duration_seconds ||= Gitlab::Metrics.histogram(:job_queue_duration_seconds, 'Request handling execution time', {}, JOB_QUEUE_DURATION_SECONDS_BUCKETS) + @job_queue_duration_seconds ||= Gitlab::Metrics.histogram(:job_queue_duration_seconds, 'Request handling execution time') end end end diff --git a/changelogs/unreleased/improve-jobs-queuing-time-metric.yml b/changelogs/unreleased/improve-jobs-queuing-time-metric.yml deleted file mode 100644 index cee8b8523fd..00000000000 --- a/changelogs/unreleased/improve-jobs-queuing-time-metric.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Partition job_queue_duration_seconds with jobs_running_for_project -merge_request: 17730 -author: -type: changed diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index fdacbe6c3f1..bca7e920de4 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -62,7 +62,6 @@ FactoryBot.define do end trait :pending do - queued_at 'Di 29. Okt 09:50:59 CET 2013' status 'pending' end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index aa7cc268dd7..97a563c1ce1 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -370,89 +370,10 @@ module Ci it_behaves_like 'validation is not active' end end - end - describe '#register_success' do - let!(:current_time) { Time.new(2018, 4, 5, 14, 0, 0) } - let!(:attempt_counter) { double('Gitlab::Metrics::NullMetric') } - let!(:job_queue_duration_seconds) { double('Gitlab::Metrics::NullMetric') } - - before do - allow(Time).to receive(:now).and_return(current_time) - - # Stub defaults for any metrics other than the ones we're testing - allow(Gitlab::Metrics).to receive(:counter) - .with(any_args) - .and_return(Gitlab::Metrics::NullMetric.instance) - allow(Gitlab::Metrics).to receive(:histogram) - .with(any_args) - .and_return(Gitlab::Metrics::NullMetric.instance) - - # Stub tested metrics - allow(Gitlab::Metrics).to receive(:counter) - .with(:job_register_attempts_total, anything) - .and_return(attempt_counter) - allow(Gitlab::Metrics).to receive(:histogram) - .with(:job_queue_duration_seconds, anything, anything, anything) - .and_return(job_queue_duration_seconds) - - project.update(shared_runners_enabled: true) - pending_job.update(created_at: current_time - 3600, queued_at: current_time - 1800) + def execute(runner) + described_class.new(runner).execute.build end - - shared_examples 'metrics collector' do - it 'increments attempt counter' do - allow(job_queue_duration_seconds).to receive(:observe) - expect(attempt_counter).to receive(:increment) - - execute(runner) - end - - it 'counts job queuing time histogram with expected labels' do - allow(attempt_counter).to receive(:increment) - expect(job_queue_duration_seconds).to receive(:observe) - .with({ shared_runner: expected_shared_runner, - jobs_running_for_project: expected_jobs_running_for_project_first_job }, 1800) - - execute(runner) - end - - context 'when project already has running jobs' do - let!(:build2) { create( :ci_build, :running, pipeline: pipeline, runner: shared_runner) } - let!(:build3) { create( :ci_build, :running, pipeline: pipeline, runner: shared_runner) } - - it 'counts job queuing time histogram with expected labels' do - allow(attempt_counter).to receive(:increment) - expect(job_queue_duration_seconds).to receive(:observe) - .with({ shared_runner: expected_shared_runner, - jobs_running_for_project: expected_jobs_running_for_project_third_job }, 1800) - - execute(runner) - end - end - end - - context 'when shared runner is used' do - let(:runner) { shared_runner } - let(:expected_shared_runner) { true } - let(:expected_jobs_running_for_project_first_job) { 0 } - let(:expected_jobs_running_for_project_third_job) { 2 } - - it_behaves_like 'metrics collector' - end - - context 'when specific runner is used' do - let(:runner) { specific_runner } - let(:expected_shared_runner) { false } - let(:expected_jobs_running_for_project_first_job) { '+Inf' } - let(:expected_jobs_running_for_project_third_job) { '+Inf' } - - it_behaves_like 'metrics collector' - end - end - - def execute(runner) - described_class.new(runner).execute.build end end end -- cgit v1.2.1 From c393a44f2fa5e161c8055bc589f10d4ee2e1d8a0 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 11 Apr 2018 13:47:37 +0200 Subject: Update VERSION to 10.7.0-rc4 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 6dab54c311b..05fd90cdbd6 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -10.7.0-rc3 +10.7.0-rc4 -- cgit v1.2.1