From afea2df15121c0fa4d36f16b050421f681564b64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Wed, 14 Dec 2016 12:14:25 +0100 Subject: First attempt --- lib/ci/api/builds.rb | 17 +++++++++++++++++ lib/ci/api/helpers.rb | 3 +++ 2 files changed, 20 insertions(+) diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index ed87a2603e8..1ec4584a13f 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -16,6 +16,15 @@ module Ci not_found! unless current_runner.active? update_runner_info + last_update = Gitlab::Redis.with { |redis| redis.get(current_runner_redis_key)} + + if params[:last_update] != "" + if :last_update == last_update + headers 'X-GitLab-Last-Update', last_update + return build_not_found! + end + end + build = Ci::RegisterBuildService.new.execute(current_runner) if build @@ -26,6 +35,14 @@ module Ci else Gitlab::Metrics.add_event(:build_not_found) + if last_update == "" + Gitlab::Redis.with do |redis] + new_update = Time.new.inspect + redis.set(current_runner_redis_key, new_update, ex: 60.minutes) + headers 'X-GitLab-Last-Update', new_update + end + end + build_not_found! end end diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb index e608f5f6cad..ef80e0df5d3 100644 --- a/lib/ci/api/helpers.rb +++ b/lib/ci/api/helpers.rb @@ -60,6 +60,9 @@ module Ci @runner ||= Runner.find_by_token(params[:token].to_s) end + def current_runner_redis_key + @runner_redis_key ||= "#{current_runner.token}_#{current_runner.tag_list}" + def get_runner_version_from_params return unless params["info"].present? attributes_for_keys(["name", "version", "revision", "platform", "architecture"], params["info"]) -- cgit v1.2.1 From a6ab8a34093ccd2de7286797ccc639c1cbf267fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Wed, 14 Dec 2016 16:06:50 +0100 Subject: Add BuildQueueWorker for injecting redis-keys --- app/models/ci/build.rb | 6 ++++++ app/workers/build_queue_worker.rb | 18 ++++++++++++++++++ lib/ci/api/helpers.rb | 1 + 3 files changed, 25 insertions(+) create mode 100644 app/workers/build_queue_worker.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 88c46076df6..cab9393344b 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -81,6 +81,12 @@ module Ci end state_machine :status do + after_transition any => [:pending] do |build| + build.run_after_commit do + BuildQueueWorker.perform_async(id) + end + end + after_transition pending: :running do |build| build.run_after_commit do BuildHooksWorker.perform_async(id) diff --git a/app/workers/build_queue_worker.rb b/app/workers/build_queue_worker.rb new file mode 100644 index 00000000000..f19290b0c1a --- /dev/null +++ b/app/workers/build_queue_worker.rb @@ -0,0 +1,18 @@ +class BuildQueueWorker + include Sidekiq::Worker + include BuildQueue + + def perform(build_id) + Ci::Build.find_by(id: build_id).try do |build| + project.runners.select do |runner| + if runner.can_pick?(build) + # Inject last_update into Redis + Gitlab::Redis.with do |redis] + new_update = Time.new.inspect + redis.set(current_runner_redis_key, new_update, ex: 60.minutes) + end + end + end + end + end +end diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb index ef80e0df5d3..74e1871619e 100644 --- a/lib/ci/api/helpers.rb +++ b/lib/ci/api/helpers.rb @@ -62,6 +62,7 @@ module Ci def current_runner_redis_key @runner_redis_key ||= "#{current_runner.token}_#{current_runner.tag_list}" + end def get_runner_version_from_params return unless params["info"].present? -- cgit v1.2.1 From 0d43815620f96a061d7bc5c2c53748cac879c518 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Wed, 14 Dec 2016 16:33:16 +0100 Subject: This is needed as well... --- app/workers/build_queue_worker.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/workers/build_queue_worker.rb b/app/workers/build_queue_worker.rb index f19290b0c1a..1ec020039cf 100644 --- a/app/workers/build_queue_worker.rb +++ b/app/workers/build_queue_worker.rb @@ -9,10 +9,17 @@ class BuildQueueWorker # Inject last_update into Redis Gitlab::Redis.with do |redis] new_update = Time.new.inspect - redis.set(current_runner_redis_key, new_update, ex: 60.minutes) + redis.set(runner_redis_key(runner), new_update, ex: 60.minutes) end end end end end + + private + + def runner_redis_key(runner) + "#{runner.token}_#{runner.tag_list}" + end + end -- cgit v1.2.1 From aa224c13fc8d9712e634dcb3843c73d3838fb20a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Wed, 14 Dec 2016 16:39:40 +0100 Subject: typo-o --- app/workers/build_queue_worker.rb | 2 +- lib/ci/api/builds.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/workers/build_queue_worker.rb b/app/workers/build_queue_worker.rb index 1ec020039cf..4cb910e90ca 100644 --- a/app/workers/build_queue_worker.rb +++ b/app/workers/build_queue_worker.rb @@ -7,7 +7,7 @@ class BuildQueueWorker project.runners.select do |runner| if runner.can_pick?(build) # Inject last_update into Redis - Gitlab::Redis.with do |redis] + Gitlab::Redis.with do |redis| new_update = Time.new.inspect redis.set(runner_redis_key(runner), new_update, ex: 60.minutes) end diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index 1ec4584a13f..acb0ca8719f 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -36,7 +36,7 @@ module Ci Gitlab::Metrics.add_event(:build_not_found) if last_update == "" - Gitlab::Redis.with do |redis] + Gitlab::Redis.with do |redis| new_update = Time.new.inspect redis.set(current_runner_redis_key, new_update, ex: 60.minutes) headers 'X-GitLab-Last-Update', new_update -- cgit v1.2.1 From 692426b13ecff871890a853bebef19d63ee4985e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Wed, 14 Dec 2016 17:05:09 +0100 Subject: Use correct variables --- app/workers/build_queue_worker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/workers/build_queue_worker.rb b/app/workers/build_queue_worker.rb index 4cb910e90ca..141c1543aeb 100644 --- a/app/workers/build_queue_worker.rb +++ b/app/workers/build_queue_worker.rb @@ -4,7 +4,7 @@ class BuildQueueWorker def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| - project.runners.select do |runner| + build.project.runners.select do |runner| if runner.can_pick?(build) # Inject last_update into Redis Gitlab::Redis.with do |redis| -- cgit v1.2.1 From 771dd68cdff9c1f57082fd99af5c22c3af96e7c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Thu, 15 Dec 2016 22:29:47 +0100 Subject: linting --- app/models/ci/build.rb | 2 +- lib/ci/api/builds.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index cab9393344b..5c84833b806 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -83,7 +83,7 @@ module Ci state_machine :status do after_transition any => [:pending] do |build| build.run_after_commit do - BuildQueueWorker.perform_async(id) + BuildQueueWorker.perform_async(id) end end diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index acb0ca8719f..df32f64ef83 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -18,8 +18,8 @@ module Ci last_update = Gitlab::Redis.with { |redis| redis.get(current_runner_redis_key)} - if params[:last_update] != "" - if :last_update == last_update + if params[:last_update].present? + if params[:last_update] == last_update headers 'X-GitLab-Last-Update', last_update return build_not_found! end -- cgit v1.2.1 From e179544342247c9a227c1ef3c3e709a0449158e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Thu, 15 Dec 2016 22:30:31 +0100 Subject: Move redis-logic into Ci::Runner --- app/models/ci/runner.rb | 16 ++++++++++++++++ lib/ci/api/builds.rb | 11 +++-------- lib/ci/api/helpers.rb | 4 ---- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 123930273e0..30f5a2ee092 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -21,6 +21,8 @@ module Ci scope :online, ->() { where('contacted_at > ?', LAST_CONTACT_TIME) } scope :ordered, ->() { order(id: :desc) } + after_save :tick_update + scope :owned_or_shared, ->(project_id) do joins('LEFT JOIN ci_runner_projects ON ci_runner_projects.runner_id = ci_runners.id') .where("ci_runner_projects.gl_project_id = :project_id OR ci_runners.is_shared = true", project_id: project_id) @@ -122,8 +124,22 @@ module Ci ] end + def tick_update + new_update = Time.new.inspect + Gitlab::Redis.with { |redis| redis.set(redis_key, new_update, ex: 60.minutes) } + new_update + end + + def last_build_queue_update + Gitlab::Redis.with { |redis| redis.get(redis_key) } + end + private + def redis_key + "runner:build_queue:#{self.id}" + end + def tag_constraints unless has_tags? || run_untagged? errors.add(:tags_list, diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index df32f64ef83..fb14f043a18 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -16,7 +16,7 @@ module Ci not_found! unless current_runner.active? update_runner_info - last_update = Gitlab::Redis.with { |redis| redis.get(current_runner_redis_key)} + last_update = current_runner.last_build_queue_update if params[:last_update].present? if params[:last_update] == last_update @@ -35,13 +35,8 @@ module Ci else Gitlab::Metrics.add_event(:build_not_found) - if last_update == "" - Gitlab::Redis.with do |redis| - new_update = Time.new.inspect - redis.set(current_runner_redis_key, new_update, ex: 60.minutes) - headers 'X-GitLab-Last-Update', new_update - end - end + new_update = current_runner.tick_update + headers 'X-GitLab-Last-Update', new_update build_not_found! end diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb index 74e1871619e..e608f5f6cad 100644 --- a/lib/ci/api/helpers.rb +++ b/lib/ci/api/helpers.rb @@ -60,10 +60,6 @@ module Ci @runner ||= Runner.find_by_token(params[:token].to_s) end - def current_runner_redis_key - @runner_redis_key ||= "#{current_runner.token}_#{current_runner.tag_list}" - end - def get_runner_version_from_params return unless params["info"].present? attributes_for_keys(["name", "version", "revision", "platform", "architecture"], params["info"]) -- cgit v1.2.1 From a20681c093d5444d24c54f41fd49a98ef919cd42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Thu, 15 Dec 2016 22:37:52 +0100 Subject: Update Runners in a Service --- app/services/ci/update_build_queue_service.rb | 12 ++++++++++++ app/workers/build_queue_worker.rb | 16 +--------------- 2 files changed, 13 insertions(+), 15 deletions(-) create mode 100644 app/services/ci/update_build_queue_service.rb diff --git a/app/services/ci/update_build_queue_service.rb b/app/services/ci/update_build_queue_service.rb new file mode 100644 index 00000000000..348ba6f5cc4 --- /dev/null +++ b/app/services/ci/update_build_queue_service.rb @@ -0,0 +1,12 @@ +module Ci + class UpdateBuildQueueService < BaseService + + def execute(build) + build.project.runners.select do |runner| + if runner.can_pick?(build) + runner.tick_update + end + end + end + end +end diff --git a/app/workers/build_queue_worker.rb b/app/workers/build_queue_worker.rb index 141c1543aeb..8cc99eabdfe 100644 --- a/app/workers/build_queue_worker.rb +++ b/app/workers/build_queue_worker.rb @@ -4,22 +4,8 @@ class BuildQueueWorker def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| - build.project.runners.select do |runner| - if runner.can_pick?(build) - # Inject last_update into Redis - Gitlab::Redis.with do |redis| - new_update = Time.new.inspect - redis.set(runner_redis_key(runner), new_update, ex: 60.minutes) - end - end - end + UpdateBuildQueueService.new(build) end end - private - - def runner_redis_key(runner) - "#{runner.token}_#{runner.tag_list}" - end - end -- cgit v1.2.1 From 811addf2d16bd912efa095b6b5a5f93d620089ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Fri, 16 Dec 2016 01:04:18 +0100 Subject: #NamingThings --- app/models/ci/runner.rb | 21 ++++++++++++++++----- app/services/ci/update_build_queue_service.rb | 1 - app/workers/build_queue_worker.rb | 3 +-- lib/ci/api/builds.rb | 12 ++++-------- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 30f5a2ee092..4f96f4f3645 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -124,19 +124,30 @@ module Ci ] end - def tick_update + def tick_runner_queue new_update = Time.new.inspect - Gitlab::Redis.with { |redis| redis.set(redis_key, new_update, ex: 60.minutes) } + Gitlab::Redis.with { |redis| redis.set(runner_queue_key, new_update, ex: 60.minutes) } new_update end - def last_build_queue_update - Gitlab::Redis.with { |redis| redis.get(redis_key) } + def ensure_runner_queue_value + Gitlab::Redis.with do |redis| + value = redis.get(runner_queue_key) + if value == "" + value = Time.new.inspect + redis.set(runner_queue_key, value, ex: 60.minutes) + end + value + end + end + + def is_runner_queue_value_latest?(value) + ensure_runner_queue_value == value if value.present? end private - def redis_key + def runner_queue_key "runner:build_queue:#{self.id}" end diff --git a/app/services/ci/update_build_queue_service.rb b/app/services/ci/update_build_queue_service.rb index 348ba6f5cc4..dd1097f5dbe 100644 --- a/app/services/ci/update_build_queue_service.rb +++ b/app/services/ci/update_build_queue_service.rb @@ -1,6 +1,5 @@ module Ci class UpdateBuildQueueService < BaseService - def execute(build) build.project.runners.select do |runner| if runner.can_pick?(build) diff --git a/app/workers/build_queue_worker.rb b/app/workers/build_queue_worker.rb index 8cc99eabdfe..c0ceb8d423a 100644 --- a/app/workers/build_queue_worker.rb +++ b/app/workers/build_queue_worker.rb @@ -4,8 +4,7 @@ class BuildQueueWorker def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| - UpdateBuildQueueService.new(build) + UpdateBuildQueueService.execute(build) end end - end diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index fb14f043a18..b1b66313092 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -16,13 +16,9 @@ module Ci not_found! unless current_runner.active? update_runner_info - last_update = current_runner.last_build_queue_update - - if params[:last_update].present? - if params[:last_update] == last_update - headers 'X-GitLab-Last-Update', last_update - return build_not_found! - end + if current_runner.is_runner_queue_value_latest?(params[:last_update]) + headers 'X-GitLab-Last-Update', params[:last_update] + return build_not_found! end build = Ci::RegisterBuildService.new.execute(current_runner) @@ -35,7 +31,7 @@ module Ci else Gitlab::Metrics.add_event(:build_not_found) - new_update = current_runner.tick_update + new_update = current_runner.ensure_runner_queue_value headers 'X-GitLab-Last-Update', new_update build_not_found! -- cgit v1.2.1 From 94b2df022449352953a2d5e607de1a3d31e88b47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Fri, 16 Dec 2016 01:20:05 +0100 Subject: Make ensure_runner_queue_value atomic --- app/models/ci/runner.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 4f96f4f3645..a26a67739a1 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -132,12 +132,14 @@ module Ci def ensure_runner_queue_value Gitlab::Redis.with do |redis| - value = redis.get(runner_queue_key) - if value == "" - value = Time.new.inspect - redis.set(runner_queue_key, value, ex: 60.minutes) + redis.multi do + value = redis.get(runner_queue_key) + if value == "" + value = Time.new.inspect + redis.set(runner_queue_key, value, ex: 60.minutes) + end + value end - value end end -- cgit v1.2.1 From f6263e2ee7288ff1b53b551053911f31ddf846b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Fri, 16 Dec 2016 16:29:11 +0100 Subject: Don't use redis.multi --- app/models/ci/runner.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index a26a67739a1..e880ea9a880 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -21,7 +21,7 @@ module Ci scope :online, ->() { where('contacted_at > ?', LAST_CONTACT_TIME) } scope :ordered, ->() { order(id: :desc) } - after_save :tick_update + after_save :tick_runner_queue scope :owned_or_shared, ->(project_id) do joins('LEFT JOIN ci_runner_projects ON ci_runner_projects.runner_id = ci_runners.id') @@ -132,14 +132,8 @@ module Ci def ensure_runner_queue_value Gitlab::Redis.with do |redis| - redis.multi do - value = redis.get(runner_queue_key) - if value == "" - value = Time.new.inspect - redis.set(runner_queue_key, value, ex: 60.minutes) - end - value - end + redis.set(runner_queue_key, value, ex: 60.minutes, nx: true) + redis.get(runner_queue_key) end end -- cgit v1.2.1 From 24f0a45b0f8558071987ec1d70446f8e9f7df0ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Fri, 16 Dec 2016 16:32:16 +0100 Subject: Do things in the correct order --- app/models/ci/runner.rb | 1 + lib/ci/api/builds.rb | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index e880ea9a880..221da8adf32 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -132,6 +132,7 @@ module Ci def ensure_runner_queue_value Gitlab::Redis.with do |redis| + value = Time.new.inspect redis.set(runner_queue_key, value, ex: 60.minutes, nx: true) redis.get(runner_queue_key) end diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index b1b66313092..8264210c460 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -21,6 +21,8 @@ module Ci return build_not_found! end + new_update = current_runner.ensure_runner_queue_value + build = Ci::RegisterBuildService.new.execute(current_runner) if build @@ -31,7 +33,6 @@ module Ci else Gitlab::Metrics.add_event(:build_not_found) - new_update = current_runner.ensure_runner_queue_value headers 'X-GitLab-Last-Update', new_update build_not_found! -- cgit v1.2.1 From fb713071db822cd5946e201d749ebee42ecbae0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Fri, 16 Dec 2016 16:33:08 +0100 Subject: Use Token instead of ID --- app/models/ci/runner.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 221da8adf32..d5b5a84d477 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -145,7 +145,7 @@ module Ci private def runner_queue_key - "runner:build_queue:#{self.id}" + "runner:build_queue:#{self.token}" end def tag_constraints -- cgit v1.2.1 From b0b05f811cc11472798661de713e2564cae2e9d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Fri, 16 Dec 2016 16:39:40 +0100 Subject: Make expire-time a constant, correct function in Service --- app/models/ci/runner.rb | 5 +++-- app/services/ci/update_build_queue_service.rb | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index d5b5a84d477..850be67d98f 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -2,6 +2,7 @@ module Ci class Runner < ActiveRecord::Base extend Ci::Model + LAST_UPDATE_EXPIRE_TIME = 60.minutes LAST_CONTACT_TIME = 1.hour.ago AVAILABLE_SCOPES = %w[specific shared active paused online] FORM_EDITABLE = %i[description tag_list active run_untagged locked] @@ -126,14 +127,14 @@ module Ci def tick_runner_queue new_update = Time.new.inspect - Gitlab::Redis.with { |redis| redis.set(runner_queue_key, new_update, ex: 60.minutes) } + Gitlab::Redis.with { |redis| redis.set(runner_queue_key, new_update, ex: LAST_UPDATE_EXPIRE_TIME) } new_update end def ensure_runner_queue_value Gitlab::Redis.with do |redis| value = Time.new.inspect - redis.set(runner_queue_key, value, ex: 60.minutes, nx: true) + redis.set(runner_queue_key, value, ex: LAST_UPDATE_EXPIRE_TIME, nx: true) redis.get(runner_queue_key) end end diff --git a/app/services/ci/update_build_queue_service.rb b/app/services/ci/update_build_queue_service.rb index dd1097f5dbe..b324863feef 100644 --- a/app/services/ci/update_build_queue_service.rb +++ b/app/services/ci/update_build_queue_service.rb @@ -3,7 +3,7 @@ module Ci def execute(build) build.project.runners.select do |runner| if runner.can_pick?(build) - runner.tick_update + runner.tick_runner_queue end end end -- cgit v1.2.1 From 74a48ecd6bce154dff9704575ba3d0824a73e8e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Fri, 16 Dec 2016 17:42:35 +0100 Subject: Change name of expire constant --- app/models/ci/runner.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 850be67d98f..d13e54fcc16 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -2,7 +2,7 @@ module Ci class Runner < ActiveRecord::Base extend Ci::Model - LAST_UPDATE_EXPIRE_TIME = 60.minutes + RUNNER_QUEUE_EXPIRY_TIME = 60.minutes LAST_CONTACT_TIME = 1.hour.ago AVAILABLE_SCOPES = %w[specific shared active paused online] FORM_EDITABLE = %i[description tag_list active run_untagged locked] @@ -127,14 +127,14 @@ module Ci def tick_runner_queue new_update = Time.new.inspect - Gitlab::Redis.with { |redis| redis.set(runner_queue_key, new_update, ex: LAST_UPDATE_EXPIRE_TIME) } + Gitlab::Redis.with { |redis| redis.set(runner_queue_key, new_update, ex: RUNNER_QUEUE_EXPIRY_TIME) } new_update end def ensure_runner_queue_value Gitlab::Redis.with do |redis| value = Time.new.inspect - redis.set(runner_queue_key, value, ex: LAST_UPDATE_EXPIRE_TIME, nx: true) + redis.set(runner_queue_key, value, ex: RUNNER_QUEUE_EXPIRY_TIME, nx: true) redis.get(runner_queue_key) end end -- cgit v1.2.1 From f35336a1e6b1eb750a501a5d54396816f4800e69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Wed, 21 Dec 2016 04:27:03 +0100 Subject: Fixed broken build --- app/services/ci/update_build_queue_service.rb | 2 +- app/workers/build_queue_worker.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/ci/update_build_queue_service.rb b/app/services/ci/update_build_queue_service.rb index b324863feef..704b872ddd0 100644 --- a/app/services/ci/update_build_queue_service.rb +++ b/app/services/ci/update_build_queue_service.rb @@ -1,5 +1,5 @@ module Ci - class UpdateBuildQueueService < BaseService + class UpdateBuildQueueService def execute(build) build.project.runners.select do |runner| if runner.can_pick?(build) diff --git a/app/workers/build_queue_worker.rb b/app/workers/build_queue_worker.rb index c0ceb8d423a..fa9e097e40a 100644 --- a/app/workers/build_queue_worker.rb +++ b/app/workers/build_queue_worker.rb @@ -4,7 +4,7 @@ class BuildQueueWorker def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| - UpdateBuildQueueService.execute(build) + Ci::UpdateBuildQueueService.new.execute(build) end end end -- cgit v1.2.1 From 932bcb4d32ee89922b6e7123a9c18489b36cc6e5 Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Mon, 2 Jan 2017 10:25:28 +0000 Subject: Added isAuthenticate to differentiate between ineligible messages --- app/assets/javascripts/u2f/authenticate.js.es6 | 2 +- app/assets/javascripts/u2f/error.js | 16 ++++++++-------- app/assets/javascripts/u2f/register.js | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/u2f/authenticate.js.es6 b/app/assets/javascripts/u2f/authenticate.js.es6 index 2b992109a8c..f22d1f9b300 100644 --- a/app/assets/javascripts/u2f/authenticate.js.es6 +++ b/app/assets/javascripts/u2f/authenticate.js.es6 @@ -57,7 +57,7 @@ return function(response) { var error; if (response.errorCode) { - error = new U2FError(response.errorCode); + error = new U2FError(response.errorCode, 'authenticate'); return _this.renderError(error); } else { return _this.renderAuthenticated(JSON.stringify(response)); diff --git a/app/assets/javascripts/u2f/error.js b/app/assets/javascripts/u2f/error.js index bb9942a3aa0..50340d9174a 100644 --- a/app/assets/javascripts/u2f/error.js +++ b/app/assets/javascripts/u2f/error.js @@ -5,21 +5,21 @@ var bind = function(fn, me){ return function(){ return fn.apply(me, arguments); }; }; this.U2FError = (function() { - function U2FError(errorCode) { + function U2FError(errorCode, u2fFlowType) { this.errorCode = errorCode; this.message = bind(this.message, this); this.httpsDisabled = window.location.protocol !== 'https:'; + this.u2fFlowType = u2fFlowType; } U2FError.prototype.message = function() { - switch (false) { - case !(this.errorCode === u2f.ErrorCodes.BAD_REQUEST && this.httpsDisabled): - return "U2F only works with HTTPS-enabled websites. Contact your administrator for more details."; - case this.errorCode !== u2f.ErrorCodes.DEVICE_INELIGIBLE: - return "This device has already been registered with us."; - default: - return "There was a problem communicating with your device."; + if (this.errorCode === u2f.ErrorCodes.BAD_REQUEST && this.httpsDisabled) { + return 'U2F only works with HTTPS-enabled websites. Contact your administrator for more details.'; + } else if (this.errorCode === u2f.ErrorCodes.DEVICE_INELIGIBLE) { + if (this.u2fFlowType === 'authenticate') return 'This device has not been registered with us.'; + if (this.u2fFlowType === 'register') return 'This device has already been registered with us.'; } + return "There was a problem communicating with your device."; }; return U2FError; diff --git a/app/assets/javascripts/u2f/register.js b/app/assets/javascripts/u2f/register.js index 050c9bfc02e..92014afa593 100644 --- a/app/assets/javascripts/u2f/register.js +++ b/app/assets/javascripts/u2f/register.js @@ -39,7 +39,7 @@ return function(response) { var error; if (response.errorCode) { - error = new U2FError(response.errorCode); + error = new U2FError(response.errorCode, 'register'); return _this.renderError(error); } else { return _this.renderRegistered(JSON.stringify(response)); -- cgit v1.2.1 From 8c9a4ed373f4b517aeae669e64023dc52c8d704a Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 4 Jan 2017 17:46:56 +0800 Subject: WIP: Add tests and make sure that headers are set * We realized that headers were not set whenever we give 204 because `render_api_error!` doesn't preserve the headers. * We also realized that `update_runner_info` would be called in POST /builds/register every time therefore runner is updated every time, ticking the queue, making this last_update didn't work very well, and the test would be failing due to that. --- lib/api/helpers.rb | 2 +- lib/ci/api/builds.rb | 4 ++-- spec/models/build_spec.rb | 10 ++++++++++ spec/models/ci/runner_spec.rb | 33 +++++++++++++++++++++++++++++++++ spec/requests/ci/api/builds_spec.rb | 32 ++++++++++++++++++++++++++++++-- 5 files changed, 76 insertions(+), 5 deletions(-) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 746849ef4c0..3324001c468 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -229,7 +229,7 @@ module API end def render_api_error!(message, status) - error!({ 'message' => message }, status) + error!({ 'message' => message }, status, header) end def handle_api_exception(exception) diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index 8264210c460..de3e224bcee 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -17,7 +17,7 @@ module Ci update_runner_info if current_runner.is_runner_queue_value_latest?(params[:last_update]) - headers 'X-GitLab-Last-Update', params[:last_update] + header 'X-GitLab-Last-Update', params[:last_update] return build_not_found! end @@ -33,7 +33,7 @@ module Ci else Gitlab::Metrics.add_event(:build_not_found) - headers 'X-GitLab-Last-Update', new_update + header 'X-GitLab-Last-Update', new_update build_not_found! end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index d4970e38f7c..e902d9ef73d 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -1180,4 +1180,14 @@ describe Ci::Build, models: true do it { is_expected.to eq('review/master') } end end + + describe 'State transition: any => [:pending]' do + let(:build) { create(:ci_build, :created) } + + it 'queues BuildQueueWorker' do + expect(BuildQueueWorker).to receive(:perform_async).with(build.id) + + build.enqueue + end + end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ef65eb99328..06eaa6d04d9 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -263,6 +263,39 @@ describe Ci::Runner, models: true do end end + describe '#tick_runner_queue' do + let(:runner) { create(:ci_runner) } + + it 'returns a new last_update value' do + expect(runner.tick_runner_queue).not_to be_empty + end + end + + describe '#ensure_runner_queue_value' do + let(:runner) { create(:ci_runner) } + + it 'sets a new last_update value when it is called the first time' do + last_update = runner.ensure_runner_queue_value + + expect_value_in_redis(last_update) + end + + it 'does not change if it is not expired and called again' do + last_update = runner.ensure_runner_queue_value + + expect(runner.ensure_runner_queue_value).to eq(last_update) + expect_value_in_redis(last_update) + end + + def expect_value_in_redis(last_update) + Gitlab::Redis.with do |redis| + runner_queue_key = runner.send(:runner_queue_key) + + expect(redis.get(runner_queue_key)).to eq(last_update) + end + end + end + describe '.assignable_for' do let(:runner) { create(:ci_runner) } let(:project) { create(:project) } diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 80652129928..8ccae0d5bff 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -5,6 +5,7 @@ describe Ci::API::Builds do let(:runner) { FactoryGirl.create(:ci_runner, tag_list: ["mysql", "ruby"]) } let(:project) { FactoryGirl.create(:empty_project) } + let(:last_update) { nil } describe "Builds API for runners" do let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') } @@ -24,7 +25,31 @@ describe Ci::API::Builds do shared_examples 'no builds available' do context 'when runner sends version in User-Agent' do context 'for stable version' do - it { expect(response).to have_http_status(204) } + it 'gives 204 and set X-GitLab-Last-Update' do + expect(response).to have_http_status(204) + expect(response.header).to have_key('X-GitLab-Last-Update') + end + end + + context 'when last_update is up-to-date' do + let(:last_update) { runner.ensure_runner_queue_value } + + it 'gives 204 and set the same X-GitLab-Last-Update' do + expect(response).to have_http_status(204) + expect(response.header['X-GitLab-Last-Update']) + .to eq(last_update) + end + end + + context 'when last_update is outdated' do + let(:last_update) { runner.ensure_runner_queue_value } + let!(:new_update) { runner.tick_runner_queue } + + it 'gives 204 and set a new X-GitLab-Last-Update' do + expect(response).to have_http_status(204) + expect(response.header['X-GitLab-Last-Update']) + .to eq(new_update) + end end context 'for beta version' do @@ -44,6 +69,7 @@ describe Ci::API::Builds do register_builds info: { platform: :darwin } expect(response).to have_http_status(201) + expect(response.headers).not_to have_key('X-GitLab-Last-Update') expect(json_response['sha']).to eq(build.sha) expect(runner.reload.platform).to eq("darwin") expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] }) @@ -219,7 +245,9 @@ describe Ci::API::Builds do end def register_builds(token = runner.token, **params) - post ci_api("/builds/register"), params.merge(token: token), { 'User-Agent' => user_agent } + new_params = params.merge(token: token, last_update: last_update) + + post ci_api("/builds/register"), new_params, { 'User-Agent' => user_agent } end end -- cgit v1.2.1 From 047107a4ab7d04efa8c30367d39ec896685dcd43 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Thu, 12 Jan 2017 12:17:14 -0500 Subject: Add margin to math blocks --- app/assets/stylesheets/framework/typography.scss | 2 +- changelogs/unreleased/26472-math-margin.yml | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/26472-math-margin.yml diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index bd58a26f429..54958973f15 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -10,7 +10,7 @@ max-width: 100%; } - *:first-child { + *:first-child:not(.katex-display) { margin-top: 0; } diff --git a/changelogs/unreleased/26472-math-margin.yml b/changelogs/unreleased/26472-math-margin.yml new file mode 100644 index 00000000000..3999f521558 --- /dev/null +++ b/changelogs/unreleased/26472-math-margin.yml @@ -0,0 +1,4 @@ +--- +title: Add margin to markdown math blocks +merge_request: +author: -- cgit v1.2.1 From 2f0bbd3d1dcc24ae7cf3fdbaa0b8db7f60e365ba Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 5 Jan 2017 10:41:15 +0000 Subject: Adds .catch to the request in order to handle erros and show a feedback to the user --- app/assets/javascripts/environments/components/environment.js.es6 | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/environments/components/environment.js.es6 b/app/assets/javascripts/environments/components/environment.js.es6 index 8b6fafb6104..80384ea8d72 100644 --- a/app/assets/javascripts/environments/components/environment.js.es6 +++ b/app/assets/javascripts/environments/components/environment.js.es6 @@ -1,6 +1,7 @@ -/* eslint-disable no-param-reassign */ +/* eslint-disable no-param-reassign, no-new */ /* global Vue */ /* global EnvironmentsService */ +/* global Flash */ //= require vue //= require vue-resource @@ -121,6 +122,10 @@ .then((json) => { this.store.storeEnvironments(json); this.isLoading = false; + }) + .catch(() => { + this.isLoading = false; + new Flash('An error occurred while fetching the environments.', 'alert'); }); }, -- cgit v1.2.1 From 463fddeafc945e4be249ba74ed510190ff9cedb6 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 5 Jan 2017 14:51:29 +0000 Subject: Adds tests Adds changelog entry Finishes tests Fix eslint errors Fix teaspoon test timing out --- .../environments/components/environment.js.es6 | 4 +- .../environments/stores/environments_store.js.es6 | 4 +- .../25507-handle-errors-environment-list.yml | 4 + .../environments/environment_spec.js.es6 | 127 +++++++++++++++++++++ spec/javascripts/environments/mock_data.js.es6 | 14 +++ .../fixtures/environments/environments.html.haml | 2 +- 6 files changed, 150 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/25507-handle-errors-environment-list.yml create mode 100644 spec/javascripts/environments/environment_spec.js.es6 diff --git a/app/assets/javascripts/environments/components/environment.js.es6 b/app/assets/javascripts/environments/components/environment.js.es6 index 80384ea8d72..6e450df3c41 100644 --- a/app/assets/javascripts/environments/components/environment.js.es6 +++ b/app/assets/javascripts/environments/components/environment.js.es6 @@ -193,7 +193,7 @@
-

