From 65caacc600bc0a507a313e8be45021512260906e Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Mon, 11 Jun 2018 13:36:36 +0200 Subject: [PATCH 1/8] fix and add missing api specs --- spec/requests/api/runners_spec.rb | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 3ebdb54f71f..c2fed09a5d2 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -112,7 +112,7 @@ describe API::Runners do end it 'avoids filtering if scope is invalid' do - get api('/runners?scope=unknown', admin) + get api('/runners/all?scope=unknown', admin) expect(response).to have_gitlab_http_status(400) end end @@ -587,6 +587,22 @@ describe API::Runners do expect(json_response[0]).to have_key('ip_address') expect(shared).to be_truthy end + + it 'filters runners by scope' do + get api("/projects/#{project.id}/runners?scope=specific", user) + + 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 + 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 end context 'authorized user without maintainer privileges' do From 4b6619cfd3ca127d728d7277cac3da8ed54b99b0 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Mon, 11 Jun 2018 13:37:47 +0200 Subject: [PATCH 2/8] add type param to runners api --- app/models/ci/runner.rb | 27 ++++++++++--------- doc/api/runners.md | 4 +++ lib/api/runners.rb | 22 ++++++++++++--- spec/requests/api/runners_spec.rb | 45 +++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 16 deletions(-) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index eabb41c29d7..043f03b7873 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -8,12 +8,24 @@ module Ci include RedisCacheable include ChronicDurationAttribute + 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 @@ -88,17 +100,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 diff --git a/doc/api/runners.md b/doc/api/runners.md index 66476e7db64..7763e6b2913 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -11,11 +11,13 @@ Get a list of specific runners available to the user. ``` GET /runners GET /runners?scope=active +GET /runners?type=project_type ``` | 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 | +| `type` | string | no | The type of runners to show, one of: `instance_type`, `group_type`, `project_type` | ``` curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/runners" @@ -56,11 +58,13 @@ is restricted to users with `admin` privileges. ``` GET /runners/all GET /runners/all?scope=online +GET /runners/all?type=project_type ``` | 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 | +| `type` | string | no | The type of runners to show, one of: `instance_type`, `group_type`, `project_type` | ``` curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/runners/all" diff --git a/lib/api/runners.rb b/lib/api/runners.rb index 30abd0b63e9..d24ff8641e1 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -11,10 +11,15 @@ 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' 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) + present paginate(runners), with: Entities::Runner end @@ -24,11 +29,17 @@ 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' 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) + present paginate(runners), with: Entities::Runner end @@ -116,10 +127,15 @@ 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' 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) + present paginate(runners), with: Entities::Runner end diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index c2fed09a5d2..3939500d7ca 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -55,6 +55,21 @@ describe API::Runners 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 end context 'unauthorized user' do @@ -115,6 +130,21 @@ describe API::Runners 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 end context 'unauthorized user' do @@ -603,6 +633,21 @@ describe API::Runners 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 end context 'authorized user without maintainer privileges' do From 048b4469e8c46327e3e3e9bbdfd7ef9bd27e047f Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Mon, 11 Jun 2018 14:58:40 +0200 Subject: [PATCH 3/8] cleanup runners api specs --- spec/requests/api/runners_spec.rb | 156 +++++++++++++++++------------- 1 file changed, 88 insertions(+), 68 deletions(-) diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 3939500d7ca..3a318a0334f 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -25,30 +25,34 @@ describe API::Runners do describe 'GET /runners' do context 'authorized user' do + it 'returns response status and headers' do + get api('/runners', user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + end + it 'returns user available runners' 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 + 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) + create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project]) + + get api('/runners?scope=paused', user) - 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 + + expect(json_response).to match_array [ + a_hash_including('description' => 'Inactive project runner') + ] end it 'avoids filtering if scope is invalid' do @@ -84,16 +88,67 @@ 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 + + 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 + + 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 end context 'without admin privileges' do @@ -103,48 +158,6 @@ describe API::Runners do expect(response).to have_gitlab_http_status(403) 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 - end - - 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 end context 'unauthorized user' do @@ -607,26 +620,33 @@ describe API::Runners do describe 'GET /projects/:id/runners' do context 'authorized user with maintainer privileges' do - it "returns project's runners" do - get api("/projects/#{project.id}/runners", user) + it 'returns response status and headers' do + get api('/runners/all', 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_truthy + end + + it 'returns all runners' do + get api("/projects/#{project.id}/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' => 'Shared runner') + ] end it 'filters runners by scope' do get api("/projects/#{project.id}/runners?scope=specific", user) - 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 + + 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 From 82546888e81488ba7b2a60a970a1cc31febcc1e9 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Mon, 11 Jun 2018 15:17:46 +0200 Subject: [PATCH 4/8] add status param to runners api --- doc/api/runners.md | 4 +++ lib/api/runners.rb | 9 ++++++ spec/requests/api/runners_spec.rb | 48 +++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/doc/api/runners.md b/doc/api/runners.md index 7763e6b2913..7a67b648b07 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -12,12 +12,14 @@ 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 | | `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" @@ -59,12 +61,14 @@ 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 | | `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" diff --git a/lib/api/runners.rb b/lib/api/runners.rb index d24ff8641e1..9bcdfc8cb15 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -13,12 +13,15 @@ module API 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 = 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 @@ -31,6 +34,8 @@ module API 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 @@ -39,6 +44,7 @@ module API 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 @@ -129,12 +135,15 @@ module API 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 = 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 diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 3a318a0334f..49a79d2ccf9 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -74,6 +74,22 @@ describe API::Runners do 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 @@ -149,6 +165,22 @@ describe API::Runners do 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 context 'without admin privileges' do @@ -668,6 +700,22 @@ describe API::Runners do 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 context 'authorized user without maintainer privileges' do From 641012c6383a1facf79b1c57a7163cb519b477de Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Mon, 11 Jun 2018 15:25:00 +0200 Subject: [PATCH 5/8] add missing params to runners api doc --- doc/api/runners.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/doc/api/runners.md b/doc/api/runners.md index 7a67b648b07..54568607a87 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -344,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 | 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" From dc1e0f6bd835536702948dd84cd2addf9f293ab9 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Mon, 11 Jun 2018 15:25:39 +0200 Subject: [PATCH 6/8] deprecate scope param for runners api --- doc/api/runners.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/runners.md b/doc/api/runners.md index 54568607a87..71ecb6606c1 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -17,7 +17,7 @@ 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` | @@ -66,7 +66,7 @@ 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` | @@ -352,7 +352,7 @@ GET /projects/:id/runners?status=active | 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 | 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` | From a1094e2316936ed455766feb7e3e3cdafcd29b53 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Mon, 11 Jun 2018 16:25:53 +0200 Subject: [PATCH 7/8] add type filter to admin runners page --- ...dmin_runners_filtered_search_token_keys.js | 7 ++ .../filtered_search_dropdown_manager.js | 5 ++ app/finders/admin/runners_finder.rb | 18 ++++- app/views/admin/runners/index.html.haml | 9 +++ spec/features/admin/admin_runners_spec.rb | 74 +++++++++++++++---- spec/finders/admin/runners_finder_spec.rb | 8 ++ 6 files changed, 104 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/filtered_search/admin_runners_filtered_search_token_keys.js b/app/assets/javascripts/filtered_search/admin_runners_filtered_search_token_keys.js index 1f9c3f41e52..f22f741ee0a 100644 --- a/app/assets/javascripts/filtered_search/admin_runners_filtered_search_token_keys.js +++ b/app/assets/javascripts/filtered_search/admin_runners_filtered_search_token_keys.js @@ -7,6 +7,13 @@ const tokenKeys = [{ symbol: '', icon: 'signal', tag: 'status', +}, { + key: 'type', + type: 'string', + param: 'type', + symbol: '', + icon: 'cube', + tag: 'type', }]; const AdminRunnersFilteredSearchTokenKeys = new FilteredSearchTokenKeys(tokenKeys); diff --git a/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js b/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js index a750647f8be..207616b9de2 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js +++ b/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) => { diff --git a/app/finders/admin/runners_finder.rb b/app/finders/admin/runners_finder.rb index 3c2d7ee7d76..fbb1cfc5c66 100644 --- a/app/finders/admin/runners_finder.rb +++ b/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 diff --git a/app/views/admin/runners/index.html.haml b/app/views/admin/runners/index.html.haml index 4dc076c95c5..75481da7a51 100644 --- a/app/views/admin/runners/index.html.haml +++ b/app/views/admin/runners/index.html.haml @@ -82,12 +82,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 diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index 026dea8d22c..5db232e4365 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/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) + 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 + 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' + expect(page).to have_content 'runner-project' + expect(page).to have_content 'runner-group' - 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' + 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 diff --git a/spec/finders/admin/runners_finder_spec.rb b/spec/finders/admin/runners_finder_spec.rb index 1e9793a5e0a..0b2325cc7ca 100644 --- a/spec/finders/admin/runners_finder_spec.rb +++ b/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 From 2466a51407205cc16f19c2d055ca21032675e1da Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Thu, 20 Sep 2018 14:54:35 +0200 Subject: [PATCH 8/8] add changelog --- .../unreleased/feature-runner-type-filter-for-admin-view.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/feature-runner-type-filter-for-admin-view.yml diff --git a/changelogs/unreleased/feature-runner-type-filter-for-admin-view.yml b/changelogs/unreleased/feature-runner-type-filter-for-admin-view.yml new file mode 100644 index 00000000000..e7812cd0944 --- /dev/null +++ b/changelogs/unreleased/feature-runner-type-filter-for-admin-view.yml @@ -0,0 +1,5 @@ +--- +title: Add a type filter to the admin runners view +merge_request: 19649 +author: Alexis Reigel +type: added