From d11e1cf4543f6e1c4500332aa84aadb1326e9f1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 3 Apr 2019 15:09:26 -0500 Subject: [PATCH] Add support to purchase extra CI minutes * extra_shared_runners_minutes_limit column was added to namespaces. * extra minutes should roll over to the next month in case customer has not used all their extra minutes --- app/services/groups/base_service.rb | 6 ++ app/services/groups/create_service.rb | 2 + app/services/groups/update_service.rb | 1 + db/schema.rb | 4 +- doc/api/groups.md | 3 + doc/api/users.md | 3 + ee/app/helpers/ee/namespaces_helper.rb | 35 ++++++-- ee/app/models/ee/namespace.rb | 21 ++++- ee/app/models/ee/user.rb | 1 + ee/app/models/namespace_statistics.rb | 15 +++- ee/app/services/ee/ci/register_job_service.rb | 3 +- ee/app/services/ee/groups/create_service.rb | 8 ++ ee/app/services/ee/groups/update_service.rb | 8 ++ .../groups/pipeline_quota/index.html.haml | 14 ++- ...tra_shared_runners_minutes_quota.html.haml | 12 +++ .../namespaces/pipelines_quota/_list.haml | 2 + .../clear_shared_runners_minutes_worker.rb | 13 +++ ...4-add-on-runner-minutes-for-gitlab-com.yml | 5 ++ ...red_runners_minutes_limit_to_namespaces.rb | 9 ++ ...ers_minutes_limit_columns_on_namespaces.rb | 20 +++++ ee/lib/ee/api/entities.rb | 3 + ee/lib/ee/api/groups.rb | 6 -- ee/lib/ee/api/namespaces.rb | 3 +- ee/spec/helpers/ee/namespaces_helper_spec.rb | 48 +++++++++++ ee/spec/models/namespace_spec.rb | 86 +++++++++++++++++++ ee/spec/models/namespace_statistics_spec.rb | 37 ++++++++ ee/spec/requests/api/namespaces_spec.rb | 4 +- .../services/ci/register_job_service_spec.rb | 39 +++++++++ .../services/groups/create_service_spec.rb | 26 ++++++ .../services/groups/update_service_spec.rb | 24 ++++++ ...lear_shared_runners_minutes_worker_spec.rb | 44 ++++++++++ lib/api/groups.rb | 1 + lib/api/users.rb | 1 + locale/gitlab.pot | 3 + spec/requests/api/groups_spec.rb | 9 -- 35 files changed, 483 insertions(+), 36 deletions(-) create mode 100644 ee/app/views/namespaces/pipelines_quota/_extra_shared_runners_minutes_quota.html.haml create mode 100644 ee/changelogs/unreleased/3314-add-on-runner-minutes-for-gitlab-com.yml create mode 100644 ee/db/migrate/20190304020812_add_extra_shared_runners_minutes_limit_to_namespaces.rb create mode 100644 ee/db/migrate/20190328210840_add_indexes_to_shared_runners_minutes_limit_and_extra_shared_runners_minutes_limit_columns_on_namespaces.rb diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb index 8c8acce5ca5a3f..019cd047ae9a58 100644 --- a/app/services/groups/base_service.rb +++ b/app/services/groups/base_service.rb @@ -7,5 +7,11 @@ class BaseService < ::BaseService def initialize(group, user, params = {}) @group, @current_user, @params = group, user, params.dup end + + private + + def remove_unallowed_params + # overridden in EE + end end end diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index cdc205fafcf42b..0bcdb9c192a289 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -8,6 +8,8 @@ def initialize(user, params = {}) end def execute + remove_unallowed_params + @group = Group.new(params) after_build_hook(@group, params) diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 2e849887296b12..bac711cc3be873 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -6,6 +6,7 @@ class UpdateService < Groups::BaseService def execute reject_parent_id! + remove_unallowed_params return false unless valid_visibility_level_change?(group, params[:visibility_level]) diff --git a/db/schema.rb b/db/schema.rb index 1274a4200fd9e6..e9192852a8e9c5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190325165127) do +ActiveRecord::Schema.define(version: 20190328210840) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1972,6 +1972,7 @@ t.string "runners_token_encrypted" t.integer "custom_project_templates_group_id" t.boolean "auto_devops_enabled" + t.integer "extra_shared_runners_minutes_limit" t.index ["created_at"], name: "index_namespaces_on_created_at", using: :btree t.index ["custom_project_templates_group_id", "type"], name: "index_namespaces_on_custom_project_templates_group_id_and_type", where: "(custom_project_templates_group_id IS NOT NULL)", using: :btree t.index ["file_template_project_id"], name: "index_namespaces_on_file_template_project_id", using: :btree @@ -1987,6 +1988,7 @@ t.index ["require_two_factor_authentication"], name: "index_namespaces_on_require_two_factor_authentication", using: :btree t.index ["runners_token"], name: "index_namespaces_on_runners_token", unique: true, using: :btree t.index ["runners_token_encrypted"], name: "index_namespaces_on_runners_token_encrypted", unique: true, using: :btree + t.index ["shared_runners_minutes_limit", "extra_shared_runners_minutes_limit"], name: "index_namespaces_on_shared_and_extra_runners_minutes_limit", using: :btree t.index ["trial_ends_on"], name: "index_namespaces_on_trial_ends_on", where: "(trial_ends_on IS NOT NULL)", using: :btree t.index ["type"], name: "index_namespaces_on_type", using: :btree end diff --git a/doc/api/groups.md b/doc/api/groups.md index 3be0937eacab3b..f596950dc80bc8 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -240,6 +240,7 @@ Example response: "file_template_project_id": 1, "parent_id": null, "shared_runners_minutes_limit": 133, + "extra_shared_runners_minutes_limit": 133, "projects": [ { "id": 7, @@ -420,6 +421,7 @@ Parameters: | `request_access_enabled` | boolean | no | Allow users to request member access. | | `parent_id` | integer | no | The parent group id for creating nested group. | | `shared_runners_minutes_limit` | integer | no | (admin-only) Pipeline minutes quota for this group. | +| `extra_shared_runners_minutes_limit` | integer | no | (admin-only) Extra pipeline minutes quota for this group. | ## Transfer project to group @@ -457,6 +459,7 @@ PUT /groups/:id | `request_access_enabled` | boolean | no | Allow users to request member access. | | `file_template_project_id` | integer | no | **(Premium)** The ID of a project to load custom file templates from | | `shared_runners_minutes_limit` | integer | no | (admin-only) Pipeline minutes quota for this group | +| `extra_shared_runners_minutes_limit` | integer | no | (admin-only) Extra pipeline minutes quota for this group | ```bash curl --request PUT --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/groups/5?name=Experimental" diff --git a/doc/api/users.md b/doc/api/users.md index ddf7f4d4879eb3..a47e79425ff734 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -261,6 +261,7 @@ Parameters: "external": false, "private_profile": false, "shared_runners_minutes_limit": 133 + "extra_shared_runners_minutes_limit": 133 } ``` @@ -303,6 +304,7 @@ Parameters: - `avatar` (optional) - Image file for user's avatar - `private_profile` (optional) - User's profile is private - true or false - `shared_runners_minutes_limit` (optional) - Pipeline minutes quota for this user +- `extra_shared_runners_minutes_limit` (optional) - Extra pipeline minutes quota for this user ## User modification @@ -334,6 +336,7 @@ Parameters: - `skip_reconfirmation` (optional) - Skip reconfirmation - true or false (default) - `external` (optional) - Flags the user as external - true or false(default) - `shared_runners_minutes_limit` (optional) - Pipeline minutes quota for this user +- `extra_shared_runners_minutes_limit` (optional) - Extra pipeline minutes quota for this user - `avatar` (optional) - Image file for user's avatar - `private_profile` (optional) - User's profile is private - true or false diff --git a/ee/app/helpers/ee/namespaces_helper.rb b/ee/app/helpers/ee/namespaces_helper.rb index ef1b1e390c2327..0a1efdc88f9ae8 100644 --- a/ee/app/helpers/ee/namespaces_helper.rb +++ b/ee/app/helpers/ee/namespaces_helper.rb @@ -2,11 +2,21 @@ module EE module NamespacesHelper + def namespace_extra_shared_runner_limits_quota(namespace) + limit = namespace.extra_shared_runners_minutes_limit.to_i + used = namespace.extra_shared_runners_minutes.to_i + status = namespace.extra_shared_runners_minutes_used? ? 'over_quota' : 'under_quota' + + content_tag(:span, class: "shared_runners_limit_#{status}") do + "#{used} / #{limit}" + end + end + def namespace_shared_runner_limits_quota(namespace) - used = namespace.shared_runners_minutes.to_i + used = namespace.shared_runners_minutes(include_extra: false).to_i if namespace.shared_runners_minutes_limit_enabled? - limit = namespace.actual_shared_runners_minutes_limit + limit = namespace.actual_shared_runners_minutes_limit(include_extra: false) status = namespace.shared_runners_minutes_used? ? 'over_quota' : 'under_quota' else limit = 'Unlimited' @@ -18,15 +28,21 @@ def namespace_shared_runner_limits_quota(namespace) end end + def namespace_extra_shared_runner_limits_percent_used(namespace) + limit = namespace.extra_shared_runners_minutes_limit.to_i + + return 0 if limit.zero? + + 100 * namespace.extra_shared_runners_minutes.to_i / limit + end + def namespace_shared_runner_limits_percent_used(namespace) return 0 unless namespace.shared_runners_minutes_limit_enabled? - 100 * namespace.shared_runners_minutes.to_i / namespace.actual_shared_runners_minutes_limit + 100 * namespace.shared_runners_minutes(include_extra: false).to_i / namespace.actual_shared_runners_minutes_limit(include_extra: false) end - def namespace_shared_runner_limits_progress_bar(namespace) - percent = [namespace_shared_runner_limits_percent_used(namespace), 100].min - + def namespace_shared_runner_usage_progress_bar(percent) status = if percent == 100 'danger' @@ -46,6 +62,13 @@ def namespace_shared_runner_limits_progress_bar(namespace) end end + def namespace_shared_runner_limits_progress_bar(namespace, extra: false) + used = extra ? namespace_extra_shared_runner_limits_percent_used(namespace) : namespace_shared_runner_limits_percent_used(namespace) + percent = [used, 100].min + + namespace_shared_runner_usage_progress_bar(percent) + end + # rubocop: disable CodeReuse/ActiveRecord def namespaces_options_with_developer_maintainer_access(options = {}) selected = options.delete(:selected) || :current_user diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb index c1cabaa4c460aa..3a197c9c1b49b6 100644 --- a/ee/app/models/ee/namespace.rb +++ b/ee/app/models/ee/namespace.rb @@ -36,9 +36,11 @@ module Namespace accepts_nested_attributes_for :gitlab_subscription scope :with_plan, -> { where.not(plan_id: nil) } + scope :with_shared_runners_minutes_limit, -> { where("namespaces.shared_runners_minutes_limit > 0") } + scope :with_extra_shared_runners_minutes_limit, -> { where("namespaces.extra_shared_runners_minutes_limit > 0") } delegate :shared_runners_minutes, :shared_runners_seconds, :shared_runners_seconds_last_reset, - to: :namespace_statistics, allow_nil: true + :extra_shared_runners_minutes, to: :namespace_statistics, allow_nil: true # Opportunistically clear the +file_template_project_id+ if invalid before_validation :clear_file_template_project_id @@ -151,9 +153,14 @@ def shared_runner_minutes_supported? end end - def actual_shared_runners_minutes_limit - shared_runners_minutes_limit || - ::Gitlab::CurrentSettings.shared_runners_minutes + def actual_shared_runners_minutes_limit(include_extra: true) + extra_minutes = include_extra ? extra_shared_runners_minutes_limit.to_i : 0 + + if shared_runners_minutes_limit + shared_runners_minutes_limit + extra_minutes + else + ::Gitlab::CurrentSettings.shared_runners_minutes + extra_minutes + end end def shared_runners_minutes_limit_enabled? @@ -167,6 +174,12 @@ def shared_runners_minutes_used? shared_runners_minutes.to_i >= actual_shared_runners_minutes_limit end + def extra_shared_runners_minutes_used? + shared_runners_minutes_limit_enabled? && + extra_shared_runners_minutes_limit && + extra_shared_runners_minutes.to_i >= extra_shared_runners_minutes_limit + end + def shared_runners_enabled? if ::Feature.enabled?(:shared_runner_minutes_on_root_namespace) all_projects.with_shared_runners.any? diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index 4210f9501eefa7..2f9a9388e28b86 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -26,6 +26,7 @@ module User validate :cannot_be_admin_and_auditor delegate :shared_runners_minutes_limit, :shared_runners_minutes_limit=, + :extra_shared_runners_minutes_limit, :extra_shared_runners_minutes_limit=, to: :namespace has_many :reviews, foreign_key: :author_id, inverse_of: :author diff --git a/ee/app/models/namespace_statistics.rb b/ee/app/models/namespace_statistics.rb index 78b8960323854d..d4873e4de9618d 100644 --- a/ee/app/models/namespace_statistics.rb +++ b/ee/app/models/namespace_statistics.rb @@ -5,7 +5,18 @@ class NamespaceStatistics < ApplicationRecord validates :namespace, presence: true - def shared_runners_minutes - shared_runners_seconds.to_i / 60 + def shared_runners_minutes(include_extra: true) + minutes = shared_runners_seconds.to_i / 60 + + include_extra ? minutes : minutes - extra_shared_runners_minutes + end + + def extra_shared_runners_minutes + limit = namespace.shared_runners_minutes_limit.to_i + extra_limit = namespace.extra_shared_runners_minutes_limit.to_i + + return 0 if extra_limit.zero? || shared_runners_minutes <= limit + + shared_runners_minutes - limit end end diff --git a/ee/app/services/ee/ci/register_job_service.rb b/ee/app/services/ee/ci/register_job_service.rb index ad619f28c484ab..7bab8d315a39ff 100644 --- a/ee/app/services/ee/ci/register_job_service.rb +++ b/ee/app/services/ee/ci/register_job_service.rb @@ -41,7 +41,8 @@ def builds_check_limit 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', + 'COALESCE(namespace_statistics.shared_runners_seconds, 0) < ' \ + 'COALESCE((namespaces.shared_runners_minutes_limit + COALESCE(namespaces.extra_shared_runners_minutes_limit, 0)), ?, 0) * 60', application_shared_runners_minutes, application_shared_runners_minutes) .select('1') end diff --git a/ee/app/services/ee/groups/create_service.rb b/ee/app/services/ee/groups/create_service.rb index 7d49d6f28fca5d..9194df913209cf 100644 --- a/ee/app/services/ee/groups/create_service.rb +++ b/ee/app/services/ee/groups/create_service.rb @@ -19,6 +19,14 @@ def after_build_hook(group, params) group.repository_size_limit = ::Gitlab::Utils.try_megabytes_to_bytes(limit) if limit end + override :remove_unallowed_params + def remove_unallowed_params + unless current_user&.admin? + params.delete(:shared_runners_minutes_limit) + params.delete(:extra_shared_runners_minutes_limit) + end + end + def log_audit_event ::AuditEventService.new( current_user, diff --git a/ee/app/services/ee/groups/update_service.rb b/ee/app/services/ee/groups/update_service.rb index 381dac7480df07..d91efa9cbab7bd 100644 --- a/ee/app/services/ee/groups/update_service.rb +++ b/ee/app/services/ee/groups/update_service.rb @@ -24,6 +24,14 @@ def before_assignment_hook(group, params) group.repository_size_limit = ::Gitlab::Utils.try_megabytes_to_bytes(limit) if limit end + override :remove_unallowed_params + def remove_unallowed_params + unless current_user&.admin? + params.delete(:shared_runners_minutes_limit) + params.delete(:extra_shared_runners_minutes_limit) + end + end + def changes_file_template_project_id? return false unless params.key?(:file_template_project_id) diff --git a/ee/app/views/groups/pipeline_quota/index.html.haml b/ee/app/views/groups/pipeline_quota/index.html.haml index c18a4ba0261e03..0de858e72bf354 100644 --- a/ee/app/views/groups/pipeline_quota/index.html.haml +++ b/ee/app/views/groups/pipeline_quota/index.html.haml @@ -3,10 +3,16 @@ %h3.page-title Group pipelines quota = link_to icon('question-circle'), help_page_path("user/admin_area/settings/continuous_integration", anchor: "shared-runners-build-minutes-quota"), target: '_blank' -%p.light - Monthly pipeline minutes usage across shared Runners for the - %strong= @group.name - group + +.row + .col-sm-6 + Monthly pipeline minutes usage across shared Runners for the + %strong= @group.name + group + + .col-sm-6 + %p.text-right + = link_to 'Buy additional minutes', 'https://customers.gitlab.com/subscriptions', target: '_blank', class: 'btn btn-inverted btn-success right text-right' = render "namespaces/pipelines_quota/list", locals: { namespace: @group, projects: @projects } diff --git a/ee/app/views/namespaces/pipelines_quota/_extra_shared_runners_minutes_quota.html.haml b/ee/app/views/namespaces/pipelines_quota/_extra_shared_runners_minutes_quota.html.haml new file mode 100644 index 00000000000000..b47d8e59cf022c --- /dev/null +++ b/ee/app/views/namespaces/pipelines_quota/_extra_shared_runners_minutes_quota.html.haml @@ -0,0 +1,12 @@ +- return unless Gitlab.com? && namespace.shared_runners_minutes_limit_enabled? + +.row + .col-sm-6 + %strong + = _("Aditional minutes") + %div + = namespace_extra_shared_runner_limits_quota(namespace) + minutes + .col-sm-6.right + #{namespace_extra_shared_runner_limits_percent_used(namespace)}% used + = namespace_shared_runner_limits_progress_bar(namespace, extra: true) diff --git a/ee/app/views/namespaces/pipelines_quota/_list.haml b/ee/app/views/namespaces/pipelines_quota/_list.haml index 1c9a43b71e0e5b..659452dccde82f 100644 --- a/ee/app/views/namespaces/pipelines_quota/_list.haml +++ b/ee/app/views/namespaces/pipelines_quota/_list.haml @@ -21,6 +21,8 @@ Unlimited = namespace_shared_runner_limits_progress_bar(namespace) + = render 'namespaces/pipelines_quota/extra_shared_runners_minutes_quota', namespace: namespace + %table.table.pipeline-project-metrics %thead %tr diff --git a/ee/app/workers/clear_shared_runners_minutes_worker.rb b/ee/app/workers/clear_shared_runners_minutes_worker.rb index 4a1a73a3e9aefb..cfc6a4533d307a 100644 --- a/ee/app/workers/clear_shared_runners_minutes_worker.rb +++ b/ee/app/workers/clear_shared_runners_minutes_worker.rb @@ -10,6 +10,15 @@ class ClearSharedRunnersMinutesWorker def perform return unless try_obtain_lease + if Gitlab::Database.postgresql? + # Using UPDATE with a joined table is not supported in MySql + Namespace.with_shared_runners_minutes_limit + .with_extra_shared_runners_minutes_limit + .where('namespace_statistics.namespace_id = namespaces.id') + .where('namespace_statistics.shared_runners_seconds > (namespaces.shared_runners_minutes_limit * 60)') + .update_all("extra_shared_runners_minutes_limit = #{extra_minutes_left_sql} FROM namespace_statistics") + end + NamespaceStatistics.where.not(shared_runners_seconds: 0) .update_all( shared_runners_seconds: 0, @@ -24,6 +33,10 @@ def perform private + def extra_minutes_left_sql + "GREATEST((namespaces.shared_runners_minutes_limit + namespaces.extra_shared_runners_minutes_limit) - ROUND(namespace_statistics.shared_runners_seconds / 60.0), 0)" + end + def try_obtain_lease Gitlab::ExclusiveLease.new('gitlab_clear_shared_runners_minutes_worker', timeout: LEASE_TIMEOUT).try_obtain diff --git a/ee/changelogs/unreleased/3314-add-on-runner-minutes-for-gitlab-com.yml b/ee/changelogs/unreleased/3314-add-on-runner-minutes-for-gitlab-com.yml new file mode 100644 index 00000000000000..9a0910b4ae8a8c --- /dev/null +++ b/ee/changelogs/unreleased/3314-add-on-runner-minutes-for-gitlab-com.yml @@ -0,0 +1,5 @@ +--- +title: Add ability to purchase extra CI minutes +merge_request: 9815 +author: +type: added diff --git a/ee/db/migrate/20190304020812_add_extra_shared_runners_minutes_limit_to_namespaces.rb b/ee/db/migrate/20190304020812_add_extra_shared_runners_minutes_limit_to_namespaces.rb new file mode 100644 index 00000000000000..c411461f9f6fa0 --- /dev/null +++ b/ee/db/migrate/20190304020812_add_extra_shared_runners_minutes_limit_to_namespaces.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddExtraSharedRunnersMinutesLimitToNamespaces < ActiveRecord::Migration[5.0] + DOWNTIME = false + + def change + add_column :namespaces, :extra_shared_runners_minutes_limit, :integer + end +end diff --git a/ee/db/migrate/20190328210840_add_indexes_to_shared_runners_minutes_limit_and_extra_shared_runners_minutes_limit_columns_on_namespaces.rb b/ee/db/migrate/20190328210840_add_indexes_to_shared_runners_minutes_limit_and_extra_shared_runners_minutes_limit_columns_on_namespaces.rb new file mode 100644 index 00000000000000..070282c01dfece --- /dev/null +++ b/ee/db/migrate/20190328210840_add_indexes_to_shared_runners_minutes_limit_and_extra_shared_runners_minutes_limit_columns_on_namespaces.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddIndexesToSharedRunnersMinutesLimitAndExtraSharedRunnersMinutesLimitColumnsOnNamespaces < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + DOWNTIME = false + + def up + add_concurrent_index :namespaces, [:shared_runners_minutes_limit, :extra_shared_runners_minutes_limit], name: 'index_namespaces_on_shared_and_extra_runners_minutes_limit' + end + + def down + remove_concurrent_index :namespaces, [:shared_runners_minutes_limit, :extra_shared_runners_minutes_limit] + end +end diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb index c275204c2303f7..d1bcf3d23ab5ca 100644 --- a/ee/lib/ee/api/entities.rb +++ b/ee/lib/ee/api/entities.rb @@ -26,6 +26,7 @@ module UserPublic prepended do expose :shared_runners_minutes_limit + expose :extra_shared_runners_minutes_limit end end @@ -66,6 +67,7 @@ module GroupDetail prepended do expose :shared_runners_minutes_limit + expose :extra_shared_runners_minutes_limit end end @@ -107,6 +109,7 @@ module Namespace prepended do expose :shared_runners_minutes_limit, if: ->(_, options) { options[:current_user]&.admin? } + expose :extra_shared_runners_minutes_limit, if: ->(_, options) { options[:current_user]&.admin? } expose :billable_members_count do |namespace, options| namespace.billable_members_count(options[:requested_hosted_plan]) end diff --git a/ee/lib/ee/api/groups.rb b/ee/lib/ee/api/groups.rb index 1ab650027b5282..dc7fc7cc1f252e 100644 --- a/ee/lib/ee/api/groups.rb +++ b/ee/lib/ee/api/groups.rb @@ -40,12 +40,6 @@ def create_group override :update_group def update_group(group) - if params[:shared_runners_minutes_limit].present? && - group.shared_runners_minutes_limit.to_i != - params[:shared_runners_minutes_limit].to_i - authenticated_as_admin! - end - params.delete(:file_template_project_id) unless group.feature_available?(:custom_file_templates_for_namespace) diff --git a/ee/lib/ee/api/namespaces.rb b/ee/lib/ee/api/namespaces.rb index 315df4fee8a6cf..d861c728ad4be7 100644 --- a/ee/lib/ee/api/namespaces.rb +++ b/ee/lib/ee/api/namespaces.rb @@ -38,6 +38,7 @@ def custom_namespace_present_options params do optional :plan, type: String, desc: "Namespace or Group plan" optional :shared_runners_minutes_limit, type: Integer, desc: "Pipeline minutes quota for this namespace" + optional :extra_shared_runners_minutes_limit, type: Integer, desc: "Extra pipeline minutes for this namespace" optional :trial_ends_on, type: Date, desc: "Trial expiration date" end put ':id' do @@ -47,7 +48,7 @@ def custom_namespace_present_options break not_found!('Namespace') unless namespace - if namespace.update(declared_params) + if namespace.update(declared_params(include_missing: false)) present namespace, with: ::API::Entities::Namespace, current_user: current_user else render_validation_error!(namespace) diff --git a/ee/spec/helpers/ee/namespaces_helper_spec.rb b/ee/spec/helpers/ee/namespaces_helper_spec.rb index 9e94165d6d6fb1..3efb39ff17b98f 100644 --- a/ee/spec/helpers/ee/namespaces_helper_spec.rb +++ b/ee/spec/helpers/ee/namespaces_helper_spec.rb @@ -79,4 +79,52 @@ end end end + + describe '#namespace_shared_runner_limits_quota' do + context "when it's unlimited" do + before do + allow(user_group).to receive(:shared_runners_minutes_limit_enabled?).and_return(false) + end + + it 'returns Unlimited for the limit section' do + expect(helper.namespace_shared_runner_limits_quota(user_group)).to match(%r{0 / Unlimited}) + end + + it 'returns the proper value for the used section' do + allow(user_group).to receive(:shared_runners_minutes).and_return(100) + + expect(helper.namespace_shared_runner_limits_quota(user_group)).to match(%r{100 / Unlimited}) + end + end + + context "when it's limited" do + before do + allow(user_group).to receive(:shared_runners_minutes_limit_enabled?).and_return(true) + allow(user_group).to receive(:shared_runners_minutes).and_return(100) + + user_group.update!(shared_runners_minutes_limit: 500) + end + + it 'returns the proper values for used and limit sections' do + expect(helper.namespace_shared_runner_limits_quota(user_group)).to match(%r{100 / 500}) + end + end + end + + describe '#namespace_extra_shared_runner_limits_quota' do + context 'when extra minutes are assigned' do + it 'returns the proper values for used and limit sections' do + allow(user_group).to receive(:extra_shared_runners_minutes).and_return(50) + user_group.update!(extra_shared_runners_minutes_limit: 100) + + expect(helper.namespace_extra_shared_runner_limits_quota(user_group)).to match(%r{50 / 100}) + end + end + + context 'when extra minutes are not assigned' do + it 'returns the proper values for used and limit sections' do + expect(helper.namespace_extra_shared_runner_limits_quota(user_group)).to match(%r{0 / 0}) + end + end + end end diff --git a/ee/spec/models/namespace_spec.rb b/ee/spec/models/namespace_spec.rb index a129da0377d3b2..5c5959dabbe7fd 100644 --- a/ee/spec/models/namespace_spec.rb +++ b/ee/spec/models/namespace_spec.rb @@ -13,6 +13,7 @@ it { is_expected.to have_one(:gitlab_subscription).dependent(:destroy) } it { is_expected.to belong_to(:plan) } + it { is_expected.to delegate_method(:extra_shared_runners_minutes).to(:namespace_statistics) } it { is_expected.to delegate_method(:shared_runners_minutes).to(:namespace_statistics) } it { is_expected.to delegate_method(:shared_runners_seconds).to(:namespace_statistics) } it { is_expected.to delegate_method(:shared_runners_seconds_last_reset).to(:namespace_statistics) } @@ -379,6 +380,20 @@ is_expected.to eq(500) end end + + context 'when extra minutes limit is set' do + before do + namespace.update_attribute(:extra_shared_runners_minutes_limit, 100) + end + + it 'returns the extra minutes by default' do + is_expected.to eq(1100) + end + + it 'can exclude the extra minutes if required' do + expect(namespace.actual_shared_runners_minutes_limit(include_extra: false)).to eq(1000) + end + end end end @@ -498,6 +513,77 @@ end end + describe '#extra_shared_runners_minutes_used?' do + subject { namespace.extra_shared_runners_minutes_used? } + + context 'with project' do + let!(:project) do + create(:project, namespace: namespace, shared_runners_enabled: true) + end + + context 'shared_runners_minutes_limit is not enabled' do + before do + allow(namespace).to receive(:shared_runners_minutes_limit_enabled?).and_return(false) + end + + it { is_expected.to be_falsey } + end + + context 'shared_runners_minutes_limit is enabled' do + context 'when limit is defined' do + before do + namespace.update_attribute(:extra_shared_runners_minutes_limit, 100) + end + + context "when usage is below the quota" do + before do + allow(namespace).to receive(:extra_shared_runners_minutes).and_return(50) + end + + it { is_expected.to be_falsey } + end + + context "when usage is above the quota" do + before do + allow(namespace).to receive(:extra_shared_runners_minutes).and_return(101) + end + + it { is_expected.to be_truthy } + end + + context 'and main limit is unlimited' do + before do + namespace.update_attribute(:shared_runners_minutes_limit, 0) + end + + context "and it's above the quota" do + it { is_expected.to be_falsey } + end + end + end + + context 'without limit' do + before do + namespace.update_attribute(:shared_runners_minutes_limit, 100) + namespace.update_attribute(:extra_shared_runners_minutes_limit, nil) + end + + context 'when main usage is above the quota' do + before do + allow(namespace).to receive(:shared_runners_minutes).and_return(101) + end + + it { is_expected.to be_falsey } + end + end + end + end + + context 'without project' do + it { is_expected.to be_falsey } + end + end + describe '#shared_runners_minutes_used?' do subject { namespace.shared_runners_minutes_used? } diff --git a/ee/spec/models/namespace_statistics_spec.rb b/ee/spec/models/namespace_statistics_spec.rb index 68bf41621558c9..e69fc80da62779 100644 --- a/ee/spec/models/namespace_statistics_spec.rb +++ b/ee/spec/models/namespace_statistics_spec.rb @@ -10,4 +10,41 @@ it { expect(namespace_statistics.shared_runners_minutes).to eq(2) } end + + describe '#extra_shared_runners_minutes' do + subject { namespace_statistics.extra_shared_runners_minutes } + + let(:namespace) { create(:namespace, shared_runners_minutes_limit: 100) } + let(:namespace_statistics) { create(:namespace_statistics, namespace: namespace) } + + context 'when limit is defined' do + before do + namespace.update_attribute(:extra_shared_runners_minutes_limit, 50) + end + + context 'when usage is above the main quota' do + before do + namespace_statistics.update_attribute(:shared_runners_seconds, 101 * 60) + end + + it { is_expected.to eq(1) } + end + + context 'when usage is below the main quota' do + before do + namespace_statistics.update_attribute(:shared_runners_seconds, 99 * 60) + end + + it { is_expected.to eq(0) } + end + end + + context 'without limit' do + before do + namespace.update_attribute(:extra_shared_runners_minutes_limit, nil) + end + + it { is_expected.to eq(0) } + end + end end diff --git a/ee/spec/requests/api/namespaces_spec.rb b/ee/spec/requests/api/namespaces_spec.rb index a655bf4852b4d7..b89d6af0342d04 100644 --- a/ee/spec/requests/api/namespaces_spec.rb +++ b/ee/spec/requests/api/namespaces_spec.rb @@ -20,11 +20,11 @@ expect(group_kind_json_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path', 'parent_id', 'members_count_with_descendants', 'plan', 'shared_runners_minutes_limit', - 'billable_members_count') + 'extra_shared_runners_minutes_limit', 'billable_members_count') expect(user_kind_json_response.keys).to contain_exactly('id', 'kind', 'name', 'path', 'full_path', 'parent_id', 'plan', 'shared_runners_minutes_limit', - 'billable_members_count') + 'extra_shared_runners_minutes_limit', 'billable_members_count') end end diff --git a/ee/spec/services/ci/register_job_service_spec.rb b/ee/spec/services/ci/register_job_service_spec.rb index bc423bf72a1cf5..2acafcbd0a9d01 100644 --- a/ee/spec/services/ci/register_job_service_spec.rb +++ b/ee/spec/services/ci/register_job_service_spec.rb @@ -147,6 +147,45 @@ end end + context 'for project with shared runners when limit is set only on namespace' do + let(:build) { execute(shared_runner) } + let(:runners_seconds_used) { 0 } + + before do + project.update(shared_runners_enabled: true) + project.namespace.update(shared_runners_minutes_limit: 100) + project.namespace.create_namespace_statistics(shared_runners_seconds: runners_seconds_used) + end + + context 'when we are under the limit' do + let(:runners_seconds_used) { 5000 } + + it "does return a build" do + expect(build).not_to be_nil + end + end + + context 'when we are over the limit' do + let(:runners_seconds_used) { 6001 } + + it "does not return a build" do + expect(build).to be_nil + end + end + + context 'when namespace has extra minutes' do + let(:runners_seconds_used) { 6001 } + + before do + project.namespace.update(extra_shared_runners_minutes_limit: 5) + end + + it "does return a build" do + expect(build).not_to be_nil + end + end + end + def execute(runner) described_class.new(runner).execute.build end diff --git a/ee/spec/services/groups/create_service_spec.rb b/ee/spec/services/groups/create_service_spec.rb index fb38313a734e13..f8ff4a8e07210e 100644 --- a/ee/spec/services/groups/create_service_spec.rb +++ b/ee/spec/services/groups/create_service_spec.rb @@ -57,6 +57,32 @@ end end + context 'updating protected params' do + let(:attrs) do + group_params.merge(shared_runners_minutes_limit: 1000, extra_shared_runners_minutes_limit: 100) + end + + context 'as an admin' do + let(:user) { create(:admin) } + + it 'updates the attributes' do + group = create_group(user, attrs) + + expect(group.shared_runners_minutes_limit).to eq(1000) + expect(group.extra_shared_runners_minutes_limit).to eq(100) + end + end + + context 'as a regular user' do + it 'ignores the attributes' do + group = create_group(user, attrs) + + expect(group.shared_runners_minutes_limit).to be_nil + expect(group.extra_shared_runners_minutes_limit).to be_nil + end + end + end + def create_group(user, opts) described_class.new(user, opts).execute end diff --git a/ee/spec/services/groups/update_service_spec.rb b/ee/spec/services/groups/update_service_spec.rb index 53e56da5779942..71b45808f1c67e 100644 --- a/ee/spec/services/groups/update_service_spec.rb +++ b/ee/spec/services/groups/update_service_spec.rb @@ -154,6 +154,30 @@ def update_file_template_project_id(id) end end + context 'updating protected params' do + let(:attrs) { { shared_runners_minutes_limit: 1000, extra_shared_runners_minutes_limit: 100 } } + + context 'as an admin' do + let(:user) { create(:admin) } + + it 'updates the attributes' do + update_group(group, user, attrs) + + expect(group.shared_runners_minutes_limit).to eq(1000) + expect(group.extra_shared_runners_minutes_limit).to eq(100) + end + end + + context 'as a regular user' do + it 'ignores the attributes' do + update_group(group, user, attrs) + + expect(group.shared_runners_minutes_limit).to be_nil + expect(group.extra_shared_runners_minutes_limit).to be_nil + end + end + end + def update_group(group, user, opts) Groups::UpdateService.new(group, user, opts).execute end diff --git a/ee/spec/workers/clear_shared_runners_minutes_worker_spec.rb b/ee/spec/workers/clear_shared_runners_minutes_worker_spec.rb index 5db460697df0d3..635553d7cb5d1e 100644 --- a/ee/spec/workers/clear_shared_runners_minutes_worker_spec.rb +++ b/ee/spec/workers/clear_shared_runners_minutes_worker_spec.rb @@ -47,5 +47,49 @@ expect(statistics.reload.shared_runners_seconds_last_reset).to be_like_time(Time.now) end end + + context 'when namespace has extra shared runner minutes', :postgresql do + let!(:namespace) do + create(:namespace, shared_runners_minutes_limit: 100, extra_shared_runners_minutes_limit: 10 ) + end + + let!(:statistics) do + create(:namespace_statistics, namespace: namespace, shared_runners_seconds: minutes_used * 60) + end + + let(:minutes_used) { 0 } + + context 'when consumption is below the default quota' do + let(:minutes_used) { 50 } + + it 'does not modify the extra minutes quota' do + subject + + expect(namespace.reload.extra_shared_runners_minutes_limit).to eq(10) + end + end + + context 'when consumption is above the default quota' do + context 'when all extra minutes are used' do + let(:minutes_used) { 115 } + + it 'sets extra minutes to 0' do + subject + + expect(namespace.reload.extra_shared_runners_minutes_limit).to eq(0) + end + end + + context 'when some extra minutes are used' do + let(:minutes_used) { 105 } + + it 'it discounts the extra minutes used' do + subject + + expect(namespace.reload.extra_shared_runners_minutes_limit).to eq(5) + end + end + end + end end end diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 8feb81aca1887e..a23bed2bd6628a 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -26,6 +26,7 @@ class Groups < Grape::API optional :ldap_cn, type: String, desc: 'LDAP Common Name' optional :ldap_access, type: Integer, desc: 'A valid access level' optional :shared_runners_minutes_limit, type: Integer, desc: '(admin-only) Pipeline minutes quota for this group' + optional :extra_shared_runners_minutes_limit, type: Integer, desc: '(admin-only) Extra pipeline minutes quota for this group' all_or_none_of :ldap_cn, :ldap_access end end diff --git a/lib/api/users.rb b/lib/api/users.rb index 776329622e2424..2f23e33bd4af25 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -54,6 +54,7 @@ def reorder_users(users) if Gitlab.ee? optional :shared_runners_minutes_limit, type: Integer, desc: 'Pipeline minutes quota for this user' + optional :extra_shared_runners_minutes_limit, type: Integer, desc: '(admin-only) Extra pipeline minutes quota for this user' end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8c842c17d31e58..b065e4db0e9022 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -617,6 +617,9 @@ msgstr "" msgid "Additional text" msgstr "" +msgid "Aditional minutes" +msgstr "" + msgid "Admin Area" msgstr "" diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index df120b91ea1a7e..1650bfbd67f2db 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -467,15 +467,6 @@ def response_project_ids(json_response, key) expect(response).to have_gitlab_http_status(404) end - # EE - it 'returns 403 for updating shared_runners_minutes_limit' do - expect do - put api("/groups/#{group1.id}", user1), params: { shared_runners_minutes_limit: 133 } - end.not_to change { group1.shared_runners_minutes_limit } - - expect(response).to have_gitlab_http_status(403) - end - it 'returns 200 if shared_runners_minutes_limit is not changing' do group1.update(shared_runners_minutes_limit: 133) -- GitLab