Merge branch 'mo-update-ci-pending-build-shared-runners' into 'master'

Update Ci::PendingBuild when shared runners is toggled

See merge request gitlab-org/gitlab!69119

app/models/ci/pending_build.rb

@@ -2,6 +2,8 @@
 
 module Ci
   class PendingBuild < Ci::ApplicationRecord
+    include EachBatch
+
     belongs_to :project
     belongs_to :build, class_name: 'Ci::Build'
     belongs_to :namespace, inverse_of: :pending_builds, class_name: 'Namespace'

app/models/project.rb

@@ -339,6 +339,7 @@ class Project < ApplicationRecord
   # build traces. Currently there's no efficient way of removing this data in
   # bulk that doesn't involve loading the rows into memory. As a result we're
   # still using `dependent: :destroy` here.
+  has_many :pending_builds, class_name: 'Ci::PendingBuild'
   has_many :builds, class_name: 'Ci::Build', inverse_of: :project, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :processables, class_name: 'Ci::Processable', inverse_of: :project
   has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks

app/services/ci/update_pending_build_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class UpdatePendingBuildService
+    VALID_PARAMS = %i[instance_runners_enabled].freeze
+
+    InvalidParamsError = Class.new(StandardError)
+    InvalidModelError = Class.new(StandardError)
+
+    def initialize(model, update_params)
+      @model = model
+      @update_params = update_params
+
+      validations!
+    end
+
+    def execute
+      return unless ::Feature.enabled?(:ci_pending_builds_maintain_shared_runners_data, @model, default_enabled: :yaml)
+
+      @model.pending_builds.each_batch do |relation|
+        relation.update_all(@update_params)
+      end
+    end
+
+    private
+
+    def validations!
+      validate_model! && validate_params!
+    end
+
+    def validate_model!
+      raise InvalidModelError unless @model.is_a?(::Project) || @model.is_a?(::Group)
+
+      true
+    end
+
+    def validate_params!
+      extra_params = @update_params.keys - VALID_PARAMS
+
+      raise InvalidParamsError, "Unvalid params: #{extra_params.join(', ')}" unless extra_params.empty?
+
+      true
+    end
+  end
+end

app/services/groups/update_shared_runners_service.rb

@@ -8,6 +8,7 @@ module Groups
       validate_params
 
       update_shared_runners
+      update_pending_builds!
 
       success
 
@@ -26,5 +27,13 @@ module Groups
     def update_shared_runners
       group.update_shared_runners_setting!(params[:shared_runners_setting])
     end
+
+    def update_pending_builds!
+      return unless group.previous_changes.include?('shared_runners_enabled')
+
+      update_params = { instance_runners_enabled: group.shared_runners_enabled }
+
+      ::Ci::UpdatePendingBuildService.new(group, update_params).execute
+    end
   end
 end

app/services/projects/update_service.rb

@@ -105,6 +105,7 @@ module Projects
       end
 
       update_pages_config if changing_pages_related_config?
+      update_pending_builds if shared_runners_toggled?
     end
 
     def after_rename_service(project)
@@ -178,6 +179,16 @@ module Projects
 
       params[:topic_list] ||= topic_list if topic_list
     end
+
+    def update_pending_builds
+      update_params = { instance_runners_enabled: project.shared_runners_enabled }
+
+      ::Ci::UpdatePendingBuildService.new(project, update_params).execute
+    end
+
+    def shared_runners_toggled?
+      project.previous_changes.include?('shared_runners_enabled')
+    end
   end
 end
 

spec/lib/gitlab/import_export/all_models.yml

@@ -589,6 +589,7 @@ project:
 - timelogs
 - error_tracking_errors
 - error_tracking_client_keys
+- pending_builds
 award_emoji:
 - awardable
 - user

spec/models/project_spec.rb

@@ -137,6 +137,7 @@ RSpec.describe Project, factory_default: :keep do
     it { is_expected.to have_many(:timelogs) }
     it { is_expected.to have_many(:error_tracking_errors).class_name('ErrorTracking::Error') }
     it { is_expected.to have_many(:error_tracking_client_keys).class_name('ErrorTracking::ClientKey') }
+    it { is_expected.to have_many(:pending_builds).class_name('Ci::PendingBuild') }
 
     # GitLab Pages
     it { is_expected.to have_many(:pages_domains) }

