simplify runner selection

don't differentiate between the different runner types, instead we rely
on the Runner model to provide the available projects.
scheduling is now applied to all runners equally.

app/models/ci/runner.rb

@@ -94,6 +94,24 @@ module Ci
       self.token = SecureRandom.hex(15) if self.token.blank?
     end
+    def accessible_projects
+      accessible_projects =
+        if shared?
+          Project.with_shared_runners
+        elsif project?
+          projects
+        elsif group?
+          hierarchy_groups = Gitlab::GroupHierarchy.new(groups).base_and_descendants
+          Project.where(namespace_id: hierarchy_groups)
+        else
+          Project.none
+        end
+
+      accessible_projects
+        .with_builds_enabled
+        .without_deleted
+    end
+
     def assign_to(project, current_user = nil)
       self.is_shared = false if shared?
       self.save

app/services/ci/register_job_service.rb

@@ -14,14 +14,7 @@ module Ci
     end
     def execute
-      builds =
-        if runner.shared?
-          builds_for_shared_runner
-        elsif runner.group?
-          builds_for_group_runner
-        else
-          builds_for_specific_runner
-        end
+      builds = builds_for_runner
       valid = true
@@ -70,62 +63,27 @@ module Ci
     private
-    def builds_for_shared_runner
-      builds_for_scheduled_runner(
-        running_builds_for_shared_runners,
-        projects: { shared_runners_enabled: true }
-      )
-    end
-
-    def builds_for_group_runner
-      builds_for_scheduled_runner(
-        running_builds_for_group_runners,
-        projects: { group_runners_enabled: true }
-      )
-    end
-
-    def builds_for_scheduled_runner(build_join, project_where)
-      project_where = project_where.deep_merge(projects: { pending_delete: false })
-
-      # don't run projects which have not enabled group runners and builds
-      builds = new_builds
-        .joins(:project).where(project_where)
-        .joins('LEFT JOIN project_features ON ci_builds.project_id = project_features.project_id')
-        .where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0')
-
-      # Implement fair scheduling
-      # this returns builds that are ordered by number of running builds
-      # we prefer projects that don't use group runners at all
-      builds
-        .joins("LEFT JOIN (#{build_join.to_sql}) AS project_builds ON ci_builds.project_id=project_builds.project_id")
-        .order('COALESCE(project_builds.running_builds, 0) ASC', 'ci_builds.id ASC')
-    end
-
-    def builds_for_specific_runner
-      new_builds.where(project: runner.projects.without_deleted.with_builds_enabled).order('created_at ASC')
+    def builds_for_runner
+      new_builds
+        .joins("LEFT JOIN (#{running_projects.to_sql}) AS running_projects ON ci_builds.project_id=running_projects.project_id")
+        .order('COALESCE(running_projects.running_builds, 0) ASC', 'ci_builds.id ASC')
     end
-    def running_builds_for_shared_runners
-      running_builds_for_runners(Ci::Runner.shared)
-    end
-
-    def running_builds_for_group_runners
-      running_builds_for_runners(Ci::Runner.joins(:runner_groups))
+    # New builds from the accessible projects
+    def new_builds
+      filter_builds(Ci::Build.pending.unstarted)
     end
-    def running_builds_for_runners(runners)
-      Ci::Build.running.where(runner: runners)
+    # Count running builds from the accessible projects
+    def running_projects
+      filter_builds(Ci::Build.running)
         .group(:project_id).select(:project_id, 'count(*) AS running_builds')
     end
-    def new_builds
-      builds = Ci::Build.pending.unstarted
+    # Filter the builds from the accessible projects
+    def filter_builds(builds)
       builds = builds.ref_protected if runner.ref_protected?
-      builds
-    end
-
-    def shared_runner_build_limits_feature_enabled?
-      ENV['DISABLE_SHARED_RUNNER_BUILD_MINUTES_LIMIT'].to_s != 'true'
+      builds.where(project: runner.accessible_projects)
     end
     def register_failure

spec/models/ci/runner_spec.rb

@@ -776,4 +776,71 @@ describe Ci::Runner do
       expect(runner.project?).to be true
     end
   end
