Merge branch 'pedropombeiro/347208/filter-by-active-state' into 'master'

GraphQL: Allow filtering runners by active status

See merge request gitlab-org/gitlab!76108

app/finders/ci/runners_finder.rb

@@ -15,6 +15,7 @@ module Ci
 
     def execute
       search!
+      filter_by_active!
       filter_by_status!
       filter_by_runner_type!
       filter_by_tag_list!
@@ -60,6 +61,10 @@ module Ci
                  end
     end
 
+    def filter_by_active!
+      @runners = @runners.active(@params[:active]) if @params.include?(:active)
+    end
+
     def filter_by_status!
       filter_by!(:status_status, Ci::Runner::AVAILABLE_STATUSES)
     end

app/graphql/resolvers/ci/runners_resolver.rb

@@ -7,6 +7,10 @@ module Resolvers
 
       type Types::Ci::RunnerType.connection_type, null: true
 
+      argument :active, ::GraphQL::Types::Boolean,
+               required: false,
+               description: 'Filter runners by active (true) or paused (false) status.'
+
       argument :status, ::Types::Ci::RunnerStatusEnum,
                required: false,
                description: 'Filter runners by status.'
@@ -38,6 +42,7 @@ module Resolvers
 
       def runners_finder_params(params)
         {
+          active: params[:active],
           status_status: params[:status]&.to_s,
           type_type: params[:type],
           tag_name: params[:tag_list],

app/models/ci/runner.rb

@@ -60,8 +60,8 @@ module Ci
 
     before_save :ensure_token
 
-    scope :active, -> { where(active: true) }
-    scope :paused, -> { where(active: false) }
+    scope :active, -> (value = true) { where(active: value) }
+    scope :paused, -> { active(false) }
     scope :online, -> { where('contacted_at > ?', online_contact_time_deadline) }
     scope :recent, -> { where('ci_runners.created_at >= :date OR ci_runners.contacted_at >= :date', date: stale_deadline) }
     scope :stale, -> { where('ci_runners.created_at < :date AND (ci_runners.contacted_at IS NULL OR ci_runners.contacted_at < :date)', date: stale_deadline) }

db/migrate/20211207154413_add_ci_runners_index_on_created_at_where_active_is_false.rb

+# frozen_string_literal: true
+
+class AddCiRunnersIndexOnCreatedAtWhereActiveIsFalse < Gitlab::Database::Migration[1.0]
+  INDEX_NAME = 'index_ci_runners_on_created_at_and_id_where_inactive'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :ci_runners, [:created_at, :id], where: 'active = FALSE', order: { created_at: :desc, id: :desc }, name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :ci_runners, INDEX_NAME
+  end
+end

db/migrate/20211207154414_add_ci_runners_index_on_contacted_at_where_active_is_false.rb

+# frozen_string_literal: true
+
+class AddCiRunnersIndexOnContactedAtWhereActiveIsFalse < Gitlab::Database::Migration[1.0]
+  INDEX_NAME = 'index_ci_runners_on_contacted_at_and_id_where_inactive'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :ci_runners, [:contacted_at, :id], where: 'active = FALSE', order: { contacted_at: :desc, id: :desc }, name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :ci_runners, INDEX_NAME
+  end
+end

db/schema_migrations/20211207154413

+98098b41864158fc4de3b8fe42603b2c0c5c2fbc664397c431712311bdaa3621
\ No newline at end of file

db/schema_migrations/20211207154414

+278907a15d04b455aa852eb9d17000c6b353be6ef78a8dcc2e71a9772a6e43ea
\ No newline at end of file

db/structure.sql

@@ -25610,10 +25610,14 @@ CREATE INDEX index_ci_runner_projects_on_runner_id ON ci_runner_projects USING b
 
 CREATE INDEX index_ci_runners_on_contacted_at_and_id_desc ON ci_runners USING btree (contacted_at, id DESC);
 
+CREATE INDEX index_ci_runners_on_contacted_at_and_id_where_inactive ON ci_runners USING btree (contacted_at DESC, id DESC) WHERE (active = false);
+
 CREATE INDEX index_ci_runners_on_contacted_at_desc_and_id_desc ON ci_runners USING btree (contacted_at DESC, id DESC);
 
 CREATE INDEX index_ci_runners_on_created_at_and_id_desc ON ci_runners USING btree (created_at, id DESC);
 
+CREATE INDEX index_ci_runners_on_created_at_and_id_where_inactive ON ci_runners USING btree (created_at DESC, id DESC) WHERE (active = false);
+
 CREATE INDEX index_ci_runners_on_created_at_desc_and_id_desc ON ci_runners USING btree (created_at DESC, id DESC);
 
 CREATE INDEX index_ci_runners_on_description_trigram ON ci_runners USING gin (description gin_trgm_ops);

doc/api/graphql/reference/index.md

@@ -370,6 +370,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 
 | Name | Type | Description |
 | ---- | ---- | ----------- |
+| <a id="queryrunnersactive"></a>`active` | [`Boolean`](#boolean) | Filter runners by active (true) or paused (false) status. |
 | <a id="queryrunnerssearch"></a>`search` | [`String`](#string) | Filter by full token or partial text in description field. |
 | <a id="queryrunnerssort"></a>`sort` | [`CiRunnerSort`](#cirunnersort) | Sort order of results. |
 | <a id="queryrunnersstatus"></a>`status` | [`CiRunnerStatus`](#cirunnerstatus) | Filter runners by status. |
@@ -10917,6 +10918,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 
 | Name | Type | Description |
 | ---- | ---- | ----------- |
+| <a id="grouprunnersactive"></a>`active` | [`Boolean`](#boolean) | Filter runners by active (true) or paused (false) status. |
 | <a id="grouprunnersmembership"></a>`membership` | [`RunnerMembershipFilter`](#runnermembershipfilter) | Control which runners to include in the results. |
 | <a id="grouprunnerssearch"></a>`search` | [`String`](#string) | Filter by full token or partial text in description field. |
 | <a id="grouprunnerssort"></a>`sort` | [`CiRunnerSort`](#cirunnersort) | Sort order of results. |

spec/finders/ci/runners_finder_spec.rb

@@ -59,6 +59,20 @@ RSpec.describe Ci::RunnersFinder do
           end
         end
 
+        context 'by active status' do
+          it 'with active set as false calls the corresponding scope on Ci::Runner with false' do
+            expect(Ci::Runner).to receive(:active).with(false).and_call_original
+
+            described_class.new(current_user: admin, params: { active: false }).execute
+          end
+
+          it 'with active set as true calls the corresponding scope on Ci::Runner with true' do
+            expect(Ci::Runner).to receive(:active).with(true).and_call_original
+
+            described_class.new(current_user: admin, params: { active: true }).execute
+          end
+        end
+
         context 'by runner type' do
           it 'calls the corresponding scope on Ci::Runner' do
             expect(Ci::Runner).to receive(:project_type).and_call_original
@@ -263,7 +277,15 @@ RSpec.describe Ci::RunnersFinder do
               let(:extra_params) { { search: 'runner_project_search' } }
 
               it 'returns correct runner' do
-                expect(subject).to eq([runner_project_3])
+                expect(subject).to match_array([runner_project_3])
+              end
+            end
+
+            context 'by active status' do
+              let(:extra_params) { { active: false } }
+
+              it 'returns correct runner' do
+                expect(subject).to match_array([runner_sub_group_1])
               end
             end
 
@@ -271,7 +293,7 @@ RSpec.describe Ci::RunnersFinder do
               let(:extra_params) { { status_status: 'paused' } }
 
               it 'returns correct runner' do
-                expect(subject).to eq([runner_sub_group_1])
+                expect(subject).to match_array([runner_sub_group_1])
               end
             end
 
@@ -279,7 +301,7 @@ RSpec.describe Ci::RunnersFinder do
               let(:extra_params) { { tag_name: %w[runner_tag] } }
 
               it 'returns correct runner' do
-                expect(subject).to eq([runner_project_5])
+                expect(subject).to match_array([runner_project_5])
               end
             end
 

spec/graphql/resolvers/ci/runners_resolver_spec.rb

@@ -45,6 +45,7 @@ RSpec.describe Resolvers::Ci::RunnersResolver do
       let(:finder) { instance_double(::Ci::RunnersFinder) }
       let(:args) do
         {
+          active: true,
           status: 'active',
           type: :instance_type,
           tag_list: ['active_runner'],
@@ -55,6 +56,7 @@ RSpec.describe Resolvers::Ci::RunnersResolver do
 
       let(:expected_params) do
         {
+          active: true,
           status_status: 'active',
           type_type: :instance_type,
           tag_name: ['active_runner'],

spec/models/ci/runner_spec.rb

@@ -302,6 +302,44 @@ RSpec.describe Ci::Runner do
     it { is_expected.to eq([runner1, runner3, runner4])}
   end
 
+  describe '.active' do
+    subject { described_class.active(active_value) }
+
+    let!(:runner1) { create(:ci_runner, :instance, active: false) }
+    let!(:runner2) { create(:ci_runner, :instance) }
+
+    context 'with active_value set to false' do
+      let(:active_value) { false }
+
+      it 'returns inactive runners' do
+        is_expected.to match_array([runner1])
+      end
+    end
+
+    context 'with active_value set to true' do
+      let(:active_value) { true }
+
+      it 'returns active runners' do
+        is_expected.to match_array([runner2])
+      end
+    end
+  end
+
+  describe '.paused' do
+    before do
+      expect(described_class).to receive(:active).with(false).and_call_original
+    end
+
+    subject { described_class.paused }
+
+    let!(:runner1) { create(:ci_runner, :instance, active: false) }
+    let!(:runner2) { create(:ci_runner, :instance) }
+
+    it 'returns inactive runners' do
+      is_expected.to match_array([runner1])
+    end
+  end
+
   describe '.stale' do
     subject { described_class.stale }