diff options
68 files changed, 1021 insertions, 189 deletions
diff --git a/.eslintrc.yml b/.eslintrc.yml index 6b9a1ce62c0..2799358be88 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -88,7 +88,7 @@ rules: - pattern: test_fixtures/** group: internal alphabetize: - order: asc + order: ignore overrides: - files: - '**/spec/**/*' diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index b96a2607552..4962268c8e9 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -1,5 +1,4 @@ /* global $ */ -/* eslint-disable import/order */ import jQuery from 'jquery'; import Cookies from 'js-cookie'; diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 1a0cec3c935..d1c9e772f50 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -82,7 +82,8 @@ module Ci # Merge requests for which the current pipeline is running against # the merge request's latest commit. has_many :merge_requests_as_head_pipeline, foreign_key: "head_pipeline_id", class_name: 'MergeRequest' - + has_many :package_build_infos, class_name: 'Packages::BuildInfo', dependent: :nullify, inverse_of: :pipeline # rubocop:disable Cop/ActiveRecordDependent + has_many :package_file_build_infos, class_name: 'Packages::PackageFileBuildInfo', dependent: :nullify, inverse_of: :pipeline # rubocop:disable Cop/ActiveRecordDependent has_many :pending_builds, -> { pending }, foreign_key: :commit_id, class_name: 'Ci::Build', inverse_of: :pipeline has_many :failed_builds, -> { latest.failed }, foreign_key: :commit_id, class_name: 'Ci::Build', inverse_of: :pipeline has_many :retryable_builds, -> { latest.failed_or_canceled.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build', inverse_of: :pipeline diff --git a/app/models/ci/resource_group.rb b/app/models/ci/resource_group.rb index 8a7456041e6..946fce3473f 100644 --- a/app/models/ci/resource_group.rb +++ b/app/models/ci/resource_group.rb @@ -14,6 +14,12 @@ module Ci before_create :ensure_resource + enum process_mode: { + unordered: 0, + oldest_first: 1, + newest_first: 2 + } + ## # NOTE: This is concurrency-safe method that the subquery in the `UPDATE` # works as explicit locking. @@ -25,8 +31,34 @@ module Ci resources.retained_by(processable).update_all(build_id: nil) > 0 end + def upcoming_processables + if unordered? || Feature.disabled?(:ci_resource_group_process_modes, project, default_enabled: :yaml) + processables.waiting_for_resource + elsif oldest_first? + processables.waiting_for_resource_or_upcoming + .order(Arel.sql("commit_id ASC, #{sort_by_job_status}")) + elsif newest_first? + processables.waiting_for_resource_or_upcoming + .order(Arel.sql("commit_id DESC, #{sort_by_job_status}")) + else + Ci::Processable.none + end + end + private + # In order to avoid deadlock, we do NOT specify the job execution order in the same pipeline. + # The system processes wherever ready to transition to `pending` status from `waiting_for_resource`. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/202186 for more information. + def sort_by_job_status + <<~SQL + CASE status + WHEN 'waiting_for_resource' THEN 0 + ELSE 1 + END ASC + SQL + end + def ensure_resource # Currently we only support one resource per group, which means # maximum one build can be set to the resource group, thus builds diff --git a/app/models/concerns/ci/has_status.rb b/app/models/concerns/ci/has_status.rb index c1299e3d468..8d715279da8 100644 --- a/app/models/concerns/ci/has_status.rb +++ b/app/models/concerns/ci/has_status.rb @@ -95,6 +95,7 @@ module Ci scope :failed_or_canceled, -> { with_status(:failed, :canceled) } scope :complete, -> { with_status(completed_statuses) } scope :incomplete, -> { without_statuses(completed_statuses) } + scope :waiting_for_resource_or_upcoming, -> { with_status(:created, :scheduled, :waiting_for_resource) } scope :cancelable, -> do where(status: [:running, :waiting_for_resource, :preparing, :pending, :created, :scheduled]) diff --git a/app/policies/ci/resource_group_policy.rb b/app/policies/ci/resource_group_policy.rb new file mode 100644 index 00000000000..ef384265b11 --- /dev/null +++ b/app/policies/ci/resource_group_policy.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Ci + class ResourceGroupPolicy < BasePolicy + delegate { @subject.project } + end +end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 54b11ea6041..c9f71e247ae 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -357,6 +357,8 @@ class ProjectPolicy < BasePolicy enable :update_commit_status enable :create_build enable :update_build + enable :read_resource_group + enable :update_resource_group enable :create_merge_request_from enable :create_wiki enable :push_code diff --git a/app/services/ci/resource_groups/assign_resource_from_resource_group_service.rb b/app/services/ci/resource_groups/assign_resource_from_resource_group_service.rb index 1d329fe7b53..dfd97498fc8 100644 --- a/app/services/ci/resource_groups/assign_resource_from_resource_group_service.rb +++ b/app/services/ci/resource_groups/assign_resource_from_resource_group_service.rb @@ -9,7 +9,7 @@ module Ci free_resources = resource_group.resources.free.count - resource_group.processables.waiting_for_resource.take(free_resources).each do |processable| + resource_group.upcoming_processables.take(free_resources).each do |processable| processable.enqueue_waiting_for_resource end end diff --git a/app/views/admin/application_settings/_files_limits.html.haml b/app/views/admin/application_settings/_files_limits.html.haml deleted file mode 100644 index 9cd12fa1caa..00000000000 --- a/app/views/admin/application_settings/_files_limits.html.haml +++ /dev/null @@ -1,34 +0,0 @@ -= gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-files-limits-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) - - %fieldset - %legend.h5.gl-border-none - = _('Unauthenticated API request rate limit') - .form-group - = f.gitlab_ui_checkbox_component :throttle_unauthenticated_files_api_enabled, - _('Enable unauthenticated API request rate limit'), - help_text: _('Helps reduce request volume (e.g. from crawlers or abusive bots)'), - checkbox_options: { data: { qa_selector: 'throttle_unauthenticated_files_api_checkbox' } } - .form-group - = f.label :throttle_unauthenticated_files_api_requests_per_period, 'Max unauthenticated API requests per period per IP', class: 'label-bold' - = f.number_field :throttle_unauthenticated_files_api_requests_per_period, class: 'form-control gl-form-input' - .form-group - = f.label :throttle_unauthenticated_files_api_period_in_seconds, 'Unauthenticated API rate limit period in seconds', class: 'label-bold' - = f.number_field :throttle_unauthenticated_files_api_period_in_seconds, class: 'form-control gl-form-input' - - %fieldset - %legend.h5.gl-border-none - = _('Authenticated API request rate limit') - .form-group - = f.gitlab_ui_checkbox_component :throttle_authenticated_files_api_enabled, - _('Enable authenticated API request rate limit'), - help_text: _('Helps reduce request volume (e.g. from crawlers or abusive bots)'), - checkbox_options: { data: { qa_selector: 'throttle_authenticated_files_api_checkbox' } } - .form-group - = f.label :throttle_authenticated_files_api_requests_per_period, 'Max authenticated API requests per period per user', class: 'label-bold' - = f.number_field :throttle_authenticated_files_api_requests_per_period, class: 'form-control gl-form-input' - .form-group - = f.label :throttle_authenticated_files_api_period_in_seconds, 'Authenticated API rate limit period in seconds', class: 'label-bold' - = f.number_field :throttle_authenticated_files_api_period_in_seconds, class: 'form-control gl-form-input' - - = f.submit 'Save changes', class: "gl-button btn btn-confirm", data: { qa_selector: 'save_changes_button' } diff --git a/app/views/admin/application_settings/_network_rate_limits.html.haml b/app/views/admin/application_settings/_network_rate_limits.html.haml new file mode 100644 index 00000000000..f1857a9749a --- /dev/null +++ b/app/views/admin/application_settings/_network_rate_limits.html.haml @@ -0,0 +1,33 @@ += gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: anchor), html: { class: 'fieldset-form' } do |f| + = form_errors(@application_setting) + + %fieldset + = _("Rate limits can help reduce request volume (like from crawlers or abusive bots).") + + %fieldset + .form-group + = f.gitlab_ui_checkbox_component :"throttle_unauthenticated_#{setting_fragment}_enabled", + _('Enable unauthenticated API request rate limit'), + checkbox_options: { data: { qa_selector: "throttle_unauthenticated_#{setting_fragment}_checkbox" } }, + label_options: { class: 'label-bold' } + .form-group + = f.label :"throttle_unauthenticated_#{setting_fragment}_requests_per_period", _('Maximum unauthenticated API requests per rate limit period per IP'), class: 'label-bold' + = f.number_field :"throttle_unauthenticated_#{setting_fragment}_requests_per_period", class: 'form-control gl-form-input' + .form-group + = f.label :"throttle_unauthenticated_#{setting_fragment}_period_in_seconds", _('Unauthenticated API rate limit period in seconds'), class: 'label-bold' + = f.number_field :"throttle_unauthenticated_#{setting_fragment}_period_in_seconds", class: 'form-control gl-form-input' + + %fieldset + .form-group + = f.gitlab_ui_checkbox_component :"throttle_authenticated_#{setting_fragment}_enabled", + _('Enable authenticated API request rate limit'), + checkbox_options: { data: { qa_selector: "throttle_authenticated_#{setting_fragment}_checkbox" } }, + label_options: { class: 'label-bold' } + .form-group + = f.label :"throttle_authenticated_#{setting_fragment}_requests_per_period", _('Maximum authenticated API requests per rate limit period per user'), class: 'label-bold' + = f.number_field :"throttle_authenticated_#{setting_fragment}_requests_per_period", class: 'form-control gl-form-input' + .form-group + = f.label :"throttle_authenticated_#{setting_fragment}_period_in_seconds", _('Authenticated API rate limit period in seconds'), class: 'label-bold' + = f.number_field :"throttle_authenticated_#{setting_fragment}_period_in_seconds", class: 'form-control gl-form-input' + + = f.submit _('Save changes'), class: "gl-button btn btn-confirm", data: { qa_selector: 'save_changes_button' } diff --git a/app/views/admin/application_settings/_package_registry_limits.html.haml b/app/views/admin/application_settings/_package_registry_limits.html.haml deleted file mode 100644 index 8769171c9e0..00000000000 --- a/app/views/admin/application_settings/_package_registry_limits.html.haml +++ /dev/null @@ -1,32 +0,0 @@ -= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-packages-limits-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) - - %fieldset - = _("The package registry rate limits can help reduce request volume (like from crawlers or abusive bots).") - - %fieldset - .form-group - .form-check - = f.check_box :throttle_unauthenticated_packages_api_enabled, class: 'form-check-input', data: { qa_selector: 'throttle_unauthenticated_packages_api_checkbox' } - = f.label :throttle_unauthenticated_packages_api_enabled, class: 'form-check-label label-bold' do - = _('Enable unauthenticated API request rate limit') - .form-group - = f.label :throttle_unauthenticated_packages_api_requests_per_period, _('Maximum unauthenticated API requests per rate limit period per IP'), class: 'label-bold' - = f.number_field :throttle_unauthenticated_packages_api_requests_per_period, class: 'form-control gl-form-input' - .form-group - = f.label :throttle_unauthenticated_packages_api_period_in_seconds, _('Unauthenticated API rate limit period in seconds'), class: 'label-bold' - = f.number_field :throttle_unauthenticated_packages_api_period_in_seconds, class: 'form-control gl-form-input' - %hr - .form-group - .form-check - = f.check_box :throttle_authenticated_packages_api_enabled, class: 'form-check-input', data: { qa_selector: 'throttle_authenticated_packages_api_checkbox' } - = f.label :throttle_authenticated_packages_api_enabled, class: 'form-check-label label-bold' do - = _('Enable authenticated API request rate limit') - .form-group - = f.label :throttle_authenticated_packages_api_requests_per_period, _('Maximum authenticated API requests per rate limit period per user'), class: 'label-bold' - = f.number_field :throttle_authenticated_packages_api_requests_per_period, class: 'form-control gl-form-input' - .form-group - = f.label :throttle_authenticated_packages_api_period_in_seconds, _('Authenticated API rate limit period in seconds'), class: 'label-bold' - = f.number_field :throttle_authenticated_packages_api_period_in_seconds, class: 'form-control gl-form-input' - - = f.submit _('Save changes'), class: "gl-button btn btn-confirm", data: { qa_selector: 'save_changes_button' } diff --git a/app/views/admin/application_settings/network.html.haml b/app/views/admin/application_settings/network.html.haml index 8dff2bc36cb..2d37b6e0385 100644 --- a/app/views/admin/application_settings/network.html.haml +++ b/app/views/admin/application_settings/network.html.haml @@ -35,9 +35,10 @@ = _('Set rate limits for package registry API requests that supersede the general user and IP rate limits.') = link_to _('Learn more.'), help_page_path('user/admin_area/settings/package_registry_rate_limits.md'), target: '_blank', rel: 'noopener noreferrer' .settings-content - = render 'package_registry_limits' + = render partial: 'network_rate_limits', locals: { anchor: 'js-packages-limits-settings', setting_fragment: 'packages_api' } + - if Feature.enabled?(:files_api_throttling, default_enabled: :yaml) - %section.settings.as-files-limits.no-animate#js-files-limits-settings{ class: ('expanded' if expanded_by_default?), data: { testid: 'files-limits-settings' } } + %section.settings.as-files-limits.no-animate#js-files-limits-settings{ class: ('expanded' if expanded_by_default?) } .settings-header %h4 = _('Files API Rate Limits') @@ -46,7 +47,7 @@ %p = _('Configure specific limits for Files API requests that supersede the general user and IP rate limits.') .settings-content - = render 'files_limits' + = render partial: 'network_rate_limits', locals: { anchor: 'js-files-limits-settings', setting_fragment: 'files_api' } %section.settings.as-git-lfs-limits.no-animate#js-git-lfs-limits-settings{ class: ('expanded' if expanded_by_default?), data: { qa_selector: 'git_lfs_limits_content' } } .settings-header diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 955674b52a4..51c37942977 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -1890,7 +1890,7 @@ :tags: [] - :name: default :worker_name: - :feature_category: + :feature_category: :not_owned :has_external_dependencies: :urgency: :resource_boundary: @@ -2215,7 +2215,7 @@ :tags: [] - :name: mailers :worker_name: ActionMailer::MailDeliveryJob - :feature_category: :issue_tracking + :feature_category: :not_owned :has_external_dependencies: :urgency: low :resource_boundary: diff --git a/app/workers/concerns/worker_attributes.rb b/app/workers/concerns/worker_attributes.rb index eebea30655c..6f91418e38c 100644 --- a/app/workers/concerns/worker_attributes.rb +++ b/app/workers/concerns/worker_attributes.rb @@ -46,8 +46,14 @@ module WorkerAttributes set_class_attribute(:feature_category, :not_owned) end + # Special case: if a worker is not owned, get the feature category + # (if present) from the calling context. def get_feature_category - get_class_attribute(:feature_category) + feature_category = get_class_attribute(:feature_category) + + return feature_category unless feature_category == :not_owned + + Gitlab::ApplicationContext.current_context_attribute('meta.feature_category') || feature_category end def feature_category_not_owned? diff --git a/config/feature_flags/development/ci_resource_group_process_modes.yml b/config/feature_flags/development/ci_resource_group_process_modes.yml new file mode 100644 index 00000000000..12d2945c5cc --- /dev/null +++ b/config/feature_flags/development/ci_resource_group_process_modes.yml @@ -0,0 +1,8 @@ +--- +name: ci_resource_group_process_modes +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67015 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/340380 +milestone: '14.3' +type: development +group: group::release +default_enabled: false diff --git a/db/migrate/20210826145509_add_function_for_inserting_deleted_records.rb b/db/migrate/20210826145509_add_function_for_inserting_deleted_records.rb index ef688cdfd8c..631cc27c8c0 100644 --- a/db/migrate/20210826145509_add_function_for_inserting_deleted_records.rb +++ b/db/migrate/20210826145509_add_function_for_inserting_deleted_records.rb @@ -6,7 +6,7 @@ class AddFunctionForInsertingDeletedRecords < ActiveRecord::Migration[6.1] def up execute(<<~SQL) - CREATE FUNCTION #{DELETED_RECORDS_INSERT_FUNCTION_NAME}() + CREATE OR REPLACE FUNCTION #{DELETED_RECORDS_INSERT_FUNCTION_NAME}() RETURNS TRIGGER AS $$ BEGIN diff --git a/db/migrate/20210916132547_add_process_mode_to_resource_groups.rb b/db/migrate/20210916132547_add_process_mode_to_resource_groups.rb new file mode 100644 index 00000000000..6bac264fcf4 --- /dev/null +++ b/db/migrate/20210916132547_add_process_mode_to_resource_groups.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddProcessModeToResourceGroups < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + PROCESS_MODE_UNORDERED = 0 + + def up + add_column :ci_resource_groups, :process_mode, :integer, default: PROCESS_MODE_UNORDERED, null: false, limit: 2 + end + + def down + remove_column :ci_resource_groups, :process_mode + end +end diff --git a/db/migrate/20210917153645_remove_pipeline_fk_from_packages_build_infos.rb b/db/migrate/20210917153645_remove_pipeline_fk_from_packages_build_infos.rb new file mode 100644 index 00000000000..456788de521 --- /dev/null +++ b/db/migrate/20210917153645_remove_pipeline_fk_from_packages_build_infos.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class RemovePipelineFkFromPackagesBuildInfos < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + with_lock_retries do + remove_foreign_key_if_exists(:packages_build_infos, :ci_pipelines) + end + end + + def down + add_concurrent_foreign_key(:packages_build_infos, :ci_pipelines, column: :pipeline_id, on_delete: :nullify) + end +end diff --git a/db/migrate/20210917153905_remove_pipeline_fk_from_packages_package_file_build_infos.rb b/db/migrate/20210917153905_remove_pipeline_fk_from_packages_package_file_build_infos.rb new file mode 100644 index 00000000000..187ddc8a088 --- /dev/null +++ b/db/migrate/20210917153905_remove_pipeline_fk_from_packages_package_file_build_infos.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class RemovePipelineFkFromPackagesPackageFileBuildInfos < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + with_lock_retries do + remove_foreign_key_if_exists(:packages_package_file_build_infos, :ci_pipelines) + end + end + + def down + add_concurrent_foreign_key(:packages_package_file_build_infos, :ci_pipelines, column: :pipeline_id, on_delete: :nullify) + end +end diff --git a/db/schema_migrations/20210916132547 b/db/schema_migrations/20210916132547 new file mode 100644 index 00000000000..69932a531d9 --- /dev/null +++ b/db/schema_migrations/20210916132547 @@ -0,0 +1 @@ +d0953fdbaa6cf656e298ea482b3e3f931254276cb2285cffafba3d94b0626d3f
\ No newline at end of file diff --git a/db/schema_migrations/20210917153645 b/db/schema_migrations/20210917153645 new file mode 100644 index 00000000000..483c6153a24 --- /dev/null +++ b/db/schema_migrations/20210917153645 @@ -0,0 +1 @@ +fb4c7ce2ed33b3843fbaaf34ea6dbb6db52776039db62b0ab0bb880744f615d1
\ No newline at end of file diff --git a/db/schema_migrations/20210917153905 b/db/schema_migrations/20210917153905 new file mode 100644 index 00000000000..a4e424dfad3 --- /dev/null +++ b/db/schema_migrations/20210917153905 @@ -0,0 +1 @@ +3f0ac2bbfdfe6a2c05043e02ec383bdc7787f86741d6b2df7da2cc6a7aef9ce3
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 80e4fe99c38..2120c64fe89 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11904,7 +11904,8 @@ CREATE TABLE ci_resource_groups ( created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, project_id bigint NOT NULL, - key character varying(255) NOT NULL + key character varying(255) NOT NULL, + process_mode smallint DEFAULT 0 NOT NULL ); CREATE SEQUENCE ci_resource_groups_id_seq @@ -28373,9 +28374,6 @@ ALTER TABLE ONLY project_deploy_tokens ALTER TABLE ONLY analytics_cycle_analytics_project_stages ADD CONSTRAINT fk_rails_1722574860 FOREIGN KEY (start_event_label_id) REFERENCES labels(id) ON DELETE CASCADE; -ALTER TABLE ONLY packages_build_infos - ADD CONSTRAINT fk_rails_17a9a0dffc FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE SET NULL; - ALTER TABLE ONLY security_orchestration_policy_rule_schedules ADD CONSTRAINT fk_rails_17ade83f17 FOREIGN KEY (security_orchestration_policy_configuration_id) REFERENCES security_orchestration_policy_configurations(id) ON DELETE CASCADE; @@ -28640,9 +28638,6 @@ ALTER TABLE ONLY snippet_user_mentions ALTER TABLE ONLY clusters_applications_helm ADD CONSTRAINT fk_rails_3e2b1c06bc FOREIGN KEY (cluster_id) REFERENCES clusters(id) ON DELETE CASCADE; -ALTER TABLE ONLY packages_package_file_build_infos - ADD CONSTRAINT fk_rails_3e3f630188 FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE SET NULL; - ALTER TABLE ONLY epic_user_mentions ADD CONSTRAINT fk_rails_3eaf4d88cc FOREIGN KEY (epic_id) REFERENCES epics(id) ON DELETE CASCADE; diff --git a/doc/api/merge_request_approvals.md b/doc/api/merge_request_approvals.md index b4403e1d9b9..998fb534b7b 100644 --- a/doc/api/merge_request_approvals.md +++ b/doc/api/merge_request_approvals.md @@ -294,6 +294,7 @@ POST /projects/:id/approval_rules | `id` | integer or string | yes | The ID or [URL-encoded path of a project](index.md#namespaced-path-encoding) | | `name` | string | yes | The name of the approval rule | | `approvals_required` | integer | yes | The number of required approvals for this rule | +| `rule_type` | string | no | The type of rule. `any_approver` is a pre-configured default rule with `approvals_required` at `0`. Other rules are `regular`. | `user_ids` | Array | no | The ids of users as approvers | | `group_ids` | Array | no | The ids of groups as approvers | | `protected_branch_ids` | Array | no | **(PREMIUM)** The ids of protected branches to scope the rule by | @@ -379,6 +380,23 @@ POST /projects/:id/approval_rules } ``` +You can increase the default number of 0 required approvers like this: + +```shell +curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" \ + --header 'Content-Type: application/json' \ + --data '{"name": "Any name", "rule_type": "any_approver", "approvals_required": 2}' +``` + +Another example is creating an additional, user-specific rule: + +```shell +curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" \ + --header 'Content-Type: application/json' \ + --data '{"name": "Name of your rule", "approvals_required": 3, "user_ids": [123, 456, 789]}' \ + https://gitlab.example.com/api/v4/projects/<project_id>/approval_rules +``` + ### Update project-level rule > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/11877) in GitLab 12.3. diff --git a/doc/api/resource_groups.md b/doc/api/resource_groups.md new file mode 100644 index 00000000000..ce4fa33d7f2 --- /dev/null +++ b/doc/api/resource_groups.md @@ -0,0 +1,70 @@ +--- +stage: Release +group: Release +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +type: concepts, howto +--- + +# Resource Groups API + +You can read more about [controling the job concurrency with resource groups](../ci/resource_groups/index.md). + +## Get a specific resource group + +```plaintext +GET /projects/:id/resource_groups/:key +``` + +| Attribute | Type | Required | Description | +|-----------|---------|----------|---------------------| +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user | +| `key` | string | yes | The key of the resource group | + +```shell +curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/1/resource_groups/production" +``` + +Example of response + +```json +{ + "id": 3, + "key": "production", + "process_mode": "unordered", + "created_at": "2021-09-01T08:04:59.650Z", + "updated_at": "2021-09-01T08:04:59.650Z" +} +``` + +## Edit an existing resource group + +Updates an existing resource group's properties. + +It returns `200` if the resource group was successfully updated. In case of an error, a status code `400` is returned. + +```plaintext +PUT /projects/:id/resource_groups/:key +``` + +| Attribute | Type | Required | Description | +| --------------- | ------- | --------------------------------- | ------------------------------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user | +| `key` | string | yes | The key of the resource group | +| `process_mode` | string | no | The process mode of the resource group. One of `unordered`, `oldest_first` or `newest_first`. Read [process modes](../ci/resource_groups/index.md#process-modes) for more information. | + +```shell +curl --request PUT --data "process_mode=oldest_first" \ + --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/1/resource_groups/production" +``` + +Example response: + +```json +{ + "id": 3, + "key": "production", + "process_mode": "oldest_first", + "created_at": "2021-09-01T08:04:59.650Z", + "updated_at": "2021-09-01T08:13:38.679Z" +} +``` diff --git a/doc/ci/environments/deployment_safety.md b/doc/ci/environments/deployment_safety.md index 1b34b520007..78f30b29e06 100644 --- a/doc/ci/environments/deployment_safety.md +++ b/doc/ci/environments/deployment_safety.md @@ -60,7 +60,7 @@ The improved pipeline flow **after** using the resource group: 1. `deploy` job in Pipeline-A finishes. 1. `deploy` job in Pipeline-B starts running. -For more information, see [`resource_group` keyword in `.gitlab-ci.yml`](../yaml/index.md#resource_group). +For more information, see [Resource Group documentation](../resource_groups/index.md). ## Skip outdated deployment jobs diff --git a/doc/ci/resource_groups/index.md b/doc/ci/resource_groups/index.md new file mode 100644 index 00000000000..e5ff919a70c --- /dev/null +++ b/doc/ci/resource_groups/index.md @@ -0,0 +1,168 @@ +--- +stage: Release +group: Release +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +description: Control the job concurrency in GitLab CI/CD +--- + +# Resource Group **(FREE)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/15536) in GitLab 12.7. + +By default, pipelines in GitLab CI/CD run in parallel. The parallelization is an important factor to improve +the feedback loop in merge requests, however, there are some situations that +you may want to limit the concurrency on deployment +jobs to run them one by one. +Resource Group allows you to strategically control +the concurrency of the jobs for optimizing your continuous deployments workflow with safety. + +## Add a resource group + +Provided that you have the following pipeline configuration (`.gitlab-ci.yml` file in your repository): + +```yaml +build: + stage: build + script: echo "Your build script" + +deploy: + stage: deploy + script: echo "Your deployment script" + environment: production +``` + +Every time you push a new commit to a branch, it runs a new pipeline that has +two jobs `build` and `deploy`. But if you push multiple commits in a short interval, multiple +pipelines start running simultaneously, for example: + +- The first pipeline runs the jobs `build` -> `deploy` +- The second pipeline runs the jobs `build` -> `deploy` + +In this case, the `deploy` jobs across different pipelines could run concurrently +to the `production` environment. Running multiple deployment scripts to the same +infrastructure could harm/confuse the instance and leave it in a corrupted state in the worst case. + +In order to ensure that a `deploy` job runs once at a time, you can specify +[`resource_group` keyword](../yaml/index.md#resource_group) to the concurrency sensitive job: + +```yaml +deploy: + ... + resource_group: production +``` + +With this configuration, the safety on the deployments is assured while you +can still run `build` jobs concurrently for maximizing the pipeline efficency. + +## Requirements + +- The basic knowledge of the [GitLab CI/CD pipelines](../pipelines/index.md) +- The basic knowledge of the [GitLab Environments and Deployments](../environments/index.md) +- [Developer role](../../user/permissions.md) (or above) in the project to configure CI/CD pipelines. + +### Limitations + +Only one resource can be attached to a resource group. + +## Process modes + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/202186) in GitLab 14.3. + +FLAG: +On self-managed GitLab, by default this feature is not available. +To make it available, ask an administrator to [enable the `ci_resource_group_process_modes` flag](../../administration/feature_flags.md). +On GitLab.com, this feature is not available. +The feature is not ready for production use. + +You can choose a process mode to strategically control the job concurrency for your deployment preferences. +The following modes are supported: + +- **Unordered:** This is the default process mode that limits the concurrency on running jobs. + It's the easiest option to use and useful when you don't care about the execution order + of the jobs. It starts processing the jobs whenever a job ready to run. +- **Oldest first:** This process mode limits the concurrency of the jobs. When a resource is free, + it picks the first job from the list of upcoming jobs (`created`, `scheduled`, or `waiting_for_resource` state) + that are sorted by pipeline ID in ascending order. + + This mode is useful when you want to ensure that the jobs are executed from the oldest pipeline. + This is less efficient compared to the `unordered` mode in terms of the pipeline efficiency, + but safer for continuous deployments. + +- **Newest first:** This process mode limits the concurrency of the jobs. When a resource is free, + it picks the first job from the list of upcoming jobs (`created`, `scheduled` or `waiting_for_resource` state) + that are sorted by pipeline ID in descending order. + + This mode is useful when you want to ensure that the jobs are executed from the newest pipeline and + cancel all of the old deploy jobs with the [skip outdated deployment jobs](../environments/deployment_safety.md#skip-outdated-deployment-jobs) feature. + This is the most efficient option in terms of the pipeline efficiency, but you must ensure that each deployment job is idempotent. + +### Change the process mode + +To change the process mode of a resource group, you need to use the API and +send a request to [edit an existing resource group](../../api/resource_groups.md#edit-an-existing-resource-group) +by specifying the `process_mode`: + +- `unordered` +- `oldest_first` +- `newest_first` + +### An example of difference between the process modes + +Consider the following `.gitlab-ci.yml`, where we have two jobs `build` and `deploy` +each running in their own stage, and the `deploy` job has a resource group set to +`production`: + +```yaml +build: + stage: build + script: echo "Your build script" + +deploy: + stage: deploy + script: echo "Your deployment script" + environment: production + resource_group: production +``` + +If three commits are pushed to the project in a short interval, that means that three +pipelines run almost at the same time: + +- The first pipeline runs the jobs `build` -> `deploy`. Let's call this deployment job `deploy-1`. +- The second pipeline runs the jobs `build` -> `deploy`. Let's call this deployment job `deploy-2`. +- The third pipeline runs the jobs `build` -> `deploy`. Let's call this deployment job `deploy-3`. + +Depending on the process mode of the resource group: + +- If the process mode is set to `unordered`: + - `deploy-1`, `deploy-2`, and `deploy-3` do not run in parallel. + - There is no guarantee on the job execution order, for example, `deploy-1` could run before or after `deploy-3` runs. +- If the process mode is `oldest_first`: + - `deploy-1`, `deploy-2`, and `deploy-3` do not run in parallel. + - `deploy-1` runs first, `deploy-2` runs second, and `deploy-3` runs last. +- If the process mode is `newest_first`: + - `deploy-1`, `deploy-2`, and `deploy-3` do not run in parallel. + - `deploy-3` runs first, `deploy-2` runs second and `deploy-1` runs last. + +## Pipeline-level concurrency control with Cross-Project/Parent-Child pipelines + +See the how to [control the pipeline concurrency in cross-project pipelines](../yaml/index.md#pipeline-level-concurrency-control-with-cross-projectparent-child-pipelines). + +## API + +See the [API documentation](../../api/resource_groups.md). + +## Related features + +Read more how you can use GitLab for [safe deployments](../environments/deployment_safety.md). + +<!-- ## Troubleshooting + +Include any troubleshooting steps that you can foresee. If you know beforehand what issues +one might have when setting this up, or when something is changed, or on upgrading, it's +important to describe those, too. Think of things that may go wrong and include them here. +This is important to minimize requests for support, and to avoid doc comments with +questions that you know someone might ask. + +Each scenario can be a third-level heading, e.g. `### Getting error message X`. +If you have none to add when creating a doc, leave this section in place +but commented out to help encourage others to add to it in the future. --> diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index fb5748788f7..ab4891f8dd6 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -3859,7 +3859,7 @@ can be deployed to, but there can be only one deployment per device at any given The `resource_group` value can only contain letters, digits, `-`, `_`, `/`, `$`, `{`, `}`, `.`, and spaces. It can't start or end with `/`. -For more information, see [Deployments Safety](../environments/deployment_safety.md). +For more information, see [Resource Group documentation](../resource_groups/index.md). #### Pipeline-level concurrency control with Cross-Project/Parent-Child pipelines diff --git a/doc/development/feature_categorization/index.md b/doc/development/feature_categorization/index.md index 2f0f8101b53..07d1a981855 100644 --- a/doc/development/feature_categorization/index.md +++ b/doc/development/feature_categorization/index.md @@ -72,6 +72,11 @@ class SomeCrossCuttingConcernWorker end ``` +Workers marked as not owned workers will, when possible, use the +category of their caller (worker or HTTP endpoint) in metrics and logs. +For instance, `ReactiveCachingWorker` can have multiple feature +categories in metrics and logs. + ## Rails controllers Specifying feature categories on controller actions can be done using diff --git a/doc/development/service_ping/metrics_dictionary.md b/doc/development/service_ping/metrics_dictionary.md index 8dc2d2255d1..a2eb0ddb821 100644 --- a/doc/development/service_ping/metrics_dictionary.md +++ b/doc/development/service_ping/metrics_dictionary.md @@ -6,7 +6,9 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Metrics Dictionary Guide -This guide describes the [Metrics Dictionary](https://gitlab-org.gitlab.io/growth/product-intelligence/metric-dictionary) and how it's implemented. +[Service Ping](index.md) metrics are defined in the +[Metrics Dictionary](https://gitlab-org.gitlab.io/growth/product-intelligence/metric-dictionary). +This guide describes the dictionary and how it's implemented. ## Metrics Definition and validation diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index 76687db3a3f..035d3ce385f 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -998,7 +998,7 @@ it like so: import Subject from '~/feature/the_subject.vue'; // Force Jest to transpile and cache -// eslint-disable-next-line import/order, no-unused-vars +// eslint-disable-next-line no-unused-vars import _Thing from '~/feature/path/to/thing.vue'; ``` diff --git a/doc/user/application_security/container_scanning/index.md b/doc/user/application_security/container_scanning/index.md index 2b3d4dbfc0a..0e5399f9d56 100644 --- a/doc/user/application_security/container_scanning/index.md +++ b/doc/user/application_security/container_scanning/index.md @@ -397,7 +397,7 @@ For details on saving and transporting Docker images as a file, see Docker's doc We recommend that you set up a [scheduled pipeline](../../../ci/pipelines/schedules.md) to fetch the latest vulnerabilities database on a preset schedule. Automating this with a pipeline means you do not have to do it manually each time. You can use the -following `.gitlab-yml.ci` example as a template. +following `.gitlab-ci.yml` example as a template. ```yaml variables: diff --git a/doc/user/group/index.md b/doc/user/group/index.md index caf874cfe28..1b1118aa2f3 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -732,6 +732,31 @@ The group's new subgroups have push rules set for them based on either: - The closest parent group with push rules defined. - Push rules set at the instance level, if no parent groups have push rules defined. +## Group approval rules **(ULTIMATE)** + +> Introduced in GitLab 13.9. [Deployed behind the `group_merge_request_approval_settings_feature_flag` flag](../../administration/feature_flags.md), disabled by default. + +FLAG: +On self-managed GitLab, by default this feature is not available. To make it available, +per group, ask an administrator to [enable the `group_merge_request_approval_settings_feature_flag` flag](../../administration/feature_flags.md). +The feature is not ready for production use. + +Group approval rules are an in-development feature that provides an interface for managing +[project merge request approval rules](../project/merge_requests/approvals/index.md) at the +top-level group level. + +The feature is limited to the user interface but the following merge requests extend the UI to: + +- [Enforce group settings on projects](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69750). +- [Cascade group settings onto projects](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68476). + +To view the merge request approval rules UI for a group: + +1. Go to the top-level group's **Settings > General** page. +1. Expand the **Merge request approvals** section. +1. Select the settings you want. +1. Select **Save changes**. + ## Related topics - [Group wikis](../project/wiki/index.md) diff --git a/doc/user/markdown.md b/doc/user/markdown.md index 5eab5fcb513..5e600b6e0d1 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -524,7 +524,7 @@ GitLab Flavored Markdown recognizes the following: | specific user | `@user_name` | | | | specific group | `@group_name` | | | | entire team | `@all` | | | -| project | `namespace/project` | | | +| project | `namespace/project>` | | | | issue | ``#123`` | `namespace/project#123` | `project#123` | | merge request | `!123` | `namespace/project!123` | `project!123` | | snippet | `$123` | `namespace/project$123` | `project$123` | diff --git a/lib/api/api.rb b/lib/api/api.rb index d0d96858f61..281f23ff5a2 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -157,6 +157,7 @@ module API mount ::API::Ci::Jobs mount ::API::Ci::Pipelines mount ::API::Ci::PipelineSchedules + mount ::API::Ci::ResourceGroups mount ::API::Ci::Runner mount ::API::Ci::Runners mount ::API::Ci::Triggers diff --git a/lib/api/ci/resource_groups.rb b/lib/api/ci/resource_groups.rb new file mode 100644 index 00000000000..02a5e92bd83 --- /dev/null +++ b/lib/api/ci/resource_groups.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module API + module Ci + class ResourceGroups < ::API::Base + before { authenticate! } + + feature_category :continuous_delivery + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + desc 'Get a single resource group' do + success Entities::Ci::ResourceGroup + end + params do + requires :key, type: String, desc: 'The key of the resource group' + end + get ':id/resource_groups/:key' do + authorize! :read_resource_group, resource_group + + present resource_group, with: Entities::Ci::ResourceGroup + end + + desc 'Edit a resource group' do + success Entities::Ci::ResourceGroup + end + params do + requires :key, type: String, desc: 'The key of the resource group' + optional :process_mode, type: String, desc: 'The process mode', + values: ::Ci::ResourceGroup.process_modes.keys + end + put ':id/resource_groups/:key' do + not_found! unless ::Feature.enabled?(:ci_resource_group_process_modes, user_project, default_enabled: :yaml) + authorize! :update_resource_group, resource_group + + if resource_group.update(declared_params(include_missing: false)) + present resource_group, with: Entities::Ci::ResourceGroup + else + render_validation_error!(resource_group) + end + end + end + + helpers do + def resource_group + @resource_group ||= user_project.resource_groups.find_by_key!(params[:key]) + end + end + end + end +end diff --git a/lib/api/entities/ci/resource_group.rb b/lib/api/entities/ci/resource_group.rb new file mode 100644 index 00000000000..0afadfa9e2a --- /dev/null +++ b/lib/api/entities/ci/resource_group.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module API + module Entities + module Ci + class ResourceGroup < Grape::Entity + expose :id, :key, :process_mode, :created_at, :updated_at + end + end + end +end diff --git a/lib/gitlab/sidekiq_config.rb b/lib/gitlab/sidekiq_config.rb index bd6b80530c3..5663c51bb7a 100644 --- a/lib/gitlab/sidekiq_config.rb +++ b/lib/gitlab/sidekiq_config.rb @@ -23,12 +23,12 @@ module Gitlab DEFAULT_WORKERS = { '_' => DummyWorker.new( queue: 'default', - weight: 1, tags: [] + weight: 1, + tags: [] ), 'ActionMailer::MailDeliveryJob' => DummyWorker.new( name: 'ActionMailer::MailDeliveryJob', queue: 'mailers', - feature_category: :issue_tracking, urgency: 'low', weight: 2, tags: [] diff --git a/lib/gitlab/sidekiq_config/dummy_worker.rb b/lib/gitlab/sidekiq_config/dummy_worker.rb index b7f53da8e00..49696e913cf 100644 --- a/lib/gitlab/sidekiq_config/dummy_worker.rb +++ b/lib/gitlab/sidekiq_config/dummy_worker.rb @@ -6,7 +6,6 @@ module Gitlab class DummyWorker ATTRIBUTE_METHODS = { name: :name, - feature_category: :get_feature_category, has_external_dependencies: :worker_has_external_dependencies?, urgency: :get_urgency, resource_boundary: :get_worker_resource_boundary, @@ -27,6 +26,20 @@ module Gitlab nil end + # All dummy workers are unowned; get the feature category from the + # context if available. + def get_feature_category + Gitlab::ApplicationContext.current_context_attribute('meta.feature_category') || :not_owned + end + + def get_worker_context + nil + end + + def context_for_arguments(*) + nil + end + ATTRIBUTE_METHODS.each do |attribute, meth| define_method meth do @attributes[attribute] diff --git a/lib/gitlab/sidekiq_middleware.rb b/lib/gitlab/sidekiq_middleware.rb index d084e9e9d7e..5ad4d7dcbb3 100644 --- a/lib/gitlab/sidekiq_middleware.rb +++ b/lib/gitlab/sidekiq_middleware.rb @@ -13,6 +13,13 @@ module Gitlab chain.add ::Gitlab::SidekiqMiddleware::SizeLimiter::Server chain.add ::Gitlab::SidekiqMiddleware::Monitor + # Labkit wraps the job in the `Labkit::Context` resurrected from + # the job-hash. We need properties from the context for + # recording metrics, so this needs to be before + # `::Gitlab::SidekiqMiddleware::ServerMetrics` (if we're using + # that). + chain.add ::Labkit::Middleware::Sidekiq::Server + if metrics chain.add ::Gitlab::SidekiqMiddleware::ServerMetrics @@ -24,7 +31,6 @@ module Gitlab chain.add ::Gitlab::SidekiqMiddleware::RequestStoreMiddleware chain.add ::Gitlab::SidekiqMiddleware::ExtraDoneLogMetadata chain.add ::Gitlab::SidekiqMiddleware::BatchLoader - chain.add ::Labkit::Middleware::Sidekiq::Server chain.add ::Gitlab::SidekiqMiddleware::InstrumentationLogger chain.add ::Gitlab::SidekiqMiddleware::AdminMode::Server chain.add ::Gitlab::SidekiqVersioning::Middleware diff --git a/lib/gitlab/sidekiq_middleware/client_metrics.rb b/lib/gitlab/sidekiq_middleware/client_metrics.rb index e3cc7b28c41..02d3bd8562b 100644 --- a/lib/gitlab/sidekiq_middleware/client_metrics.rb +++ b/lib/gitlab/sidekiq_middleware/client_metrics.rb @@ -14,6 +14,7 @@ module Gitlab def call(worker_class, job, queue, _redis_pool) # worker_class can either be the string or class of the worker being enqueued. worker_class = worker_class.safe_constantize if worker_class.respond_to?(:safe_constantize) + labels = create_labels(worker_class, queue, job) labels[:scheduling] = job.key?('at') ? 'delayed' : 'immediate' diff --git a/lib/gitlab/sidekiq_middleware/metrics_helper.rb b/lib/gitlab/sidekiq_middleware/metrics_helper.rb index 66930a34319..207d2d769b2 100644 --- a/lib/gitlab/sidekiq_middleware/metrics_helper.rb +++ b/lib/gitlab/sidekiq_middleware/metrics_helper.rb @@ -3,14 +3,21 @@ module Gitlab module SidekiqMiddleware module MetricsHelper + include ::Gitlab::SidekiqMiddleware::WorkerContext + TRUE_LABEL = "yes" FALSE_LABEL = "no" private def create_labels(worker_class, queue, job) - worker_name = (job['wrapped'].presence || worker_class).to_s - worker = find_worker(worker_name, worker_class) + worker = find_worker(worker_class, job) + + # This should never happen: we should always be able to find a + # worker class for a given Sidekiq job. But if we can't, we + # shouldn't blow up here, because we want to record this in our + # metrics. + worker_name = worker.try(:name) || worker.class.name labels = { queue: queue.to_s, worker: worker_name, @@ -23,9 +30,7 @@ module Gitlab labels[:urgency] = worker.get_urgency.to_s labels[:external_dependencies] = bool_as_label(worker.worker_has_external_dependencies?) - - feature_category = worker.get_feature_category - labels[:feature_category] = feature_category.to_s + labels[:feature_category] = worker.get_feature_category.to_s resource_boundary = worker.get_worker_resource_boundary labels[:boundary] = resource_boundary == :unknown ? "" : resource_boundary.to_s @@ -36,10 +41,6 @@ module Gitlab def bool_as_label(value) value ? TRUE_LABEL : FALSE_LABEL end - - def find_worker(worker_name, worker_class) - Gitlab::SidekiqConfig::DEFAULT_WORKERS.fetch(worker_name, worker_class) - end end end end diff --git a/lib/gitlab/sidekiq_middleware/worker_context.rb b/lib/gitlab/sidekiq_middleware/worker_context.rb index 897a9211948..a5d92cf699c 100644 --- a/lib/gitlab/sidekiq_middleware/worker_context.rb +++ b/lib/gitlab/sidekiq_middleware/worker_context.rb @@ -10,6 +10,12 @@ module Gitlab context_or_nil.use(&block) end + + def find_worker(worker_class, job) + worker_name = (job['wrapped'].presence || worker_class).to_s + + Gitlab::SidekiqConfig::DEFAULT_WORKERS[worker_name]&.klass || worker_class + end end end end diff --git a/lib/gitlab/sidekiq_middleware/worker_context/client.rb b/lib/gitlab/sidekiq_middleware/worker_context/client.rb index 1a899b27ea3..4c33d2bfd31 100644 --- a/lib/gitlab/sidekiq_middleware/worker_context/client.rb +++ b/lib/gitlab/sidekiq_middleware/worker_context/client.rb @@ -7,20 +7,15 @@ module Gitlab include Gitlab::SidekiqMiddleware::WorkerContext def call(worker_class_or_name, job, _queue, _redis_pool, &block) - worker_class = worker_class_or_name.to_s.safe_constantize + worker_class = find_worker(worker_class_or_name.to_s.safe_constantize, job) - # Mailers can't be constantized like this + # This is not a worker we know about, perhaps from a gem return yield unless worker_class - return yield unless worker_class.include?(::ApplicationWorker) + return yield unless worker_class.respond_to?(:context_for_arguments) context_for_args = worker_class.context_for_arguments(job['args']) - wrap_in_optional_context(context_for_args) do - # This should be inside the context for the arguments so - # that we don't override the feature category on the worker - # with the one from the caller. - Gitlab::ApplicationContext.with_context(feature_category: worker_class.get_feature_category.to_s, &block) - end + wrap_in_optional_context(context_for_args, &block) end end end diff --git a/lib/gitlab/sidekiq_middleware/worker_context/server.rb b/lib/gitlab/sidekiq_middleware/worker_context/server.rb index 2d8fd8002d2..d026f4918c6 100644 --- a/lib/gitlab/sidekiq_middleware/worker_context/server.rb +++ b/lib/gitlab/sidekiq_middleware/worker_context/server.rb @@ -7,7 +7,7 @@ module Gitlab include Gitlab::SidekiqMiddleware::WorkerContext def call(worker, job, _queue, &block) - worker_class = worker.class + worker_class = find_worker(worker.class, job) # This is not a worker we know about, perhaps from a gem return yield unless worker_class.respond_to?(:get_worker_context) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 73fd4f3aece..336e0e72c09 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4765,9 +4765,6 @@ msgstr "" msgid "Authenticated API rate limit period in seconds" msgstr "" -msgid "Authenticated API request rate limit" -msgstr "" - msgid "Authenticated API requests" msgstr "" @@ -16681,9 +16678,6 @@ msgstr "" msgid "Helps prevent malicious users hide their activity" msgstr "" -msgid "Helps reduce request volume (e.g. from crawlers or abusive bots)" -msgstr "" - msgid "Helps reduce request volume (for example, from crawlers or abusive bots)" msgstr "" @@ -33764,9 +33758,6 @@ msgstr "" msgid "The number of times an upload record could not find its file" msgstr "" -msgid "The package registry rate limits can help reduce request volume (like from crawlers or abusive bots)." -msgstr "" - msgid "The page could not be displayed because it timed out." msgstr "" @@ -36038,9 +36029,6 @@ msgstr "" msgid "Unauthenticated API rate limit period in seconds" msgstr "" -msgid "Unauthenticated API request rate limit" -msgstr "" - msgid "Unauthenticated requests" msgstr "" diff --git a/package.json b/package.json index da980bb3b77..2d29bcdbc4f 100644 --- a/package.json +++ b/package.json @@ -113,7 +113,7 @@ "codesandbox-api": "0.0.23", "compression-webpack-plugin": "^5.0.2", "copy-webpack-plugin": "^6.4.1", - "core-js": "^3.17.3", + "core-js": "^3.18.0", "cron-validator": "^1.1.1", "cropper": "^2.3.0", "css-loader": "^2.1.1", diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index c7739e2ff5f..15ee2f355d8 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -67,6 +67,8 @@ RSpec.describe 'Database schema' do oauth_access_tokens: %w[resource_owner_id application_id], oauth_applications: %w[owner_id], open_project_tracker_data: %w[closed_status_id], + packages_build_infos: %w[pipeline_id], + packages_package_file_build_infos: %w[pipeline_id], product_analytics_events_experimental: %w[event_id txn_id user_id], project_group_links: %w[group_id], project_statistics: %w[namespace_id], diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index b25fc9f257a..5f55983beb8 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -602,18 +602,47 @@ RSpec.describe 'Admin updates settings' do expect(current_settings.issues_create_limit).to eq(0) end - it 'changes Files API rate limits settings' do - visit network_admin_application_settings_path + shared_examples 'regular throttle rate limit settings' do + it 'changes rate limit settings' do + visit network_admin_application_settings_path - page.within('[data-testid="files-limits-settings"]') do - check 'Enable unauthenticated API request rate limit' - fill_in 'Max unauthenticated API requests per period per IP', with: 10 - click_button 'Save changes' + page.within(".#{selector}") do + check 'Enable unauthenticated API request rate limit' + fill_in 'Maximum unauthenticated API requests per rate limit period per IP', with: 12 + fill_in 'Unauthenticated API rate limit period in seconds', with: 34 + + check 'Enable authenticated API request rate limit' + fill_in 'Maximum authenticated API requests per rate limit period per user', with: 56 + fill_in 'Authenticated API rate limit period in seconds', with: 78 + + click_button 'Save changes' + end + + expect(page).to have_content "Application settings saved successfully" + + expect(current_settings).to have_attributes( + "throttle_unauthenticated_#{fragment}_enabled" => true, + "throttle_unauthenticated_#{fragment}_requests_per_period" => 12, + "throttle_unauthenticated_#{fragment}_period_in_seconds" => 34, + "throttle_authenticated_#{fragment}_enabled" => true, + "throttle_authenticated_#{fragment}_requests_per_period" => 56, + "throttle_authenticated_#{fragment}_period_in_seconds" => 78 + ) end + end - expect(page).to have_content "Application settings saved successfully" - expect(current_settings.throttle_unauthenticated_files_api_enabled).to be true - expect(current_settings.throttle_unauthenticated_files_api_requests_per_period).to eq(10) + context 'Package Registry API rate limits' do + let(:selector) { 'as-packages-limits' } + let(:fragment) { :packages_api } + + include_examples 'regular throttle rate limit settings' + end + + context 'Files API rate limits' do + let(:selector) { 'as-files-limits' } + let(:fragment) { :files_api } + + include_examples 'regular throttle rate limit settings' end end diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js index 9dc82bbdc93..9b63f84e617 100644 --- a/spec/frontend/diffs/components/app_spec.js +++ b/spec/frontend/diffs/components/app_spec.js @@ -13,11 +13,8 @@ import DiffFile from '~/diffs/components/diff_file.vue'; import NoChanges from '~/diffs/components/no_changes.vue'; import TreeList from '~/diffs/components/tree_list.vue'; -/* eslint-disable import/order */ -/* You know what: sometimes alphabetical isn't the best order */ import CollapsedFilesWarning from '~/diffs/components/collapsed_files_warning.vue'; import HiddenFilesWarning from '~/diffs/components/hidden_files_warning.vue'; -/* eslint-enable import/order */ import axios from '~/lib/utils/axios_utils'; import * as urlUtils from '~/lib/utils/url_utility'; diff --git a/spec/frontend/filtered_search/dropdown_user_spec.js b/spec/frontend/filtered_search/dropdown_user_spec.js index 961587f7146..5a7bfcd9caf 100644 --- a/spec/frontend/filtered_search/dropdown_user_spec.js +++ b/spec/frontend/filtered_search/dropdown_user_spec.js @@ -1,7 +1,6 @@ import DropdownUtils from '~/filtered_search/dropdown_utils'; // TODO: Moving this line up throws an error about `FilteredSearchDropdown` // being undefined in test. See gitlab-org/gitlab#321476 for more info. -// eslint-disable-next-line import/order import DropdownUser from '~/filtered_search/dropdown_user'; import FilteredSearchTokenizer from '~/filtered_search/filtered_search_tokenizer'; import IssuableFilteredTokenKeys from '~/filtered_search/issuable_filtered_search_token_keys'; diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 006f8a39f9c..d89af1521a2 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1631,10 +1631,19 @@ RSpec.describe Gitlab::Database::MigrationHelpers do let(:worker) do Class.new do include Sidekiq::Worker + sidekiq_options queue: 'test' + + def self.name + 'WorkerClass' + end end end + before do + stub_const(worker.name, worker) + end + describe '#sidekiq_queue_length' do context 'when queue is empty' do it 'returns zero' do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 614aa55c3c5..9915e7412dc 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -272,6 +272,8 @@ ci_pipelines: - dast_profiles_pipeline - dast_site_profile - dast_site_profiles_pipeline +- package_build_infos +- package_file_build_infos ci_refs: - project - ci_pipelines diff --git a/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb index 698758a13fd..de5360d2dcd 100644 --- a/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb @@ -63,5 +63,18 @@ RSpec.describe Gitlab::SidekiqMiddleware::ClientMetrics do Sidekiq::Testing.inline! { TestWorker.perform_in(1.second) } end end + + context 'when the worker class cannot be found' do + it 'increments enqueued jobs metric with the worker labels set to NilClass' do + test_anonymous_worker = Class.new(TestWorker) + + expect(enqueued_jobs_metric).to receive(:increment).with(a_hash_including(worker: 'NilClass'), 1) + + # Sidekiq won't be able to create an instance of this class + expect do + Sidekiq::Testing.inline! { test_anonymous_worker.perform_async } + end.to raise_error(NameError) + end + end end end diff --git a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb index cae0bb6b167..dc0b4239120 100644 --- a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb @@ -211,6 +211,9 @@ RSpec.describe Gitlab::SidekiqMiddleware::ServerMetrics do end end + include_context 'server metrics with mocked prometheus' + include_context 'server metrics call' + before do stub_const('TestWorker', Class.new) TestWorker.class_eval do @@ -234,9 +237,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::ServerMetrics do end end - include_context 'server metrics with mocked prometheus' - include_context 'server metrics call' - shared_context 'worker declaring data consistency' do let(:worker_class) { LBTestWorker } @@ -308,5 +308,67 @@ RSpec.describe Gitlab::SidekiqMiddleware::ServerMetrics do end end end + + context 'feature attribution' do + let(:test_worker) do + category = worker_category + + Class.new do + include Sidekiq::Worker + include WorkerAttributes + + if category + feature_category category + else + feature_category_not_owned! + end + + def perform + end + end + end + + let(:context_category) { 'continuous_integration' } + let(:job) { { 'meta.feature_category' => 'continuous_integration' } } + + before do + stub_const('TestWorker', test_worker) + end + + around do |example| + with_sidekiq_server_middleware do |chain| + Gitlab::SidekiqMiddleware.server_configurator( + metrics: true, + arguments_logger: false, + memory_killer: false + ).call(chain) + + Sidekiq::Testing.inline! { example.run } + end + end + + include_context 'server metrics with mocked prometheus' + include_context 'server metrics call' + + context 'when a worker has a feature category' do + let(:worker_category) { 'authentication_and_authorization' } + + it 'uses that category for metrics' do + expect(completion_seconds_metric).to receive(:observe).with(a_hash_including(feature_category: worker_category), anything) + + TestWorker.process_job(job) + end + end + + context 'when a worker does not have a feature category' do + let(:worker_category) { nil } + + it 'uses the category from the context for metrics' do + expect(completion_seconds_metric).to receive(:observe).with(a_hash_including(feature_category: context_category), anything) + + TestWorker.process_job(job) + end + end + end end # rubocop: enable RSpec/MultipleMemoizedHelpers diff --git a/spec/lib/gitlab/sidekiq_middleware/worker_context/client_spec.rb b/spec/lib/gitlab/sidekiq_middleware/worker_context/client_spec.rb index d6cc787f53d..7ef85cf25f6 100644 --- a/spec/lib/gitlab/sidekiq_middleware/worker_context/client_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/worker_context/client_spec.rb @@ -11,8 +11,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Client do include ApplicationWorker - feature_category :issue_tracking - def self.job_for_args(args) jobs.find { |job| job['args'] == args } end @@ -23,7 +21,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Client do end before do - stub_const('TestWithContextWorker', worker_class) + stub_const(worker_class.name, worker_class) end describe "#call" do @@ -43,39 +41,5 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Client do expect(job1['meta.user']).to eq(user_per_job['job1'].username) expect(job2['meta.user']).to eq(user_per_job['job2'].username) end - - context 'when the feature category is set in the context_proc' do - it 'takes the feature category from the worker, not the caller' do - TestWithContextWorker.bulk_perform_async_with_contexts( - %w(job1 job2), - arguments_proc: -> (name) { [name, 1, 2, 3] }, - context_proc: -> (_) { { feature_category: 'code_review' } } - ) - - job1 = TestWithContextWorker.job_for_args(['job1', 1, 2, 3]) - job2 = TestWithContextWorker.job_for_args(['job2', 1, 2, 3]) - - expect(job1['meta.feature_category']).to eq('issue_tracking') - expect(job2['meta.feature_category']).to eq('issue_tracking') - end - end - - context 'when the feature category is already set in the surrounding block' do - it 'takes the feature category from the worker, not the caller' do - Gitlab::ApplicationContext.with_context(feature_category: 'authentication_and_authorization') do - TestWithContextWorker.bulk_perform_async_with_contexts( - %w(job1 job2), - arguments_proc: -> (name) { [name, 1, 2, 3] }, - context_proc: -> (_) { {} } - ) - end - - job1 = TestWithContextWorker.job_for_args(['job1', 1, 2, 3]) - job2 = TestWithContextWorker.job_for_args(['job2', 1, 2, 3]) - - expect(job1['meta.feature_category']).to eq('issue_tracking') - expect(job2['meta.feature_category']).to eq('issue_tracking') - end - end end end diff --git a/spec/lib/gitlab/sidekiq_middleware/worker_context/server_spec.rb b/spec/lib/gitlab/sidekiq_middleware/worker_context/server_spec.rb index f736a7db774..377ff6fd166 100644 --- a/spec/lib/gitlab/sidekiq_middleware/worker_context/server_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/worker_context/server_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Server do - let(:worker_class) do + let(:test_worker) do Class.new do def self.name "TestWorker" @@ -23,6 +23,16 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Server do end end + let(:not_owned_worker) do + Class.new(test_worker) do + def self.name + "NotOwnedWorker" + end + + feature_category_not_owned! + end + end + let(:other_worker) do Class.new do def self.name @@ -37,7 +47,8 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Server do end before do - stub_const("TestWorker", worker_class) + stub_const("TestWorker", test_worker) + stub_const("NotOwnedWorker", not_owned_worker) stub_const("OtherWorker", other_worker) end @@ -57,10 +68,24 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Server do expect(TestWorker.contexts['identifier'].keys).not_to include('meta.user') end - it 'takes the feature category from the worker' do - TestWorker.perform_async('identifier', 1) + context 'feature category' do + it 'takes the feature category from the worker' do + Gitlab::ApplicationContext.with_context(feature_category: 'authentication_and_authorization') do + TestWorker.perform_async('identifier', 1) + end + + expect(TestWorker.contexts['identifier']).to include('meta.feature_category' => 'foo') + end - expect(TestWorker.contexts['identifier']).to include('meta.feature_category' => 'foo') + context 'when the worker is not owned' do + it 'takes the feature category from the surrounding context' do + Gitlab::ApplicationContext.with_context(feature_category: 'authentication_and_authorization') do + NotOwnedWorker.perform_async('identifier', 1) + end + + expect(NotOwnedWorker.contexts['identifier']).to include('meta.feature_category' => 'authentication_and_authorization') + end + end end it "doesn't fail for unknown workers" do diff --git a/spec/lib/gitlab/sidekiq_middleware_spec.rb b/spec/lib/gitlab/sidekiq_middleware_spec.rb index 8285cf960d2..dde364098db 100644 --- a/spec/lib/gitlab/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware_spec.rb @@ -58,13 +58,13 @@ RSpec.describe Gitlab::SidekiqMiddleware do let(:all_sidekiq_middlewares) do [ ::Gitlab::SidekiqMiddleware::Monitor, + ::Labkit::Middleware::Sidekiq::Server, ::Gitlab::SidekiqMiddleware::ServerMetrics, ::Gitlab::SidekiqMiddleware::ArgumentsLogger, ::Gitlab::SidekiqMiddleware::MemoryKiller, ::Gitlab::SidekiqMiddleware::RequestStoreMiddleware, ::Gitlab::SidekiqMiddleware::ExtraDoneLogMetadata, ::Gitlab::SidekiqMiddleware::BatchLoader, - ::Labkit::Middleware::Sidekiq::Server, ::Gitlab::SidekiqMiddleware::InstrumentationLogger, ::Gitlab::SidekiqMiddleware::AdminMode::Server, ::Gitlab::SidekiqVersioning::Middleware, diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 1007d64438f..a8395a83177 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -35,6 +35,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it { is_expected.to have_many(:sourced_pipelines) } it { is_expected.to have_many(:triggered_pipelines) } it { is_expected.to have_many(:pipeline_artifacts) } + it { is_expected.to have_many(:package_build_infos).dependent(:nullify).inverse_of(:pipeline) } + it { is_expected.to have_many(:package_file_build_infos).dependent(:nullify).inverse_of(:pipeline) } it { is_expected.to have_one(:chat_data) } it { is_expected.to have_one(:source_pipeline) } diff --git a/spec/models/ci/resource_group_spec.rb b/spec/models/ci/resource_group_spec.rb index 50a786419f2..18ee2d3d7fa 100644 --- a/spec/models/ci/resource_group_spec.rb +++ b/spec/models/ci/resource_group_spec.rb @@ -85,4 +85,77 @@ RSpec.describe Ci::ResourceGroup do end end end + + describe '#upcoming_processables' do + subject { resource_group.upcoming_processables } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:pipeline_1) { create(:ci_pipeline, project: project) } + let_it_be(:pipeline_2) { create(:ci_pipeline, project: project) } + + let!(:resource_group) { create(:ci_resource_group, process_mode: process_mode, project: project) } + + Ci::HasStatus::STATUSES_ENUM.keys.each do |status| + let!("build_1_#{status}") { create(:ci_build, pipeline: pipeline_1, status: status, resource_group: resource_group) } + let!("build_2_#{status}") { create(:ci_build, pipeline: pipeline_2, status: status, resource_group: resource_group) } + end + + context 'when process mode is unordered' do + let(:process_mode) { :unordered } + + it 'returns correct jobs in an indeterministic order' do + expect(subject).to contain_exactly(build_1_waiting_for_resource, build_2_waiting_for_resource) + end + end + + context 'when process mode is oldest_first' do + let(:process_mode) { :oldest_first } + + it 'returns correct jobs in a specific order' do + expect(subject[0]).to eq(build_1_waiting_for_resource) + expect(subject[1..2]).to contain_exactly(build_1_created, build_1_scheduled) + expect(subject[3]).to eq(build_2_waiting_for_resource) + expect(subject[4..5]).to contain_exactly(build_2_created, build_2_scheduled) + end + + context 'when ci_resource_group_process_modes feature flag is disabled' do + it 'returns correct jobs in an indeterministic order' do + stub_feature_flags(ci_resource_group_process_modes: false) + + expect(subject).to contain_exactly(build_1_waiting_for_resource, build_2_waiting_for_resource) + end + end + end + + context 'when process mode is newest_first' do + let(:process_mode) { :newest_first } + + it 'returns correct jobs in a specific order' do + expect(subject[0]).to eq(build_2_waiting_for_resource) + expect(subject[1..2]).to contain_exactly(build_2_created, build_2_scheduled) + expect(subject[3]).to eq(build_1_waiting_for_resource) + expect(subject[4..5]).to contain_exactly(build_1_created, build_1_scheduled) + end + + context 'when ci_resource_group_process_modes feature flag is disabled' do + it 'returns correct jobs in an indeterministic order' do + stub_feature_flags(ci_resource_group_process_modes: false) + + expect(subject).to contain_exactly(build_1_waiting_for_resource, build_2_waiting_for_resource) + end + end + end + + context 'when process mode is unknown' do + let(:process_mode) { :unordered } + + before do + resource_group.update_column(:process_mode, 3) + end + + it 'returns empty' do + is_expected.to be_empty + end + end + end end diff --git a/spec/models/concerns/ci/has_status_spec.rb b/spec/models/concerns/ci/has_status_spec.rb index 0709a050056..9dfc7d84f89 100644 --- a/spec/models/concerns/ci/has_status_spec.rb +++ b/spec/models/concerns/ci/has_status_spec.rb @@ -363,6 +363,18 @@ RSpec.describe Ci::HasStatus do it_behaves_like 'not containing the job', status end end + + describe '.waiting_for_resource_or_upcoming' do + subject { CommitStatus.waiting_for_resource_or_upcoming } + + %i[created scheduled waiting_for_resource].each do |status| + it_behaves_like 'containing the job', status + end + + %i[running failed success canceled].each do |status| + it_behaves_like 'not containing the job', status + end + end end describe '::DEFAULT_STATUS' do diff --git a/spec/requests/api/ci/resource_groups_spec.rb b/spec/requests/api/ci/resource_groups_spec.rb new file mode 100644 index 00000000000..c48b7cd769c --- /dev/null +++ b/spec/requests/api/ci/resource_groups_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Ci::ResourceGroups do + let_it_be(:project) { create(:project) } + let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + let_it_be(:reporter) { create(:user).tap { |u| project.add_reporter(u) } } + + let(:user) { developer } + + describe 'GET /projects/:id/resource_groups/:key' do + subject { get api("/projects/#{project.id}/resource_groups/#{key}", user) } + + let!(:resource_group) { create(:ci_resource_group, project: project) } + let(:key) { resource_group.key } + + it 'returns a resource group', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['id']).to eq(resource_group.id) + expect(json_response['key']).to eq(resource_group.key) + expect(json_response['process_mode']).to eq(resource_group.process_mode) + expect(Time.parse(json_response['created_at'])).to be_like_time(resource_group.created_at) + expect(Time.parse(json_response['updated_at'])).to be_like_time(resource_group.updated_at) + end + + context 'when user is reporter' do + let(:user) { reporter } + + it 'returns forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when there is no corresponding resource group' do + let(:key) { 'unknown' } + + it 'returns not found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'PUT /projects/:id/resource_groups/:key' do + subject { put api("/projects/#{project.id}/resource_groups/#{key}", user), params: params } + + let!(:resource_group) { create(:ci_resource_group, project: project) } + let(:key) { resource_group.key } + let(:params) { { process_mode: :oldest_first } } + + it 'changes the process mode of a resource group' do + expect { subject } + .to change { resource_group.reload.process_mode }.from('unordered').to('oldest_first') + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['process_mode']).to eq('oldest_first') + end + + context 'when ci_resource_group_process_modes feature flag is disabled' do + before do + stub_feature_flags(ci_resource_group_process_modes: false) + end + + it 'returns not found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with invalid parameter' do + let(:params) { { process_mode: :unknown } } + + it 'returns bad request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when user is reporter' do + let(:user) { reporter } + + it 'returns forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when there is no corresponding resource group' do + let(:key) { 'unknown' } + + it 'returns not found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/services/ci/resource_groups/assign_resource_from_resource_group_service_spec.rb b/spec/services/ci/resource_groups/assign_resource_from_resource_group_service_spec.rb index 53aa842bc28..194203a422c 100644 --- a/spec/services/ci/resource_groups/assign_resource_from_resource_group_service_spec.rb +++ b/spec/services/ci/resource_groups/assign_resource_from_resource_group_service_spec.rb @@ -48,6 +48,92 @@ RSpec.describe Ci::ResourceGroups::AssignResourceFromResourceGroupService do expect(build).to be_pending end end + + context 'when process mode is oldest_first' do + let(:resource_group) { create(:ci_resource_group, process_mode: :oldest_first, project: project) } + + it 'requests resource' do + subject + + expect(build.reload).to be_pending + expect(build.resource).to be_present + end + + context 'when the other job exists in the newer pipeline' do + let!(:build_2) { create(:ci_build, :waiting_for_resource, project: project, user: user, resource_group: resource_group) } + + it 'requests resource for the job in the oldest pipeline' do + subject + + expect(build.reload).to be_pending + expect(build.resource).to be_present + expect(build_2.reload).to be_waiting_for_resource + expect(build_2.resource).to be_nil + end + end + + context 'when build is not `waiting_for_resource` state' do + let!(:build) { create(:ci_build, :created, project: project, user: user, resource_group: resource_group) } + + it 'attempts to request a resource' do + expect_next_found_instance_of(Ci::Build) do |job| + expect(job).to receive(:enqueue_waiting_for_resource).and_call_original + end + + subject + end + + it 'does not change the job status' do + subject + + expect(build.reload).to be_created + expect(build.resource).to be_nil + end + end + end + + context 'when process mode is newest_first' do + let(:resource_group) { create(:ci_resource_group, process_mode: :newest_first, project: project) } + + it 'requests resource' do + subject + + expect(build.reload).to be_pending + expect(build.resource).to be_present + end + + context 'when the other job exists in the newer pipeline' do + let!(:build_2) { create(:ci_build, :waiting_for_resource, project: project, user: user, resource_group: resource_group) } + + it 'requests resource for the job in the newest pipeline' do + subject + + expect(build.reload).to be_waiting_for_resource + expect(build.resource).to be_nil + expect(build_2.reload).to be_pending + expect(build_2.resource).to be_present + end + end + + context 'when build is not `waiting_for_resource` state' do + let!(:build) { create(:ci_build, :created, project: project, user: user, resource_group: resource_group) } + + it 'attempts to request a resource' do + expect_next_found_instance_of(Ci::Build) do |job| + expect(job).to receive(:enqueue_waiting_for_resource).and_call_original + end + + subject + end + + it 'does not change the job status' do + subject + + expect(build.reload).to be_created + expect(build.resource).to be_nil + end + end + end end context 'when there are no available resources' do diff --git a/spec/support/shared_contexts/lib/gitlab/sidekiq_middleware/server_metrics_shared_context.rb b/spec/support/shared_contexts/lib/gitlab/sidekiq_middleware/server_metrics_shared_context.rb index 73de631e293..37e3805e4e0 100644 --- a/spec/support/shared_contexts/lib/gitlab/sidekiq_middleware/server_metrics_shared_context.rb +++ b/spec/support/shared_contexts/lib/gitlab/sidekiq_middleware/server_metrics_shared_context.rb @@ -17,6 +17,9 @@ RSpec.shared_context 'server metrics with mocked prometheus' do let(:elasticsearch_requests_total) { double('elasticsearch calls total metric') } before do + allow(Gitlab::Metrics).to receive(:histogram).and_call_original + allow(Gitlab::Metrics).to receive(:counter).and_call_original + allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_queue_duration_seconds, anything, anything, anything).and_return(queue_duration_seconds) allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_completion_seconds, anything, anything, anything).and_return(completion_seconds_metric) allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_cpu_seconds, anything, anything, anything).and_return(user_execution_seconds_metric) diff --git a/spec/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index de1b46c65ad..d7e4864cb08 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -49,6 +49,7 @@ RSpec.shared_context 'ProjectPolicy context' do resolve_note update_build update_commit_status update_container_image update_deployment update_environment update_merge_request update_metrics_dashboard_annotation update_pipeline update_release destroy_release + read_resource_group update_resource_group ] end diff --git a/spec/workers/concerns/application_worker_spec.rb b/spec/workers/concerns/application_worker_spec.rb index ac4e4a682c8..af038c81b9e 100644 --- a/spec/workers/concerns/application_worker_spec.rb +++ b/spec/workers/concerns/application_worker_spec.rb @@ -248,6 +248,10 @@ RSpec.describe ApplicationWorker do end describe '.perform_async' do + before do + stub_const(worker.name, worker) + end + shared_examples_for 'worker utilizes load balancing capabilities' do |data_consistency| before do worker.data_consistency(data_consistency) @@ -282,6 +286,10 @@ RSpec.describe ApplicationWorker do end describe '.bulk_perform_async' do + before do + stub_const(worker.name, worker) + end + it 'enqueues jobs in bulk' do Sidekiq::Testing.fake! do worker.bulk_perform_async([['Foo', [1]], ['Foo', [2]]]) @@ -293,6 +301,10 @@ RSpec.describe ApplicationWorker do end describe '.bulk_perform_in' do + before do + stub_const(worker.name, worker) + end + context 'when delay is valid' do it 'correctly schedules jobs' do Sidekiq::Testing.fake! do diff --git a/spec/workers/concerns/worker_context_spec.rb b/spec/workers/concerns/worker_context_spec.rb index ebdb752d900..80b427b2b42 100644 --- a/spec/workers/concerns/worker_context_spec.rb +++ b/spec/workers/concerns/worker_context_spec.rb @@ -13,6 +13,10 @@ RSpec.describe WorkerContext do end end + before do + stub_const(worker.name, worker) + end + describe '.worker_context' do it 'allows modifying the context for the entire worker' do worker.worker_context(user: build_stubbed(:user)) diff --git a/yarn.lock b/yarn.lock index e603b675ba0..91c00287ed0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3819,10 +3819,10 @@ core-js-pure@^3.0.0: resolved "https://registry.yarnpkg.com/core-js-pure/-/core-js-pure-3.6.5.tgz#c79e75f5e38dbc85a662d91eea52b8256d53b813" integrity sha512-lacdXOimsiD0QyNf9BC/mxivNJ/ybBGJXQFKzRekp1WTHoVUWsUHEn+2T8GJAzzIhyOuXA+gOxCVN3l+5PLPUA== -core-js@^3.17.3: - version "3.17.3" - resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.17.3.tgz#8e8bd20e91df9951e903cabe91f9af4a0895bc1e" - integrity sha512-lyvajs+wd8N1hXfzob1LdOCCHFU4bGMbqqmLn1Q4QlCpDqWPpGf+p0nj+LNrvDDG33j0hZXw2nsvvVpHysxyNw== +core-js@^3.18.0: + version "3.18.0" + resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.18.0.tgz#9af3f4a6df9ba3428a3fb1b171f1503b3f40cc49" + integrity sha512-WJeQqq6jOYgVgg4NrXKL0KLQhi0CT4ZOCvFL+3CQ5o7I6J8HkT5wd53EadMfqTDp1so/MT1J+w2ujhWcCJtN7w== core-js@~2.3.0: version "2.3.0" |