spec/services/ci/update_pending_build_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::UpdatePendingBuildService do
+  describe '#execute' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, namespace: group) }
+    let_it_be(:pending_build_1) { create(:ci_pending_build, project: project, instance_runners_enabled: false) }
+    let_it_be(:pending_build_2) { create(:ci_pending_build, project: project, instance_runners_enabled: true) }
+    let_it_be(:update_params) { { instance_runners_enabled: true } }
+
+    subject(:service) { described_class.new(model, update_params).execute }
+
+    context 'validations' do
+      context 'when model is invalid' do
+        let(:model) { pending_build_1 }
+
+        it 'raises an error' do
+          expect { service }.to raise_error(described_class::InvalidModelError)
+        end
+      end
+
+      context 'when params is invalid' do
+        let(:model) { group }
+        let(:update_params) { { minutes_exceeded: true } }
+
+        it 'raises an error' do
+          expect { service }.to raise_error(described_class::InvalidParamsError)
+        end
+      end
+    end
+
+    context 'when model is a group with pending builds' do
+      let(:model) { group }
+
+      it 'updates all pending builds', :aggregate_failures do
+        service
+
+        expect(pending_build_1.reload.instance_runners_enabled).to be_truthy
+        expect(pending_build_2.reload.instance_runners_enabled).to be_truthy
+      end
+
+      context 'when ci_pending_builds_maintain_shared_runners_data is disabled' do
+        before do
+          stub_feature_flags(ci_pending_builds_maintain_shared_runners_data: false)
+        end
+
+        it 'does not update all pending builds', :aggregate_failures do
+          service
+
+          expect(pending_build_1.reload.instance_runners_enabled).to be_falsey
+          expect(pending_build_2.reload.instance_runners_enabled).to be_truthy
+        end
+      end
+    end
+
+    context 'when model is a project with pending builds' do
+      let(:model) { project }
+
+      it 'updates all pending builds', :aggregate_failures do
+        service
+
+        expect(pending_build_1.reload.instance_runners_enabled).to be_truthy
+        expect(pending_build_2.reload.instance_runners_enabled).to be_truthy
+      end
+
+      context 'when ci_pending_builds_maintain_shared_runners_data is disabled' do
+        before do
+          stub_feature_flags(ci_pending_builds_maintain_shared_runners_data: false)
+        end
+
+        it 'does not update all pending builds', :aggregate_failures do
+          service
+
+          expect(pending_build_1.reload.instance_runners_enabled).to be_falsey
+          expect(pending_build_2.reload.instance_runners_enabled).to be_truthy
+        end
+      end
+    end
+  end
+end

spec/services/groups/update_shared_runners_service_spec.rb

@@ -55,6 +55,31 @@ RSpec.describe Groups::UpdateSharedRunnersService do
             expect(subject[:status]).to eq(:success)
           end
         end
+
+        context 'when group has pending builds' do
+          let_it_be(:group) { create(:group, :shared_runners_disabled) }
+          let_it_be(:project) { create(:project, namespace: group, shared_runners_enabled: false) }
+          let_it_be(:pending_build_1) { create(:ci_pending_build, project: project, instance_runners_enabled: false) }
+          let_it_be(:pending_build_2) { create(:ci_pending_build, project: project, instance_runners_enabled: false) }
+
+          it 'updates pending builds for the group' do
+            subject
+
+            expect(pending_build_1.reload.instance_runners_enabled).to be_truthy
+            expect(pending_build_2.reload.instance_runners_enabled).to be_truthy
+          end
+
+          context 'when shared runners is not toggled' do
+            let(:params) { { shared_runners_setting: 'invalid_enabled' } }
+
+            it 'does not update pending builds for the group' do
+              subject
+
+              expect(pending_build_1.reload.instance_runners_enabled).to be_falsey
+              expect(pending_build_2.reload.instance_runners_enabled).to be_falsey
+            end
+          end
+        end
       end
 
       context 'disable shared Runners' do
@@ -67,6 +92,19 @@ RSpec.describe Groups::UpdateSharedRunnersService do
 
           expect(subject[:status]).to eq(:success)
         end
+
+        context 'when group has pending builds' do
+          let_it_be(:project) { create(:project, namespace: group) }
+          let_it_be(:pending_build_1) { create(:ci_pending_build, project: project, instance_runners_enabled: true) }
+          let_it_be(:pending_build_2) { create(:ci_pending_build, project: project, instance_runners_enabled: true) }
+
+          it 'updates pending builds for the group' do
+            subject
+
+            expect(pending_build_1.reload.instance_runners_enabled).to be_falsey
+            expect(pending_build_2.reload.instance_runners_enabled).to be_falsey
+          end
+        end
       end
 
       context 'allow descendants to override' do

spec/services/projects/update_service_spec.rb

@@ -441,6 +441,30 @@ RSpec.describe Projects::UpdateService do
       end
     end
 
+    context 'when updating #shared_runners', :https_pages_enabled do
+      let!(:pending_build) { create(:ci_pending_build, project: project, instance_runners_enabled: true) }
+
+      subject(:call_service) do
+        update_project(project, admin, shared_runners_enabled: shared_runners_enabled)
+      end
+
+      context 'when shared runners is toggled' do
+        let(:shared_runners_enabled) { false }
+
+        it 'updates ci pending builds' do
+          expect { call_service }.to change { pending_build.reload.instance_runners_enabled }.to(false)
+        end
+      end
+
+      context 'when shared runners is not toggled' do
+        let(:shared_runners_enabled) { true }
+
+        it 'updates ci pending builds' do
+          expect { call_service }.to not_change { pending_build.reload.instance_runners_enabled }
+        end
+      end
+    end
+
     context 'with external authorization enabled' do
       before do
         enable_external_authorization_service_check