Merge branch 'feature/runner-config-untagged-jobs' into 'master'

Add config for CI Runner that prevents it from picking untagged jobs

Closes #3456

See merge request !4039

9 files changed, 73 insertions, 50 deletions

diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb
index 8b8a7320072..7345c91f67d 100644
--- a/app/controllers/admin/runners_controller.rb
+++ b/app/controllers/admin/runners_controller.rb
@@ -9,23 +9,18 @@ class Admin::RunnersController < Admin::ApplicationController
   end
 
   def show
-    @builds = @runner.builds.order('id DESC').first(30)
-    @projects =
-      if params[:search].present?
-        ::Project.search(params[:search])
-      else
-        Project.all
-      end
-    @projects = @projects.where.not(id: @runner.projects.select(:id)) if @runner.projects.any?
-    @projects = @projects.page(params[:page]).per(30)
+    assign_builds_and_projects
   end
 
   def update
-    @runner.update_attributes(runner_params)
-
-    respond_to do |format|
-      format.js
-      format.html { redirect_to admin_runner_path(@runner) }
+    if @runner.update_attributes(runner_params)
+      respond_to do |format|
+        format.js
+        format.html { redirect_to admin_runner_path(@runner) }
+      end
+    else
+      assign_builds_and_projects
+      render 'show'
     end
   end
 
@@ -60,4 +55,16 @@ class Admin::RunnersController < Admin::ApplicationController
   def runner_params
     params.require(:runner).permit(Ci::Runner::FORM_EDITABLE)
   end
+
+  def assign_builds_and_projects
+    @builds = runner.builds.order('id DESC').first(30)
+    @projects =
+      if params[:search].present?
+        ::Project.search(params[:search])
+      else
+        Project.all
+      end
+    @projects = @projects.where.not(id: runner.projects.select(:id)) if runner.projects.any?
+    @projects = @projects.page(params[:page]).per(30)
+  end
 end
diff --git a/app/controllers/projects/runners_controller.rb b/app/controllers/projects/runners_controller.rb
index 3a9d67aff64..0b4fa572501 100644
--- a/app/controllers/projects/runners_controller.rb
+++ b/app/controllers/projects/runners_controller.rb
@@ -20,7 +20,7 @@ class Projects::RunnersController < Projects::ApplicationController
     if @runner.update_attributes(runner_params)
       redirect_to runner_path(@runner), notice: 'Runner was successfully updated.'
     else
-      redirect_to runner_path(@runner), alert: 'Runner was not updated.'
+      render 'edit'
     end
   end
 
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 50190101f2b..ff7dd44c526 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -291,9 +291,15 @@ module Ci
     end
 
     def can_be_served?(runner)
+      return false unless has_tags? || runner.run_untagged?
+
       (tag_list - runner.tag_list).empty?
     end
 
+    def has_tags?
+      tag_list.any?
+    end
+
     def any_runners_online?
       project.any_runners? { |runner| runner.active? && runner.online? && can_be_served?(runner) }
     end
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 819064f99bb..6829dc91cb9 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -4,7 +4,7 @@ module Ci
 
     LAST_CONTACT_TIME = 5.minutes.ago
     AVAILABLE_SCOPES = %w[specific shared active paused online]
-    FORM_EDITABLE = %i[description tag_list active]
+    FORM_EDITABLE = %i[description tag_list active run_untagged]
 
     has_many :builds, class_name: 'Ci::Build'
     has_many :runner_projects, dependent: :destroy, class_name: 'Ci::RunnerProject'
@@ -26,6 +26,8 @@ module Ci
         .where("ci_runner_projects.gl_project_id = :project_id OR ci_runners.is_shared = true", project_id: project_id)
     end
 
+    validate :tag_constraints
+
     acts_as_taggable
 
     # Searches for runners matching the given query.
@@ -96,5 +98,18 @@ module Ci
     def short_sha
       token[0...8] if token
     end
+
+    def has_tags?
+      tag_list.any?
+    end
+
+    private
+
+    def tag_constraints
+      unless has_tags? || run_untagged?
+        errors.add(:tags_list,
+          'can not be empty when runner is not allowed to pick untagged jobs')
+      end
+    end
   end
 end
diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml
index 4dfb3ed05bb..c3784bf7192 100644
--- a/app/views/admin/runners/show.html.haml
+++ b/app/views/admin/runners/show.html.haml
@@ -9,8 +9,6 @@
         %span.runner-state.runner-state-specific
           Specific
 
-
-
 - if @runner.shared?
   .bs-callout.bs-callout-success
     %h4 This runner will process builds from ALL UNASSIGNED projects
diff --git a/app/views/projects/runners/_form.html.haml b/app/views/projects/runners/_form.html.haml
index 2d6c964ae94..d62f5c8f131 100644
--- a/app/views/projects/runners/_form.html.haml
+++ b/app/views/projects/runners/_form.html.haml
@@ -1,4 +1,5 @@
 = form_for runner, url: runner_form_url, html: { class: 'form-horizontal' } do |f|
+  = form_errors(runner)
   .form-group
     = label :active, "Active", class: 'control-label'
     .col-sm-10
@@ -6,6 +7,12 @@
         = f.check_box :active
         %span.light Paused runners don't accept new builds
   .form-group
+    = label :run_untagged, 'Run untagged jobs', class: 'control-label'
+    .col-sm-10
+      .checkbox
+        = f.check_box :run_untagged
+        %span.light Indicates whether this runner can pick jobs without tags
+  .form-group
     = label_tag :token, class: 'control-label' do
       Token
     .col-sm-10
diff --git a/app/views/projects/runners/_runner.html.haml b/app/views/projects/runners/_runner.html.haml
index 47ec420189d..96e2aac451f 100644
--- a/app/views/projects/runners/_runner.html.haml
+++ b/app/views/projects/runners/_runner.html.haml
@@ -5,7 +5,7 @@
       - if @runners.include?(runner)
         = link_to runner.short_sha, runner_path(runner)
         %small
-          =link_to edit_namespace_project_runner_path(@project.namespace, @project, runner) do
+          = link_to edit_namespace_project_runner_path(@project.namespace, @project, runner) do
             %i.fa.fa-edit.btn
       - else
         = runner.short_sha
diff --git a/app/views/projects/runners/edit.html.haml b/app/views/projects/runners/edit.html.haml
index 771947d7908..95706888655 100644
--- a/app/views/projects/runners/edit.html.haml
+++ b/app/views/projects/runners/edit.html.haml
@@ -1,5 +1,6 @@
 - page_title "Edit", "#{@runner.description} ##{@runner.id}", "Runners"
 
 %h4 Runner ##{@runner.id}
+
 %hr
  = render 'form', runner: @runner, runner_form_url: runner_path(@runner)
diff --git a/app/views/projects/runners/show.html.haml b/app/views/projects/runners/show.html.haml
index 5bf4c09ca25..f24e1b9144e 100644
--- a/app/views/projects/runners/show.html.haml
+++ b/app/views/projects/runners/show.html.haml
@@ -17,50 +17,39 @@
         %th Property Name
         %th Value
     %tr
-      %td
-        Tags
+      %td Active
+      %td= @runner.active? ? 'Yes' : 'No'
+    %tr
+      %td Can run untagged jobs
+      %td= @runner.run_untagged? ? 'Yes' : 'No'
+    %tr
+      %td Tags
       %td
         - @runner.tag_list.each do |tag|
           %span.label.label-primary
             = tag
     %tr
-      %td
-        Name
-      %td
-        = @runner.name
+      %td Name
+      %td= @runner.name
     %tr
-      %td
-        Version
-      %td
-        = @runner.version
+      %td Version
+      %td= @runner.version
     %tr
-      %td
-        Revision
-      %td
-        = @runner.revision
+      %td Revision
+      %td= @runner.revision
     %tr
-      %td
-        Platform
-      %td
-        = @runner.platform
+      %td Platform
+      %td= @runner.platform
     %tr
-      %td
-        Architecture
-      %td
-        = @runner.architecture
+      %td Architecture
+      %td= @runner.architecture
     %tr
-      %td
-        Description
-      %td
-        = @runner.description
+      %td Description
+      %td= @runner.description
     %tr
-      %td
-        Last contact
+      %td Last contact
       %td
         - if @runner.contacted_at
           #{time_ago_in_words(@runner.contacted_at)} ago
         - else
           Never
-
-
-