+

You don't have any environments right now.

@@ -207,7 +207,7 @@ + class="btn btn-create js-new-environment-button"> New Environment

diff --git a/app/assets/javascripts/environments/stores/environments_store.js.es6 b/app/assets/javascripts/environments/stores/environments_store.js.es6 index 0204a903ab5..46e39a15ac9 100644 --- a/app/assets/javascripts/environments/stores/environments_store.js.es6 +++ b/app/assets/javascripts/environments/stores/environments_store.js.es6 @@ -59,7 +59,7 @@ if (occurs.length) { acc[acc.indexOf(occurs[0])].children.push(environment); - acc[acc.indexOf(occurs[0])].children.sort(this.sortByName); + acc[acc.indexOf(occurs[0])].children.slice().sort(this.sortByName); } else { acc.push({ name: environment.environment_type, @@ -73,7 +73,7 @@ } return acc; - }, []).sort(this.sortByName); + }, []).slice().sort(this.sortByName); this.state.environments = environmentsTree; diff --git a/changelogs/unreleased/25507-handle-errors-environment-list.yml b/changelogs/unreleased/25507-handle-errors-environment-list.yml new file mode 100644 index 00000000000..4e9794f7917 --- /dev/null +++ b/changelogs/unreleased/25507-handle-errors-environment-list.yml @@ -0,0 +1,4 @@ +--- +title: Handle HTTP errors in environment list +merge_request: +author: diff --git a/spec/javascripts/environments/environment_spec.js.es6 b/spec/javascripts/environments/environment_spec.js.es6 new file mode 100644 index 00000000000..20e11ca3738 --- /dev/null +++ b/spec/javascripts/environments/environment_spec.js.es6 @@ -0,0 +1,127 @@ +/* global Vue, environment */ + +//= require vue +//= require vue-resource +//= require flash +//= require environments/stores/environments_store +//= require environments/components/environment +//= require ./mock_data + +describe('Environment', () => { + preloadFixtures('environments/environments'); + + let component; + + beforeEach(() => { + loadFixtures('environments/environments'); + }); + + describe('successfull request', () => { + describe('without environments', () => { + const environmentsEmptyResponseInterceptor = (request, next) => { + next(request.respondWith(JSON.stringify([]), { + status: 200, + })); + }; + + beforeEach(() => { + Vue.http.interceptors.push(environmentsEmptyResponseInterceptor); + }); + + afterEach(() => { + Vue.http.interceptors = _.without( + Vue.http.interceptors, environmentsEmptyResponseInterceptor, + ); + }); + + it('should render the empty state', (done) => { + component = new gl.environmentsList.EnvironmentsComponent({ + el: document.querySelector('#environments-list-view'), + propsData: { + store: gl.environmentsList.EnvironmentsStore.create(), + }, + }); + + setTimeout(() => { + expect( + component.$el.querySelector('.js-new-environment-button').textContent, + ).toContain('New Environment'); + + expect( + component.$el.querySelector('.js-blank-state-title').textContent, + ).toContain('You don\'t have any environments right now.'); + + done(); + }, 0); + }); + }); + + describe('with environments', () => { + const environmentsResponseInterceptor = (request, next) => { + next(request.respondWith(JSON.stringify([environment]), { + status: 200, + })); + }; + + beforeEach(() => { + Vue.http.interceptors.push(environmentsResponseInterceptor); + }); + + afterEach(() => { + Vue.http.interceptors = _.without( + Vue.http.interceptors, environmentsResponseInterceptor, + ); + }); + + it('should render a table with environments', (done) => { + component = new gl.environmentsList.EnvironmentsComponent({ + el: document.querySelector('#environments-list-view'), + propsData: { + store: gl.environmentsList.EnvironmentsStore.create(), + }, + }); + + setTimeout(() => { + expect( + component.$el.querySelectorAll('table tbody tr').length, + ).toEqual(1); + done(); + }, 0); + }); + }); + }); + + describe('unsuccessfull request', () => { + const environmentsErrorResponseInterceptor = (request, next) => { + next(request.respondWith(JSON.stringify([]), { + status: 500, + })); + }; + + beforeEach(() => { + Vue.http.interceptors.push(environmentsErrorResponseInterceptor); + }); + + afterEach(() => { + Vue.http.interceptors = _.without( + Vue.http.interceptors, environmentsErrorResponseInterceptor, + ); + }); + + it('should render empty state', (done) => { + component = new gl.environmentsList.EnvironmentsComponent({ + el: document.querySelector('#environments-list-view'), + propsData: { + store: gl.environmentsList.EnvironmentsStore.create(), + }, + }); + + setTimeout(() => { + expect( + component.$el.querySelector('.js-blank-state-title').textContent, + ).toContain('You don\'t have any environments right now.'); + done(); + }, 0); + }); + }); +}); diff --git a/spec/javascripts/environments/mock_data.js.es6 b/spec/javascripts/environments/mock_data.js.es6 index 9e16bc3e6a5..8ecd01f9a83 100644 --- a/spec/javascripts/environments/mock_data.js.es6 +++ b/spec/javascripts/environments/mock_data.js.es6 @@ -133,3 +133,17 @@ const environmentsList = [ updated_at: '2016-11-07T11:11:16.525Z', }, ]; + +const environment = { + id: 4, + name: 'production', + state: 'available', + external_url: 'http://production.', + environment_type: null, + last_deployment: {}, + 'stoppable?': false, + environment_path: '/root/review-app/environments/4', + stop_path: '/root/review-app/environments/4/stop', + created_at: '2016-12-16T11:51:04.690Z', + updated_at: '2016-12-16T12:04:51.133Z', +}; diff --git a/spec/javascripts/fixtures/environments/environments.html.haml b/spec/javascripts/fixtures/environments/environments.html.haml index d89bc50c1f0..e6000fbb553 100644 --- a/spec/javascripts/fixtures/environments/environments.html.haml +++ b/spec/javascripts/fixtures/environments/environments.html.haml @@ -1,5 +1,5 @@ %div - #environments-list-view{ data: { environments_data: "https://gitlab.com/foo/environments", + #environments-list-view{ data: { environments_data: "foo/environments", "can-create-deployment" => "true", "can-read-environment" => "true", "can-create-environment" => "true", -- cgit v1.2.1 From 683ea89e04d115352b0411be79594475932edd6e Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 12 Jan 2017 14:19:34 -0500 Subject: Use bootstrap dropdown events to only make the request when the dropdown is being opened Fixes builds dropdown making request when clicked to be closed Adds MR ID to CHANGELOG Improve documentation Use bootstrap dropdown events to only make the request when the dropdown is being opened --- .../javascripts/mini_pipeline_graph_dropdown.js.es6 | 21 +++++++++++---------- .../26601-dropdown-makes-request-close.yml | 4 ++++ .../fixtures/mini_dropdown_graph.html.haml | 11 ++++++----- 3 files changed, 21 insertions(+), 15 deletions(-) create mode 100644 changelogs/unreleased/26601-dropdown-makes-request-close.yml diff --git a/app/assets/javascripts/mini_pipeline_graph_dropdown.js.es6 b/app/assets/javascripts/mini_pipeline_graph_dropdown.js.es6 index 90b3366f14b..80549532ea9 100644 --- a/app/assets/javascripts/mini_pipeline_graph_dropdown.js.es6 +++ b/app/assets/javascripts/mini_pipeline_graph_dropdown.js.es6 @@ -10,9 +10,9 @@ * The container should be the table element. * * The stage icon clicked needs to have the following HTML structure: - *
- * - *
+ *
+ * + *
*
*/ (() => { @@ -26,13 +26,11 @@ } /** - * Adds and removes the event listener. + * Adds the event listener when the dropdown is opened. + * All dropdown events are fired at the .dropdown-menu's parent element. */ bindEvents() { - const dropdownButtonSelector = 'button.js-builds-dropdown-button'; - - $(this.container).off('click', dropdownButtonSelector, this.getBuildsList) - .on('click', dropdownButtonSelector, this.getBuildsList); + $(this.container).on('shown.bs.dropdown', this.getBuildsList); } /** @@ -52,11 +50,14 @@ /** * For the clicked stage, gets the list of builds. * - * @param {Object} e + * All dropdown events have a relatedTarget property, + * whose value is the toggling anchor element. + * + * @param {Object} e bootstrap dropdown event * @return {Promise} */ getBuildsList(e) { - const button = e.currentTarget; + const button = e.relatedTarget; const endpoint = button.dataset.stageEndpoint; return $.ajax({ diff --git a/changelogs/unreleased/26601-dropdown-makes-request-close.yml b/changelogs/unreleased/26601-dropdown-makes-request-close.yml new file mode 100644 index 00000000000..a810e04376d --- /dev/null +++ b/changelogs/unreleased/26601-dropdown-makes-request-close.yml @@ -0,0 +1,4 @@ +--- +title: Fixes builds dropdown making request when clicked to be closed +merge_request: 8545 +author: diff --git a/spec/javascripts/fixtures/mini_dropdown_graph.html.haml b/spec/javascripts/fixtures/mini_dropdown_graph.html.haml index e9bf7568e95..29370b974af 100644 --- a/spec/javascripts/fixtures/mini_dropdown_graph.html.haml +++ b/spec/javascripts/fixtures/mini_dropdown_graph.html.haml @@ -1,8 +1,9 @@ -%div.js-builds-dropdown-tests - %button.dropdown.js-builds-dropdown-button{'data-stage-endpoint' => 'foobar'} +%div.js-builds-dropdown-tests.dropdown.dropdown.js-mini-pipeline-graph + %button.js-builds-dropdown-button{'data-stage-endpoint' => 'foobar', data: { toggle: 'dropdown'} } Dropdown - %div.js-builds-dropdown-container - %div.js-builds-dropdown-list - %div.js-builds-dropdown-loading.builds-dropdown-loading.hidden + %ul.dropdown-menu.mini-pipeline-graph-dropdown-menu.js-builds-dropdown-container + .js-builds-dropdown-list.scrollable-menu + + .js-builds-dropdown-loading.builds-dropdown-loading.hidden %span.fa.fa-spinner.fa-spin -- cgit v1.2.1 From b038c53073b191df2044f96d4ff5d01a35b22d83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Sat, 14 Jan 2017 00:18:21 -0500 Subject: Move default values to ApplicationSetting::DEFAULTS and use it in CurrentSettings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/models/application_setting.rb | 84 ++++++++++++++++++++------------------- lib/gitlab/current_settings.rb | 30 +------------- 2 files changed, 45 insertions(+), 69 deletions(-) diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 8fab77cda0a..e33a58d3771 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -13,6 +13,49 @@ class ApplicationSetting < ActiveRecord::Base [\r\n] # any number of newline characters }x + DEFAULTS_CE = { + after_sign_up_text: nil, + akismet_enabled: false, + container_registry_token_expire_delay: 5, + default_branch_protection: Settings.gitlab['default_branch_protection'], + default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], + default_projects_limit: Settings.gitlab['default_projects_limit'], + default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], + disabled_oauth_sign_in_sources: [], + domain_whitelist: Settings.gitlab['domain_whitelist'], + gravatar_enabled: Settings.gravatar['enabled'], + help_page_text: nil, + housekeeping_bitmaps_enabled: true, + housekeeping_enabled: true, + housekeeping_full_repack_period: 50, + housekeeping_gc_period: 200, + housekeeping_incremental_repack_period: 10, + import_sources: Gitlab::ImportSources.values, + koding_enabled: false, + koding_url: nil, + max_artifacts_size: Settings.artifacts['max_size'], + max_attachment_size: Settings.gitlab['max_attachment_size'], + plantuml_enabled: false, + plantuml_url: nil, + recaptcha_enabled: false, + repository_checks_enabled: true, + repository_storages: ['default'], + require_two_factor_authentication: false, + restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], + session_expire_delay: Settings.gitlab['session_expire_delay'], + send_user_confirmation_email: false, + shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], + shared_runners_text: nil, + sidekiq_throttling_enabled: false, + sign_in_text: nil, + signin_enabled: Settings.gitlab['signin_enabled'], + signup_enabled: Settings.gitlab['signup_enabled'], + two_factor_grace_period: 48, + user_default_external: false + } + + DEFAULTS = DEFAULTS_CE + serialize :restricted_visibility_levels serialize :import_sources serialize :disabled_oauth_sign_in_sources, Array @@ -163,46 +206,7 @@ class ApplicationSetting < ActiveRecord::Base end def self.create_from_defaults - create( - default_projects_limit: Settings.gitlab['default_projects_limit'], - default_branch_protection: Settings.gitlab['default_branch_protection'], - signup_enabled: Settings.gitlab['signup_enabled'], - signin_enabled: Settings.gitlab['signin_enabled'], - gravatar_enabled: Settings.gravatar['enabled'], - sign_in_text: nil, - after_sign_up_text: nil, - help_page_text: nil, - shared_runners_text: nil, - restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], - max_attachment_size: Settings.gitlab['max_attachment_size'], - session_expire_delay: Settings.gitlab['session_expire_delay'], - default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], - default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], - domain_whitelist: Settings.gitlab['domain_whitelist'], - import_sources: Gitlab::ImportSources.values, - shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], - max_artifacts_size: Settings.artifacts['max_size'], - require_two_factor_authentication: false, - two_factor_grace_period: 48, - recaptcha_enabled: false, - akismet_enabled: false, - koding_enabled: false, - koding_url: nil, - plantuml_enabled: false, - plantuml_url: nil, - repository_checks_enabled: true, - disabled_oauth_sign_in_sources: [], - send_user_confirmation_email: false, - container_registry_token_expire_delay: 5, - repository_storages: ['default'], - user_default_external: false, - sidekiq_throttling_enabled: false, - housekeeping_enabled: true, - housekeeping_bitmaps_enabled: true, - housekeeping_incremental_repack_period: 10, - housekeeping_full_repack_period: 50, - housekeeping_gc_period: 200, - ) + create(DEFAULTS) end def home_page_url_column_exist diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 2ff27e46d64..c4fc3709c93 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -28,35 +28,7 @@ module Gitlab end def fake_application_settings - OpenStruct.new( - default_projects_limit: Settings.gitlab['default_projects_limit'], - default_branch_protection: Settings.gitlab['default_branch_protection'], - signup_enabled: Settings.gitlab['signup_enabled'], - signin_enabled: Settings.gitlab['signin_enabled'], - gravatar_enabled: Settings.gravatar['enabled'], - koding_enabled: false, - plantuml_enabled: false, - sign_in_text: nil, - after_sign_up_text: nil, - help_page_text: nil, - shared_runners_text: nil, - restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], - max_attachment_size: Settings.gitlab['max_attachment_size'], - session_expire_delay: Settings.gitlab['session_expire_delay'], - default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], - default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], - domain_whitelist: Settings.gitlab['domain_whitelist'], - import_sources: %w[gitea github bitbucket gitlab google_code fogbugz git gitlab_project], - shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], - max_artifacts_size: Settings.artifacts['max_size'], - require_two_factor_authentication: false, - two_factor_grace_period: 48, - akismet_enabled: false, - repository_checks_enabled: true, - container_registry_token_expire_delay: 5, - user_default_external: false, - sidekiq_throttling_enabled: false, - ) + ApplicationSetting.new(ApplicationSetting::DEFAULTS) end private -- cgit v1.2.1 From f6cc29ed83921c3dce98d8c519c4826e7cc8221a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Sat, 14 Jan 2017 00:18:40 -0500 Subject: Don't persist ApplicationSetting in test env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/gitlab/current_settings.rb | 16 ++++- lib/gitlab/github_import/project_creator.rb | 4 +- spec/controllers/health_check_controller_spec.rb | 6 ++ .../admin_disables_git_access_protocol_spec.rb | 3 + spec/features/admin/admin_health_check_spec.rb | 9 ++- spec/features/admin/admin_runners_spec.rb | 3 + spec/features/admin/admin_settings_spec.rb | 5 +- .../admin/admin_uses_repository_checks_spec.rb | 9 ++- spec/lib/gitlab/current_settings_spec.rb | 68 +++++++++++++++------- spec/requests/api/internal_spec.rb | 9 +-- spec/spec_helper.rb | 1 + 11 files changed, 97 insertions(+), 36 deletions(-) diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index c4fc3709c93..c79e17b57ee 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -9,7 +9,9 @@ module Gitlab end def ensure_application_settings! - if connect_to_db? + return fake_application_settings unless connect_to_db? + + unless ENV['IN_MEMORY_APPLICATION_SETTINGS'] == 'true' begin settings = ::ApplicationSetting.current # In case Redis isn't running or the Redis UNIX socket file is not available @@ -20,15 +22,23 @@ module Gitlab settings ||= ::ApplicationSetting.create_from_defaults unless ActiveRecord::Migrator.needs_migration? end - settings || fake_application_settings + settings || in_memory_application_settings end def sidekiq_throttling_enabled? current_application_settings.sidekiq_throttling_enabled? end + def in_memory_application_settings + @in_memory_application_settings ||= ApplicationSetting.new(ApplicationSetting::DEFAULTS) + # In case migrations the application_settings table is not created yet, + # we fallback to a simple OpenStruct + rescue ActiveRecord::StatementInvalid + fake_application_settings + end + def fake_application_settings - ApplicationSetting.new(ApplicationSetting::DEFAULTS) + OpenStruct.new(ApplicationSetting::DEFAULTS) end private diff --git a/lib/gitlab/github_import/project_creator.rb b/lib/gitlab/github_import/project_creator.rb index 3f635be22ba..a55adc9b1c8 100644 --- a/lib/gitlab/github_import/project_creator.rb +++ b/lib/gitlab/github_import/project_creator.rb @@ -1,6 +1,8 @@ module Gitlab module GithubImport class ProjectCreator + include Gitlab::CurrentSettings + attr_reader :repo, :name, :namespace, :current_user, :session_data, :type def initialize(repo, name, namespace, current_user, session_data, type: 'github') @@ -34,7 +36,7 @@ module Gitlab end def visibility_level - repo.private ? Gitlab::VisibilityLevel::PRIVATE : ApplicationSetting.current.default_project_visibility + repo.private ? Gitlab::VisibilityLevel::PRIVATE : current_application_settings.default_project_visibility end # diff --git a/spec/controllers/health_check_controller_spec.rb b/spec/controllers/health_check_controller_spec.rb index 56ecf2bb644..cfe18dd4b6c 100644 --- a/spec/controllers/health_check_controller_spec.rb +++ b/spec/controllers/health_check_controller_spec.rb @@ -1,10 +1,16 @@ require 'spec_helper' describe HealthCheckController do + include StubENV + let(:token) { current_application_settings.health_check_access_token } let(:json_response) { JSON.parse(response.body) } let(:xml_response) { Hash.from_xml(response.body)['hash'] } + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + end + describe 'GET #index' do context 'when services are up but NO access token' do it 'returns a not found page' do diff --git a/spec/features/admin/admin_disables_git_access_protocol_spec.rb b/spec/features/admin/admin_disables_git_access_protocol_spec.rb index 66044b44495..e8e080ce3e2 100644 --- a/spec/features/admin/admin_disables_git_access_protocol_spec.rb +++ b/spec/features/admin/admin_disables_git_access_protocol_spec.rb @@ -1,10 +1,13 @@ require 'rails_helper' feature 'Admin disables Git access protocol', feature: true do + include StubENV + let(:project) { create(:empty_project, :empty_repo) } let(:admin) { create(:admin) } background do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') login_as(admin) end diff --git a/spec/features/admin/admin_health_check_spec.rb b/spec/features/admin/admin_health_check_spec.rb index dec2dedf2b5..f7e49a56deb 100644 --- a/spec/features/admin/admin_health_check_spec.rb +++ b/spec/features/admin/admin_health_check_spec.rb @@ -1,9 +1,11 @@ require 'spec_helper' feature "Admin Health Check", feature: true do + include StubENV include WaitForAjax before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') login_as :admin end @@ -12,11 +14,12 @@ feature "Admin Health Check", feature: true do visit admin_health_check_path end - it { page.has_text? 'Health Check' } - it { page.has_text? 'Health information can be retrieved' } - it 'has a health check access token' do + page.has_text? 'Health Check' + page.has_text? 'Health information can be retrieved' + token = current_application_settings.health_check_access_token + expect(page).to have_content("Access token is #{token}") expect(page).to have_selector('#health-check-token', text: token) end diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index d92c66b689d..f05fbe3d062 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -1,7 +1,10 @@ require 'spec_helper' describe "Admin Runners" do + include StubENV + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') login_as :admin end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 47fa2f14307..de42ab81fac 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -1,7 +1,10 @@ require 'spec_helper' feature 'Admin updates settings', feature: true do - before(:each) do + include StubENV + + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') login_as :admin visit admin_application_settings_path end diff --git a/spec/features/admin/admin_uses_repository_checks_spec.rb b/spec/features/admin/admin_uses_repository_checks_spec.rb index 661fb761809..855247de2ea 100644 --- a/spec/features/admin/admin_uses_repository_checks_spec.rb +++ b/spec/features/admin/admin_uses_repository_checks_spec.rb @@ -1,7 +1,12 @@ require 'rails_helper' feature 'Admin uses repository checks', feature: true do - before { login_as :admin } + include StubENV + + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + login_as :admin + end scenario 'to trigger a single check' do project = create(:empty_project) @@ -29,7 +34,7 @@ feature 'Admin uses repository checks', feature: true do scenario 'to clear all repository checks', js: true do visit admin_application_settings_path - + expect(RepositoryCheck::ClearWorker).to receive(:perform_async) click_link 'Clear all repository checks' diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index 004341ffd02..b01c4805a34 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -1,36 +1,64 @@ require 'spec_helper' describe Gitlab::CurrentSettings do + include StubENV + + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + end + describe '#current_application_settings' do - it 'attempts to use cached values first' do - allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true) - expect(ApplicationSetting).to receive(:current).and_return(::ApplicationSetting.create_from_defaults) - expect(ApplicationSetting).not_to receive(:last) + context 'with DB available' do + before do + allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true) + end - expect(current_application_settings).to be_a(ApplicationSetting) - end + it 'attempts to use cached values first' do + expect(ApplicationSetting).to receive(:current) + expect(ApplicationSetting).not_to receive(:last) + + expect(current_application_settings).to be_a(ApplicationSetting) + end - it 'does not attempt to connect to DB or Redis' do - allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(false) - expect(ApplicationSetting).not_to receive(:current) - expect(ApplicationSetting).not_to receive(:last) + it 'falls back to DB if Redis returns an empty value' do + expect(ApplicationSetting).to receive(:last).and_call_original - expect(current_application_settings).to eq fake_application_settings + expect(current_application_settings).to be_a(ApplicationSetting) + end + + it 'falls back to DB if Redis fails' do + expect(ApplicationSetting).to receive(:current).and_raise(::Redis::BaseError) + expect(ApplicationSetting).to receive(:last).and_call_original + + expect(current_application_settings).to be_a(ApplicationSetting) + end end - it 'falls back to DB if Redis returns an empty value' do - allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true) - expect(ApplicationSetting).to receive(:last).and_call_original + context 'with DB unavailable' do + before do + allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(false) + end - expect(current_application_settings).to be_a(ApplicationSetting) + it 'returns an in-memory ApplicationSetting object' do + expect(ApplicationSetting).not_to receive(:current) + expect(ApplicationSetting).not_to receive(:last) + + expect(current_application_settings).to be_a(OpenStruct) + end end - it 'falls back to DB if Redis fails' do - allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true) - expect(ApplicationSetting).to receive(:current).and_raise(::Redis::BaseError) - expect(ApplicationSetting).to receive(:last).and_call_original + context 'when ENV["IN_MEMORY_APPLICATION_SETTINGS"] is true' do + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'true') + end + + it 'returns an in-memory ApplicationSetting object' do + expect(ApplicationSetting).not_to receive(:current) + expect(ApplicationSetting).not_to receive(:last) - expect(current_application_settings).to be_a(ApplicationSetting) + expect(current_application_settings).to be_a(ApplicationSetting) + expect(current_application_settings).not_to be_persisted + end end end end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 35644bd8cc9..c0a01d37990 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -337,8 +337,7 @@ describe API::Internal, api: true do context 'ssh access has been disabled' do before do - settings = ::ApplicationSetting.create_from_defaults - settings.update_attribute(:enabled_git_access_protocol, 'http') + stub_application_setting(enabled_git_access_protocol: 'http') end it 'rejects the SSH push' do @@ -360,8 +359,7 @@ describe API::Internal, api: true do context 'http access has been disabled' do before do - settings = ::ApplicationSetting.create_from_defaults - settings.update_attribute(:enabled_git_access_protocol, 'ssh') + stub_application_setting(enabled_git_access_protocol: 'ssh') end it 'rejects the HTTP push' do @@ -383,8 +381,7 @@ describe API::Internal, api: true do context 'web actions are always allowed' do it 'allows WEB push' do - settings = ::ApplicationSetting.create_from_defaults - settings.update_attribute(:enabled_git_access_protocol, 'ssh') + stub_application_setting(enabled_git_access_protocol: 'ssh') project.team << [user, :developer] push(key, project, 'web') diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6ee3307512d..f78899134d5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,6 +2,7 @@ require './spec/simplecov_env' SimpleCovEnv.start! ENV["RAILS_ENV"] ||= 'test' +ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true' require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' -- cgit v1.2.1 From 725b16543d75b82fbcd858ce9d2c88fbe92cc450 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 16 Jan 2017 11:59:21 -0500 Subject: Filter environments visibility in store instead of the view in order to not get a infinite update loop in vue.js --- .../environments/components/environment.js.es6 | 45 ++--------------- .../environments/stores/environments_store.js.es6 | 59 ++++++++++++++++++++++ 2 files changed, 62 insertions(+), 42 deletions(-) diff --git a/app/assets/javascripts/environments/components/environment.js.es6 b/app/assets/javascripts/environments/components/environment.js.es6 index 6e450df3c41..fea642467fa 100644 --- a/app/assets/javascripts/environments/components/environment.js.es6 +++ b/app/assets/javascripts/environments/components/environment.js.es6 @@ -11,41 +11,6 @@ (() => { window.gl = window.gl || {}; - /** - * Given the visibility prop provided by the url query parameter and which - * changes according to the active tab we need to filter which environments - * should be visible. - * - * The environments array is a recursive tree structure and we need to filter - * both root level environments and children environments. - * - * In order to acomplish that, both `filterState` and `filterEnvironmentsByState` - * functions work together. - * The first one works as the filter that verifies if the given environment matches - * the given state. - * The second guarantees both root level and children elements are filtered as well. - */ - - const filterState = state => environment => environment.state === state && environment; - /** - * Given the filter function and the array of environments will return only - * the environments that match the state provided to the filter function. - * - * @param {Function} fn - * @param {Array} array - * @return {Array} - */ - const filterEnvironmentsByState = (fn, arr) => arr.map((item) => { - if (item.children) { - const filteredChildren = filterEnvironmentsByState(fn, item.children).filter(Boolean); - if (filteredChildren.length) { - item.children = filteredChildren; - return item; - } - } - return fn(item); - }).filter(Boolean); - gl.environmentsList.EnvironmentsComponent = Vue.component('environment-component', { props: { store: { @@ -82,10 +47,6 @@ }, computed: { - filteredEnvironments() { - return filterEnvironmentsByState(filterState(this.visibility), this.state.environments); - }, - scope() { return this.$options.getQueryParameter('scope'); }, @@ -112,7 +73,7 @@ const scope = this.$options.getQueryParameter('scope'); if (scope) { - this.visibility = scope; + this.store.storeVisibility(scope); } this.isLoading = true; @@ -213,7 +174,7 @@
+ v-if="!isLoading && state.filteredEnvironments.length > 0"> @@ -226,7 +187,7 @@ -