Merge branch 'account-shared-minutes-on-top-level-namespace' into 'master'

Account shared runner minutes to top-level namespace

Closes #3564

See merge request gitlab-org/gitlab-ee!3262

app/models/namespace.rb

@@ -213,6 +213,10 @@ class Namespace < ActiveRecord::Base
     parent.present?
   end
 
+  def root_ancestor
+    ancestors.reorder(nil).find_by(parent_id: nil)
+  end
+
   def subgroup?
     has_parent?
   end

app/models/project.rb

@@ -500,6 +500,14 @@ class Project < ActiveRecord::Base
       .base_and_ancestors(upto: top)
   end
 
+  def root_namespace
+    if namespace.has_parent?
+      namespace.root_ancestor
+    else
+      namespace
+    end
+  end
+
   def lfs_enabled?
     return namespace.lfs_enabled? if self[:lfs_enabled].nil?
 

app/views/admin/groups/_form.html.haml

@@ -20,7 +20,8 @@
 
   = render 'groups/group_admin_settings', f: f
 
-  = render 'namespaces/shared_runners_minutes_setting', f: f
+  - if @group.shared_runner_minutes_supported?
+    = render 'namespaces/shared_runners_minutes_setting', f: f
 
   - if @group.new_record?
     .form-group

changelogs/unreleased-ee/account-shared-minutes-on-top-level-namespace.yml

+---
+title: Account shared runner minutes to top-level namespace
+merge_request:
+author:
+type: fixed

ee/app/controllers/groups/pipeline_quota_controller.rb

 class Groups::PipelineQuotaController < Groups::ApplicationController
   before_action :authorize_admin_group!
+  before_action :validate_shared_runner_minutes_support!
 
   layout 'group_settings'
 
   def index
-    @projects = @group.projects.with_shared_runners_limit_enabled.page(params[:page])
+    @projects = all_projects.with_shared_runners_limit_enabled.page(params[:page])
+  end
+
+  private
+
+  def all_projects
+    if Feature.enabled?(:shared_runner_minutes_on_root_namespace)
+      @group.all_projects
+    else
+      @group.projects
+    end
+  end
+
+  def validate_shared_runner_minutes_support!
+    render_404 unless @group.shared_runner_minutes_supported?
   end
 end

ee/app/models/ee/namespace.rb

@@ -34,6 +34,7 @@ module EE
         to: :namespace_statistics, allow_nil: true
 
       validate :validate_plan_name
+      validate :validate_shared_runner_minutes_support
     end
 
     module ClassMethods
@@ -42,10 +43,6 @@ module EE
       end
     end
 
-    def root_ancestor
-      ancestors.reorder(nil).find_by(parent_id: nil)
-    end
-
     def move_dir
       raise NotImplementedError unless defined?(super)
 
@@ -96,13 +93,22 @@ module EE
       actual_plan&.name || FREE_PLAN
     end
 
+    def shared_runner_minutes_supported?
+      if has_parent?
+        !Feature.enabled?(:shared_runner_minutes_on_root_namespace)
+      else
+        true
+      end
+    end
+
     def actual_shared_runners_minutes_limit
       shared_runners_minutes_limit ||
         current_application_settings.shared_runners_minutes
     end
 
     def shared_runners_minutes_limit_enabled?
-      shared_runners_enabled? &&
+      shared_runner_minutes_supported? &&
+        shared_runners_enabled? &&
         actual_shared_runners_minutes_limit.nonzero?
     end
 
@@ -111,6 +117,14 @@ module EE
         shared_runners_minutes.to_i >= actual_shared_runners_minutes_limit
     end
 
+    def shared_runners_enabled?
+      if Feature.enabled?(:shared_runner_minutes_on_root_namespace)
+        all_projects.with_shared_runners.any?
+      else
+        projects.with_shared_runners.any?
+      end
+    end
+
     # These helper methods are required to not break the Namespace API.
     def plan=(plan_name)
       if plan_name.is_a?(String)
@@ -140,6 +154,14 @@ module EE
       end
     end
 
+    def validate_shared_runner_minutes_support
+      return if shared_runner_minutes_supported?
+
+      if shared_runners_minutes_limit_changed?
+        errors.add(:shared_runners_minutes_limit, 'is not supported for this namespace')
+      end
+    end
+
     def load_feature_available(feature)
       globally_available = License.feature_available?(feature)
 

