Update storage settings to allow extra values per shard

This will be necessary when adding gitaly settings. This version
doesn't make any functional changes, but allows us to include this
breaking change in 9.0 and add the needed extra settings in the future
with backwards compatibility

app/helpers/application_settings_helper.rb

@@ -81,8 +81,8 @@ module ApplicationSettingsHelper
   end
 
   def repository_storages_options_for_select
-    options = Gitlab.config.repositories.storages.map do |name, path|
-      ["#{name} - #{path}", name]
+    options = Gitlab.config.repositories.storages.map do |name, storage|
+      ["#{name} - #{storage['path']}", name]
     end
 
     options_for_select(options, @application_setting.repository_storages)

app/models/project.rb

@@ -434,7 +434,7 @@ class Project < ActiveRecord::Base
   end
 
   def repository_storage_path
-    Gitlab.config.repositories.storages[repository_storage]
+    Gitlab.config.repositories.storages[repository_storage]['path']
   end
 
   def team

app/models/repository.rb

@@ -58,10 +58,6 @@ class Repository
     end
   end
 
-  def self.storages
-    Gitlab.config.repositories.storages
-  end
-
   def initialize(path_with_namespace, project)
     @path_with_namespace = path_with_namespace
     @project = project

app/workers/post_receive.rb

@@ -4,8 +4,8 @@ class PostReceive
   extend Gitlab::CurrentSettings
 
   def perform(repo_path, identifier, changes)
-    if path = Gitlab.config.repositories.storages.find { |p| repo_path.start_with?(p[1].to_s) }
-      repo_path.gsub!(path[1].to_s, "")
+    if repository_storage = Gitlab.config.repositories.storages.find { |p| repo_path.start_with?(p[1]['path'].to_s) }
+      repo_path.gsub!(repository_storage[1]['path'].to_s, "")
     else
       log("Check gitlab.yml config for correct repositories.storages values. No repository storage path matches \"#{repo_path}\"")
     end

changelogs/unreleased/28447-hybrid-repository-storages.yml

+---
+title: Update storage settings to allow extra values per repository storage
+merge_request: 9597
+author:

config/gitlab.yml.example

@@ -548,7 +548,8 @@ production: &base
     # gitlab-shell invokes Dir.pwd inside the repository path and that results
     # real path not the symlink.
     storages: # You must have at least a `default` storage path.
-      default: /home/git/repositories/
+      default:
+        path: /home/git/repositories/
 
   ## Backup settings
   backup:
@@ -659,7 +660,8 @@ test:
     path: tmp/tests/gitlab-satellites/
   repositories:
     storages:
-      default: tmp/tests/repositories/
+      default:
+        path: tmp/tests/repositories/
   backup:
     path: tmp/tests/backups
   gitlab_shell:

config/initializers/1_settings.rb

