Fixed builds for specific runners #488

ayufan · 2014-10-30T10:05:28Z

Don't run projects which are assigned to specific runner by shared runner.

dzaporozhets · 2014-10-30T10:53:43Z

Old one is expected behaviour. So this looks like a feature/improvement. It makes sense for me if your project requires super-specific ENV so you cant allow other runners to serve your build. So feature looks reasonable.
But If we want to change behaviour in this way we need next:

CHANGELOG entry
Place in UI where I can see that my project wont be served by shared runners
Runner UI should say that specific runners lock project from shared runners
tests for your change

ayufan · 2014-10-30T10:58:40Z

It seems reasonable.

ayufan · 2014-10-30T11:09:13Z

I thought that specific runners are there to limit where projects can be run. I would expect project to run only on selected runner. It maybe useful in cases where server can deploy application to production servers.

Maybe we can even extend it to support specific runners per-branch basis. For example master is always executed on the selected one. I think that I saw somewhere PR for that.

dzaporozhets · 2014-10-30T11:12:40Z

We introduced specific runners so people can add own laptop/server/vps as runner for own projects only. Ex. I used this to extend build capacity for my fork in one instance. But advantages of specific runners are there to limit where projects can be run are bigger. Still this can be a breaking change for someone. So it should be properly described in UI and has CHANGELOG item

houndci-bot · 2014-11-07T14:50:23Z

lib/api/builds.rb

@@ -15,8 +15,11 @@ class Builds < Grape::API

        ActiveRecord::Base.transaction do
          build = if current_runner.shared?
-                    Build.first_pending
+                    # don't run projects which are assigned to specific runners
+                    projects = RunnerProject.distinct(:project).pluck(:project_id)


Line is too long. [82/80]

ayufan · 2014-11-10T13:27:10Z

@randx it's updated. What do you think?

dzaporozhets · 2014-11-14T16:19:02Z

app/views/projects/_specific_runner.html.haml

+    This project uses
+    %span.label.label-info specific
+    runner.
+    Builds will be run only by #{@project.runners.each.map { |r| "#" + r.id.to_s }.join(", ")}.


move logic like @project.runners.each.map { |r| "#" + r.id.to_s }.join(", ") to helper.

dzaporozhets · 2014-11-14T16:29:16Z

@ayufan I left some feedback. Also fix houndci comment please

ayufan · 2014-11-17T09:54:59Z

@randx I addressed all the comments.

dzaporozhets · 2014-11-17T10:56:43Z

@ayufan thank you

ayufan · 2015-01-02T11:57:02Z

@randx @vsizov any update?

dblessing · 2015-01-02T12:24:24Z

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

@@ -23,10 +23,10 @@
    %ul
      %li
        %span.label.label-success shared
-        \- run any builds
+        \- run builds from all non-assigned projects


Non-assigned -> unassigned

dblessing · 2015-01-02T12:29:42Z

@ayufan This looks nice. I made a few comments on grammar.

ayufan · 2015-01-02T14:03:56Z

@dblessing Thanks. Fixed.

dblessing · 2015-01-02T14:57:58Z

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

    %p If you want runner to build only specific projects you just need to enable them in table below
 - else
  .bs-callout.bs-callout-info
    %h4 This runner will process build only from ASSIGNED projects
-    %p If you want runner to build all projects you just need to disable all assignee projects
+    %p If you want runner to build non assigned projects you just need to disable all assignee projects


Whoops, missed this non-assigned.

ayufan · 2015-01-02T15:02:37Z

Fixed once again :)

dblessing · 2015-01-02T20:16:52Z

Cool, great work. Thanks.

dzaporozhets · 2015-01-03T12:08:41Z

@vsizov can you review this please?

vsizov · 2015-01-08T00:57:28Z

app/helpers/projects_helper.rb

@@ -22,4 +22,16 @@ def html_badge_code(project, ref)
    url = status_project_url(project, ref: ref, format: 'png')
    "<a href='#{project_url(project, ref: ref)}'><img src='#{url}' /></a>"
  end
+
+  def runners_for_project(project)
+    project.runners.each.map { |r| "#" + r.id.to_s }.join(", ")


let's remove each from here

ayufan · 2015-01-08T11:40:57Z

@vsizov updated

ayufan · 2015-01-12T18:33:07Z

@vsizov ?

…nner.

Fixed builds for specific runners

houndci-bot reviewed Nov 7, 2014
View reviewed changes

dzaporozhets reviewed Nov 14, 2014
View reviewed changes

dblessing reviewed Jan 2, 2015
View reviewed changes

dblessing added the Ready to merge label Jan 2, 2015

dzaporozhets assigned vsizov Jan 3, 2015

vsizov reviewed Jan 8, 2015
View reviewed changes

Don't run projects which are assigned to specific runner by shared ru…

c6e8f2c

…nner.

vsizov added a commit that referenced this pull request Jan 12, 2015

Merge pull request #488 from ayufan/topic/shared-runners

8aaab14

Fixed builds for specific runners

vsizov merged commit 8aaab14 into gitlabhq:master Jan 12, 2015

ayufan deleted the topic/shared-runners branch January 19, 2015 10:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed builds for specific runners #488

Fixed builds for specific runners #488

ayufan commented Oct 30, 2014

dzaporozhets commented Oct 30, 2014

ayufan commented Oct 30, 2014

ayufan commented Oct 30, 2014

dzaporozhets commented Oct 30, 2014

houndci-bot Nov 7, 2014

ayufan commented Nov 10, 2014

dzaporozhets Nov 14, 2014

dzaporozhets commented Nov 14, 2014

ayufan commented Nov 17, 2014

dzaporozhets commented Nov 17, 2014

ayufan commented Jan 2, 2015

dblessing Jan 2, 2015

dblessing commented Jan 2, 2015

ayufan commented Jan 2, 2015

dblessing Jan 2, 2015

ayufan commented Jan 2, 2015

dblessing commented Jan 2, 2015

dzaporozhets commented Jan 3, 2015

vsizov Jan 8, 2015

ayufan commented Jan 8, 2015

ayufan commented Jan 12, 2015

Fixed builds for specific runners #488

Fixed builds for specific runners #488

Conversation

ayufan commented Oct 30, 2014

dzaporozhets commented Oct 30, 2014

ayufan commented Oct 30, 2014

ayufan commented Oct 30, 2014

dzaporozhets commented Oct 30, 2014

houndci-bot Nov 7, 2014

Choose a reason for hiding this comment

ayufan commented Nov 10, 2014

dzaporozhets Nov 14, 2014

Choose a reason for hiding this comment

dzaporozhets commented Nov 14, 2014

ayufan commented Nov 17, 2014

dzaporozhets commented Nov 17, 2014

ayufan commented Jan 2, 2015

dblessing Jan 2, 2015

Choose a reason for hiding this comment

dblessing commented Jan 2, 2015

ayufan commented Jan 2, 2015

dblessing Jan 2, 2015

Choose a reason for hiding this comment

ayufan commented Jan 2, 2015

dblessing commented Jan 2, 2015

dzaporozhets commented Jan 3, 2015

vsizov Jan 8, 2015

Choose a reason for hiding this comment

ayufan commented Jan 8, 2015

ayufan commented Jan 12, 2015