ee/app/models/ee/project.rb

@@ -54,7 +54,7 @@ module EE
         to: :statistics, allow_nil: true
 
       delegate :actual_shared_runners_minutes_limit,
-        :shared_runners_minutes_used?, to: :namespace
+        :shared_runners_minutes_used?, to: :shared_runners_limit_namespace
 
       validates :repository_size_limit,
         numericality: { only_integer: true, greater_than_or_equal_to: 0, allow_nil: true }
@@ -82,6 +82,14 @@ module EE
       end
     end
 
+    def shared_runners_limit_namespace
+      if Feature.enabled?(:shared_runner_minutes_on_root_namespace)
+        root_namespace
+      else
+        namespace
+      end
+    end
+
     def mirror
       super && feature_available?(:repository_mirrors)
     end
@@ -190,11 +198,12 @@ module EE
     end
 
     def shared_runners_available?
-      super && !namespace.shared_runners_minutes_used?
+      super && !shared_runners_limit_namespace.shared_runners_minutes_used?
     end
 
     def shared_runners_minutes_limit_enabled?
-      !public? && shared_runners_enabled? && namespace.shared_runners_minutes_limit_enabled?
+      !public? && shared_runners_enabled? &&
+        shared_runners_limit_namespace.shared_runners_minutes_limit_enabled?
     end
 
     def feature_available?(feature, user = nil)

ee/app/services/ee/ci/register_job_service.rb

@@ -17,8 +17,7 @@ module EE
       end
 
       def builds_check_limit
