Add runners API
References #4264 (closed)
Merge request reports
Activity
mentioned in issue #4264 (closed)
Basic runners management is done. I need to add per project enabling/disabling and documentation.
/cc @ayufan
@tmaczukin I made a comment, I'll review the rest tomorrow first thing.
- lib/api/runners.rb 0 → 100644
1 module API 2 # Runners API 3 class Runners < Grape::API 4 before { authenticate! } 5 6 resource :runners do 7 # Get available shared runners 8 # 9 # Example Request: 10 # GET /runners @axil: In 00ad21db You've added an deprecation note, but
lib/ci/api/runners.rb
is not deprecated! :) It's still used by runners to register/remove themselves from GitLab. Wholelib/ci/api
will be left as an runners communication channel. Am I right, @ayufan?Edited by Tomasz Maczukin@tmaczukin Yes, you are right. We can say that at least for now it's used. It's definitely deprecated to be used by consumer API, since this is intended to be used ONLY by runners.
Added 569 commits:
- 0cbf4990...22808a22 - 549 commits from branch
master
- 99417755 - Add basic runners management API
- afc7da9a - Add feature to enable/disable runner in project
- 71154113 - Add missing methods documentation; fix rubocop reported violation
- a3c5211c - Fix runners filtering in API
- 53b56c06 - Add runners API documentation [ci-skip]
- 5f6ea6ef - Add note of deprecation in old Runners CI API
- c8cba73d - Add
Example response
above each json output - 2b41b1ef - Add some fixes in runners API documentation
- bd804649 - Split
/runners
entrypoint to/runners
and/runners/all
- ee494fed - Fix runners API spec
- 9e01b8ee - Add associated project info to runner details output
- 015e1bbd - Note that
ci/api/runners.rb
is still used - 7f0e0186 - Correct PRIVATE-TOKEN to use dash instead of underscore
- eea956df - More fixes in runners doc
- bb60fa43 - Add token to runner details output in API
- a0172414 - Modify authentication check methods in runners API
- 434fa1d9 - Limit projects to user available projects if user is not an admin
- 7167bc59 - GET /runners is for specific runners only
- ed423406 - Fix runners filtering
- 37644145 - Modify runner projects selecting method in runners API
Toggle commit list- 0cbf4990...22808a22 - 549 commits from branch
377 383 expose :name 378 384 end 379 385 386 class RunnerDetails < Runner 387 expose :tag_list 388 expose :version, :revision, :platform, :architecture 389 expose :contacted_at, as: :last_contact 390 expose :token, if: lambda { |runner, options| options[:current_user].is_admin? || !runner.is_shared? } 391 expose :projects, with: Entities::RunnerProjectDetails do |runner, options| 392 if options[:current_user].is_admin? 393 runner.projects 394 else 395 options[:current_user].authorized_projects.where(id: runner.projects) @ayufan I did this like You suggested. It still generates an union of three selects:
This query looks like a monster!
But at least the code is now more clean.This is showing all projects associated with the runner and for which the user has access. I still don't see the purpose to show the user only projects where he has
master
role.On the other hand - most of steps are using index or doing hash joins. It should not be hard for PostgreSQL. I'm only worried about those sequence scans on projects and members tables. They can be big and sequence scan can consume much IO resources. But... 75% of this query (from
Hash Agregate
to bottom) is used right now in many places - this is theUser::authorized_projects
query. So maybe I'm worrying unnecessarily.
Milestone changed to 8.5
@ayufan: I think, that this is done. There is only a question about this project list in
RunnerDetails
, but the rest is finished.Lets review it, decide what to do with
RunnerDetails
and merge it :)@tmaczukin Ok, lets follow up on this tomorrow morning :)
- doc/api/runners.md 0 → 100644
239 { 240 "active": true, 241 "description": "test-2-20150125", 242 "id": 8, 243 "is_shared": false, 244 "name": null 245 }, 246 { 247 "active": true, 248 "description": "development_runner", 249 "id": 5, 250 "is_shared": true, 251 "name": null 252 } 253 ] 254 ``` - lib/api/runners.rb 0 → 100644
120 end 121 122 helpers do 123 def filter_runners(runners, scope, options = {}) 124 return runners unless scope.present? 125 126 available_scopes = ::Ci::Runner::AVAILABLE_SCOPES 127 if options[:without] 128 available_scopes = available_scopes - options[:without] 129 end 130 131 if (available_scopes & [scope]).empty? 132 render_api_error!('Scope contains invalid value', 400) 133 end 134 135 runners.send(scope) scope
is validated against limited list, so it shouldn't have invalid value. We can useif.. elsif
orcase
constructions here, but this should work just like the above. And this is moreDRY
-like. If we will want to create new or remove some scopes, we should only modify the model (andAVAILABLE_SCOPES
const in it).filter_runners
method would still work in that case.
- spec/requests/api/runners_spec.rb 0 → 100644
8 let(:user2) { create(:user) } 9 let!(:project) { create(:project, creator_id: user.id) } 10 let!(:project2) { create(:project, creator_id: user.id) } 11 let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } 12 let!(:master2) { create(:project_member, user: user, project: project2, access_level: ProjectMember::MASTER) } 13 let!(:developer) { create(:project_member, user: user2, project: project, access_level: ProjectMember::REPORTER) } 14 let!(:shared_runner) { create(:ci_shared_runner, tag_list: ['mysql', 'ruby'], active: true) } 15 let!(:specific_runner) { create(:ci_specific_runner, tag_list: ['mysql', 'ruby']) } 16 let!(:specific_runner_project) { create(:ci_runner_project, runner: specific_runner, project: project) } 17 let!(:specific_runner2) { create(:ci_specific_runner) } 18 let!(:specific_runner2_project) { create(:ci_runner_project, runner: specific_runner2, project: project2) } 19 let!(:unused_specific_runner) { create(:ci_specific_runner) } 20 let!(:two_projects_runner) { create(:ci_specific_runner) } 21 let!(:two_projects_runner_project) { create(:ci_runner_project, runner: two_projects_runner, project: project) } 22 let!(:two_projects_runner_project2) { create(:ci_runner_project, runner: two_projects_runner, project: project2) } 23 - spec/requests/api/runners_spec.rb 0 → 100644
100 end 101 end 102 end 103 104 describe 'GET /runners/:id' do 105 context 'admin user' do 106 it "should return runner's details" do 107 get api("/runners/#{specific_runner.id}", admin) 108 109 expect(response.status).to eq(200) 110 expect(json_response['description']).to eq(specific_runner.description) 111 end 112 113 it "should return shared runner's details" do 114 get api("/runners/#{shared_runner.id}", admin) 115 I see it green. Is it ready @tmaczukin? :)
@ayufan: There is opened discussion about runner projects query (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2640/diffs#cc752c66542cbeeb21ca7a57c430836b865580c9_380_395) and scope filtering with
runner.send(scope)
(https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2640/diffs#94e215001dc1fb1699bdc278b0ff146bacc25bcd_0_135). If we decide to leave it as it is, then yes, in my opinion it is ready :)@DouweM @rspeicher Could you guys review?
@rspeicher: I've implemented and/or replied on Your comments. You can check the code again.
@DouweM Could you look?
mentioned in merge request gitlab-com/www-gitlab-com!1434 (merged)
Added 276 commits:
- e98b6335...e586858e - 240 commits from branch
master
- 128be3c0 - Add basic runners management API
- d42ced44 - Add feature to enable/disable runner in project
- 8c37f0ff - Add missing methods documentation; fix rubocop reported violation
- 53f775ae - Fix runners filtering in API
- 43c8daf3 - Add runners API documentation [ci-skip]
- 1532d28c - Add note of deprecation in old Runners CI API
-
cd62c747 - Add
Example response
above each json output - b56ee397 - Add some fixes in runners API documentation
-
81ced6f5 - Split
/runners
entrypoint to/runners
and/runners/all
- 16b3368a - Fix runners API spec
- f562a477 - Add associated project info to runner details output
-
d4f1bcdd - Note that
ci/api/runners.rb
is still used - a09ae173 - Correct PRIVATE-TOKEN to use dash instead of underscore
- dc32af95 - More fixes in runners doc
- 553bac57 - Add token to runner details output in API
- b58744cd - Modify authentication check methods in runners API
- f21b15d5 - Limit projects to user available projects if user is not an admin
- 97c88966 - GET /runners is for specific runners only
- 36e7ffea - Fix runners filtering
- 24eed1c5 - Modify runner projects selecting method in runners API
- d1ac00ae - Modify and fix output of delete and update of a runner
- dc182dc5 - Add some modifications to spec/requests/api/runners_spec.rb
- b36116f9 - Move :runner_id param to POST body when enabling specific runner in project
- 05e73356 - Update docummentation - specific runner enabling
-
4ebadb77 - iChange
name
andpath
toname_with_namespace
andpath_with_namespace
in … - 2ef196de - Change interpolation to named placeholder in owned_or_shared scope
-
957c4de9 - Reorganize
let
statements in spec/requests/api/runners_spec.rb -
d38322b8 - Change
.map{...}.inject{...}
toany?{...}
for searching shared runners - a5540b38 - Modify expectations for update runner feature
- f8f492e5 - Remove unnecessary parameters
- 7ea60c85 - Replace Entities::RunnerProjectDetails with Entities::ForkedFromProject
-
e4d2f997 - Change
last_contact
tocontacted_at
-
acfe25ed - Refactorize
ci_runner
factory andlet
definitions in runners API spec -
49d5c35b - Fix old usages of
ci_runner
factory - 98236881 - Add CHANGELOG entry
- f2f6f773 - Rename Entities::ForkedFromProject to Entities::BasicProjectDetails
Toggle commit list- e98b6335...e586858e - 240 commits from branch
mentioned in commit 1817b766
@DouweM I merged the Runners API :)
mentioned in commit 778da502
Mentioned in commit maxraab/gitlab-ce@778da502