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 @@ class Admin::RunnersController < Admin::ApplicationController 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 @@ class Admin::RunnersController < Admin::ApplicationController 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 @@ class Admin::RunnersController < Admin::ApplicationController 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 @@ class Projects::RunnersController < Projects::ApplicationController 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 @@ class Projects::RunnersController < Projects::ApplicationController 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 @@ class Projects::RunnersController < Projects::ApplicationController 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 6e58a1878c860611b287ac7a9ac96cbd4a897ba6..b4760b5baaa8b50953770dbc57dc8691a55b1ee2 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -22,8 +22,6 @@ module Ci 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) @@ -149,12 +147,6 @@ module Ci "runner:build_queue:#{self.token}" end - def form_editable_changed? - FORM_EDITABLE.any? do |editable| - public_send("#{editable}_changed?") - end - end - def tag_constraints unless has_tags? || run_untagged? errors.add(:tags_list, 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/lib/ci/api/runners.rb b/lib/ci/api/runners.rb index bcc82969eb3f0e0f97eca45335d9f17a4d741dcd..c10858f4c5e7f242b4c80952af2e0fe2fe7941b5 100644 --- a/lib/ci/api/runners.rb +++ b/lib/ci/api/runners.rb @@ -28,23 +28,27 @@ module Ci post "register" do required_attributes! [:token] - attributes = attributes_for_keys( - [:description, :tag_list, :run_untagged, :locked] - ) - + project = nil runner = if runner_registration_token_valid? # Create shared runner. Requires admin access - Ci::Runner.create(attributes.merge(is_shared: true)) + Ci::Runner.new(is_shared: true) elsif project = Project.find_by(runners_token: params[:token]) - # Create a specific runner for project. - project.runners.create(attributes) + Ci::Runner.new end return forbidden! unless runner + attributes = attributes_for_keys( + [:description, :tag_list, :run_untagged, :locked] + ).merge(get_runner_version_from_params || {}) + + Ci::UpdateRunnerService.new(runner).update(attributes) + + # Assign the specific runner for the project + project.runners << runner if project + if runner.id - runner.update(get_runner_version_from_params) present runner, with: Entities::Runner else not_found! diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 2b856ca7af7e5b5816fd8093e5f21e881c39adb6..7b993a454b72787f70af3148c7c5b47b8aa96ae9 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -291,7 +291,7 @@ describe Ci::Runner, models: true do 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 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