Commit 88a1537f authored by Corinna Wiesner's avatar Corinna Wiesner

Always check both storage feature flags

When the new feature flag :additional_repo_storage_by_namespace is
checked, we need to make sure that :namespace_storage_limit is disabled.
parent 8ba7dfae
...@@ -50,7 +50,7 @@ module EE ...@@ -50,7 +50,7 @@ module EE
::Gitlab.dev_env_or_com? && ::Gitlab.dev_env_or_com? &&
::Gitlab::CurrentSettings.automatic_purchased_storage_allocation? && ::Gitlab::CurrentSettings.automatic_purchased_storage_allocation? &&
::Feature.enabled?(:buy_storage_link) && ::Feature.enabled?(:buy_storage_link) &&
::Feature.enabled?(:additional_repo_storage_by_namespace, namespace) namespace.additional_repo_storage_by_namespace_enabled?
end end
def namespace_storage_usage_link(namespace) def namespace_storage_usage_link(namespace)
......
...@@ -33,7 +33,7 @@ module EE ...@@ -33,7 +33,7 @@ module EE
def enforce_limit? def enforce_limit?
return false unless ::Gitlab::CurrentSettings.automatic_purchased_storage_allocation? return false unless ::Gitlab::CurrentSettings.automatic_purchased_storage_allocation?
return false unless ::Feature.enabled?(:additional_repo_storage_by_namespace, root_namespace) return false unless root_namespace.additional_repo_storage_by_namespace_enabled?
true true
end end
......
...@@ -27,7 +27,7 @@ module EE ...@@ -27,7 +27,7 @@ module EE
def additional_repo_storage_available? def additional_repo_storage_available?
return false unless ::Gitlab::CurrentSettings.automatic_purchased_storage_allocation? return false unless ::Gitlab::CurrentSettings.automatic_purchased_storage_allocation?
::Feature.enabled?(:additional_repo_storage_by_namespace, namespace) namespace.additional_repo_storage_by_namespace_enabled?
end end
def total_repository_size_excess def total_repository_size_excess
......
...@@ -206,31 +206,29 @@ RSpec.describe EE::NamespaceStorageLimitAlertHelper do ...@@ -206,31 +206,29 @@ RSpec.describe EE::NamespaceStorageLimitAlertHelper do
describe '#can_purchase_storage_for_namespace?' do describe '#can_purchase_storage_for_namespace?' do
subject { helper.can_purchase_storage_for_namespace?(namespace) } subject { helper.can_purchase_storage_for_namespace?(namespace) }
let(:namespace) { build(:namespace) } let_it_be(:namespace) { build(:namespace) }
where(:is_dev_or_com, :auto_storage_allocation_enabled, :buy_storage_link_enabled, :additional_storage_enabled, :result) do where(
true | true | true | true | true is_dev_or_com: [true, false],
true | true | true | false | false auto_storage_allocation_enabled: [true, false],
true | true | false | true | false buy_storage_link_enabled: [true, false],
true | true | false | false | false namespace_storage_limit_enabled: [true, false],
true | false | true | true | false additional_storage_enabled: [true, false]
true | false | true | false | false )
true | false | false | true | false
true | false | false | false | false
false | true | true | true | false
false | true | true | false | false
false | true | false | true | false
false | true | false | false | false
false | false | true | true | false
false | false | true | false | false
false | false | false | true | false
false | false | false | false | false
end
with_them do with_them do
let(:result) do
is_dev_or_com &&
auto_storage_allocation_enabled &&
buy_storage_link_enabled &&
!namespace_storage_limit_enabled &&
additional_storage_enabled
end
before do before do
allow(::Gitlab).to receive(:dev_env_or_com?).and_return(is_dev_or_com) allow(::Gitlab).to receive(:dev_env_or_com?).and_return(is_dev_or_com)
stub_application_setting(automatic_purchased_storage_allocation: auto_storage_allocation_enabled) stub_application_setting(automatic_purchased_storage_allocation: auto_storage_allocation_enabled)
stub_feature_flags(namespace_storage_limit: namespace_storage_limit_enabled)
stub_feature_flags(additional_repo_storage_by_namespace: additional_storage_enabled) stub_feature_flags(additional_repo_storage_by_namespace: additional_storage_enabled)
stub_feature_flags(buy_storage_link: buy_storage_link_enabled) stub_feature_flags(buy_storage_link: buy_storage_link_enabled)
end end
......
...@@ -26,64 +26,60 @@ RSpec.describe Gitlab::RepositorySizeChecker do ...@@ -26,64 +26,60 @@ RSpec.describe Gitlab::RepositorySizeChecker do
allow(namespace).to receive(:total_repository_size_excess).and_return(total_repository_size_excess.megabytes) allow(namespace).to receive(:total_repository_size_excess).and_return(total_repository_size_excess.megabytes)
end end
shared_examples 'original logic (without size excess and additional storage)' do
include_examples 'checker size above limit'
include_examples 'checker size above limit (with additional storage, that would bring it under the limit)'
include_examples 'checker size not over limit'
end
describe '#above_size_limit?' do describe '#above_size_limit?' do
context 'when Gitlab app setting for automatic purchased storage allocation is not enabled' do context 'when enabled is false' do
let(:gitlab_setting_enabled) { false } let(:enabled) { false }
include_examples 'checker size above limit' it 'returns false' do
include_examples 'checker size not over limit' expect(subject.above_size_limit?).to eq(false)
end
end end
context 'with feature flag :additional_repo_storage_by_namespace enabled' do include_examples 'original logic (without size excess and additional storage)'
shared_examples 'feature flag :additional_repo_storage_by_namespace enabled' do
context 'when there is available excess storage' do
it 'returns false' do
expect(subject.above_size_limit?).to eq(false)
end
end
context 'when size is above the limit and there is no excess storage' do context 'when Gitlab app setting for automatic purchased storage allocation is not enabled' do
let(:current_size) { 100 } let(:gitlab_setting_enabled) { false }
let(:total_repository_size_excess) { 20 }
let(:additional_purchased_storage) { 10 }
it 'returns true' do include_examples 'original logic (without size excess and additional storage)'
expect(subject.above_size_limit?).to eq(true) end
end
end
it 'returns false when not over the limit' do context 'with feature flag :namespace_storage_limit disabled' do
expect(subject.above_size_limit?).to eq(false) before do
end stub_feature_flags(namespace_storage_limit: false)
end end
include_examples 'feature flag :additional_repo_storage_by_namespace enabled' context 'when there are no locked projects (total repository excess < additional storage)' do
let(:current_size) { 100 } # current_size > limit
let(:total_repository_size_excess) { 5 }
let(:additional_purchased_storage) { 10 }
context 'when only enabled for a specific namespace' do it 'returns false' do
before do expect(subject.above_size_limit?).to eq(false)
stub_feature_flags(additional_repo_storage_by_namespace: false)
end end
end
context 'when namespace is another one than the given one' do context 'when there are no locked projects (total repository excess == additional storage)' do
let(:other_namespace) { create(:namespace) } let(:current_size) { 100 } # current_size > limit
let(:total_repository_size_excess) { 10 }
before do let(:additional_purchased_storage) { 10 }
stub_feature_flags(additional_repo_storage_by_namespace: other_namespace)
end
include_examples 'checker size above limit' it 'returns false' do
include_examples 'checker size not over limit' expect(subject.above_size_limit?).to eq(false)
end end
end
context 'when namespace is the given one' do context 'when there are locked projects (total repository excess > additional storage)' do
let(:namespace) { create(:namespace, additional_purchased_storage_size: additional_purchased_storage) } let(:total_repository_size_excess) { 12 }
let(:additional_purchased_storage) { 10 }
before do
stub_feature_flags(additional_repo_storage_by_namespace: namespace)
end
include_examples 'feature flag :additional_repo_storage_by_namespace enabled' include_examples 'checker size above limit'
end include_examples 'checker size not over limit'
end end
end end
...@@ -92,17 +88,22 @@ RSpec.describe Gitlab::RepositorySizeChecker do ...@@ -92,17 +88,22 @@ RSpec.describe Gitlab::RepositorySizeChecker do
stub_feature_flags(additional_repo_storage_by_namespace: false) stub_feature_flags(additional_repo_storage_by_namespace: false)
end end
include_examples 'checker size above limit' include_examples 'original logic (without size excess and additional storage)'
include_examples 'checker size not over limit'
end end
end end
describe '#exceeded_size' do describe '#exceeded_size' do
context 'with feature flag :additional_repo_storage_by_namespace enabled' do include_examples 'checker size exceeded'
context 'when Gitlab app setting for automatic purchased storage allocation is not enabled' do
let(:gitlab_setting_enabled) { false }
include_examples 'checker size exceeded' context 'when Gitlab app setting for automatic purchased storage allocation is not enabled' do
let(:gitlab_setting_enabled) { false }
include_examples 'checker size exceeded'
end
context 'with feature flag :namespace_storage_limit disabled' do
before do
stub_feature_flags(namespace_storage_limit: false)
end end
context 'when current size + total repository size excess are below or equal to the limit + additional purchased storage' do context 'when current size + total repository size excess are below or equal to the limit + additional purchased storage' do
......
...@@ -28,14 +28,28 @@ RSpec.describe EE::Namespace::RootExcessStorageSize do ...@@ -28,14 +28,28 @@ RSpec.describe EE::Namespace::RootExcessStorageSize do
context 'when limit enforcement is on' do context 'when limit enforcement is on' do
let(:storage_allocation_enabled) { true } let(:storage_allocation_enabled) { true }
context 'when below limit' do context 'with feature flag :namespace_storage_limit disabled' do
it { is_expected.to eq(false) } before do
stub_feature_flags(namespace_storage_limit: false)
end
context 'when below limit' do
it { is_expected.to eq(false) }
end
context 'when above limit' do
let(:total_repository_size_excess) { 101.megabytes }
it { is_expected.to eq(true) }
end
end end
context 'when above limit' do context 'with feature flag :additional_repo_storage_by_namespace disabled' do
let(:total_repository_size_excess) { 101.megabytes } before do
stub_feature_flags(additional_repo_storage_by_namespace: false)
end
it { is_expected.to eq(true) } it { is_expected.to eq(false) }
end end
end end
end end
...@@ -91,14 +105,20 @@ RSpec.describe EE::Namespace::RootExcessStorageSize do ...@@ -91,14 +105,20 @@ RSpec.describe EE::Namespace::RootExcessStorageSize do
allow(Gitlab::CurrentSettings).to receive(:automatic_purchased_storage_allocation?) { storage_allocation_enabled } allow(Gitlab::CurrentSettings).to receive(:automatic_purchased_storage_allocation?) { storage_allocation_enabled }
end end
it { is_expected.to eq(true) }
context 'with application setting is disabled' do context 'with application setting is disabled' do
let(:storage_allocation_enabled) { false } let(:storage_allocation_enabled) { false }
it { is_expected.to eq(false) } it { is_expected.to eq(false) }
end end
context 'with feature flag :namespace_storage_limit disabled' do
before do
stub_feature_flags(namespace_storage_limit: false)
end
it { is_expected.to eq(true) }
end
context 'with feature flag :additional_repo_storage_by_namespace disabled' do context 'with feature flag :additional_repo_storage_by_namespace disabled' do
before do before do
stub_feature_flags(additional_repo_storage_by_namespace: false) stub_feature_flags(additional_repo_storage_by_namespace: false)
......
...@@ -8,11 +8,13 @@ RSpec.describe Namespaces::CheckExcessStorageSizeService, '#execute' do ...@@ -8,11 +8,13 @@ RSpec.describe Namespaces::CheckExcessStorageSizeService, '#execute' do
let(:service) { described_class.new(namespace, user) } let(:service) { described_class.new(namespace, user) }
let(:total_repository_size_excess) { 150.megabytes } let(:total_repository_size_excess) { 150.megabytes }
let(:additional_purchased_storage_size) { 100 } let(:additional_purchased_storage_size) { 100 }
let(:namespace_storage_limit_enabled) { false }
let(:storage_allocation_enabled) { true } let(:storage_allocation_enabled) { true }
subject(:response) { service.execute } subject(:response) { service.execute }
before do before do
stub_feature_flags(namespace_storage_limit: namespace_storage_limit_enabled)
allow(Gitlab::CurrentSettings).to receive(:automatic_purchased_storage_allocation?) { storage_allocation_enabled } allow(Gitlab::CurrentSettings).to receive(:automatic_purchased_storage_allocation?) { storage_allocation_enabled }
allow(namespace).to receive(:root_ancestor).and_return(namespace) allow(namespace).to receive(:root_ancestor).and_return(namespace)
...@@ -33,6 +35,12 @@ RSpec.describe Namespaces::CheckExcessStorageSizeService, '#execute' do ...@@ -33,6 +35,12 @@ RSpec.describe Namespaces::CheckExcessStorageSizeService, '#execute' do
it { is_expected.to be_success } it { is_expected.to be_success }
end end
context 'with feature flag :namespace_storage_limit enabled' do
let(:namespace_storage_limit_enabled) { true }
it { is_expected.to be_success }
end
end end
context 'when additional_purchased_storage_size is set to 0' do context 'when additional_purchased_storage_size is set to 0' do
......
...@@ -32,7 +32,7 @@ module Gitlab ...@@ -32,7 +32,7 @@ module Gitlab
def changes_will_exceed_size_limit?(change_size) def changes_will_exceed_size_limit?(change_size)
return false unless enabled? return false unless enabled?
change_size > limit || exceeded_size(change_size) > 0 above_size_limit? || exceeded_size(change_size) > 0
end end
# @param change_size [int] in bytes # @param change_size [int] in bytes
......
...@@ -10,6 +10,17 @@ RSpec.shared_examples 'checker size above limit' do ...@@ -10,6 +10,17 @@ RSpec.shared_examples 'checker size above limit' do
end end
end end
RSpec.shared_examples 'checker size above limit (with additional storage, that would bring it under the limit)' do
context 'when size is above the limit' do
let(:current_size) { 100 }
let(:additional_purchased_storage) { 60 }
it 'returns true' do
expect(subject.above_size_limit?).to eq(true)
end
end
end
RSpec.shared_examples 'checker size not over limit' do RSpec.shared_examples 'checker size not over limit' do
it 'returns false when not over the limit' do it 'returns false when not over the limit' do
expect(subject.above_size_limit?).to eq(false) expect(subject.above_size_limit?).to eq(false)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment