diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb index 7345c91f67df3ba174c11001edf49c434b484297..348641e5ecb5a95ac8ff2ff746fb3b97eaf82251 100644 --- a/app/controllers/admin/runners_controller.rb +++ b/app/controllers/admin/runners_controller.rb @@ -13,7 +13,7 @@ def show end def update - if @runner.update_attributes(runner_params) + if Ci::UpdateRunnerService.new(@runner).update(runner_params) respond_to do |format| format.js format.html { redirect_to admin_runner_path(@runner) } @@ -31,7 +31,7 @@ def destroy end def resume - if @runner.update_attributes(active: true) + if Ci::UpdateRunnerService.new(@runner).update(active: true) redirect_to admin_runners_path, notice: 'Runner was successfully updated.' else redirect_to admin_runners_path, alert: 'Runner was not updated.' @@ -39,7 +39,7 @@ def resume end def pause - if @runner.update_attributes(active: false) + if Ci::UpdateRunnerService.new(@runner).update(active: false) redirect_to admin_runners_path, notice: 'Runner was successfully updated.' else redirect_to admin_runners_path, alert: 'Runner was not updated.' diff --git a/app/controllers/projects/runners_controller.rb b/app/controllers/projects/runners_controller.rb index 53c36635efe1ecd779360bf14ead772aa2556b97..ff75c408beb84b025b413d380576f51198b0c582 100644 --- a/app/controllers/projects/runners_controller.rb +++ b/app/controllers/projects/runners_controller.rb @@ -16,7 +16,7 @@ def edit end def update - if @runner.update_attributes(runner_params) + if Ci::UpdateRunnerService.new(@runner).update(runner_params) redirect_to runner_path(@runner), notice: 'Runner was successfully updated.' else render 'edit' @@ -32,7 +32,7 @@ def destroy end def resume - if @runner.update_attributes(active: true) + if Ci::UpdateRunnerService.new(@runner).update(active: true) redirect_to runner_path(@runner), notice: 'Runner was successfully updated.' else redirect_to runner_path(@runner), alert: 'Runner was not updated.' @@ -40,7 +40,7 @@ def resume end def pause - if @runner.update_attributes(active: false) + if Ci::UpdateRunnerService.new(@runner).update(active: false) redirect_to runner_path(@runner), notice: 'Runner was successfully updated.' else redirect_to runner_path(@runner), alert: 'Runner was not updated.' diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index ed1843ba0053e86a9f0c772005b3a088196b110a..07a086b0acae00b1a48cfdcad69cdf03572bb764 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -22,8 +22,6 @@ class Runner < ActiveRecord::Base scope :online, ->() { where('contacted_at > ?', LAST_CONTACT_TIME) } scope :ordered, ->() { order(id: :desc) } - after_save :tick_runner_queue, if: :form_editable_changed? - 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) @@ -40,6 +38,8 @@ class Runner < ActiveRecord::Base acts_as_taggable + after_destroy :cleanup_runner_queue + # Searches for runners matching the given query. # # This method uses ILIKE on PostgreSQL and LIKE on MySQL. @@ -147,14 +147,14 @@ def is_runner_queue_value_latest?(value) private - def runner_queue_key - "runner:build_queue:#{self.token}" + def cleanup_runner_queue + Gitlab::Redis.with do |redis| + redis.del(runner_queue_key) + end end - def form_editable_changed? - FORM_EDITABLE.any? do |editable| - public_send("#{editable}_changed?") - end + def runner_queue_key + "runner:build_queue:#{self.token}" end def tag_constraints diff --git a/app/services/ci/update_runner_service.rb b/app/services/ci/update_runner_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..198b29b3a9d05c7520705622b5fbbd5421644550 --- /dev/null +++ b/app/services/ci/update_runner_service.rb @@ -0,0 +1,15 @@ +module Ci + class UpdateRunnerService + attr_reader :runner + + def initialize(runner) + @runner = runner + end + + def update(params) + runner.update(params).tap do + runner.tick_runner_queue + end + end + end +end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 3f32248e52b0d9357a50cee55c58fcf642e5425c..f8513ac8b1c1ba6d134a670ba40313c4ee95b7aa 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -290,7 +290,7 @@ let!(:last_update) { runner.ensure_runner_queue_value } before do - runner.update(description: 'new runner') + Ci::UpdateRunnerService.new(runner).update(description: 'new runner') end it 'sets a new last_update value' do @@ -318,6 +318,25 @@ def expect_value_in_redis end end + describe '#destroy' do + let(:runner) { create(:ci_runner) } + + context 'when there is a tick in the queue' do + let!(:queue_key) { runner.send(:runner_queue_key) } + + before do + runner.tick_runner_queue + runner.destroy + end + + it 'cleans up the queue' do + Gitlab::Redis.with do |redis| + expect(redis.get(queue_key)).to be_nil + end + end + end + end + describe '.assignable_for' do let(:runner) { create(:ci_runner) } let(:project) { create(:empty_project) } diff --git a/spec/services/ci/update_runner_service_spec.rb b/spec/services/ci/update_runner_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..8429881dd151298733f1fa939f3c8e5969c959ff --- /dev/null +++ b/spec/services/ci/update_runner_service_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe Ci::UpdateRunnerService, services: true do + let(:runner) { create(:ci_runner) } + + describe '#update' do + before do + allow(runner).to receive(:tick_runner_queue) + + described_class.new(runner).update(description: 'new runner') + end + + it 'updates the runner and ticking the queue' do + expect(runner.description).to eq('new runner') + expect(runner).to have_received(:tick_runner_queue) + end + end +end