+
+  describe '#accessible_projects' do
+    let!(:shared_runner) { create(:ci_runner, :shared) }
+    let!(:shared_project) { create(:project, shared_runners_enabled: true) }
+
+    let!(:project_runner) { create(:ci_runner) }
+    let!(:project_project) { create(:project, runners: [project_runner], shared_runners_enabled: false) }
+
+    let!(:group_runner) { create(:ci_runner) }
+
+    let!(:parent_group) { create(:group) }
+    let!(:parent_group_project) do
+      create(:project, group: parent_group, shared_runners_enabled: false)
+    end
+
+    let!(:group) { create :group, runners: [group_runner], parent: parent_group }
+    let!(:group_project) do
+      create(:project, group: group, shared_runners_enabled: false)
+    end
+
+    let!(:nested_group_project) do
+      nested_group = create :group, parent: group
+      create(:project, group: nested_group, shared_runners_enabled: false)
+    end
+
+    it 'returns the project with a shared runner' do
+      expect(shared_runner.reload.accessible_projects).to eq [shared_project]
+    end
+
+    it 'returns the project with a project runner' do
+      expect(project_runner.reload.accessible_projects).to eq [project_project]
+    end
+
+    it 'returns the projects with a group and nested group runner' do
+      expect(group_runner.reload.accessible_projects).to eq [group_project, nested_group_project]
+    end
+
+    context 'deleted' do
+      before do
+        shared_project.update_attributes!(pending_delete: true)
+        project_project.update_attributes!(pending_delete: true)
+        group_project.update_attributes!(pending_delete: true)
+        nested_group_project.update_attributes!(pending_delete: true)
+      end
+
+      it 'returns no projects' do
+        expect(shared_runner.reload.accessible_projects).to be_empty
+        expect(project_runner.reload.accessible_projects).to be_empty
+        expect(group_runner.reload.accessible_projects).to be_empty
+      end
+    end
+
+    context 'builds disabled' do
+      before do
+        shared_project.update_attributes!(builds_enabled: false)
+        project_project.update_attributes!(builds_enabled: false)
+        group_project.update_attributes!(builds_enabled: false)
+        nested_group_project.update_attributes!(builds_enabled: false)
+      end
+
+      it 'returns no projects' do
+        expect(shared_runner.reload.accessible_projects).to be_empty
+        expect(project_runner.reload.accessible_projects).to be_empty
+        expect(group_runner.reload.accessible_projects).to be_empty
+      end
+    end
+  end
 end

spec/services/ci/register_job_service_spec.rb

@@ -179,6 +179,7 @@ module Ci
           let!(:pipeline2) { create :ci_pipeline, project: project2 }
           let!(:project3) { create :project, group_runners_enabled: true, group: group }
           let!(:pipeline3) { create :ci_pipeline, project: project3 }
+
           let!(:build1_project1) { pending_job }
           let!(:build2_project1) { create :ci_build, pipeline: pipeline }
           let!(:build3_project1) { create :ci_build, pipeline: pipeline }
@@ -186,6 +187,36 @@ module Ci
           let!(:build2_project2) { create :ci_build, pipeline: pipeline2 }
           let!(:build1_project3) { create :ci_build, pipeline: pipeline3 }
+          # these shouldn't influence the scheduling
+          let!(:unrelated_group) { create :group }
+          let!(:unrelated_project) { create :project, group_runners_enabled: true, group: unrelated_group }
+          let!(:unrelated_pipeline) { create :ci_pipeline, project: unrelated_project }
+          let!(:build1_unrelated_project) { create :ci_build, pipeline: unrelated_pipeline }
+          let!(:unrelated_group_runner) { create :ci_runner, groups: [unrelated_group] }
+
+          it 'does not consider builds from other group runners' do
+            expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 6
+            execute(group_runner)
+
+            expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 5
+            execute(group_runner)
+
+            expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 4
+            execute(group_runner)
+
+            expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 3
+            execute(group_runner)
+
+            expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 2
+            execute(group_runner)
+
+            expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 1
+            execute(group_runner)
+
+            expect(described_class.new(group_runner).send(:builds_for_runner).count).to eq 0
+            expect(execute(group_runner)).to be_nil
+          end
+
           it 'prefers projects without builds first' do
             # it gets for one build from each of the projects
             expect(execute(group_runner)).to eq(build1_project1)
@@ -198,6 +229,8 @@ module Ci
             # in the end the third build
             expect(execute(group_runner)).to eq(build3_project1)
+
+            expect(execute(group_runner)).to be_nil
           end
           it 'equalises number of running builds' do
@@ -211,6 +244,8 @@ module Ci
             expect(execute(group_runner)).to eq(build2_project2)
             expect(execute(group_runner)).to eq(build1_project3)
             expect(execute(group_runner)).to eq(build3_project1)
+
+            expect(execute(group_runner)).to be_nil
           end
         end
@@ -247,7 +282,7 @@ module Ci
           let!(:other_build) { create :ci_build, pipeline: pipeline }
           before do
-            allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner)
+            allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_runner)
               .and_return(Ci::Build.where(id: [pending_job, other_build]))
           end
@@ -259,7 +294,7 @@ module Ci
         context 'when single build is in queue' do
           before do
-            allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner)
+            allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_runner)
               .and_return(Ci::Build.where(id: pending_job))
           end
@@ -270,7 +305,7 @@ module Ci
         context 'when there is no build in queue' do
           before do
-            allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner)
+            allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_runner)
               .and_return(Ci::Build.none)
           end