diff options
Diffstat (limited to 'spec/services/projects/container_repository')
-rw-r--r-- | spec/services/projects/container_repository/cache_tags_created_at_service_spec.rb | 133 | ||||
-rw-r--r-- | spec/services/projects/container_repository/cleanup_tags_service_spec.rb | 457 |
2 files changed, 212 insertions, 378 deletions
diff --git a/spec/services/projects/container_repository/cache_tags_created_at_service_spec.rb b/spec/services/projects/container_repository/cache_tags_created_at_service_spec.rb deleted file mode 100644 index dfe2ff9e57c..00000000000 --- a/spec/services/projects/container_repository/cache_tags_created_at_service_spec.rb +++ /dev/null @@ -1,133 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ::Projects::ContainerRepository::CacheTagsCreatedAtService, :clean_gitlab_redis_cache do - let_it_be(:dummy_tag_class) { Struct.new(:name, :created_at) } - let_it_be(:repository) { create(:container_repository) } - - let(:tags) { create_tags(5) } - let(:service) { described_class.new(repository) } - - shared_examples 'not interacting with redis' do - it 'does not interact with redis' do - expect(::Gitlab::Redis::Cache).not_to receive(:with) - - subject - end - end - - describe '#populate' do - subject { service.populate(tags) } - - context 'with tags' do - it 'gets values from redis' do - expect(::Gitlab::Redis::Cache).to receive(:with).and_call_original - - expect(subject).to eq(0) - - tags.each { |t| expect(t.created_at).to eq(nil) } - end - - context 'with cached values' do - let(:cached_tags) { tags.first(2) } - - before do - ::Gitlab::Redis::Cache.with do |redis| - cached_tags.each do |tag| - redis.set(cache_key(tag), rfc3339(10.days.ago)) - end - end - end - - it 'gets values from redis' do - expect(::Gitlab::Redis::Cache).to receive(:with).and_call_original - - expect(subject).to eq(2) - - cached_tags.each { |t| expect(t.created_at).not_to eq(nil) } - (tags - cached_tags).each { |t| expect(t.created_at).to eq(nil) } - end - end - end - - context 'with no tags' do - let(:tags) { [] } - - it_behaves_like 'not interacting with redis' - end - end - - describe '#insert' do - let(:max_ttl) { 90.days } - - subject { service.insert(tags, max_ttl) } - - context 'with tags' do - let(:tag) { tags.first } - let(:ttl) { 90.days - 3.days } - - before do - travel_to(Time.zone.local(2021, 9, 2, 12, 0, 0)) - - tag.created_at = DateTime.rfc3339(3.days.ago.rfc3339) - end - - after do - travel_back - end - - it 'inserts values in redis' do - ::Gitlab::Redis::Cache.with do |redis| - expect(redis) - .to receive(:set) - .with(cache_key(tag), rfc3339(tag.created_at), ex: ttl.to_i) - .and_call_original - end - - subject - end - - context 'with some of them already cached' do - let(:tag) { tags.first } - - before do - ::Gitlab::Redis::Cache.with do |redis| - redis.set(cache_key(tag), rfc3339(10.days.ago)) - end - service.populate(tags) - end - - it_behaves_like 'not interacting with redis' - end - end - - context 'with no tags' do - let(:tags) { [] } - - it_behaves_like 'not interacting with redis' - end - - context 'with no expires_in' do - let(:max_ttl) { nil } - - it_behaves_like 'not interacting with redis' - end - end - - def create_tags(size) - Array.new(size) do |i| - dummy_tag_class.new("Tag #{i}", nil) - end - end - - def cache_key(tag) - "container_repository:{#{repository.id}}:tag:#{tag.name}:created_at" - end - - def rfc3339(date_time) - # DateTime rfc3339 is different ActiveSupport::TimeWithZone rfc3339 - # The caching will use DateTime rfc3339 - DateTime.rfc3339(date_time.rfc3339).rfc3339 - end -end diff --git a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb index 289bbf4540e..a41ba8216cc 100644 --- a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb +++ b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb @@ -41,322 +41,320 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_ describe '#execute' do subject { service.execute } - shared_examples 'reading and removing tags' do |caching_enabled: true| - context 'when no params are specified' do - let(:params) { {} } + context 'when no params are specified' do + let(:params) { {} } - it 'does not remove anything' do - expect_any_instance_of(Projects::ContainerRepository::DeleteTagsService) - .not_to receive(:execute) - expect_no_caching + it 'does not remove anything' do + expect_any_instance_of(Projects::ContainerRepository::DeleteTagsService) + .not_to receive(:execute) + expect_no_caching - is_expected.to eq(expected_service_response(before_truncate_size: 0, after_truncate_size: 0, before_delete_size: 0)) - end + is_expected.to eq(expected_service_response(before_truncate_size: 0, after_truncate_size: 0, before_delete_size: 0)) end + end - context 'when regex matching everything is specified' do - shared_examples 'removes all matches' do - it 'does remove all tags except latest' do - expect_no_caching + context 'when regex matching everything is specified' do + shared_examples 'removes all matches' do + it 'does remove all tags except latest' do + expect_no_caching - expect_delete(%w(A Ba Bb C D E)) + expect_delete(%w(A Ba Bb C D E)) - is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C D E))) - end + is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C D E))) end + end + + let(:params) do + { 'name_regex_delete' => '.*' } + end + it_behaves_like 'removes all matches' + + context 'with deprecated name_regex param' do let(:params) do - { 'name_regex_delete' => '.*' } + { 'name_regex' => '.*' } end it_behaves_like 'removes all matches' + end + end - context 'with deprecated name_regex param' do - let(:params) do - { 'name_regex' => '.*' } - end + context 'with invalid regular expressions' do + shared_examples 'handling an invalid regex' do + it 'keeps all tags' do + expect_no_caching - it_behaves_like 'removes all matches' + expect(Projects::ContainerRepository::DeleteTagsService) + .not_to receive(:new) + + subject end - end - context 'with invalid regular expressions' do - shared_examples 'handling an invalid regex' do - it 'keeps all tags' do - expect_no_caching + it { is_expected.to eq(status: :error, message: 'invalid regex') } - expect(Projects::ContainerRepository::DeleteTagsService) - .not_to receive(:new) + it 'calls error tracking service' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).and_call_original - subject - end + subject + end + end - it { is_expected.to eq(status: :error, message: 'invalid regex') } + context 'when name_regex_delete is invalid' do + let(:params) { { 'name_regex_delete' => '*test*' } } - it 'calls error tracking service' do - expect(Gitlab::ErrorTracking).to receive(:log_exception).and_call_original + it_behaves_like 'handling an invalid regex' + end - subject - end - end + context 'when name_regex is invalid' do + let(:params) { { 'name_regex' => '*test*' } } - context 'when name_regex_delete is invalid' do - let(:params) { { 'name_regex_delete' => '*test*' } } + it_behaves_like 'handling an invalid regex' + end - it_behaves_like 'handling an invalid regex' - end + context 'when name_regex_keep is invalid' do + let(:params) { { 'name_regex_keep' => '*test*' } } - context 'when name_regex is invalid' do - let(:params) { { 'name_regex' => '*test*' } } + it_behaves_like 'handling an invalid regex' + end + end - it_behaves_like 'handling an invalid regex' - end + context 'when delete regex matching specific tags is used' do + let(:params) do + { 'name_regex_delete' => 'C|D' } + end - context 'when name_regex_keep is invalid' do - let(:params) { { 'name_regex_keep' => '*test*' } } + it 'does remove C and D' do + expect_delete(%w(C D)) - it_behaves_like 'handling an invalid regex' - end + expect_no_caching + + is_expected.to eq(expected_service_response(deleted: %w(C D), before_truncate_size: 2, after_truncate_size: 2, before_delete_size: 2)) end - context 'when delete regex matching specific tags is used' do + context 'with overriding allow regex' do let(:params) do - { 'name_regex_delete' => 'C|D' } + { 'name_regex_delete' => 'C|D', + 'name_regex_keep' => 'C' } end - it 'does remove C and D' do - expect_delete(%w(C D)) + it 'does not remove C' do + expect_delete(%w(D)) expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(C D), before_truncate_size: 2, after_truncate_size: 2, before_delete_size: 2)) + is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1)) end + end - context 'with overriding allow regex' do - let(:params) do - { 'name_regex_delete' => 'C|D', - 'name_regex_keep' => 'C' } - end + context 'with name_regex_delete overriding deprecated name_regex' do + let(:params) do + { 'name_regex' => 'C|D', + 'name_regex_delete' => 'D' } + end - it 'does not remove C' do - expect_delete(%w(D)) + it 'does not remove C' do + expect_delete(%w(D)) - expect_no_caching + expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1)) - end + is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1)) end + end + end - context 'with name_regex_delete overriding deprecated name_regex' do - let(:params) do - { 'name_regex' => 'C|D', - 'name_regex_delete' => 'D' } - end + context 'with allow regex value' do + let(:params) do + { 'name_regex_delete' => '.*', + 'name_regex_keep' => 'B.*' } + end - it 'does not remove C' do - expect_delete(%w(D)) + it 'does not remove B*' do + expect_delete(%w(A C D E)) - expect_no_caching + expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1)) - end - end + is_expected.to eq(expected_service_response(deleted: %w(A C D E), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4)) + end + end + + context 'when keeping only N tags' do + let(:params) do + { 'name_regex' => 'A|B.*|C', + 'keep_n' => 1 } end - context 'with allow regex value' do - let(:params) do - { 'name_regex_delete' => '.*', - 'name_regex_keep' => 'B.*' } - end + it 'sorts tags by date' do + expect_delete(%w(Bb Ba C)) - it 'does not remove B*' do - expect_delete(%w(A C D E)) + expect_no_caching - expect_no_caching + expect(service).to receive(:order_by_date).and_call_original - is_expected.to eq(expected_service_response(deleted: %w(A C D E), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4)) - end + is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 3)) end + end - context 'when keeping only N tags' do - let(:params) do - { 'name_regex' => 'A|B.*|C', - 'keep_n' => 1 } - end + context 'when not keeping N tags' do + let(:params) do + { 'name_regex' => 'A|B.*|C' } + end - it 'sorts tags by date' do - expect_delete(%w(Bb Ba C)) + it 'does not sort tags by date' do + expect_delete(%w(A Ba Bb C)) - expect_no_caching + expect_no_caching - expect(service).to receive(:order_by_date).and_call_original + expect(service).not_to receive(:order_by_date) - is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 3)) - end + is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4)) end + end - context 'when not keeping N tags' do - let(:params) do - { 'name_regex' => 'A|B.*|C' } - end - - it 'does not sort tags by date' do - expect_delete(%w(A Ba Bb C)) + context 'when removing keeping only 3' do + let(:params) do + { 'name_regex_delete' => '.*', + 'keep_n' => 3 } + end - expect_no_caching + it 'does remove B* and C as they are the oldest' do + expect_delete(%w(Bb Ba C)) - expect(service).not_to receive(:order_by_date) + expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4)) - end + is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3)) end + end - context 'when removing keeping only 3' do - let(:params) do - { 'name_regex_delete' => '.*', - 'keep_n' => 3 } - end + context 'when removing older than 1 day' do + let(:params) do + { 'name_regex_delete' => '.*', + 'older_than' => '1 day' } + end - it 'does remove B* and C as they are the oldest' do - expect_delete(%w(Bb Ba C)) + it 'does remove B* and C as they are older than 1 day' do + expect_delete(%w(Ba Bb C)) - expect_no_caching + expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3)) - end + is_expected.to eq(expected_service_response(deleted: %w(Ba Bb C), before_delete_size: 3)) end + end - context 'when removing older than 1 day' do - let(:params) do - { 'name_regex_delete' => '.*', - 'older_than' => '1 day' } - end + context 'when combining all parameters' do + let(:params) do + { 'name_regex_delete' => '.*', + 'keep_n' => 1, + 'older_than' => '1 day' } + end - it 'does remove B* and C as they are older than 1 day' do - expect_delete(%w(Ba Bb C)) + it 'does remove B* and C' do + expect_delete(%w(Bb Ba C)) - expect_no_caching + expect_no_caching - is_expected.to eq(expected_service_response(deleted: %w(Ba Bb C), before_delete_size: 3)) - end + is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3)) end + end + + context 'when running a container_expiration_policy' do + let(:user) { nil } - context 'when combining all parameters' do + context 'with valid container_expiration_policy param' do let(:params) do { 'name_regex_delete' => '.*', 'keep_n' => 1, - 'older_than' => '1 day' } + 'older_than' => '1 day', + 'container_expiration_policy' => true } end - it 'does remove B* and C' do - expect_delete(%w(Bb Ba C)) + it 'succeeds without a user' do + expect_delete(%w(Bb Ba C), container_expiration_policy: true) - expect_no_caching + expect_caching is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3)) end end - context 'when running a container_expiration_policy' do - let(:user) { nil } - - context 'with valid container_expiration_policy param' do - let(:params) do - { 'name_regex_delete' => '.*', - 'keep_n' => 1, - 'older_than' => '1 day', - 'container_expiration_policy' => true } - end - - it 'succeeds without a user' do - expect_delete(%w(Bb Ba C), container_expiration_policy: true) - - caching_enabled ? expect_caching : expect_no_caching - - is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3)) - end + context 'without container_expiration_policy param' do + let(:params) do + { 'name_regex_delete' => '.*', + 'keep_n' => 1, + 'older_than' => '1 day' } end - context 'without container_expiration_policy param' do - let(:params) do - { 'name_regex_delete' => '.*', - 'keep_n' => 1, - 'older_than' => '1 day' } - end - - it 'fails' do - is_expected.to eq(status: :error, message: 'access denied') - end + it 'fails' do + is_expected.to eq(status: :error, message: 'access denied') end end + end - context 'truncating the tags list' do - let(:params) do - { - 'name_regex_delete' => '.*', - 'keep_n' => 1 - } - end + context 'truncating the tags list' do + let(:params) do + { + 'name_regex_delete' => '.*', + 'keep_n' => 1 + } + end - shared_examples 'returning the response' do |status:, original_size:, before_truncate_size:, after_truncate_size:, before_delete_size:| - it 'returns the response' do - expect_no_caching + shared_examples 'returning the response' do |status:, original_size:, before_truncate_size:, after_truncate_size:, before_delete_size:| + it 'returns the response' do + expect_no_caching - result = subject + result = subject - service_response = expected_service_response( - status: status, - original_size: original_size, - before_truncate_size: before_truncate_size, - after_truncate_size: after_truncate_size, - before_delete_size: before_delete_size, - deleted: nil - ) + service_response = expected_service_response( + status: status, + original_size: original_size, + before_truncate_size: before_truncate_size, + after_truncate_size: after_truncate_size, + before_delete_size: before_delete_size, + deleted: nil + ) - expect(result).to eq(service_response) - end + expect(result).to eq(service_response) end + end - where(:feature_flag_enabled, :max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do - false | 10 | :success | :success | false - false | 10 | :error | :error | false - false | 3 | :success | :success | false - false | 3 | :error | :error | false - false | 0 | :success | :success | false - false | 0 | :error | :error | false - true | 10 | :success | :success | false - true | 10 | :error | :error | false - true | 3 | :success | :error | true - true | 3 | :error | :error | true - true | 0 | :success | :success | false - true | 0 | :error | :error | false - end + where(:feature_flag_enabled, :max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do + false | 10 | :success | :success | false + false | 10 | :error | :error | false + false | 3 | :success | :success | false + false | 3 | :error | :error | false + false | 0 | :success | :success | false + false | 0 | :error | :error | false + true | 10 | :success | :success | false + true | 10 | :error | :error | false + true | 3 | :success | :error | true + true | 3 | :error | :error | true + true | 0 | :success | :success | false + true | 0 | :error | :error | false + end - with_them do - before do - stub_feature_flags(container_registry_expiration_policies_throttling: feature_flag_enabled) - stub_application_setting(container_registry_cleanup_tags_service_max_list_size: max_list_size) - allow_next_instance_of(Projects::ContainerRepository::DeleteTagsService) do |service| - expect(service).to receive(:execute).and_return(status: delete_tags_service_status) - end + with_them do + before do + stub_feature_flags(container_registry_expiration_policies_throttling: feature_flag_enabled) + stub_application_setting(container_registry_cleanup_tags_service_max_list_size: max_list_size) + allow_next_instance_of(Projects::ContainerRepository::DeleteTagsService) do |service| + expect(service).to receive(:execute).and_return(status: delete_tags_service_status) end + end - original_size = 7 - keep_n = 1 + original_size = 7 + keep_n = 1 - it_behaves_like( - 'returning the response', - status: params[:expected_status], - original_size: original_size, - before_truncate_size: original_size - keep_n, - after_truncate_size: params[:expected_truncated] ? params[:max_list_size] + keep_n : original_size - keep_n, - before_delete_size: params[:expected_truncated] ? params[:max_list_size] : original_size - keep_n - 1 # one tag is filtered out with older_than filter - ) - end + it_behaves_like( + 'returning the response', + status: params[:expected_status], + original_size: original_size, + before_truncate_size: original_size - keep_n, + after_truncate_size: params[:expected_truncated] ? params[:max_list_size] + keep_n : original_size - keep_n, + before_delete_size: params[:expected_truncated] ? params[:max_list_size] : original_size - keep_n - 1 # one tag is filtered out with older_than filter + ) end end - context 'caching' do + context 'caching', :freeze_time do let(:params) do { 'name_regex_delete' => '.*', @@ -381,17 +379,12 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_ before do expect_delete(%w(Bb Ba C), container_expiration_policy: true) - travel_to(Time.zone.local(2021, 9, 2, 12, 0, 0)) # We froze time so we need to set the created_at stubs again stub_digest_config('sha256:configA', 1.hour.ago) stub_digest_config('sha256:configB', 5.days.ago) stub_digest_config('sha256:configC', 1.month.ago) end - after do - travel_back - end - it 'caches the created_at values' do ::Gitlab::Redis::Cache.with do |redis| expect_mget(redis, tags_and_created_ats.keys) @@ -450,32 +443,6 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_ DateTime.rfc3339(date_time.rfc3339).rfc3339 end end - - context 'with container_registry_expiration_policies_caching enabled for the project' do - before do - stub_feature_flags(container_registry_expiration_policies_caching: project) - end - - it_behaves_like 'reading and removing tags', caching_enabled: true - end - - context 'with container_registry_expiration_policies_caching disabled' do - before do - stub_feature_flags(container_registry_expiration_policies_caching: false) - end - - it_behaves_like 'reading and removing tags', caching_enabled: false - end - - context 'with container_registry_expiration_policies_caching not enabled for the project' do - let_it_be(:another_project) { create(:project) } - - before do - stub_feature_flags(container_registry_expiration_policies_caching: another_project) - end - - it_behaves_like 'reading and removing tags', caching_enabled: false - end end private |