@@ -445,8 +445,13 @@ Settings.gitlab_shell['ssh_path_prefix'] ||= Settings.send(:build_gitlab_shell_s
 #
 Settings['repositories'] ||= Settingslogic.new({})
 Settings.repositories['storages'] ||= {}
-# Setting gitlab_shell.repos_path is DEPRECATED and WILL BE REMOVED in version 9.0
-Settings.repositories.storages['default'] ||= Settings.gitlab_shell['repos_path'] || Settings.gitlab['user_home'] + '/repositories/'
+unless Settings.repositories.storages['default']
+  Settings.repositories.storages['default'] ||= {}
+  # We set the path only if the default storage doesn't exist, in case it exists
+  # but follows the pre-9.0 configuration structure. `6_validations.rb` initializer
+  # will validate all storages and throw a relevant error to the user if necessary.
+  Settings.repositories.storages['default']['path'] ||= Settings.gitlab['user_home'] + '/repositories/'
+end
 
 #
 # The repository_downloads_path is used to remove outdated repository
@@ -455,11 +460,11 @@ Settings.repositories.storages['default'] ||= Settings.gitlab_shell['repos_path'
 # data-integrity issue. In this case, we sets it to the default
 # repository_downloads_path value.
 #
-repositories_storages_path     = Settings.repositories.storages.values
+repositories_storages          = Settings.repositories.storages.values
 repository_downloads_path      = Settings.gitlab['repository_downloads_path'].to_s.gsub(/\/$/, '')
 repository_downloads_full_path = File.expand_path(repository_downloads_path, Settings.gitlab['user_home'])
 
-if repository_downloads_path.blank? || repositories_storages_path.any? { |path| [repository_downloads_path, repository_downloads_full_path].include?(path.gsub(/\/$/, '')) }
+if repository_downloads_path.blank? || repositories_storages.any? { |rs| [repository_downloads_path, repository_downloads_full_path].include?(rs['path'].gsub(/\/$/, '')) }
   Settings.gitlab['repository_downloads_path'] = File.join(Settings.shared['path'], 'cache/archive')
 end
 

config/initializers/6_validations.rb

@@ -4,8 +4,8 @@ end
 
 def find_parent_path(name, path)
   parent = Pathname.new(path).realpath.parent
-  Gitlab.config.repositories.storages.detect do |n, p|
-    name != n && Pathname.new(p).realpath == parent
+  Gitlab.config.repositories.storages.detect do |n, rs|
+    name != n && Pathname.new(rs['path']).realpath == parent
   end
 end
 
@@ -16,10 +16,22 @@ end
 def validate_storages
   storage_validation_error('No repository storage path defined') if Gitlab.config.repositories.storages.empty?
 
-  Gitlab.config.repositories.storages.each do |name, path|
+  Gitlab.config.repositories.storages.each do |name, repository_storage|
     storage_validation_error("\"#{name}\" is not a valid storage name") unless storage_name_valid?(name)
 
-    parent_name, _parent_path = find_parent_path(name, path)
+    if repository_storage.is_a?(String)
+      error = "#{name} is not a valid storage, because it has no `path` key. " \
+        "It may be configured as:\n\n#{name}:\n  path: #{repository_storage}\n\n" \
+        "Refer to gitlab.yml.example for an updated example"
+
+      storage_validation_error(error)
+    end
+
+    if !repository_storage.is_a?(Hash) || repository_storage['path'].nil?
+      storage_validation_error("#{name} is not a valid storage, because it has no `path` key. Refer to gitlab.yml.example for an updated example")
+    end
+
+    parent_name, _parent_path = find_parent_path(name, repository_storage['path'])
     if parent_name
       storage_validation_error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages")
     end

db/migrate/20140502125220_migrate_repo_size.rb

@@ -8,7 +8,7 @@ class MigrateRepoSize < ActiveRecord::Migration
     project_data.each do |project|
       id = project['id']
       namespace_path = project['namespace_path'] || ''
-      repos_path = Gitlab.config.gitlab_shell['repos_path'] || Gitlab.config.repositories.storages.default
+      repos_path = Gitlab.config.gitlab_shell['repos_path'] || Gitlab.config.repositories.storages.default['path']
       path = File.join(repos_path, namespace_path, project['project_path'] + '.git')
 
       begin

db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb

@@ -12,7 +12,7 @@ class MigrateProcessCommitWorkerJobs < ActiveRecord::Migration
     end
 
     def repository_storage_path
-      Gitlab.config.repositories.storages[repository_storage]
+      Gitlab.config.repositories.storages[repository_storage]['path']
     end
 
     def repository_path

db/migrate/20161220141214_remove_dot_git_from_group_names.rb

@@ -60,7 +60,7 @@ class RemoveDotGitFromGroupNames < ActiveRecord::Migration
 
   def move_namespace(group_id, path_was, path)
     repository_storage_paths = select_all("SELECT distinct(repository_storage) FROM projects WHERE namespace_id = #{group_id}").map do |row|
-      Gitlab.config.repositories.storages[row['repository_storage']]
+      Gitlab.config.repositories.storages[row['repository_storage']]['path']
     end.compact
 
     # Move the namespace directory in all storages paths used by member projects

db/migrate/20161226122833_remove_dot_git_from_usernames.rb

@@ -71,7 +71,7 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration
     route_exists = route_exists?(path)
 
     Gitlab.config.repositories.storages.each_value do |storage|
-      if route_exists || path_exists?(path, storage)
+      if route_exists || path_exists?(path, storage['path'])
         counter += 1
         path = "#{base}#{counter}"
 
@@ -84,7 +84,7 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration
 
   def move_namespace(namespace_id, path_was, path)
     repository_storage_paths = select_all("SELECT distinct(repository_storage) FROM projects WHERE namespace_id = #{namespace_id}").map do |row|
-      Gitlab.config.repositories.storages[row['repository_storage']]
+      Gitlab.config.repositories.storages[row['repository_storage']]['path']
     end.compact
 
     # Move the namespace directory in all storages paths used by member projects

doc/administration/repository_storage_paths.md

@@ -52,9 +52,12 @@ respectively.
       # Paths where repositories can be stored. Give the canonicalized absolute pathname.
       # NOTE: REPOS PATHS MUST NOT CONTAIN ANY SYMLINK!!!
       storages: # You must have at least a 'default' storage path.
-        default: /home/git/repositories
-        nfs: /mnt/nfs/repositories
-        cephfs: /mnt/cephfs/repositories
+        default:
+          path: /home/git/repositories
+        nfs:
+          path: /mnt/nfs/repositories
+        cephfs:
+          path: /mnt/cephfs/repositories
     ```
 
 1. [Restart GitLab] for the changes to take effect.
@@ -75,9 +78,9 @@ working, you can remove the `repos_path` line.
 
     ```ruby
     git_data_dirs({
-      "default" => "/var/opt/gitlab/git-data",
-      "nfs" => "/mnt/nfs/git-data",
-      "cephfs" => "/mnt/cephfs/git-data"
+      "default" => { "path" => "/var/opt/gitlab/git-data" },
+      "nfs" => { "path" => "/mnt/nfs/git-data" },
+      "cephfs" => { "path" => "/mnt/cephfs/git-data" }
     })
     ```
 

doc/update/8.17-to-9.0.md

+#### Configuration changes for repository storages
+
+This version introduces a new configuration structure for repository storages.
+Update your current configuration as follows, replacing with your storages names and paths:
+
+**For installations from source**
+
+1. Update your `gitlab.yml`, from
+
+  ```yaml
+  repositories:
+    storages: # You must have at least a 'default' storage path.
+      default: /home/git/repositories
+      nfs: /mnt/nfs/repositories
+      cephfs: /mnt/cephfs/repositories
+  ```
+
+  to
+
+  ```yaml
+  repositories:
+    storages: # You must have at least a 'default' storage path.
+      default:
+        path: /home/git/repositories
+      nfs:
+        path: /mnt/nfs/repositories
+      cephfs:
+        path: /mnt/cephfs/repositories
+  ```
+
+**For Omnibus installations**
+
+1. Upate your `/etc/gitlab/gitlab.rb`, from
+
+  ```ruby
+  git_data_dirs({
+    "default" => "/var/opt/gitlab/git-data",
+    "nfs" => "/mnt/nfs/git-data",
+    "cephfs" => "/mnt/cephfs/git-data"
+  })
+  ```
+
+  to
+
+  ```ruby
+  git_data_dirs({
+    "default" => { "path" => "/var/opt/gitlab/git-data" },
+    "nfs" => { "path" => "/mnt/nfs/git-data" },
+    "cephfs" => { "path" => "/mnt/cephfs/git-data" }
+  })
+  ```
+
 #### Nginx configuration
 
 Ensure you're still up-to-date with the latest NGINX configuration changes:

lib/api/helpers/internal_helpers.rb

@@ -9,11 +9,11 @@ module API
       # In addition, they may have a '.git' extension and multiple namespaces
       #
       # Transform all these cases to 'namespace/project'
-      def clean_project_path(project_path, storage_paths = Repository.storages.values)
+      def clean_project_path(project_path, storages = Gitlab.config.repositories.storages.values)
         project_path = project_path.sub(/\.git\z/, '')
 
-        storage_paths.each do |storage_path|
-          storage_path = File.expand_path(storage_path)
+        storages.each do |storage|
+          storage_path = File.expand_path(storage['path'])
 
           if project_path.start_with?(storage_path)
             project_path = project_path.sub(storage_path, '')

lib/backup/repository.rb

@@ -68,7 +68,8 @@ module Backup
     end
 
     def restore
-      Gitlab.config.repositories.storages.each do |name, path|
+      Gitlab.config.repositories.storages.each do |name, repository_storage|
+        path = repository_storage['path']
         next unless File.exist?(path)
 
         # Move repos dir to 'repositories.old' dir
@@ -199,7 +200,7 @@ module Backup
     private
 
     def repository_storage_paths_args
-      Gitlab.config.repositories.storages.values
+      Gitlab.config.repositories.storages.values.map { |rs| rs['path'] }
     end
   end
 end

lib/tasks/gitlab/check.rake

@@ -355,7 +355,8 @@ namespace :gitlab do
     def check_repo_base_exists
       puts "Repo base directory exists?"
 
-      Gitlab.config.repositories.storages.each do |name, repo_base_path|
+      Gitlab.config.repositories.storages.each do |name, repository_storage|
+        repo_base_path = repository_storage['path']
         print "#{name}... "
 
         if File.exist?(repo_base_path)
@@ -379,7 +380,8 @@ namespace :gitlab do
     def check_repo_base_is_not_symlink
       puts "Repo storage directories are symlinks?"
 
-      Gitlab.config.repositories.storages.each do |name, repo_base_path|
+      Gitlab.config.repositories.storages.each do |name, repository_storage|
+        repo_base_path = repository_storage['path']
         print "#{name}... "
 
         unless File.exist?(repo_base_path)
@@ -402,7 +404,8 @@ namespace :gitlab do
     def check_repo_base_permissions
       puts "Repo paths access is drwxrws---?"
 
-      Gitlab.config.repositories.storages.each do |name, repo_base_path|
+      Gitlab.config.repositories.storages.each do |name, repository_storage|
+        repo_base_path = repository_storage['path']
         print "#{name}... "
 
         unless File.exist?(repo_base_path)
@@ -432,7 +435,8 @@ namespace :gitlab do
       gitlab_shell_owner_group = Gitlab.config.gitlab_shell.owner_group
       puts "Repo paths owned by #{gitlab_shell_ssh_user}:#{gitlab_shell_owner_group}?"
 
-      Gitlab.config.repositories.storages.each do |name, repo_base_path|
+      Gitlab.config.repositories.storages.each do |name, repository_storage|
+        repo_base_path = repository_storage['path']
         print "#{name}... "
 
         unless File.exist?(repo_base_path)
@@ -811,8 +815,8 @@ namespace :gitlab do
   namespace :repo do
     desc "GitLab | Check the integrity of the repositories managed by GitLab"
     task check: :environment do
-      Gitlab.config.repositories.storages.each do |name, path|
-        namespace_dirs = Dir.glob(File.join(path, '*'))
+      Gitlab.config.repositories.storages.each do |name, repository_storage|
+        namespace_dirs = Dir.glob(File.join(repository_storage['path'], '*'))
 
         namespace_dirs.each do |namespace_dir|
           repo_dirs = Dir.glob(File.join(namespace_dir, '*'))

lib/tasks/gitlab/cleanup.rake

@@ -6,7 +6,8 @@ namespace :gitlab do
       remove_flag = ENV['REMOVE']
 
       namespaces = Namespace.pluck(:path)
-      Gitlab.config.repositories.storages.each do |name, git_base_path|
+      Gitlab.config.repositories.storages.each do |name, repository_storage|
+        git_base_path = repository_storage['path']
         all_dirs = Dir.glob(git_base_path + '/*')
 
         puts git_base_path.color(:yellow)
@@ -75,7 +76,8 @@ namespace :gitlab do
       warn_user_is_not_gitlab
 
       move_suffix = "+orphaned+#{Time.now.to_i}"
-      Gitlab.config.repositories.storages.each do |name, repo_root|
+      Gitlab.config.repositories.storages.each do |name, repository_storage|
+        repo_root = repository_storage['path']
         # Look for global repos (legacy, depth 1) and normal repos (depth 2)
         IO.popen(%W(find #{repo_root} -mindepth 1 -maxdepth 2 -name *.git)) do |find|
           find.each_line do |path|

lib/tasks/gitlab/import.rake

@@ -11,7 +11,8 @@ namespace :gitlab do
     #
     desc "GitLab | Import bare repositories from repositories -> storages into GitLab project instance"
     task repos: :environment do
-      Gitlab.config.repositories.storages.each do |name, git_base_path|
+      Gitlab.config.repositories.storages.each_value do |repository_storage|
+        git_base_path = repository_storage['path']
         repos_to_import = Dir.glob(git_base_path + '/**/*.git')
 
         repos_to_import.each do |repo_path|

lib/tasks/gitlab/info.rake

@@ -77,8 +77,8 @@ namespace :gitlab do
       puts "GitLab Shell".color(:yellow)
       puts "Version:\t#{gitlab_shell_version || "unknown".color(:red)}"
       puts "Repository storage paths:"
-      Gitlab.config.repositories.storages.each do |name, path|
-        puts "- #{name}: \t#{path}"
+      Gitlab.config.repositories.storages.each do |name, repository_storage|
+        puts "- #{name}: \t#{repository_storage['path']}"
       end
       puts "Hooks:\t\t#{Gitlab.config.gitlab_shell.hooks_path}"
       puts "Git:\t\t#{Gitlab.config.git.bin_path}"

lib/tasks/gitlab/task_helpers.rb

@@ -130,8 +130,8 @@ module Gitlab
     end
 
     def all_repos
-      Gitlab.config.repositories.storages.each do |name, path|
-        IO.popen(%W(find #{path} -mindepth 2 -maxdepth 2 -type d -name *.git)) do |find|
+      Gitlab.config.repositories.storages.each_value do |repository_storage|
+        IO.popen(%W(find #{repository_storage['path']} -mindepth 2 -maxdepth 2 -type d -name *.git)) do |find|
           find.each_line do |path|
             yield path.chomp
           end
@@ -140,7 +140,7 @@ module Gitlab
     end
 
     def repository_storage_paths_args
-      Gitlab.config.repositories.storages.values
+      Gitlab.config.repositories.storages.values.map { |rs| rs['path'] }
     end
 
     def user_home

spec/initializers/6_validations_spec.rb

@@ -14,7 +14,7 @@ describe '6_validations', lib: true do
 
   context 'with correct settings' do
     before do
-      mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/d')
+      mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/d' })
     end
 
     it 'passes through' do
@@ -24,7 +24,7 @@ describe '6_validations', lib: true do
 
   context 'with invalid storage names' do
     before do
-      mock_storages('name with spaces' => 'tmp/tests/paths/a/b/c')
+      mock_storages('name with spaces' => { 'path' => 'tmp/tests/paths/a/b/c' })
     end
 
     it 'throws an error' do
@@ -34,7 +34,7 @@ describe '6_validations', lib: true do
 
   context 'with nested storage paths' do
     before do
-      mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/c/d')
+      mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/c/d' })
     end
 
     it 'throws an error' do
@@ -44,7 +44,7 @@ describe '6_validations', lib: true do
 
   context 'with similar but un-nested storage paths' do
     before do
-      mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/c2')
+      mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/c2' })
     end
 
     it 'passes through' do
@@ -52,6 +52,26 @@ describe '6_validations', lib: true do
     end
   end
 
+  context 'with incomplete settings' do
+    before do
+      mock_storages('foo' => {})
+    end
+
+    it 'throws an error suggesting the user to update its settings' do
+      expect { validate_storages }.to raise_error('foo is not a valid storage, because it has no `path` key. Refer to gitlab.yml.example for an updated example. Please fix this in your gitlab.yml before starting GitLab.')
+    end
+  end
+
+  context 'with deprecated settings structure' do
+    before do
+      mock_storages('foo' => 'tmp/tests/paths/a/b/c')
+    end
+
+    it 'throws an error suggesting the user to update its settings' do
+      expect { validate_storages }.to raise_error("foo is not a valid storage, because it has no `path` key. It may be configured as:\n\nfoo:\n  path: tmp/tests/paths/a/b/c\n\nRefer to gitlab.yml.example for an updated example. Please fix this in your gitlab.yml before starting GitLab.")
+    end
+  end
+
   def mock_storages(storages)
     allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
   end

spec/models/namespace_spec.rb

@@ -163,7 +163,7 @@ describe Namespace, models: true do
 
   describe :rm_dir do
     let!(:project) { create(:empty_project, namespace: namespace) }
-    let!(:path) { File.join(Gitlab.config.repositories.storages.default, namespace.full_path) }
+    let!(:path) { File.join(Gitlab.config.repositories.storages.default['path'], namespace.full_path) }
 
     it "removes its dirs when deleted" do
       namespace.destroy

spec/models/project_spec.rb

@@ -183,7 +183,7 @@ describe Project, models: true do
       let(:project2) { build(:empty_project, repository_storage: 'missing') }
 
       before do
-        storages = { 'custom' => 'tmp/tests/custom_repositories' }
+        storages = { 'custom' => { 'path' => 'tmp/tests/custom_repositories' } }
         allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
       end
 
@@ -392,7 +392,7 @@ describe Project, models: true do
 
     before do
       FileUtils.mkdir('tmp/tests/custom_repositories')
-      storages = { 'custom' => 'tmp/tests/custom_repositories' }
+      storages = { 'custom' => { 'path' => 'tmp/tests/custom_repositories' } }
       allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
     end
 
@@ -1138,8 +1138,8 @@ describe Project, models: true do
 
     before do
       storages = {
-        'default' => 'tmp/tests/repositories',
-        'picked'  => 'tmp/tests/repositories',
+        'default' => { 'path' => 'tmp/tests/repositories' },
+        'picked'  => { 'path' => 'tmp/tests/repositories' },
       }
       allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
     end

spec/requests/api/api_internal_helpers_spec.rb

@@ -21,7 +21,7 @@ describe ::API::Helpers::InternalHelpers do
         # Relative and absolute storage paths, with and without trailing /
         ['.', './', Dir.pwd, Dir.pwd + '/'].each do |storage_path|
           context "storage path is #{storage_path}" do
-            subject { clean_project_path(project_path, [storage_path]) }
+            subject { clean_project_path(project_path, [{ 'path' => storage_path }]) }
 
             it { is_expected.to eq(expected) }
           end

spec/support/test_env.rb

@@ -145,7 +145,7 @@ module TestEnv
   end
 
   def repos_path
-    Gitlab.config.repositories.storages.default
+    Gitlab.config.repositories.storages.default['path']
   end
 
   def backup_path

spec/tasks/gitlab/backup_rake_spec.rb

@@ -227,8 +227,8 @@ describe 'gitlab:app namespace rake task' do
         FileUtils.mkdir('tmp/tests/default_storage')
         FileUtils.mkdir('tmp/tests/custom_storage')
         storages = {
-          'default' => 'tmp/tests/default_storage',
-          'custom' => 'tmp/tests/custom_storage'
+          'default' => { 'path' => 'tmp/tests/default_storage' },
+          'custom' => { 'path' => 'tmp/tests/custom_storage' }
         }
         allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
 

spec/workers/post_receive_spec.rb

@@ -116,6 +116,6 @@ describe PostReceive do
   end
 
   def pwd(project)
-    File.join(Gitlab.config.repositories.storages.default, project.path_with_namespace)
+    File.join(Gitlab.config.repositories.storages.default['path'], project.path_with_namespace)
   end
 end