Merge branch '24059-round-robin-repository-storage' into 'master'

Resolve "Introduce round-robin project creation to spread load over multiple shards"

Allow multiple shards to be enabled in the admin settings page, balancing project creation across all enabled shards.

* `f.select ..., multiple: true` isn't the most beautiful UI in the world, but switching to `collection_check_boxes` (or a facsimile thereof) isn't trivial
* Should `pick_repository_storage` be a method of `ApplicationSetting`, or `Project`? It's going to accrete logic over time so perhaps it should be its own class already?
* This is written to avoid the need for a database migration, so it is`serialize :repository_storage` without `, Array`. This is tested, but alternatives include:
  * Add a database migration
  * Write a custom Coder that will accept a String or Array in `load` and always `dump an Array.

Closes #24059

See merge request !7273

Signed-off-by: Rémy Coutable <remy@rymai.me>

3 files changed, 62 insertions, 10 deletions

diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index cc215d252f9..2b76e056f3c 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -41,14 +41,62 @@ describe ApplicationSetting, models: true do
       subject { setting }
     end
 
-    context 'repository storages inclussion' do
+    # Upgraded databases will have this sort of content
+    context 'repository_storages is a String, not an Array' do
+      before { setting.__send__(:raw_write_attribute, :repository_storages, 'default') }
+
+      it { expect(setting.repository_storages_before_type_cast).to eq('default') }
+      it { expect(setting.repository_storages).to eq(['default']) }
+    end
+
+    context 'repository storages' do
       before do
-        storages = { 'custom' => 'tmp/tests/custom_repositories' }
+        storages = {
+          'custom1' => 'tmp/tests/custom_repositories_1',
+          'custom2' => 'tmp/tests/custom_repositories_2',
+          'custom3' => 'tmp/tests/custom_repositories_3',
+
+        }
         allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
       end
 
-      it { is_expected.to allow_value('custom').for(:repository_storage) }
-      it { is_expected.not_to allow_value('alternative').for(:repository_storage) }
+      describe 'inclusion' do
+        it { is_expected.to allow_value('custom1').for(:repository_storages) }
+        it { is_expected.to allow_value(['custom2', 'custom3']).for(:repository_storages) }
+        it { is_expected.not_to allow_value('alternative').for(:repository_storages) }
+        it { is_expected.not_to allow_value(['alternative', 'custom1']).for(:repository_storages) }
+      end
+
+      describe 'presence' do
+        it { is_expected.not_to allow_value([]).for(:repository_storages) }
+        it { is_expected.not_to allow_value("").for(:repository_storages) }
+        it { is_expected.not_to allow_value(nil).for(:repository_storages) }
+      end
+
+      describe '.pick_repository_storage' do
+        it 'uses Array#sample to pick a random storage' do
+          array = double('array', sample: 'random')
+          expect(setting).to receive(:repository_storages).and_return(array)
+
+          expect(setting.pick_repository_storage).to eq('random')
+        end
+
+        describe '#repository_storage' do
+          it 'returns the first storage' do
+            setting.repository_storages = ['good', 'bad']
+
+            expect(setting.repository_storage).to eq('good')
+          end
+        end
+
+        describe '#repository_storage=' do
+          it 'overwrites repository_storages' do
+            setting.repository_storage = 'overwritten'
+
+            expect(setting.repository_storages).to eq(['overwritten'])
+          end
+        end
+      end
     end
   end
 
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index aef277357cf..0245897938c 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -837,16 +837,19 @@ describe Project, models: true do
   context 'repository storage by default' do
     let(:project) { create(:empty_project) }
 
-    subject { project.repository_storage }
-
     before do
-      storages = { 'alternative_storage' => '/some/path' }
+      storages = {
+        'default' => 'tmp/tests/repositories',
+        'picked'  => 'tmp/tests/repositories',
+      }
       allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
-      stub_application_setting(repository_storage: 'alternative_storage')
-      allow_any_instance_of(Project).to receive(:ensure_dir_exist).and_return(true)
     end
 
-    it { is_expected.to eq('alternative_storage') }
+    it 'picks storage from ApplicationSetting' do
+      expect_any_instance_of(ApplicationSetting).to receive(:pick_repository_storage).and_return('picked')
+
+      expect(project.repository_storage).to eq('picked')
+    end
   end
 
   context 'shared runners by default' do
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index f4903d8e0be..096a8ebab70 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -33,6 +33,7 @@ describe API::API, 'Settings', api: true  do
         expect(json_response['default_projects_limit']).to eq(3)
         expect(json_response['signin_enabled']).to be_falsey
         expect(json_response['repository_storage']).to eq('custom')
+        expect(json_response['repository_storages']).to eq(['custom'])
         expect(json_response['koding_enabled']).to be_truthy
         expect(json_response['koding_url']).to eq('http://koding.example.com')
       end