diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-20 09:11:13 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-20 09:11:13 +0300 |
commit | 24e54a8f10e88aafa48c8a7dc548576939e6612b (patch) | |
tree | fe5d0061aa21a8d80511b70a48c4579233d458b1 | |
parent | 1f4126278ce0310e11bcc607dafff5ab462dca2d (diff) |
Add latest changes from gitlab-org/gitlab@master
-rw-r--r-- | db/post_migrate/20230215213349_add_sync_index_on_packages_package_file_filename.rb | 21 | ||||
-rw-r--r-- | db/schema_migrations/20230215213349 | 1 | ||||
-rw-r--r-- | db/structure.sql | 2 | ||||
-rw-r--r-- | doc/development/redis/new_redis_instance.md | 64 | ||||
-rw-r--r-- | lib/gitlab/redis/multi_store.rb | 103 | ||||
-rw-r--r-- | spec/features/abuse_report_spec.rb | 19 | ||||
-rw-r--r-- | spec/frontend/repository/components/preview/__snapshots__/index_spec.js.snap | 42 | ||||
-rw-r--r-- | spec/frontend/repository/components/preview/index_spec.js | 93 | ||||
-rw-r--r-- | spec/lib/gitlab/redis/multi_store_spec.rb | 219 | ||||
-rw-r--r-- | spec/requests/projects/google_cloud/deployments_controller_spec.rb | 112 | ||||
-rw-r--r-- | spec/support/helpers/gitaly_setup.rb | 17 | ||||
-rw-r--r-- | spec/support/shared_examples/features/abuse_report_shared_examples.rb | 14 |
12 files changed, 268 insertions, 439 deletions
diff --git a/db/post_migrate/20230215213349_add_sync_index_on_packages_package_file_filename.rb b/db/post_migrate/20230215213349_add_sync_index_on_packages_package_file_filename.rb new file mode 100644 index 00000000000..9a2e7abbbc1 --- /dev/null +++ b/db/post_migrate/20230215213349_add_sync_index_on_packages_package_file_filename.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddSyncIndexOnPackagesPackageFileFilename < Gitlab::Database::Migration[2.1] + INDEX_NAME = 'index_packages_package_files_on_file_name' + + disable_ddl_transaction! + + def up + add_concurrent_index( + :packages_package_files, + :file_name, + name: INDEX_NAME, + using: :gin, + opclass: { description: :gin_trgm_ops } + ) + end + + def down + remove_concurrent_index_by_name :packages_package_files, INDEX_NAME + end +end diff --git a/db/schema_migrations/20230215213349 b/db/schema_migrations/20230215213349 new file mode 100644 index 00000000000..0512fd3c7ea --- /dev/null +++ b/db/schema_migrations/20230215213349 @@ -0,0 +1 @@ +9b8521de286e8c363497c7854c530c7fcaf5aecb193a89addf7e15704ae271f9
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 62794dc529d..7e394f6bb7e 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -31061,6 +31061,8 @@ CREATE INDEX index_packages_package_file_build_infos_on_package_file_id ON packa CREATE INDEX index_packages_package_file_build_infos_on_pipeline_id ON packages_package_file_build_infos USING btree (pipeline_id); +CREATE INDEX index_packages_package_files_on_file_name ON packages_package_files USING gin (file_name gin_trgm_ops); + CREATE INDEX index_packages_package_files_on_file_store ON packages_package_files USING btree (file_store); CREATE INDEX index_packages_package_files_on_id_for_cleanup ON packages_package_files USING btree (id) WHERE (status = 1); diff --git a/doc/development/redis/new_redis_instance.md b/doc/development/redis/new_redis_instance.md index 0a030a0877f..c1f69f4d103 100644 --- a/doc/development/redis/new_redis_instance.md +++ b/doc/development/redis/new_redis_instance.md @@ -177,19 +177,15 @@ bin/feature-flag use_primary_store_as_default_for_foo ``` By enabling `use_primary_and_secondary_stores_for_foo` feature flag, our `Gitlab::Redis::Foo` will use `MultiStore` to write to both new Redis instance -and the [old (fallback-instance)](#fallback-instance). -If we fail to fetch data from the new instance, we will fallback and read from the old Redis instance. -We can monitor logs for `Gitlab::Redis::MultiStore::ReadFromPrimaryError`, and also the Prometheus counter `gitlab_redis_multi_store_read_fallback_total`. +and the [old (fallback-instance)](#fallback-instance). All read commands are performed only on the default store which is controlled using the +`use_primary_store_as_default_for_foo` feature flag. By enabling `use_primary_store_as_default_for_foo` feature flag, +the `MultiStore` uses `primary_store` (new instance) as default Redis store. For `pipelined` commands (`pipelined` and `multi`), we execute the entire operation in both stores and then compare the results. If they differ, we emit a `Gitlab::Redis::MultiStore:PipelinedDiffError` error, and track it in the `gitlab_redis_multi_store_pipelined_diff_error_total` Prometheus counter. -Once we stop seeing those errors, this means that we are no longer relying on the data stored on the old Redis store. -At this point, we are probably safe to move the traffic to the new Redis store. - -By enabling `use_primary_store_as_default_for_foo` feature flag, the `MultiStore` will use `primary_store` (new instance) as default Redis store. - -Once this feature flag is enabled, we can disable `use_primary_and_secondary_stores_for_foo` feature flag. +After a period of time for the new store to be populated, we can perform external validation to compare the state of both stores. +Upon satisfactory validation results, we are probably safe to move the traffic to the new Redis store. We can disable `use_primary_and_secondary_stores_for_foo` feature flag. This will allow the MultiStore to read and write only from the primary Redis store (new store), moving all the traffic to the new Redis store. Once we have moved all our traffic to the primary store, our data migration is complete. @@ -206,17 +202,44 @@ MultiStore implements read and write Redis commands separately. - `smembers` - `scard` +- 'exists' +- 'exists?' +- 'get' +- 'hexists' +- 'hget' +- 'hgetall' +- 'hlen' +- 'hmget' +- 'hscan_each' +- 'mapped_hmget' +- 'mget' +- 'scan_each' +- 'scard' +- 'sismember' +- 'smembers' +- 'sscan' +- 'sscan_each' +- 'ttl' +- 'zscan_each' + ##### Write commands -- `set` -- `setnx` -- `setex` -- `sadd` -- `srem` -- `del` -- `pipelined` -- `flushdb` -- `rpush` +- 'del' +- 'eval' +- 'expire' +- 'flushdb' +- 'hdel' +- 'hset' +- 'incr' +- 'incrby' +- 'mapped_hmset' +- 'rpush' +- 'sadd' +- 'set' +- 'setex' +- 'setnx' +- 'srem' +- 'unlink' ##### `pipelined` commands @@ -227,7 +250,8 @@ Thus, excluding the Redis operations performed, the block should be idempotent. - `multi` When a command outside of the supported list is used, `method_missing` will pass it to the old Redis instance and keep track of it. -This ensures that anything unexpected behaves like it would before. +This ensures that anything unexpected behaves like it would before. In development or test environment, an error would be raised for early +detection. NOTE: By tracking `gitlab_redis_multi_store_method_missing_total` counter and `Gitlab::Redis::MultiStore::MethodMissingError`, @@ -237,7 +261,6 @@ a developer will need to add an implementation for missing Redis commands before | error | message | |---------------------------------------------------|---------------------------------------------------------------------------------------------| -| `Gitlab::Redis::MultiStore::ReadFromPrimaryError` | Value not found on the Redis primary store. Read from the Redis secondary store successful. | | `Gitlab::Redis::MultiStore::PipelinedDiffError` | `pipelined` command executed on both stores successfully but results differ between them. | | `Gitlab::Redis::MultiStore::MethodMissingError` | Method missing. Falling back to execute method on the Redis secondary store. | @@ -245,7 +268,6 @@ a developer will need to add an implementation for missing Redis commands before | Metrics name | Type | Labels | Description | |-------------------------------------------------------|--------------------|----------------------------|----------------------------------------------------------| -| `gitlab_redis_multi_store_read_fallback_total` | Prometheus Counter | `command`, `instance_name` | Client side Redis MultiStore reading fallback total | | `gitlab_redis_multi_store_pipelined_diff_error_total` | Prometheus Counter | `command`, `instance_name` | Redis MultiStore `pipelined` command diff between stores | | `gitlab_redis_multi_store_method_missing_total` | Prometheus Counter | `command`, `instance_name` | Client side Redis MultiStore method missing total | diff --git a/lib/gitlab/redis/multi_store.rb b/lib/gitlab/redis/multi_store.rb index a102267d52b..9571e2f92e6 100644 --- a/lib/gitlab/redis/multi_store.rb +++ b/lib/gitlab/redis/multi_store.rb @@ -5,12 +5,6 @@ module Gitlab class MultiStore include Gitlab::Utils::StrongMemoize - class ReadFromPrimaryError < StandardError - def message - 'Value not found on the redis primary store. Read from the redis secondary store successful.' - end - end - class PipelinedDiffError < StandardError def initialize(result_primary, result_secondary) @result_primary = result_primary @@ -32,41 +26,33 @@ module Gitlab attr_reader :primary_store, :secondary_store, :instance_name - FAILED_TO_READ_ERROR_MESSAGE = 'Failed to read from the redis primary_store.' + FAILED_TO_READ_ERROR_MESSAGE = 'Failed to read from the redis default_store.' FAILED_TO_WRITE_ERROR_MESSAGE = 'Failed to write to the redis primary_store.' FAILED_TO_RUN_PIPELINE = 'Failed to execute pipeline on the redis primary_store.' SKIP_LOG_METHOD_MISSING_FOR_COMMANDS = %i[info].freeze - # For ENUMERATOR_CACHE_HIT_VALIDATOR and READ_CACHE_HIT_VALIDATOR, - # we define procs to validate cache hit. The only other acceptable value is nil, - # in the case of errors being raised. - # - # If a command has no empty response, set ->(val) { true } - # - # Ref: https://www.rubydoc.info/github/redis/redis-rb/Redis/Commands - # - READ_CACHE_HIT_VALIDATOR = { - exists: ->(val) { val != 0 }, - exists?: ->(val) { val }, - get: ->(val) { !val.nil? }, - hexists: ->(val) { val }, - hget: ->(val) { !val.nil? }, - hgetall: ->(val) { val.is_a?(Hash) && !val.empty? }, - hlen: ->(val) { val != 0 }, - hmget: ->(val) { val.is_a?(Array) && !val.compact.empty? }, - hscan_each: ->(val) { val.is_a?(Enumerator) && !val.first.nil? }, - mapped_hmget: ->(val) { val.is_a?(Hash) && !val.compact.empty? }, - mget: ->(val) { val.is_a?(Array) && !val.compact.empty? }, - scan_each: ->(val) { val.is_a?(Enumerator) && !val.first.nil? }, - scard: ->(val) { val != 0 }, - sismember: ->(val) { val }, - smembers: ->(val) { val.is_a?(Array) && !val.empty? }, - sscan: ->(val) { val != ['0', []] }, - sscan_each: ->(val) { val.is_a?(Enumerator) && !val.first.nil? }, - ttl: ->(val) { val != 0 && val != -2 }, # ttl returns -2 if the key does not exist. See https://redis.io/commands/ttl/ - zscan_each: ->(val) { val.is_a?(Enumerator) && !val.first.nil? } - }.freeze + READ_COMMANDS = %i[ + exists + exists? + get + hexists + hget + hgetall + hlen + hmget + hscan_each + mapped_hmget + mget + scan_each + scard + sismember + smembers + sscan + sscan_each + ttl + zscan_each + ].freeze WRITE_COMMANDS = %i[ del @@ -111,7 +97,7 @@ module Gitlab end # rubocop:disable GitlabSecurity/PublicSend - READ_CACHE_HIT_VALIDATOR.each_key do |name| + READ_COMMANDS.each do |name| define_method(name) do |*args, **kwargs, &block| if use_primary_and_secondary_stores? read_command(name, *args, **kwargs, &block) @@ -186,12 +172,6 @@ module Gitlab @pipelined_command_error.increment(command: command_name, instance_name: instance_name) end - def increment_read_fallback_count(command_name) - @read_fallback_counter ||= Gitlab::Metrics.counter(:gitlab_redis_multi_store_read_fallback_total, - 'Client side Redis MultiStore reading fallback') - @read_fallback_counter.increment(command: command_name, instance_name: instance_name) - end - def increment_method_missing_count(command_name) @method_missing_counter ||= Gitlab::Metrics.counter(:gitlab_redis_multi_store_method_missing_total, 'Client side Redis MultiStore method missing') @@ -247,7 +227,7 @@ module Gitlab if @instance send_command(@instance, command_name, *args, **kwargs, &block) else - read_one_with_fallback(command_name, *args, **kwargs, &block) + read_from_default(command_name, *args, **kwargs, &block) end end @@ -259,35 +239,12 @@ module Gitlab end end - def read_one_with_fallback(command_name, *args, **kwargs, &block) - begin - value = send_command(default_store, command_name, *args, **kwargs, &block) - rescue StandardError => e - log_error(e, command_name, - multi_store_error_message: FAILED_TO_READ_ERROR_MESSAGE) - end - - return value if block.nil? && cache_hit?(command_name, value) - - fallback_read(command_name, *args, **kwargs, &block) - end - - def cache_hit?(command, value) - validator = READ_CACHE_HIT_VALIDATOR[command] - return false unless validator - - !value.nil? && validator.call(value) - end - - def fallback_read(command_name, *args, **kwargs, &block) - value = send_command(fallback_store, command_name, *args, **kwargs, &block) - - if value - log_error(ReadFromPrimaryError.new, command_name) - increment_read_fallback_count(command_name) - end - - value + def read_from_default(command_name, *args, **kwargs, &block) + send_command(default_store, command_name, *args, **kwargs, &block) + rescue StandardError => e + log_error(e, command_name, + multi_store_error_message: FAILED_TO_READ_ERROR_MESSAGE) + raise end def write_both(command_name, *args, **kwargs, &block) diff --git a/spec/features/abuse_report_spec.rb b/spec/features/abuse_report_spec.rb index 1267025a7bf..474ab4c7b8e 100644 --- a/spec/features/abuse_report_spec.rb +++ b/spec/features/abuse_report_spec.rb @@ -15,25 +15,6 @@ RSpec.describe 'Abuse reports', :js, feature_category: :insider_threat do end describe 'report abuse to administrator' do - shared_examples 'reports the user with an abuse category' do - it do - fill_and_submit_abuse_category_form - fill_and_submit_report_abuse_form - - expect(page).to have_content 'Thank you for your report' - end - end - - shared_examples 'reports the user without an abuse category' do - it do - click_link 'Report abuse to administrator' - - fill_and_submit_report_abuse_form - - expect(page).to have_content 'Thank you for your report' - end - end - context 'when reporting an issue for abuse' do before do visit project_issue_path(project, issue) diff --git a/spec/frontend/repository/components/preview/__snapshots__/index_spec.js.snap b/spec/frontend/repository/components/preview/__snapshots__/index_spec.js.snap deleted file mode 100644 index 48a4feca1e5..00000000000 --- a/spec/frontend/repository/components/preview/__snapshots__/index_spec.js.snap +++ /dev/null @@ -1,42 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Repository file preview component renders file HTML 1`] = ` -<article - class="file-holder limited-width-container readme-holder" -> - <div - class="js-file-title file-title-flex-parent" - > - <div - class="file-header-content" - > - <gl-icon-stub - name="doc-text" - size="16" - /> - - <gl-link-stub - href="http://test.com" - > - <strong> - README.md - </strong> - </gl-link-stub> - </div> - </div> - - <div - class="blob-viewer" - data-qa-selector="blob_viewer_content" - itemprop="about" - > - <div> - <div - class="blob" - > - test - </div> - </div> - </div> -</article> -`; diff --git a/spec/frontend/repository/components/preview/index_spec.js b/spec/frontend/repository/components/preview/index_spec.js index d4c746b67d6..416cc7d360a 100644 --- a/spec/frontend/repository/components/preview/index_spec.js +++ b/spec/frontend/repository/components/preview/index_spec.js @@ -1,77 +1,64 @@ import { GlLoadingIcon } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; -import { nextTick } from 'vue'; +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createMockApollo from 'helpers/mock_apollo_helper'; import { handleLocationHash } from '~/lib/utils/common_utils'; +import waitForPromises from 'helpers/wait_for_promises'; import Preview from '~/repository/components/preview/index.vue'; +const PROPS_DATA = { + blob: { + webPath: 'http://test.com', + name: 'README.md', + }, +}; + +const MOCK_README_DATA = { + __typename: 'ReadmeFile', + html: '<div class="blob">test</div>', +}; + jest.mock('~/lib/utils/common_utils'); -let vm; -let $apollo; +Vue.use(VueApollo); + +let wrapper; +let mockApollo; +let mockReadmeData; -function factory(blob, loading) { - $apollo = { - queries: { - readme: { - query: jest.fn().mockReturnValue(Promise.resolve({})), - loading, - }, - }, - }; +const mockResolvers = { + Query: { + readme: () => mockReadmeData(), + }, +}; - vm = shallowMount(Preview, { - propsData: { - blob, - }, - mocks: { - $apollo, - }, +function createComponent() { + mockApollo = createMockApollo([], mockResolvers); + + return shallowMount(Preview, { + propsData: PROPS_DATA, + apolloProvider: mockApollo, }); } describe('Repository file preview component', () => { - afterEach(() => { - vm.destroy(); + beforeEach(() => { + mockReadmeData = jest.fn(); + wrapper = createComponent(); + mockReadmeData.mockResolvedValue(MOCK_README_DATA); }); - it('renders file HTML', async () => { - factory({ - webPath: 'http://test.com', - name: 'README.md', - }); - - // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details - // eslint-disable-next-line no-restricted-syntax - vm.setData({ readme: { html: '<div class="blob">test</div>' } }); - - await nextTick(); - expect(vm.element).toMatchSnapshot(); + afterEach(() => { + wrapper.destroy(); }); it('handles hash after render', async () => { - factory({ - webPath: 'http://test.com', - name: 'README.md', - }); - - // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details - // eslint-disable-next-line no-restricted-syntax - vm.setData({ readme: { html: '<div class="blob">test</div>' } }); - - await nextTick(); + await waitForPromises(); expect(handleLocationHash).toHaveBeenCalled(); }); it('renders loading icon', async () => { - factory( - { - webPath: 'http://test.com', - name: 'README.md', - }, - true, - ); - - await nextTick(); - expect(vm.findComponent(GlLoadingIcon).exists()).toBe(true); + expect(wrapper.findComponent(GlLoadingIcon).exists()).toBe(true); }); }); diff --git a/spec/lib/gitlab/redis/multi_store_spec.rb b/spec/lib/gitlab/redis/multi_store_spec.rb index 423a7e80ead..baf2546fc5c 100644 --- a/spec/lib/gitlab/redis/multi_store_spec.rb +++ b/spec/lib/gitlab/redis/multi_store_spec.rb @@ -210,47 +210,6 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do end end - RSpec.shared_examples_for 'fallback read from the non-default store' do - let(:counter) { Gitlab::Metrics::NullMetric.instance } - - before do - allow(Gitlab::Metrics).to receive(:counter).and_return(counter) - end - - it 'fallback and execute on secondary instance' do - expect(multi_store.fallback_store).to receive(name).with(*expected_args).and_call_original - - subject - end - - it 'logs the ReadFromPrimaryError' do - expect(Gitlab::ErrorTracking).to receive(:log_exception).with( - an_instance_of(Gitlab::Redis::MultiStore::ReadFromPrimaryError), - hash_including(command_name: name, instance_name: instance_name) - ) - - subject - end - - it 'increment read fallback count metrics' do - expect(counter).to receive(:increment).with(command: name, instance_name: instance_name) - - subject - end - - include_examples 'reads correct value' - - context 'when fallback read from the secondary instance raises an exception' do - before do - allow(multi_store.fallback_store).to receive(name).with(*expected_args).and_raise(StandardError) - end - - it 'fails with exception' do - expect { subject }.to raise_error(StandardError) - end - end - end - RSpec.shared_examples_for 'secondary store' do it 'execute on the secondary instance' do expect(secondary_store).to receive(name).with(*expected_args).and_call_original @@ -283,31 +242,21 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do subject end - unless params[:block] - it 'does not execute on the secondary store' do - expect(secondary_store).not_to receive(name) - - subject - end - end - include_examples 'reads correct value' end - context 'when reading from primary instance is raising an exception' do + context 'when reading from default instance is raising an exception' do before do allow(multi_store.default_store).to receive(name).with(*expected_args).and_raise(StandardError) allow(Gitlab::ErrorTracking).to receive(:log_exception) end - it 'logs the exception' do + it 'logs the exception and re-raises the error' do expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(StandardError), hash_including(:multi_store_error_message, instance_name: instance_name, command_name: name)) - subject + expect { subject }.to raise_error(an_instance_of(StandardError)) end - - include_examples 'fallback read from the non-default store' end context 'when reading from empty default instance' do @@ -316,7 +265,9 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do multi_store.default_store.flushdb end - include_examples 'fallback read from the non-default store' + it 'does not call the fallback store' do + expect(multi_store.fallback_store).not_to receive(name) + end end context 'when the command is executed within pipelined block' do @@ -346,16 +297,16 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do end context 'when block is provided' do - it 'both stores yields to the block' do + it 'only default store yields to the block' do expect(primary_store).to receive(name).and_yield(value) - expect(secondary_store).to receive(name).and_yield(value) + expect(secondary_store).not_to receive(name).and_yield(value) subject end - it 'both stores to execute' do + it 'only default store to execute' do expect(primary_store).to receive(name).with(*expected_args).and_call_original - expect(secondary_store).to receive(name).with(*expected_args).and_call_original + expect(secondary_store).not_to receive(name).with(*expected_args).and_call_original subject end @@ -421,27 +372,19 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do subject do multi_store.mget(values) do |v| multi_store.sadd(skey, v) - multi_store.scard(skey) - end - end - - RSpec.shared_examples_for 'primary instance executes block' do - it 'ensures primary instance is executing the block' do - expect(primary_store).to receive(:send).with(:mget, values).and_call_original - expect(primary_store).to receive(:send).with(:sadd, skey, %w[1 2 3]).and_call_original - expect(primary_store).to receive(:send).with(:scard, skey).and_call_original - - expect(secondary_store).to receive(:send).with(:mget, values).and_call_original - expect(secondary_store).to receive(:send).with(:sadd, skey, %w[10 20 30]).and_call_original - expect(secondary_store).to receive(:send).with(:scard, skey).and_call_original - - subject end end context 'when using both stores' do context 'when primary instance is default store' do - it_behaves_like 'primary instance executes block' + it 'ensures primary instance is executing the block' do + expect(primary_store).to receive(:send).with(:mget, values).and_call_original + expect(primary_store).to receive(:send).with(:sadd, skey, %w[1 2 3]).and_call_original + + expect(secondary_store).not_to receive(:send) + + subject + end end context 'when secondary instance is default store' do @@ -449,8 +392,14 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do stub_feature_flags(use_primary_store_as_default_for_test_store: false) end - # multistore read still favours the primary store - it_behaves_like 'primary instance executes block' + it 'ensures secondary instance is executing the block' do + expect(primary_store).not_to receive(:send) + + expect(secondary_store).to receive(:send).with(:mget, values).and_call_original + expect(secondary_store).to receive(:send).with(:sadd, skey, %w[10 20 30]).and_call_original + + subject + end end end @@ -465,7 +414,6 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do expect(primary_store).to receive(:send).with(:mget, values).and_call_original expect(primary_store).to receive(:send).with(:sadd, skey, %w[1 2 3]).and_call_original - expect(primary_store).to receive(:send).with(:scard, skey).and_call_original subject end @@ -479,7 +427,6 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do it 'ensures only secondary instance is executing the block' do expect(secondary_store).to receive(:send).with(:mget, values).and_call_original expect(secondary_store).to receive(:send).with(:sadd, skey, %w[10 20 30]).and_call_original - expect(secondary_store).to receive(:send).with(:scard, skey).and_call_original expect(primary_store).not_to receive(:send) @@ -668,120 +615,6 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do end # rubocop:enable RSpec/MultipleMemoizedHelpers - context 'with ENUMERATOR_COMMANDS redis commands' do - let_it_be(:hkey) { "redis:hash" } - let_it_be(:skey) { "redis:set" } - let_it_be(:zkey) { "redis:sortedset" } - let_it_be(:rvalue) { "value1" } - let_it_be(:scan_kwargs) { { match: 'redis:hash' } } - - where(:case_name, :name, :args, :kwargs) do - 'execute :scan_each command' | :scan_each | nil | ref(:scan_kwargs) - 'execute :sscan_each command' | :sscan_each | ref(:skey) | {} - 'execute :hscan_each command' | :hscan_each | ref(:hkey) | {} - 'execute :zscan_each command' | :zscan_each | ref(:zkey) | {} - end - - before(:all) do - primary_store.hset(hkey, rvalue, 1) - primary_store.sadd?(skey, rvalue) - primary_store.zadd(zkey, 1, rvalue) - - secondary_store.hset(hkey, rvalue, 1) - secondary_store.sadd?(skey, rvalue) - secondary_store.zadd(zkey, 1, rvalue) - end - - RSpec.shared_examples_for 'enumerator commands execution' do |both_stores, default_primary| - context 'without block passed in' do - subject do - multi_store.send(name, *args, **kwargs) - end - - it 'returns an enumerator' do - expect(subject).to be_instance_of(Enumerator) - end - end - - context 'with block passed in' do - subject do - multi_store.send(name, *args, **kwargs) { |key| multi_store.incr(rvalue) } - end - - it 'returns nil' do - expect(subject).to eq(nil) - end - - it 'runs block on correct Redis instance' do - if both_stores - expect(primary_store).to receive(name).with(*expected_args).and_call_original - expect(secondary_store).to receive(name).with(*expected_args).and_call_original - - expect(primary_store).to receive(:incr).with(rvalue) - expect(secondary_store).to receive(:incr).with(rvalue) - elsif default_primary - expect(primary_store).to receive(name).with(*expected_args).and_call_original - expect(primary_store).to receive(:incr).with(rvalue) - - expect(secondary_store).not_to receive(name) - expect(secondary_store).not_to receive(:incr).with(rvalue) - else - expect(secondary_store).to receive(name).with(*expected_args).and_call_original - expect(secondary_store).to receive(:incr).with(rvalue) - - expect(primary_store).not_to receive(name) - expect(primary_store).not_to receive(:incr).with(rvalue) - end - - subject - end - end - end - - with_them do - describe name.to_s do - let(:expected_args) { kwargs.present? ? [*args, { **kwargs }] : Array(args) } - - before do - allow(primary_store).to receive(name).and_call_original - allow(secondary_store).to receive(name).and_call_original - end - - context 'when only using 1 store' do - before do - stub_feature_flags(use_primary_and_secondary_stores_for_test_store: false) - end - - context 'when using secondary store as default' do - before do - stub_feature_flags(use_primary_store_as_default_for_test_store: false) - end - - it_behaves_like 'enumerator commands execution', false, false - end - - context 'when using primary store as default' do - it_behaves_like 'enumerator commands execution', false, true - end - end - - context 'when using both stores' do - context 'when using secondary store as default' do - before do - stub_feature_flags(use_primary_store_as_default_for_test_store: false) - end - - it_behaves_like 'enumerator commands execution', true, false - end - - context 'when using primary store as default' do - it_behaves_like 'enumerator commands execution', true, true - end - end - end - end - end - RSpec.shared_examples_for 'pipelined command' do |name| let_it_be(:key1) { "redis:{1}:key_a" } let_it_be(:value1) { "redis_value1" } diff --git a/spec/requests/projects/google_cloud/deployments_controller_spec.rb b/spec/requests/projects/google_cloud/deployments_controller_spec.rb index d564a31f835..14214b8fdfb 100644 --- a/spec/requests/projects/google_cloud/deployments_controller_spec.rb +++ b/spec/requests/projects/google_cloud/deployments_controller_spec.rb @@ -108,66 +108,104 @@ RSpec.describe Projects::GoogleCloud::DeploymentsController, feature_category: : end end - it 'redirects to google cloud deployments on enable service error' do - get url - - expect(response).to redirect_to(project_google_cloud_deployments_path(project)) - # since GPC_PROJECT_ID is not set, enable cloud run service should return an error - expect_snowplow_event( - category: 'Projects::GoogleCloud::DeploymentsController', - action: 'error_enable_services', - label: nil, - project: project, - user: user_maintainer - ) - end + context 'when enable service fails' do + before do + allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |service| + allow(service) + .to receive(:execute) + .and_return( + status: :error, + message: 'No GCP projects found. Configure a service account or GCP_PROJECT_ID ci variable' + ) + end + end - it 'redirects to google cloud deployments with error' do - mock_gcp_error = Google::Apis::ClientError.new('some_error') + it 'redirects to google cloud deployments and tracks event on enable service error' do + get url - allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |service| - allow(service).to receive(:execute).and_raise(mock_gcp_error) + expect(response).to redirect_to(project_google_cloud_deployments_path(project)) + # since GPC_PROJECT_ID is not set, enable cloud run service should return an error + expect_snowplow_event( + category: 'Projects::GoogleCloud::DeploymentsController', + action: 'error_enable_services', + label: nil, + project: project, + user: user_maintainer + ) end - get url + it 'shows a flash alert' do + get url - expect(response).to redirect_to(project_google_cloud_deployments_path(project)) - expect_snowplow_event( - category: 'Projects::GoogleCloud::DeploymentsController', - action: 'error_google_api', - label: nil, - project: project, - user: user_maintainer - ) + expect(flash[:alert]) + .to eq('No GCP projects found. Configure a service account or GCP_PROJECT_ID ci variable') + end end - context 'GCP_PROJECT_IDs are defined' do - it 'redirects to google_cloud deployments on generate pipeline error' do - allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |enable_cloud_run_service| - allow(enable_cloud_run_service).to receive(:execute).and_return({ status: :success }) - end + context 'when enable service raises an error' do + before do + mock_gcp_error = Google::Apis::ClientError.new('some_error') - allow_next_instance_of(GoogleCloud::GeneratePipelineService) do |generate_pipeline_service| - allow(generate_pipeline_service).to receive(:execute).and_return({ status: :error }) + allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |service| + allow(service).to receive(:execute).and_raise(mock_gcp_error) end + end + it 'redirects to google cloud deployments with error' do get url expect(response).to redirect_to(project_google_cloud_deployments_path(project)) expect_snowplow_event( category: 'Projects::GoogleCloud::DeploymentsController', - action: 'error_generate_cloudrun_pipeline', + action: 'error_google_api', label: nil, project: project, user: user_maintainer ) end - it 'redirects to create merge request form' do - allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |service| - allow(service).to receive(:execute).and_return({ status: :success }) + it 'shows a flash warning' do + get url + + expect(flash[:warning]).to eq(format(_('Google Cloud Error - %{error}'), error: 'some_error')) + end + end + + context 'GCP_PROJECT_IDs are defined' do + before do + allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |enable_cloud_run_service| + allow(enable_cloud_run_service).to receive(:execute).and_return({ status: :success }) + end + end + + context 'when generate pipeline service fails' do + before do + allow_next_instance_of(GoogleCloud::GeneratePipelineService) do |generate_pipeline_service| + allow(generate_pipeline_service).to receive(:execute).and_return({ status: :error }) + end + end + + it 'redirects to google_cloud deployments and tracks event on generate pipeline error' do + get url + + expect(response).to redirect_to(project_google_cloud_deployments_path(project)) + expect_snowplow_event( + category: 'Projects::GoogleCloud::DeploymentsController', + action: 'error_generate_cloudrun_pipeline', + label: nil, + project: project, + user: user_maintainer + ) + end + + it 'shows a flash alert' do + get url + + expect(flash[:alert]).to eq('Failed to generate pipeline') end + end + it 'redirects to create merge request form' do allow_next_instance_of(GoogleCloud::GeneratePipelineService) do |service| allow(service).to receive(:execute).and_return({ status: :success }) end diff --git a/spec/support/helpers/gitaly_setup.rb b/spec/support/helpers/gitaly_setup.rb index 398a2a20f2f..bf3c67a1818 100644 --- a/spec/support/helpers/gitaly_setup.rb +++ b/spec/support/helpers/gitaly_setup.rb @@ -198,8 +198,23 @@ module GitalySetup end LOGGER.debug "Checking gitaly-ruby bundle...\n" + + bundle_install unless bundle_check + + abort 'bundle check failed' unless bundle_check + end + + def bundle_check + bundle_cmd('check') + end + + def bundle_install + bundle_cmd('install') + end + + def bundle_cmd(cmd) out = ENV['CI'] ? $stdout : '/dev/null' - abort 'bundle check failed' unless system(env, 'bundle', 'check', out: out, chdir: gemfile_dir) + system(env, 'bundle', cmd, out: out, chdir: gemfile_dir) end def connect_proc(toml) diff --git a/spec/support/shared_examples/features/abuse_report_shared_examples.rb b/spec/support/shared_examples/features/abuse_report_shared_examples.rb new file mode 100644 index 00000000000..7a520fb0cd2 --- /dev/null +++ b/spec/support/shared_examples/features/abuse_report_shared_examples.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'reports the user with an abuse category' do + it 'creates abuse report' do + click_button 'Report abuse to administrator' + choose "They're posting spam." + click_button 'Next' + + fill_in 'abuse_report_message', with: 'This user sends spam' + click_button 'Send report' + + expect(page).to have_content 'Thank you for your report' + end +end |