diff options
author | Nick Thomas <nick@gitlab.com> | 2016-11-03 17:12:20 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2016-11-04 07:01:08 +0300 |
commit | c1388d0efb79b755c06b9db19fc4ad47af7cf0a3 (patch) | |
tree | 4f1bbfca15dc287d2b14a68f4e5167c18f5a1b10 | |
parent | 3a8a7c1251cef4098ffbc54718ba21736c5e2800 (diff) |
Allow multiple repository storage shards to be enabled, and automatically round-robin between them
-rw-r--r-- | app/controllers/admin/application_settings_controller.rb | 2 | ||||
-rw-r--r-- | app/helpers/application_settings_helper.rb | 4 | ||||
-rw-r--r-- | app/models/application_setting.rb | 39 | ||||
-rw-r--r-- | app/models/project.rb | 2 | ||||
-rw-r--r-- | app/views/admin/application_settings/_form.html.haml | 4 | ||||
-rw-r--r-- | db/migrate/20161103171205_rename_repository_storage_column.rb | 29 | ||||
-rw-r--r-- | db/schema.rb | 4 | ||||
-rw-r--r-- | lib/api/entities.rb | 1 | ||||
-rw-r--r-- | lib/api/settings.rb | 4 | ||||
-rw-r--r-- | spec/models/application_setting_spec.rb | 56 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 15 | ||||
-rw-r--r-- | spec/requests/api/settings_spec.rb | 1 |
12 files changed, 137 insertions, 24 deletions
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 6ef7cf0bae6..86e808314f4 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -116,8 +116,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :metrics_packet_size, :send_user_confirmation_email, :container_registry_token_expire_delay, - :repository_storage, :enabled_git_access_protocol, + repository_storages: [], restricted_visibility_levels: [], import_sources: [], disabled_oauth_sign_in_sources: [] diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 6229384817b..45a567a1eba 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -93,11 +93,11 @@ module ApplicationSettingsHelper end end - def repository_storage_options_for_select + def repository_storages_options_for_select options = Gitlab.config.repositories.storages.map do |name, path| ["#{name} - #{path}", name] end - options_for_select(options, @application_setting.repository_storage) + options_for_select(options, @application_setting.repository_storages) end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index c99aa7772bb..6e7a90e7d9c 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -18,6 +18,7 @@ class ApplicationSetting < ActiveRecord::Base serialize :disabled_oauth_sign_in_sources, Array serialize :domain_whitelist, Array serialize :domain_blacklist, Array + serialize :repository_storages cache_markdown_field :sign_in_text cache_markdown_field :help_page_text @@ -74,9 +75,8 @@ class ApplicationSetting < ActiveRecord::Base presence: true, numericality: { only_integer: true, greater_than: 0 } - validates :repository_storage, - presence: true, - inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } } + validates :repository_storages, presence: true + validate :check_repository_storages validates :enabled_git_access_protocol, inclusion: { in: %w(ssh http), allow_blank: true, allow_nil: true } @@ -166,7 +166,7 @@ class ApplicationSetting < ActiveRecord::Base disabled_oauth_sign_in_sources: [], send_user_confirmation_email: false, container_registry_token_expire_delay: 5, - repository_storage: 'default', + repository_storages: ['default'], user_default_external: false, ) end @@ -201,6 +201,29 @@ class ApplicationSetting < ActiveRecord::Base self.domain_blacklist_raw = file.read end + def repository_storages + value = read_attribute(:repository_storages) + value = [value] if value.is_a?(String) + value = [] if value.nil? + + value + end + + # repository_storage is still required in the API. Remove in 9.0 + def repository_storage + repository_storages.first + end + + def repository_storage=(value) + self.repository_storages = [value] + end + + # Choose one of the available repository storage options. Currently all have + # equal weighting. + def pick_repository_storage + repository_storages.sample + end + def runners_registration_token ensure_runners_registration_token! end @@ -208,4 +231,12 @@ class ApplicationSetting < ActiveRecord::Base def health_check_access_token ensure_health_check_access_token! end + + private + + def check_repository_storages + invalid = repository_storages - Gitlab.config.repositories.storages.keys + errors.add(:repository_storages, "can't include: #{invalid.join(", ")}") unless + invalid.empty? + end end diff --git a/app/models/project.rb b/app/models/project.rb index d5512dfaf9c..cf931f64c03 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -28,7 +28,7 @@ class Project < ActiveRecord::Base default_value_for :archived, false default_value_for :visibility_level, gitlab_config_features.visibility_level default_value_for :container_registry_enabled, gitlab_config_features.container_registry - default_value_for(:repository_storage) { current_application_settings.repository_storage } + default_value_for(:repository_storage) { current_application_settings.pick_repository_storage } default_value_for(:shared_runners_enabled) { current_application_settings.shared_runners_enabled } default_value_for :issues_enabled, gitlab_config_features.issues default_value_for :merge_requests_enabled, gitlab_config_features.merge_requests diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index c4c68cd7891..28003e5f509 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -353,9 +353,9 @@ %fieldset %legend Repository Storage .form-group - = f.label :repository_storage, 'Storage path for new projects', class: 'control-label col-sm-2' + = f.label :repository_storages, 'Storage paths for new projects', class: 'control-label col-sm-2' .col-sm-10 - = f.select :repository_storage, repository_storage_options_for_select, {}, class: 'form-control' + = f.select :repository_storages, repository_storages_options_for_select, {include_hidden: false}, multiple: true, class: 'form-control' .help-block Manage repository storage paths. Learn more in the = succeed "." do diff --git a/db/migrate/20161103171205_rename_repository_storage_column.rb b/db/migrate/20161103171205_rename_repository_storage_column.rb new file mode 100644 index 00000000000..e9f992793b4 --- /dev/null +++ b/db/migrate/20161103171205_rename_repository_storage_column.rb @@ -0,0 +1,29 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RenameRepositoryStorageColumn < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + rename_column :application_settings, :repository_storage, :repository_storages + end +end diff --git a/db/schema.rb b/db/schema.rb index 54b5fc83be0..dc088925d97 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161025231710) do +ActiveRecord::Schema.define(version: 20161103171205) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -88,7 +88,7 @@ ActiveRecord::Schema.define(version: 20161025231710) do t.integer "container_registry_token_expire_delay", default: 5 t.text "after_sign_up_text" t.boolean "user_default_external", default: false, null: false - t.string "repository_storage", default: "default" + t.string "repository_storages", default: "default" t.string "enabled_git_access_protocol" t.boolean "domain_blacklist_enabled", default: false t.text "domain_blacklist" diff --git a/lib/api/entities.rb b/lib/api/entities.rb index d52496451a2..1f378ba1635 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -509,6 +509,7 @@ module API expose :after_sign_out_path expose :container_registry_token_expire_delay expose :repository_storage + expose :repository_storages expose :koding_enabled expose :koding_url end diff --git a/lib/api/settings.rb b/lib/api/settings.rb index c885fcd7ea3..c4cb1c7924a 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -17,12 +17,12 @@ module API present current_settings, with: Entities::ApplicationSetting end - # Modify applicaiton settings + # Modify application settings # # Example Request: # PUT /application/settings put "application/settings" do - attributes = current_settings.attributes.keys - ["id"] + attributes = ["repository_storage"] + current_settings.attributes.keys - ["id"] attrs = attributes_for_keys(attributes) if current_settings.update_attributes(attrs) 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 |