-        ::Namespace.reorder(nil)
-          .where('namespaces.id = projects.namespace_id')
+        all_namespaces
           .joins('LEFT JOIN namespace_statistics ON namespace_statistics.namespace_id = namespaces.id')
           .where('COALESCE(namespaces.shared_runners_minutes_limit, ?, 0) = 0 OR ' \
             'COALESCE(namespace_statistics.shared_runners_seconds, 0) < COALESCE(namespaces.shared_runners_minutes_limit, ?, 0) * 60',
@@ -26,6 +25,16 @@ module EE
           .select('1')
       end
 
+      def all_namespaces
+        namespaces = ::Namespace.reorder(nil).where('namespaces.id = projects.namespace_id')
+
+        if Feature.enabled?(:shared_runner_minutes_on_root_namespace)
+          namespaces = ::Gitlab::GroupHierarchy.new(namespaces).roots
+        end
+
+        namespaces
+      end
+
       def application_shared_runners_minutes
         current_application_settings.shared_runners_minutes
       end

ee/app/services/update_build_minutes_service.rb

@@ -18,10 +18,10 @@ class UpdateBuildMinutesService < BaseService
   end
 
   def project_statistics
-    project.statistics || project.create_statistics(namespace: namespace)
+    project.statistics || project.create_statistics(namespace: project.namespace)
   end
 
   def namespace
-    project.namespace
+    project.shared_runners_limit_namespace
   end
 end

ee/app/views/namespaces/pipelines_quota/_list.haml

@@ -33,7 +33,7 @@
         %td
           .avatar-container.s20.hidden-xs
             = project_icon(project, alt: '', class: 'avatar project-avatar s20')
-          %strong= link_to project.name, project
+          %strong= link_to project.full_name, project
         %td
           = project.shared_runners_minutes
     - if projects.blank?

ee/app/views/projects/jobs/_shared_runner_limit_warning.html.haml

-- if project.namespace.shared_runners_minutes_used?
-  - quota_used = project.namespace.shared_runners_minutes
-  - quota_limit = project.namespace.actual_shared_runners_minutes_limit
+- if project.shared_runners_limit_namespace.shared_runners_minutes_used?
+  - quota_used = project.shared_runners_limit_namespace.shared_runners_minutes
+  - quota_limit = project.shared_runners_limit_namespace.actual_shared_runners_minutes_limit
   .bs-callout.bs-callout-warning
     %p
       You have used all your shared Runners pipeline minutes.

ee/app/views/shared/_shared_runners_minutes_limit.html.haml

 - project = local_assigns.fetch(:project, nil)
-- namespace = local_assigns.fetch(:namespace, project && project.namespace)
+- namespace = local_assigns.fetch(:namespace, project && project.shared_runners_limit_namespace)
 - scope = (project || namespace).full_path
 - has_limit = (project || namespace).shared_runners_minutes_limit_enabled?
 - can_see_status = project.nil? || can?(current_user, :create_pipeline, project)

lib/gitlab/group_hierarchy.rb

@@ -33,6 +33,10 @@ module Gitlab
       base_and_ancestors(upto: upto).where.not(id: ancestors_base.select(:id))
     end
 
+    def roots
+      base_and_ancestors.where(namespaces: { parent_id: nil })
+    end
+
     # Returns a relation that includes the ancestors_base set of groups
     # and all their ancestors (recursively).
     #

spec/features/groups/pipeline_quota_spec.rb → spec/ee/spec/features/groups/pipeline_quota_spec.rb

@@ -95,8 +95,56 @@ feature 'Groups > Pipeline Quota' do
       end
 
       page.within('.pipeline-project-metrics') do
-        expect(page).to have_content(project.name)
-        expect(page).not_to have_content(other_project.name)
+        expect(page).to have_content(project.full_name)
+        expect(page).not_to have_content(other_project.full_name)
+      end
+    end
+  end
+
+  context 'with shared_runner_minutes_on_root_namespace disabled' do
+    before do
+      stub_feature_flags(shared_runner_minutes_on_root_namespace: false)
+    end
+
+    context 'when accessing group with subgroups' do
+      let(:group) { create(:group, :with_used_build_minutes_limit) }
+      let!(:subgroup) { create(:group, parent: group) }
+      let!(:subproject) { create(:project, namespace: subgroup, shared_runners_enabled: true) }
+
+      it 'does not show project of subgroup' do
+        visit_pipeline_quota_page
+
+        expect(page).to have_content(project.full_name)
+        expect(page).not_to have_content(subproject.full_name)
+      end
+    end
+  end
+
+  context 'with shared_runner_minutes_on_root_namespace enabled', :nested_groups do
+    before do
+      stub_feature_flags(shared_runner_minutes_on_root_namespace: true)
+    end
+
+    context 'when accessing subgroup' do
+      let(:root_ancestor) { create(:group) }
+      let(:group) { create(:group, parent: root_ancestor) }
+
+      it 'does not show subproject' do
+        visit_pipeline_quota_page
+
+        expect(page).to have_http_status(:not_found)
+      end
+    end
+
+    context 'when accesing root group' do
+      let!(:subgroup) { create(:group, parent: group) }
+      let!(:subproject) { create(:project, namespace: subgroup, shared_runners_enabled: true) }
+
+      it 'does show projects of subgroup' do
+        visit_pipeline_quota_page
+
+        expect(page).to have_content(project.full_name)
+        expect(page).to have_content(subproject.full_name)
       end
     end
   end

spec/ee/spec/models/ee/namespace_spec.rb

@@ -63,6 +63,34 @@ describe Namespace do
         end
       end
     end
+
+    describe '#validate_shared_runner_minutes_support' do
+      before do
+        stub_feature_flags(shared_runner_minutes_on_root_namespace: true)
+      end
+
+      context 'when changing :shared_runners_minutes_limit' do
+        before do
+          namespace.shared_runners_minutes_limit = 100
+        end
+
+        context 'when group is subgroup' do
+          set(:root_ancestor) { create(:group) }
+          let(:namespace) { create(:namespace, parent: root_ancestor) }
+
+          it 'is invalid' do
+            expect(namespace).not_to be_valid
+            expect(namespace.errors[:shared_runners_minutes_limit]).to include('is not supported for this namespace')
+          end
+        end
+
+        context 'when group is root' do
+          it 'is valid' do
+            expect(namespace).to be_valid
+          end
+        end
+      end
+    end
   end
 
   describe '#move_dir' do
@@ -295,6 +323,42 @@ describe Namespace do
     end
   end
 
+  describe '#shared_runner_minutes_supported?' do
+    subject { namespace.shared_runner_minutes_supported? }
+
+    context 'when is subgroup' do
+      before do
+        namespace.parent = build(:group)
+      end
+
+      context 'when shared_runner_minutes_on_root_namespace is disabled' do
+        before do
+          stub_feature_flags(shared_runner_minutes_on_root_namespace: false)
+        end
+
+        it 'returns true' do
+          is_expected.to eq(true)
+        end
+      end
+
+      context 'when shared_runner_minutes_on_root_namespace is enabled', :nested_groups do
+        before do
+          stub_feature_flags(shared_runner_minutes_on_root_namespace: true)
+        end
+
+        it 'returns false' do
+          is_expected.to eq(false)
+        end
+      end
+    end
+
+    context 'when is root' do
+      it 'returns true' do
+        is_expected.to eq(true)
+      end
+    end
+  end
+
   describe '#shared_runners_minutes_limit_enabled?' do
     subject { namespace.shared_runners_minutes_limit_enabled? }
 
@@ -315,6 +379,15 @@ describe Namespace do
         end
 
         it { is_expected.to be_truthy }
+
+        context 'when is subgroup', :nested_groups do
+          before do
+            stub_feature_flags(shared_runner_minutes_on_root_namespace: true)
+            namespace.parent = build(:group)
+          end
+
+          it { is_expected.to be_falsey }
+        end
       end
     end
 
@@ -323,6 +396,49 @@ describe Namespace do
     end
   end
 
+  describe '#shared_runners_enabled?' do
+    subject { namespace.shared_runners_enabled? }
+
+    context 'subgroup with shared runners enabled project' do
+      let(:subgroup) { create(:group, parent: namespace) }
+      let!(:subproject) { create(:project, namespace: subgroup, shared_runners_enabled: true) }
+
+      context 'when shared_runner_minutes_on_root_namespace is disabled' do
+        before do
+          stub_feature_flags(shared_runner_minutes_on_root_namespace: false)
+        end
+
+        it "returns false" do
+          is_expected.to eq(false)
+        end
+      end
+
+      context 'when shared_runner_minutes_on_root_namespace is enabled', :nested_groups do
+        before do
+          stub_feature_flags(shared_runner_minutes_on_root_namespace: true)
+        end
+
+        it "returns true" do
+          is_expected.to eq(true)
+        end
+      end
+    end
+
+    context 'group with shared runners enabled project' do
+      let!(:project) { create(:project, namespace: namespace, shared_runners_enabled: true) }
+
+      it "returns true" do
+        is_expected.to eq(true)
+      end
+    end
+
+    context 'group without projects' do
+      it "returns false" do
+        is_expected.to eq(false)
+      end
+    end
+  end
+
   describe '#shared_runners_minutes_used?' do
     subject { namespace.shared_runners_minutes_used? }
 
@@ -363,19 +479,6 @@ describe Namespace do
     end
   end
 
-  describe '#root_ancestor' do
-    it 'returns the top most ancestor', :nested_groups do
-      root_group = create(:group)
-      nested_group = create(:group, parent: root_group)
-      deep_nested_group = create(:group, parent: nested_group)
-      very_deep_nested_group = create(:group, parent: deep_nested_group)
-
-      expect(nested_group.root_ancestor).to eq(root_group)
-      expect(deep_nested_group.root_ancestor).to eq(root_group)
-      expect(very_deep_nested_group.root_ancestor).to eq(root_group)
-    end
-  end
-
   describe '#actual_plan' do
     context 'when namespace has a plan associated' do
       before do

spec/ee/spec/models/ee/project_spec.rb

@@ -8,9 +8,9 @@ describe Project do
     it { is_expected.to delegate_method(:shared_runners_seconds).to(:statistics) }
     it { is_expected.to delegate_method(:shared_runners_seconds_last_reset).to(:statistics) }
 
-    it { is_expected.to delegate_method(:actual_shared_runners_minutes_limit).to(:namespace) }
-    it { is_expected.to delegate_method(:shared_runners_minutes_limit_enabled?).to(:namespace) }
-    it { is_expected.to delegate_method(:shared_runners_minutes_used?).to(:namespace) }
+    it { is_expected.to delegate_method(:actual_shared_runners_minutes_limit).to(:shared_runners_limit_namespace) }
+    it { is_expected.to delegate_method(:shared_runners_minutes_limit_enabled?).to(:shared_runners_limit_namespace) }
+    it { is_expected.to delegate_method(:shared_runners_minutes_used?).to(:shared_runners_limit_namespace) }
 
     it { is_expected.to have_one(:mirror_data).class_name('ProjectMirrorData') }
     it { is_expected.to have_many(:path_locks) }
@@ -532,6 +532,34 @@ describe Project do
     end
   end
 
+  describe '#shared_runners_limit_namespace' do
+    set(:root_ancestor) { create(:group) }
+    set(:group) { create(:group, parent: root_ancestor) }
+    let(:project) { create(:project, namespace: group) }
+
+    subject { project.shared_runners_limit_namespace }
+
+    context 'when shared_runner_minutes_on_root_namespace is disabled' do
+      before do
+        stub_feature_flags(shared_runner_minutes_on_root_namespace: false)
+      end
+
+      it 'returns parent namespace' do
+        is_expected.to eq(group)
+      end
+    end
+
+    context 'when shared_runner_minutes_on_root_namespace is enabled', :nested_groups do
+      before do
+        stub_feature_flags(shared_runner_minutes_on_root_namespace: true)
+      end
+
+      it 'returns root namespace' do
+        is_expected.to eq(root_ancestor)
+      end
+    end
+  end
+
   describe '#shared_runners_minutes_limit_enabled?' do
     let(:project) { create(:project) }
 

spec/ee/spec/services/ee/ci/register_job_service_spec.rb

@@ -62,6 +62,66 @@ module Ci
             end
           end
         end
+
+        context 'when group is subgroup' do
+          let!(:root_ancestor) { create(:group) }
+          let!(:group) { create(:group, parent: root_ancestor) }
+          let!(:project) { create :project, shared_runners_enabled: true, group: group }
+          let(:build) { execute(shared_runner) }
+
+          context 'when shared_runner_minutes_on_root_namespace is disabled' do
+            before do
+              stub_feature_flags(shared_runner_minutes_on_root_namespace: false)
+            end
+
+            it "does return a build" do
+              expect(build).not_to be_nil
+            end
+
+            context 'when we are over limit on subnamespace' do
+              before do
+                group.create_namespace_statistics(
+                  shared_runners_seconds: 6001)
+              end
+
+              it "does not return a build" do
+                expect(build).to be_nil
+              end
+            end
+          end
+
+          context 'when shared_runner_minutes_on_root_namespace is enabled', :nested_groups do
+            before do
+              stub_feature_flags(shared_runner_minutes_on_root_namespace: true)
+            end
+
+            it "does return a build" do
+              expect(build).not_to be_nil
+            end
+
+            context 'when we are over limit on subnamespace' do
+              before do
+                group.create_namespace_statistics(
+                  shared_runners_seconds: 6001)
+              end
+
+              it "limit is ignored and build is returned" do
+                expect(build).not_to be_nil
+              end
+            end
+
+            context 'when we are over limit on root namespace' do
+              before do
+                root_ancestor.create_namespace_statistics(
+                  shared_runners_seconds: 6001)
+              end
+
+              it "does not return a build" do
+                expect(build).to be_nil
+              end
+            end
+          end
+        end
       end
 
       def execute(runner)

spec/ee/spec/views/admin/groups/_form.html.haml_spec.rb

+require 'spec_helper'
+
+describe 'admin/groups/_form' do
+  set(:admin) { create(:admin) }
+
+  before do
+    assign(:group, group)
+    allow(view).to receive(:can?) { true }
+    allow(view).to receive(:current_user) { admin }
+    allow(view).to receive(:visibility_level) { group.visibility_level }
+  end
+
+  describe 'when :shared_runner_minutes_on_root_namespace is disabled' do
+    before do
+      stub_feature_flags(shared_runner_minutes_on_root_namespace: false)
+    end
+
+    context 'when sub group is used' do
+      let(:root_ancestor) { create(:group) }
+      let(:group) { build(:group, parent: root_ancestor) }
+
+      it 'renders shared_runners_minutes_setting' do
+        render
+
+        expect(rendered).to render_template('namespaces/_shared_runners_minutes_setting')
+      end
+    end
+  end
+
+  describe 'when :shared_runner_minutes_on_root_namespace is enabled', :nested_groups do
+    before do
+      stub_feature_flags(shared_runner_minutes_on_root_namespace: true)
+    end
+
+    context 'when sub group is used' do
+      let(:root_ancestor) { create(:group) }
+      let(:group) { build(:group, parent: root_ancestor) }
+
+      it 'does not render shared_runners_minutes_setting' do
+        render
+
+        expect(rendered).not_to render_template('namespaces/_shared_runners_minutes_setting')
+      end
+    end
+
+    context 'when root group is used' do
+      let(:group) { build(:group) }
+
+      it 'does not render shared_runners_minutes_setting' do
+        render
+
+        expect(rendered).to render_template('namespaces/_shared_runners_minutes_setting')
+      end
+    end
+  end
+end

spec/lib/gitlab/group_hierarchy_spec.rb

@@ -83,6 +83,20 @@ describe Gitlab::GroupHierarchy, :postgresql do
     end
   end
 
+  describe '#root' do
+    it 'includes only the roots' do
+      relation = described_class.new(Group.where(id: child2)).roots
+
+      expect(relation).to contain_exactly(parent)
+    end
+
+    it 'when quering parent it includes parent' do
+      relation = described_class.new(Group.where(id: parent)).roots
+
+      expect(relation).to contain_exactly(parent)
+    end
+  end
+
   describe '#all_groups' do
     let(:relation) do
       described_class.new(Group.where(id: child1.id)).all_groups

spec/models/namespace_spec.rb

@@ -675,4 +675,17 @@ describe Namespace do
       expect(other_namespace.find_fork_of(project)).to eq(other_fork)
     end
   end
+
+  describe '#root_ancestor' do
+    it 'returns the top most ancestor', :nested_groups do
+      root_group = create(:group)
+      nested_group = create(:group, parent: root_group)
+      deep_nested_group = create(:group, parent: nested_group)
+      very_deep_nested_group = create(:group, parent: deep_nested_group)
+
+      expect(nested_group.root_ancestor).to eq(root_group)
+      expect(deep_nested_group.root_ancestor).to eq(root_group)
+      expect(very_deep_nested_group.root_ancestor).to eq(root_group)
+    end
+  end
 end

spec/models/project_spec.rb

@@ -3474,4 +3474,27 @@ describe Project do
       expect(project.wiki_repository_exists?).to eq(false)
     end
   end
+
+  describe '#root_namespace' do
+    let(:project) { build(:project, namespace: parent) }
+
+    subject { project.root_namespace }
+
+    context 'when namespace has parent group' do
+      let(:root_ancestor) { create(:group) }
+      let(:parent) { build(:group, parent: root_ancestor) }
+
+      it 'returns root ancestor' do
+        is_expected.to eq(root_ancestor)
+      end
+    end
+
+    context 'when namespace is root ancestor' do
+      let(:parent) { build(:group) }
+
+      it 'returns current namespace' do
+        is_expected.to eq(parent)
+      end
+    end
+  end
 end

spec/services/update_build_minutes_service_spec.rb

@@ -42,6 +42,40 @@ describe UpdateBuildMinutesService do
             .to eq(100 + build.duration.to_i)
         end
       end
+
+      context 'when namespace is subgroup' do
+        let(:root_ancestor) { create(:group, shared_runners_minutes_limit: 100) }
+
+        context 'when shared_runner_minutes_on_root_namespace is disabled' do
+          let(:namespace) { create(:namespace, parent: root_ancestor, shared_runners_minutes_limit: 100) }
+
+          before do
+            stub_feature_flags(shared_runner_minutes_on_root_namespace: false)
+          end
+
+          it 'creates a statistics in current namespace' do
+            subject
+
+            expect(namespace.namespace_statistics.reload.shared_runners_seconds)
+              .to eq(build.duration.to_i)
+          end
+        end
+
+        context 'when shared_runner_minutes_on_root_namespace is enabled', :nested_groups do
+          let(:namespace) { create(:namespace, parent: root_ancestor) }
+
+          before do
+            stub_feature_flags(shared_runner_minutes_on_root_namespace: true)
+          end
+
+          it 'creates a statistics in root namespace' do
+            subject
+
+            expect(root_ancestor.namespace_statistics.reload.shared_runners_seconds)
+              .to eq(build.duration.to_i)
+          end
+        end
+      end
     end
 
     context 'for specific runner' do