diff options
20 files changed, 248 insertions, 172 deletions
diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 76191a923bf..fefadab16e4 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -161,7 +161,7 @@ review-qa-smoke: review-qa-all: extends: - .review-qa-base - - .review:rules:mr-only-manual + - .review:rules:review-qa-all parallel: 5 script: - export KNAPSACK_REPORT_PATH=knapsack/master_report.json @@ -198,7 +198,7 @@ review-performance: parallel-spec-reports: extends: - - .review:rules:mr-only-manual + - .review:rules:review-qa-all image: ${GITLAB_DEPENDENCY_PROXY}ruby:2.7-alpine stage: post-qa dependencies: ["review-qa-all"] diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index e76b0f2d07f..b8b37c530f2 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -1007,9 +1007,12 @@ - <<: *if-dot-com-gitlab-org-merge-request changes: *frontend-patterns - <<: *if-dot-com-gitlab-org-merge-request - changes: *code-qa-patterns + changes: *code-patterns when: manual allow_failure: true + - <<: *if-dot-com-gitlab-org-merge-request + changes: *qa-patterns + allow_failure: true - <<: *if-dot-com-gitlab-org-schedule .review:rules:review-deploy: @@ -1022,9 +1025,12 @@ changes: *frontend-patterns allow_failure: true - <<: *if-dot-com-gitlab-org-merge-request - changes: *code-qa-patterns + changes: *code-patterns when: manual allow_failure: true + - <<: *if-dot-com-gitlab-org-merge-request + changes: *qa-patterns + allow_failure: true - <<: *if-dot-com-gitlab-org-schedule allow_failure: true @@ -1067,14 +1073,17 @@ when: manual allow_failure: true -.review:rules:mr-only-manual: +.review:rules:review-qa-all: rules: - <<: *if-not-ee when: never - <<: *if-dot-com-gitlab-org-merge-request - changes: *code-qa-patterns + changes: *code-patterns when: manual allow_failure: true + - <<: *if-dot-com-gitlab-org-merge-request + changes: *qa-patterns + allow_failure: true .review:rules:review-cleanup: rules: diff --git a/.gitlab/issue_templates/Query Performance Investigation.md b/.gitlab/issue_templates/Query Performance Investigation.md index ddd361e4f2f..354cdb1bfe1 100644 --- a/.gitlab/issue_templates/Query Performance Investigation.md +++ b/.gitlab/issue_templates/Query Performance Investigation.md @@ -10,6 +10,16 @@ As the name implies, the purpose of the template is to detail underperforming qu - [ ] Provide [priority and severity labels](https://about.gitlab.com/handbook/engineering/quality/issue-triage/#availability) - [ ] If this requires immediate attention cc `@gitlab-org/database-team` and reach out in the #g_database slack channel +### SQL Statement + +```sql + +``` + +### Data from Elastic + +Instructions on collecting data from [PostgreSQL slow logs stored in Elasticsearch](https://gitlab.com/gitlab-com/runbooks/-/merge_requests/3361/diffs) + ### Requested Data points Please provide as many of these fields as possible when submitting a query performance report. @@ -20,7 +30,6 @@ Please provide as many of these fields as possible when submitting a query perfo - Database time relative to total database time - Source of calls (Sidekiq, WebAPI, etc) - Query ID -- SQL Statement - Query Plan - Query Example - Total number of calls (relative) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 68a01828592..0cbfc674111 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -6f0bfe7c326aeabcb5c3bc35bd3994160d20d6c3 +6314c40a2e0ae9d43eb4c01026d0aef57c10d91c diff --git a/app/experiments/members/invite_email_experiment.rb b/app/experiments/members/invite_email_experiment.rb index a0f4dee2866..6a7d2b110d3 100644 --- a/app/experiments/members/invite_email_experiment.rb +++ b/app/experiments/members/invite_email_experiment.rb @@ -8,7 +8,82 @@ module Members INVITE_TYPE = 'initial_email' def resolve_variant_name - Strategy::RoundRobin.new(feature_flag_name, %i[avatar permission_info control]).execute + RoundRobin.new(feature_flag_name, %i[avatar permission_info control]).execute + end + end + + class RoundRobin + CacheError = Class.new(StandardError) + + COUNTER_EXPIRE_TIME = 86400 # one day + + def initialize(key, variants) + @key = key + @variants = variants + end + + def execute + increment_counter + resolve_variant_name + end + + # When the counter would expire + # + # @api private Used internally by SRE and debugging purpose + # @return [Integer] Number in seconds until expiration or false if never + def counter_expires_in + Gitlab::Redis::SharedState.with do |redis| + redis.ttl(key) + end + end + + # Return the actual counter value + # + # @return [Integer] value + def counter_value + Gitlab::Redis::SharedState.with do |redis| + (redis.get(key) || 0).to_i + end + end + + # Reset the counter + # + # @private Used internally by SRE and debugging purpose + # @return [Boolean] whether reset was a success + def reset! + redis_cmd do |redis| + redis.del(key) + end + end + + private + + attr_reader :key, :variants + + # Increase the counter + # + # @return [Boolean] whether operation was a success + def increment_counter + redis_cmd do |redis| + redis.incr(key) + redis.expire(key, COUNTER_EXPIRE_TIME) + end + end + + def resolve_variant_name + remainder = counter_value % variants.size + + variants[remainder] + end + + def redis_cmd + Gitlab::Redis::SharedState.with { |redis| yield(redis) } + + true + rescue CacheError => e + Gitlab::AppLogger.warn("GitLab: An unexpected error occurred in writing to Redis: #{e}") + + false end end end diff --git a/app/experiments/strategy/round_robin.rb b/app/experiments/strategy/round_robin.rb deleted file mode 100644 index 7b80c0e984d..00000000000 --- a/app/experiments/strategy/round_robin.rb +++ /dev/null @@ -1,78 +0,0 @@ -# frozen_string_literal: true - -module Strategy - class RoundRobin - CacheError = Class.new(StandardError) - - COUNTER_EXPIRE_TIME = 86400 # one day - - def initialize(key, variants) - @key = key - @variants = variants - end - - def execute - increment_counter - resolve_variant_name - end - - # When the counter would expire - # - # @api private Used internally by SRE and debugging purpose - # @return [Integer] Number in seconds until expiration or false if never - def counter_expires_in - Gitlab::Redis::SharedState.with do |redis| - redis.ttl(key) - end - end - - # Return the actual counter value - # - # @return [Integer] value - def counter_value - Gitlab::Redis::SharedState.with do |redis| - (redis.get(key) || 0).to_i - end - end - - # Reset the counter - # - # @private Used internally by SRE and debugging purpose - # @return [Boolean] whether reset was a success - def reset! - redis_cmd do |redis| - redis.del(key) - end - end - - private - - attr_reader :key, :variants - - # Increase the counter - # - # @return [Boolean] whether operation was a success - def increment_counter - redis_cmd do |redis| - redis.incr(key) - redis.expire(key, COUNTER_EXPIRE_TIME) - end - end - - def resolve_variant_name - remainder = counter_value % variants.size - - variants[remainder] - end - - def redis_cmd - Gitlab::Redis::SharedState.with { |redis| yield(redis) } - - true - rescue CacheError => e - Gitlab::AppLogger.warn("GitLab: An unexpected error occurred in writing to Redis: #{e}") - - false - end - end -end diff --git a/app/services/boards/destroy_service.rb b/app/services/boards/destroy_service.rb index 8f3d4b58b7b..0b1cd61b119 100644 --- a/app/services/boards/destroy_service.rb +++ b/app/services/boards/destroy_service.rb @@ -3,7 +3,7 @@ module Boards class DestroyService < Boards::BaseService def execute(board) - if parent.boards.size == 1 + if boards.size == 1 return ServiceResponse.error(message: "The board could not be deleted, because the parent doesn't have any other boards.") end @@ -11,5 +11,11 @@ module Boards ServiceResponse.success end + + private + + def boards + parent.boards + end end end diff --git a/app/views/projects/settings/_archive.html.haml b/app/views/projects/settings/_archive.html.haml index 4300ebb4852..5e0f24cea21 100644 --- a/app/views/projects/settings/_archive.html.haml +++ b/app/views/projects/settings/_archive.html.haml @@ -11,7 +11,7 @@ %p= _("Unarchiving the project will restore its members' ability to make changes to it. The repository can be committed to, and issues, comments, and other entities can be created. %{strong_start}Once active, this project shows up in the search and on the dashboard.%{strong_end} %{link_start}Learn more.%{link_end}").html_safe % { strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe, link_start: link_start, link_end: '</a>'.html_safe } = link_to _('Unarchive project'), unarchive_project_path(@project), data: { confirm: _("Are you sure that you want to unarchive this project?"), qa_selector: 'unarchive_project_link' }, - method: :post, class: "gl-button btn btn-success" + method: :post, class: "gl-button btn btn-confirm" - else - link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: help_page_path('user/project/settings/index', anchor: 'archiving-a-project') } %p= _("Archiving the project will make it entirely read only. It is hidden from the dashboard and doesn't show up in searches. %{strong_start}The repository cannot be committed to, and no issues, comments, or other entities can be created.%{strong_end} %{link_start}Learn more.%{link_end}").html_safe % { strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe, link_start: link_start, link_end: '</a>'.html_safe } diff --git a/app/views/projects/settings/_general.html.haml b/app/views/projects/settings/_general.html.haml index d1a95886115..6d74ff2305f 100644 --- a/app/views/projects/settings/_general.html.haml +++ b/app/views/projects/settings/_general.html.haml @@ -39,4 +39,4 @@ %hr = link_to _('Remove avatar'), project_avatar_path(@project), data: { confirm: _('Avatar will be removed. Are you sure?')}, method: :delete, class: 'btn btn-link' - = f.submit _('Save changes'), class: "gl-button btn btn-success gl-mt-6", data: { qa_selector: 'save_naming_topics_avatar_button' } + = f.submit _('Save changes'), class: "gl-button btn btn-confirm gl-mt-6", data: { qa_selector: 'save_naming_topics_avatar_button' } diff --git a/app/views/projects/settings/ci_cd/_autodevops_form.html.haml b/app/views/projects/settings/ci_cd/_autodevops_form.html.haml index 5f79dd3d4bb..68e4bed8b9a 100644 --- a/app/views/projects/settings/ci_cd/_autodevops_form.html.haml +++ b/app/views/projects/settings/ci_cd/_autodevops_form.html.haml @@ -54,4 +54,4 @@ = s_('CICD|Automatic deployment to staging, manual deployment to production') = link_to sprite_icon('question-o'), help_page_path('topics/autodevops/customize.md', anchor: 'incremental-rollout-to-production'), target: '_blank' - = f.submit _('Save changes'), class: "btn gl-button btn-success gl-mt-5", data: { qa_selector: 'save_changes_button' } + = f.submit _('Save changes'), class: "btn gl-button btn-confirm gl-mt-5", data: { qa_selector: 'save_changes_button' } diff --git a/app/views/projects/settings/ci_cd/_form.html.haml b/app/views/projects/settings/ci_cd/_form.html.haml index 3b0073848a6..c4b5c23be13 100644 --- a/app/views/projects/settings/ci_cd/_form.html.haml +++ b/app/views/projects/settings/ci_cd/_form.html.haml @@ -96,7 +96,7 @@ = html_escape(_('The regular expression used to find test coverage output in the job log. For example, use %{regex} for Simplecov (Ruby). Leave blank to disable.')) % { regex: '<code>\(\d+.\d+%\)</code>'.html_safe } = link_to sprite_icon('question-o'), help_page_path('ci/pipelines/settings', anchor: 'test-coverage-parsing'), target: '_blank' - = f.submit _('Save changes'), class: "btn gl-button btn-success", data: { qa_selector: 'save_general_pipelines_changes_button' } + = f.submit _('Save changes'), class: "btn gl-button btn-confirm", data: { qa_selector: 'save_general_pipelines_changes_button' } %hr diff --git a/app/views/projects/settings/operations/_configuration_banner.html.haml b/app/views/projects/settings/operations/_configuration_banner.html.haml index 888625689f1..8551aa5380e 100644 --- a/app/views/projects/settings/operations/_configuration_banner.html.haml +++ b/app/views/projects/settings/operations/_configuration_banner.html.haml @@ -21,4 +21,4 @@ .col-sm-10 %p.gl-mt-3 = s_('PrometheusService|Monitor your project’s environments by deploying and configuring Prometheus on your clusters.') - = link_to s_('PrometheusService|Install Prometheus on clusters'), project_clusters_path(project), class: 'btn btn-success' + = link_to s_('PrometheusService|Install Prometheus on clusters'), project_clusters_path(project), class: 'gl-button btn btn-confirm' diff --git a/app/views/projects/settings/operations/_tracing.html.haml b/app/views/projects/settings/operations/_tracing.html.haml index 7a597b309c8..8bc9a383a73 100644 --- a/app/views/projects/settings/operations/_tracing.html.haml +++ b/app/views/projects/settings/operations/_tracing.html.haml @@ -30,4 +30,4 @@ - link_start_tag = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: jaeger_help_url } - link_end_tag = "#{sprite_icon('external-link', css_class: 'ml-1 vertical-align-middle')}</a>".html_safe = _("For more information, please review %{link_start_tag}Jaeger's configuration doc%{link_end_tag}").html_safe % { link_start_tag: link_start_tag, link_end_tag: link_end_tag } - = f.submit _('Save changes'), class: 'btn btn-success' + = f.submit _('Save changes'), class: 'gl-button btn btn-confirm' diff --git a/changelogs/unreleased/btn-confirm-projects-settings.yml b/changelogs/unreleased/btn-confirm-projects-settings.yml new file mode 100644 index 00000000000..84531f61c07 --- /dev/null +++ b/changelogs/unreleased/btn-confirm-projects-settings.yml @@ -0,0 +1,5 @@ +--- +title: Move from btn-success to btn-confirm in projects/settings directory +merge_request: 56938 +author: Yogi (@yo) +type: changed diff --git a/config/feature_flags/development/gitlab_org_sitemap.yml b/config/feature_flags/development/gitlab_org_sitemap.yml deleted file mode 100644 index e0f10124d58..00000000000 --- a/config/feature_flags/development/gitlab_org_sitemap.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: gitlab_org_sitemap -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46661 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/276915 -milestone: '13.6' -type: development -group: group::editor -default_enabled: false diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 3c258841607..7dd29c05d89 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -2372,6 +2372,16 @@ Autogenerated return type of DestroyContainerRepositoryTags. | `deletedTagNames` | [`[String!]!`](#string) | Deleted container repository tags. | | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +### `DestroyEpicBoardPayload` + +Autogenerated return type of DestroyEpicBoard. + +| Field | Type | Description | +| ----- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `epicBoard` | [`EpicBoard`](#epicboard) | Epic board after mutation. | +| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | + ### `DestroyNotePayload` Autogenerated return type of DestroyNote. diff --git a/spec/experiments/members/invite_email_experiment_spec.rb b/spec/experiments/members/invite_email_experiment_spec.rb index 539230e39b9..a9a269347e0 100644 --- a/spec/experiments/members/invite_email_experiment_spec.rb +++ b/spec/experiments/members/invite_email_experiment_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Members::InviteEmailExperiment do +RSpec.describe Members::InviteEmailExperiment, :clean_gitlab_redis_shared_state do subject(:invite_email) { experiment('members/invite_email', **context) } let(:context) { { actor: double('Member', created_by: double('User', avatar_url: '_avatar_url_')) } } @@ -23,7 +23,7 @@ RSpec.describe Members::InviteEmailExperiment do end end - describe "variant resolution", :clean_gitlab_redis_shared_state do + describe "variant resolution" do it "proves out round robin in variant selection", :aggregate_failures do instance_1 = described_class.new('members/invite_email', **context) allow(instance_1).to receive(:enabled?).and_return(true) @@ -45,4 +45,69 @@ RSpec.describe Members::InviteEmailExperiment do expect(instance_3.variant.name).to eq('avatar') end end + + describe Members::RoundRobin do + subject(:round_robin) { Members::RoundRobin.new('_key_', %i[variant1 variant2]) } + + describe "execute" do + context "when there are 2 variants" do + it "proves out round robin in selection", :aggregate_failures do + expect(round_robin.execute).to eq :variant2 + expect(round_robin.execute).to eq :variant1 + expect(round_robin.execute).to eq :variant2 + end + end + + context "when there are more than 2 variants" do + subject(:round_robin) { Members::RoundRobin.new('_key_', %i[variant1 variant2 variant3]) } + + it "proves out round robin in selection", :aggregate_failures do + expect(round_robin.execute).to eq :variant2 + expect(round_robin.execute).to eq :variant3 + expect(round_robin.execute).to eq :variant1 + + expect(round_robin.execute).to eq :variant2 + expect(round_robin.execute).to eq :variant3 + expect(round_robin.execute).to eq :variant1 + end + end + + context "when writing to cache fails" do + subject(:round_robin) { Members::RoundRobin.new('_key_', []) } + + it "raises an error and logs" do + allow(Gitlab::Redis::SharedState).to receive(:with).and_raise(Members::RoundRobin::CacheError) + expect(Gitlab::AppLogger).to receive(:warn) + + expect { round_robin.execute }.to raise_error(Members::RoundRobin::CacheError) + end + end + end + + describe "#counter_expires_in" do + it 'displays the expiration time in seconds' do + round_robin.execute + + expect(round_robin.counter_expires_in).to be_between(0, described_class::COUNTER_EXPIRE_TIME) + end + end + + describe '#value' do + it 'get the count' do + expect(round_robin.counter_value).to eq(0) + + round_robin.execute + + expect(round_robin.counter_value).to eq(1) + end + end + + describe '#reset!' do + it 'resets the count down to zero' do + 3.times { round_robin.execute } + + expect { round_robin.reset! }.to change { round_robin.counter_value }.from(3).to(0) + end + end + end end diff --git a/spec/experiments/strategy/round_robin_spec.rb b/spec/experiments/strategy/round_robin_spec.rb deleted file mode 100644 index f837a4701b2..00000000000 --- a/spec/experiments/strategy/round_robin_spec.rb +++ /dev/null @@ -1,68 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Strategy::RoundRobin, :clean_gitlab_redis_shared_state do - subject(:round_robin) { described_class.new('_key_', %i[variant1 variant2]) } - - describe "execute" do - context "when there are 2 variants" do - it "proves out round robin in selection", :aggregate_failures do - expect(round_robin.execute).to eq :variant2 - expect(round_robin.execute).to eq :variant1 - expect(round_robin.execute).to eq :variant2 - end - end - - context "when there are more than 2 variants" do - subject(:round_robin) { described_class.new('_key_', %i[variant1 variant2 variant3]) } - - it "proves out round robin in selection", :aggregate_failures do - expect(round_robin.execute).to eq :variant2 - expect(round_robin.execute).to eq :variant3 - expect(round_robin.execute).to eq :variant1 - - expect(round_robin.execute).to eq :variant2 - expect(round_robin.execute).to eq :variant3 - expect(round_robin.execute).to eq :variant1 - end - end - - context "when writing to cache fails" do - subject(:round_robin) { described_class.new('_key_', []) } - - it "raises an error and logs" do - allow(Gitlab::Redis::SharedState).to receive(:with).and_raise(Strategy::RoundRobin::CacheError) - expect(Gitlab::AppLogger).to receive(:warn) - - expect { round_robin.execute }.to raise_error(Strategy::RoundRobin::CacheError) - end - end - end - - describe "#counter_expires_in" do - it 'displays the expiration time in seconds' do - round_robin.execute - - expect(round_robin.counter_expires_in).to be_between(0, described_class::COUNTER_EXPIRE_TIME) - end - end - - describe '#value' do - it 'get the count' do - expect(round_robin.counter_value).to eq(0) - - round_robin.execute - - expect(round_robin.counter_value).to eq(1) - end - end - - describe '#reset!' do - it 'resets the count down to zero' do - 3.times { round_robin.execute } - - expect { round_robin.reset! }.to change { round_robin.counter_value }.from(3).to(0) - end - end -end diff --git a/spec/services/boards/destroy_service_spec.rb b/spec/services/boards/destroy_service_spec.rb new file mode 100644 index 00000000000..ec0d264d726 --- /dev/null +++ b/spec/services/boards/destroy_service_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Boards::DestroyService do + context 'with project board' do + let_it_be(:parent) { create(:project) } + let(:boards) { parent.boards } + let(:board_factory) { :board } + + it_behaves_like 'board destroy service' + end + + context 'with group board' do + let_it_be(:parent) { create(:group) } + let(:boards) { parent.boards } + let(:board_factory) { :board } + + it_behaves_like 'board destroy service' + end +end diff --git a/spec/support/shared_examples/boards/destroy_service_shared_examples.rb b/spec/support/shared_examples/boards/destroy_service_shared_examples.rb new file mode 100644 index 00000000000..33bae3da44b --- /dev/null +++ b/spec/support/shared_examples/boards/destroy_service_shared_examples.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'board destroy service' do + describe '#execute' do + let(:parent_type) { parent.is_a?(Project) ? :project : :group } + let!(:board) { create(board_factory, parent_type => parent) } + + subject(:service) { described_class.new(parent, double) } + + context 'when there is more than one board' do + let!(:board2) { create(board_factory, parent_type => parent) } + + it 'destroys the board' do + create(board_factory, parent_type => parent) + + expect do + expect(service.execute(board)).to be_success + end.to change(boards, :count).by(-1) + end + end + + context 'when there is only one board' do + it 'does not remove board' do + expect do + expect(service.execute(board)).to be_error + end.not_to change(boards, :count) + end + end + end +end |