Merge branch 'feature/runner-type-filter-for-admin-view' into 'master'

Feature: Runner type filter for admin view

See merge request gitlab-org/gitlab-ce!19649

app/assets/javascripts/filtered_search/admin_runners_filtered_search_token_keys.js

@@ -7,6 +7,13 @@ const tokenKeys = [{
   symbol: '',
   icon: 'messages',
   tag: 'status',
+}, {
+  key: 'type',
+  type: 'string',
+  param: 'type',
+  symbol: '',
+  icon: 'cube',
+  tag: 'type',
 }];
 const AdminRunnersFilteredSearchTokenKeys = new FilteredSearchTokenKeys(tokenKeys);

app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js

@@ -96,6 +96,11 @@ export default class FilteredSearchDropdownManager {
         gl: NullDropdown,
         element: this.container.querySelector('#js-dropdown-admin-runner-status'),
       },
+      type: {
+        reference: null,
+        gl: NullDropdown,
+        element: this.container.querySelector('#js-dropdown-admin-runner-type'),
+      },
     };
     supportedTokens.forEach((type) => {

app/finders/admin/runners_finder.rb

@@ -10,6 +10,7 @@ class Admin::RunnersFinder < UnionFinder
   def execute
     search!
     filter_by_status!
+    filter_by_runner_type!
     sort!
     paginate!
@@ -36,10 +37,11 @@ class Admin::RunnersFinder < UnionFinder
   end
   def filter_by_status!
-    status = @params[:status_status]
-    if status.present? && Ci::Runner::AVAILABLE_STATUSES.include?(status)
-      @runners = @runners.public_send(status) # rubocop:disable GitlabSecurity/PublicSend
-    end
+    filter_by!(:status_status, Ci::Runner::AVAILABLE_STATUSES)
+  end
+
+  def filter_by_runner_type!
+    filter_by!(:type_type, Ci::Runner::AVAILABLE_TYPES)
   end
   def sort!
@@ -49,4 +51,12 @@ class Admin::RunnersFinder < UnionFinder
   def paginate!
     @runners = @runners.page(@params[:page]).per(NUMBER_OF_RUNNERS_PER_PAGE)
   end
+
+  def filter_by!(scope_name, available_scopes)
+    scope = @params[scope_name]
+
+    if scope.present? && available_scopes.include?(scope)
+      @runners = @runners.public_send(scope) # rubocop:disable GitlabSecurity/PublicSend
+    end
+  end
 end

app/models/ci/runner.rb

@@ -9,12 +9,24 @@ module Ci
     include ChronicDurationAttribute
     include FromUnion
+    enum access_level: {
+      not_protected: 0,
+      ref_protected: 1
+    }
+
+    enum runner_type: {
+      instance_type: 1,
+      group_type: 2,
+      project_type: 3
+    }
+
     RUNNER_QUEUE_EXPIRY_TIME = 60.minutes
     ONLINE_CONTACT_TIMEOUT = 1.hour
     UPDATE_DB_RUNNER_INFO_EVERY = 40.minutes
-    AVAILABLE_TYPES = %w[specific shared].freeze
+    AVAILABLE_TYPES_LEGACY = %w[specific shared].freeze
+    AVAILABLE_TYPES = runner_types.keys.freeze
     AVAILABLE_STATUSES = %w[active paused online offline].freeze
-    AVAILABLE_SCOPES = (AVAILABLE_TYPES + AVAILABLE_STATUSES).freeze
+    AVAILABLE_SCOPES = (AVAILABLE_TYPES_LEGACY + AVAILABLE_TYPES + AVAILABLE_STATUSES).freeze
     FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level maximum_timeout_human_readable].freeze
     ignore_column :is_shared
@@ -97,17 +109,6 @@ module Ci
     after_destroy :cleanup_runner_queue
-    enum access_level: {
-      not_protected: 0,
-      ref_protected: 1
-    }
-
-    enum runner_type: {
-      instance_type: 1,
-      group_type: 2,
-      project_type: 3
-    }
-
     cached_attr_reader :version, :revision, :platform, :architecture, :ip_address, :contacted_at
     chronic_duration_attr :maximum_timeout_human_readable, :maximum_timeout

app/views/admin/runners/index.html.haml

@@ -83,12 +83,21 @@
                       {{hint}}
                     %span.js-filter-tag.dropdown-light-content
                       {{tag}}
+
             #js-dropdown-admin-runner-status.filtered-search-input-dropdown-menu.dropdown-menu
               %ul{ data: { dropdown: true } }
                 - Ci::Runner::AVAILABLE_STATUSES.each do |status|
                   %li.filter-dropdown-item{ data: { value: status } }
                     = button_tag class: %w[btn btn-link] do
                       = status.titleize
+
+            #js-dropdown-admin-runner-type.filtered-search-input-dropdown-menu.dropdown-menu
+              %ul{ data: { dropdown: true } }
+                - Ci::Runner::AVAILABLE_TYPES.each do |runner_type|
+                  %li.filter-dropdown-item{ data: { value: runner_type } }
+                    = button_tag class: %w[btn btn-link] do
+                      = runner_type.titleize
+
           = button_tag class: %w[clear-search hidden] do
             = icon('times')
         .filter-dropdown-container

changelogs/unreleased/feature-runner-type-filter-for-admin-view.yml

+---
+title: Add a type filter to the admin runners view
+merge_request: 19649
+author: Alexis Reigel
+type: added

doc/api/runners.md

@@ -11,11 +11,15 @@ Get a list of specific runners available to the user.
 ```
 GET /runners
 GET /runners?scope=active
+GET /runners?type=project_type
+GET /runners?status=active
 ```
 | Attribute | Type    | Required | Description         |
 |-----------|---------|----------|---------------------|
-| `scope`   | string  | no       | The scope of specific runners to show, one of: `active`, `paused`, `online`, `offline`; showing all runners if none provided |
+| `scope`   | string  | no       | Deprecated: Use `type` or `status` instead. The scope of specific runners to show, one of: `active`, `paused`, `online`, `offline`; showing all runners if none provided |
+| `type`    | string  | no       | The type of runners to show, one of: `instance_type`, `group_type`, `project_type` |
+| `status`  | string  | no       | The status of runners to show, one of: `active`, `paused`, `online`, `offline` |
 ```
 curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/runners"
@@ -56,11 +60,15 @@ is restricted to users with `admin` privileges.
 ```
 GET /runners/all
 GET /runners/all?scope=online
+GET /runners/all?type=project_type
+GET /runners/all?status=active
 ```
 | Attribute | Type    | Required | Description         |
 |-----------|---------|----------|---------------------|
-| `scope`   | string  | no       | The scope of runners to show, one of: `specific`, `shared`, `active`, `paused`, `online`, `offline`; showing all runners if none provided |
+| `scope`   | string  | no       | Deprecated: Use `type` or `status` instead. The scope of runners to show, one of: `specific`, `shared`, `active`, `paused`, `online`, `offline`; showing all runners if none provided |
+| `type`    | string  | no       | The type of runners to show, one of: `instance_type`, `group_type`, `project_type` |
+| `status`  | string  | no       | The status of runners to show, one of: `active`, `paused`, `online`, `offline` |
 ```
 curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/runners/all"
@@ -336,11 +344,17 @@ usage is enabled in the project's settings.
 ```
 GET /projects/:id/runners
+GET /projects/:id/runners?scope=active
+GET /projects/:id/runners?type=project_type
+GET /projects/:id/runners?status=active
 ```
-| Attribute | Type    | Required | Description         |
-|-----------|---------|----------|---------------------|
+| Attribute | Type           | Required | Description         |
+|-----------|----------------|----------|---------------------|
 | `id`      | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
+| `scope`   | string         | no       | Deprecated: Use `type` or `status` instead. The scope of specific runners to show, one of: `active`, `paused`, `online`, `offline`; showing all runners if none provided |
+| `type`    | string         | no       | The type of runners to show, one of: `instance_type`, `group_type`, `project_type` |
+| `status`  | string         | no       | The status of runners to show, one of: `active`, `paused`, `online`, `offline` |
 ```
 curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/9/runners"

lib/api/runners.rb

@@ -11,10 +11,18 @@ module API
       params do
         optional :scope, type: String, values: Ci::Runner::AVAILABLE_STATUSES,
                          desc: 'The scope of specific runners to show'
+        optional :type, type: String, values: Ci::Runner::AVAILABLE_TYPES,
+                        desc: 'The type of the runners to show'
+        optional :status, type: String, values: Ci::Runner::AVAILABLE_STATUSES,
+                          desc: 'The status of the runners to show'
         use :pagination
       end
       get do
-        runners = filter_runners(current_user.ci_owned_runners, params[:scope], allowed_scopes: Ci::Runner::AVAILABLE_STATUSES)
+        runners = current_user.ci_owned_runners
+        runners = filter_runners(runners, params[:scope], allowed_scopes: Ci::Runner::AVAILABLE_STATUSES)
+        runners = filter_runners(runners, params[:type], allowed_scopes: Ci::Runner::AVAILABLE_TYPES)
+        runners = filter_runners(runners, params[:status], allowed_scopes: Ci::Runner::AVAILABLE_STATUSES)
+
         present paginate(runners), with: Entities::Runner
       end
@@ -24,11 +32,20 @@ module API
       params do
         optional :scope, type: String, values: Ci::Runner::AVAILABLE_SCOPES,
                          desc: 'The scope of specific runners to show'
+        optional :type, type: String, values: Ci::Runner::AVAILABLE_TYPES,
+                        desc: 'The type of the runners to show'
+        optional :status, type: String, values: Ci::Runner::AVAILABLE_STATUSES,
+                          desc: 'The status of the runners to show'
         use :pagination
       end
       get 'all' do
         authenticated_as_admin!
-        runners = filter_runners(Ci::Runner.all, params[:scope])
+
+        runners = Ci::Runner.all
+        runners = filter_runners(runners, params[:scope])
+        runners = filter_runners(runners, params[:type], allowed_scopes: Ci::Runner::AVAILABLE_TYPES)
+        runners = filter_runners(runners, params[:status], allowed_scopes: Ci::Runner::AVAILABLE_STATUSES)
+
         present paginate(runners), with: Entities::Runner
       end
@@ -116,10 +133,18 @@ module API
       params do
         optional :scope, type: String, values: Ci::Runner::AVAILABLE_SCOPES,
                          desc: 'The scope of specific runners to show'
+        optional :type, type: String, values: Ci::Runner::AVAILABLE_TYPES,
+                        desc: 'The type of the runners to show'
+        optional :status, type: String, values: Ci::Runner::AVAILABLE_STATUSES,
+                          desc: 'The status of the runners to show'
         use :pagination
       end
       get ':id/runners' do
-        runners = filter_runners(Ci::Runner.owned_or_instance_wide(user_project.id), params[:scope])
+        runners = Ci::Runner.owned_or_instance_wide(user_project.id)
+        runners = filter_runners(runners, params[:scope])
+        runners = filter_runners(runners, params[:type], allowed_scopes: Ci::Runner::AVAILABLE_TYPES)
+        runners = filter_runners(runners, params[:status], allowed_scopes: Ci::Runner::AVAILABLE_STATUSES)
+
         present paginate(runners), with: Entities::Runner
       end

spec/features/admin/admin_runners_spec.rb

@@ -73,24 +73,72 @@ describe "Admin Runners" do
           expect(page).to have_text 'No runners found'
         end
+
+        it 'shows correct runner when status is selected and search term is entered' do
+          create(:ci_runner, description: 'runner-a-1', active: true)
+          create(:ci_runner, description: 'runner-a-2', active: false)
+          create(:ci_runner, description: 'runner-b-1', active: true)
+
+          visit admin_runners_path
+
+          input_filtered_search_keys('status:active')
+          expect(page).to have_content 'runner-a-1'
+          expect(page).to have_content 'runner-b-1'
+          expect(page).not_to have_content 'runner-a-2'
+
+          input_filtered_search_keys('status:active runner-a')
+          expect(page).to have_content 'runner-a-1'
+          expect(page).not_to have_content 'runner-b-1'
+          expect(page).not_to have_content 'runner-a-2'
+        end
       end
-      it 'shows correct runner when status is selected and search term is entered', :js do
-        create(:ci_runner, description: 'runner-a-1', active: true)
-        create(:ci_runner, description: 'runner-a-2', active: false)
-        create(:ci_runner, description: 'runner-b-1', active: true)
-        visit admin_runners_path
-        input_filtered_search_keys('status:active')
-        expect(page).to have_content 'runner-a-1'
-        expect(page).to have_content 'runner-b-1'
-        expect(page).not_to have_content 'runner-a-2'
-        input_filtered_search_keys('status:active runner-a')
-        expect(page).to have_content 'runner-a-1'
-        expect(page).not_to have_content 'runner-b-1'
-        expect(page).not_to have_content 'runner-a-2'
+      describe 'filter by type', :js do
+        it 'shows correct runner when type matches' do
+          create :ci_runner, :project, description: 'runner-project'
+          create :ci_runner, :group, description: 'runner-group'
+          visit admin_runners_path
+
+          expect(page).to have_content 'runner-project'
+          expect(page).to have_content 'runner-group'
+          input_filtered_search_keys('type:project_type')
+          expect(page).to have_content 'runner-project'
+          expect(page).not_to have_content 'runner-group'
+        end
+
+        it 'shows no runner when type does not match' do
+          create :ci_runner, :project, description: 'runner-project'
+          create :ci_runner, :group, description: 'runner-group'
+          visit admin_runners_path
+
+          input_filtered_search_keys('type:instance_type')
+
+          expect(page).not_to have_content 'runner-project'
+          expect(page).not_to have_content 'runner-group'
+
+          expect(page).to have_text 'No runners found'
+        end
+
+        it 'shows correct runner when type is selected and search term is entered' do
+          create :ci_runner, :project, description: 'runner-a-1'
+          create :ci_runner, :instance, description: 'runner-a-2'
+          create :ci_runner, :project, description: 'runner-b-1'
+
+          visit admin_runners_path
+
+          input_filtered_search_keys('type:project_type')
+          expect(page).to have_content 'runner-a-1'
+          expect(page).to have_content 'runner-b-1'
+          expect(page).not_to have_content 'runner-a-2'
+
+          input_filtered_search_keys('type:project_type runner-a')
+          expect(page).to have_content 'runner-a-1'
+          expect(page).not_to have_content 'runner-b-1'
+          expect(page).not_to have_content 'runner-a-2'
+        end
       end
       it 'sorts by last contact date', :js do

spec/finders/admin/runners_finder_spec.rb

@@ -29,6 +29,14 @@ describe Admin::RunnersFinder do
       end
     end
+    context 'filter by runner type' do
+      it 'calls the corresponding scope on Ci::Runner' do
+        expect(Ci::Runner).to receive(:project_type).and_call_original
+
+        described_class.new(params: { type_type: 'project_type' }).execute
+      end
+    end
+
     context 'sort' do
       context 'without sort param' do
         it 'sorts by created_at' do

spec/requests/api/runners_spec.rb

@@ -25,36 +25,71 @@ describe API::Runners do
   describe 'GET /runners' do
     context 'authorized user' do
-      it 'returns user available runners' do
+      it 'returns response status and headers' do
         get api('/runners', user)
-        shared = json_response.any? { |r| r['is_shared'] }
-        descriptions = json_response.map { |runner| runner['description'] }
         expect(response).to have_gitlab_http_status(200)
         expect(response).to include_pagination_headers
-        expect(json_response).to be_an Array
-        expect(json_response[0]).to have_key('ip_address')
-        expect(descriptions).to contain_exactly(
-          'Project runner', 'Two projects runner', 'Group runner'
-        )
-        expect(shared).to be_falsey
+      end
+
+      it 'returns user available runners' do
+        get api('/runners', user)
+
+        expect(json_response).to match_array [
+          a_hash_including('description' => 'Project runner'),
+          a_hash_including('description' => 'Two projects runner'),
+          a_hash_including('description' => 'Group runner')
+        ]
       end
       it 'filters runners by scope' do
-        get api('/runners?scope=active', user)
-        shared = json_response.any? { |r| r['is_shared'] }
+        create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project])
+
+        get api('/runners?scope=paused', user)
         expect(response).to have_gitlab_http_status(200)
         expect(response).to include_pagination_headers
-        expect(json_response).to be_an Array
-        expect(json_response[0]).to have_key('ip_address')
-        expect(shared).to be_falsey
+
+        expect(json_response).to match_array [
+          a_hash_including('description' => 'Inactive project runner')
+        ]
       end
       it 'avoids filtering if scope is invalid' do
         get api('/runners?scope=unknown', user)
         expect(response).to have_gitlab_http_status(400)
       end
+
+      it 'filters runners by type' do
+        get api('/runners?type=project_type', user)
+
+        expect(json_response).to match_array [
+          a_hash_including('description' => 'Project runner'),
+          a_hash_including('description' => 'Two projects runner')
+        ]
+      end
+
+      it 'does not filter by invalid type' do
+        get api('/runners?type=bogus', user)
+
+        expect(response).to have_gitlab_http_status(400)
+      end
+
+      it 'filters runners by status' do
+        create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project])
+
+        get api('/runners?status=paused', user)
+
+        expect(json_response).to match_array [
+          a_hash_including('description' => 'Inactive project runner')
+        ]
+      end
+
+      it 'does not filter by invalid status' do
+        get api('/runners?status=bogus', user)
+
+        expect(response).to have_gitlab_http_status(400)
+      end
     end
     context 'unauthorized user' do
@@ -69,51 +104,91 @@ describe API::Runners do
   describe 'GET /runners/all' do
     context 'authorized user' do
       context 'with admin privileges' do
+        it 'returns response status and headers' do
+          get api('/runners/all', admin)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response).to include_pagination_headers
+        end
+
         it 'returns all runners' do
           get api('/runners/all', admin)
-          shared = json_response.any? { |r| r['is_shared'] }
+          expect(json_response).to match_array [
+            a_hash_including('description' => 'Project runner'),
+            a_hash_including('description' => 'Two projects runner'),
+            a_hash_including('description' => 'Group runner'),
+            a_hash_including('description' => 'Shared runner')
+          ]
+        end
+
+        it 'filters runners by scope' do
+          get api('/runners/all?scope=shared', admin)
+
+          shared = json_response.all? { |r| r['is_shared'] }
           expect(response).to have_gitlab_http_status(200)
           expect(response).to include_pagination_headers
           expect(json_response).to be_an Array
           expect(json_response[0]).to have_key('ip_address')
           expect(shared).to be_truthy
         end
-      end
-      context 'without admin privileges' do
-        it 'does not return runners list' do
-          get api('/runners/all', user)
-          expect(response).to have_gitlab_http_status(403)
+        it 'filters runners by scope' do
+          get api('/runners/all?scope=specific', admin)
+          expect(response).to have_gitlab_http_status(200)
+          expect(response).to include_pagination_headers
+
+          expect(json_response).to match_array [
+            a_hash_including('description' => 'Project runner'),
+            a_hash_including('description' => 'Two projects runner'),
+            a_hash_including('description' => 'Group runner')
+          ]
         end
-      end
-      it 'filters runners by scope' do
-        get api('/runners/all?scope=shared', admin)
-        shared = json_response.all? { |r| r['is_shared'] }
-        expect(response).to have_gitlab_http_status(200)
-        expect(response).to include_pagination_headers
-        expect(json_response).to be_an Array
-        expect(json_response[0]).to have_key('ip_address')
-        expect(shared).to be_truthy
-      end
-      it 'filters runners by scope' do
-        get api('/runners/all?scope=specific', admin)
-        shared = json_response.any? { |r| r['is_shared'] }
-        expect(response).to have_gitlab_http_status(200)
-        expect(response).to include_pagination_headers
-        expect(json_response).to be_an Array
-        expect(json_response[0]).to have_key('ip_address')
-        expect(shared).to be_falsey
+        it 'avoids filtering if scope is invalid' do
+          get api('/runners/all?scope=unknown', admin)
+          expect(response).to have_gitlab_http_status(400)
+        end
+        it 'filters runners by type' do
+          get api('/runners/all?type=project_type', admin)
+          expect(json_response).to match_array [
+            a_hash_including('description' => 'Project runner'),
+            a_hash_including('description' => 'Two projects runner')
+          ]
+        end
+        it 'does not filter by invalid type' do
+          get api('/runners/all?type=bogus', admin)
+
+          expect(response).to have_gitlab_http_status(400)
+        end
+
+        it 'filters runners by status' do
+          create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project])
+
+          get api('/runners/all?status=paused', admin)
+
+          expect(json_response).to match_array [
+            a_hash_including('description' => 'Inactive project runner')
+          ]
+        end
+
+        it 'does not filter by invalid status' do
+          get api('/runners/all?status=bogus', admin)
+
+          expect(response).to have_gitlab_http_status(400)
+        end
       end
-      it 'avoids filtering if scope is invalid' do
-        get api('/runners?scope=unknown', admin)
-        expect(response).to have_gitlab_http_status(400)
+      context 'without admin privileges' do
+        it 'does not return runners list' do
+          get api('/runners/all', user)
+
+          expect(response).to have_gitlab_http_status(403)
+        end
       end
     end
@@ -577,15 +652,69 @@ describe API::Runners do
   describe 'GET /projects/:id/runners' do
     context 'authorized user with maintainer privileges' do
-      it "returns project's runners" do
+      it 'returns response status and headers' do
+        get api('/runners/all', admin)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+      end
+
+      it 'returns all runners' do
         get api("/projects/#{project.id}/runners", user)
-        shared = json_response.any? { |r| r['is_shared'] }
+        expect(json_response).to match_array [
+          a_hash_including('description' => 'Project runner'),
+          a_hash_including('description' => 'Two projects runner'),
+          a_hash_including('description' => 'Shared runner')
+        ]
+      end
+
+      it 'filters runners by scope' do
+        get api("/projects/#{project.id}/runners?scope=specific", user)
+
         expect(response).to have_gitlab_http_status(200)
         expect(response).to include_pagination_headers
-        expect(json_response).to be_an Array
-        expect(json_response[0]).to have_key('ip_address')
-        expect(shared).to be_truthy
+
+        expect(json_response).to match_array [
+          a_hash_including('description' => 'Project runner'),
+          a_hash_including('description' => 'Two projects runner')
+        ]
+      end
+
+      it 'avoids filtering if scope is invalid' do
+        get api("/projects/#{project.id}/runners?scope=unknown", user)
+        expect(response).to have_gitlab_http_status(400)
+      end
+
+      it 'filters runners by type' do
+        get api("/projects/#{project.id}/runners?type=project_type", user)
+
+        expect(json_response).to match_array [
+          a_hash_including('description' => 'Project runner'),
+          a_hash_including('description' => 'Two projects runner')
+        ]
+      end
+
+      it 'does not filter by invalid type' do
+        get api("/projects/#{project.id}/runners?type=bogus", user)
+
+        expect(response).to have_gitlab_http_status(400)
+      end
+
+      it 'filters runners by status' do
+        create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project])
+
+        get api("/projects/#{project.id}/runners?status=paused", user)
+
+        expect(json_response).to match_array [
+          a_hash_including('description' => 'Inactive project runner')
+        ]
+      end
+
+      it 'does not filter by invalid status' do
+        get api("/projects/#{project.id}/runners?status=bogus", user)
+
+        expect(response).to have_gitlab_http_status(400)
       end
     end