From 6cfaab3f7646b45aac7c5bf8609e777d17ce4897 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 16 Jan 2017 09:11:36 +0100 Subject: Fix remove dot git migration failing when user has no projects - Fixed typo - Fixed migration when there are no projects and path is nil - Added path rollback that was missing if there was a SQL error --- ...20161226122833_remove_dot_git_from_usernames.rb | 39 ++++++++++++++++------ 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb index 7d97339581f..6e4b164b772 100644 --- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb +++ b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb @@ -15,15 +15,26 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration path_was = user['username'] path_was_wildcard = quote_string("#{path_was}/%") - path = move_namespace(namespace_id, path_was, path) - - execute "UPDATE routes SET path = '#{path}' WHERE source_type = 'Namespace' AND source_id = #{namespace_id}" - execute "UPDATE namespaces SET path = '#{path}' WHERE id = #{namespace_id}" - execute "UPDATE users SET username = '#{path}' WHERE id = #{id}" - - select_all("SELECT id, path FROM routes WHERE path LIKE '#{path_was_wildcard}'").each do |route| - new_path = "#{path}/#{route['path'].split('/').last}" - execute "UPDATE routes SET path = '#{new_path}' WHERE id = #{route['id']}" + # It's possible for `move_namespace` to return nil if the given namespace + # has nothing on storage (i.e., they never made a project). + path = move_namespace(namespace_id, path_was) || rename_path(path_was) + + begin + execute "UPDATE routes SET path = '#{path}' WHERE source_type = 'Namespace' AND source_id = #{namespace_id}" + execute "UPDATE namespaces SET path = '#{path}' WHERE id = #{namespace_id}" + execute "UPDATE users SET username = '#{path}' WHERE id = #{id}" + + select_all("SELECT id, path FROM routes WHERE path LIKE '#{path_was_wildcard}'").each do |route| + new_path = "#{path}/#{route['path'].split('/').last}" + execute "UPDATE routes SET path = '#{new_path}' WHERE id = #{route['id']}" + end + rescue => e + # Move namespace path back, if it has been moved already. + unless path_exists?(repository_storage_path, path_was) + gitlab_shell.mv_namespace(repository_storage_path, path, path_was) + end + + raise e end end end @@ -87,7 +98,15 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration end end - Gitlab::UploadsTransfer.new.rename_namespace(path_was, path) + begin + Gitlab::UploadsTransfer.new.rename_namespace(path_was, path) + rescue => e + if path.nil? + say("Couldn't find a storage path for #{namespace_id}, #{path_was} -- skipping") + else + raise e + end + end path end -- cgit v1.2.1 From 7d03a55c98dfdc6b27df903b056427089aa0dc95 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 16 Jan 2017 09:49:58 +0100 Subject: Remove rollback and fixed a couple of issues --- .../20161226122833_remove_dot_git_from_usernames.rb | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb index 6e4b164b772..82ad7fd1b35 100644 --- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb +++ b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb @@ -17,7 +17,7 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration # It's possible for `move_namespace` to return nil if the given namespace # has nothing on storage (i.e., they never made a project). - path = move_namespace(namespace_id, path_was) || rename_path(path_was) + path = move_namespace(namespace_id, path_was) || new_path(path_was) begin execute "UPDATE routes SET path = '#{path}' WHERE source_type = 'Namespace' AND source_id = #{namespace_id}" @@ -29,10 +29,7 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration execute "UPDATE routes SET path = '#{new_path}' WHERE id = #{route['id']}" end rescue => e - # Move namespace path back, if it has been moved already. - unless path_exists?(repository_storage_path, path_was) - gitlab_shell.mv_namespace(repository_storage_path, path, path_was) - end + say("Couldn't update routes for path #{path_was} to #{path}") raise e end @@ -55,13 +52,13 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration select_all("SELECT id, path FROM routes WHERE path = '#{quote_string(path)}'").present? end - def path_exists?(repository_storage_path, path) - gitlab_shell.exists?(repository_storage_path, path) + def path_exists?(path, repository_storage_path) + repository_storage_path && gitlab_shell.exists?(repository_storage_path, path) end # Accepts invalid path like test.git and returns test_git or # test_git1 if test_git already taken - def rename_path(repository_storage_path, path) + def new_path(path, repository_storage_path = nil) # To stay closer with original name and reduce risk of duplicates # we rename suffix instead of removing it path = path.sub(/\.git\z/, '_git') @@ -69,7 +66,7 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration counter = 0 base = path - while route_exists?(path) || path_exists?(repository_storage_path, path) + while route_exists?(path) || path_exists?(path, repository_storage_path) counter += 1 path = "#{base}#{counter}" end @@ -77,7 +74,7 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration path end - def move_namespace(namespace_id, path_was, path) + def move_namespace(namespace_id, path_was) 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']] end.compact @@ -87,7 +84,7 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration # Ensure old directory exists before moving it gitlab_shell.add_namespace(repository_storage_path, path_was) - path = quote_string(rename_path(repository_storage_path, path_was)) + path = quote_string(new_path(path_was, repository_storage_path)) unless gitlab_shell.mv_namespace(repository_storage_path, path_was, path) Rails.logger.error "Exception moving path #{repository_storage_path} from #{path_was} to #{path}" -- cgit v1.2.1 From a9402708b2668be9cc4ef5fac65f5116648f042b Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 16 Jan 2017 10:40:29 +0100 Subject: fix nil path error --- db/migrate/20161226122833_remove_dot_git_from_usernames.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb index 82ad7fd1b35..edf9914308c 100644 --- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb +++ b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb @@ -79,6 +79,8 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration Gitlab.config.repositories.storages[row['repository_storage']] end.compact + path = nil + # Move the namespace directory in all storages paths used by member projects repository_storage_paths.each do |repository_storage_path| # Ensure old directory exists before moving it -- cgit v1.2.1 From 9302fbd08c55dc7a98bc07031ac1f5eb6105cbb0 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 16 Jan 2017 12:59:36 +0100 Subject: cope with namespace duplicated paths in any storage --- ...20161226122833_remove_dot_git_from_usernames.rb | 32 ++++++++++++---------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb index edf9914308c..479cb544088 100644 --- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb +++ b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb @@ -14,10 +14,9 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration namespace_id = user['namespace_id'] path_was = user['username'] path_was_wildcard = quote_string("#{path_was}/%") + path = quote_string(new_path(path_was)) - # It's possible for `move_namespace` to return nil if the given namespace - # has nothing on storage (i.e., they never made a project). - path = move_namespace(namespace_id, path_was) || new_path(path_was) + move_namespace(namespace_id, path_was, path) begin execute "UPDATE routes SET path = '#{path}' WHERE source_type = 'Namespace' AND source_id = #{namespace_id}" @@ -30,6 +29,8 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration end rescue => e say("Couldn't update routes for path #{path_was} to #{path}") + # Move namespace back + move_namespace(namespace_id, path, path_was) raise e end @@ -58,36 +59,39 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration # Accepts invalid path like test.git and returns test_git or # test_git1 if test_git already taken - def new_path(path, repository_storage_path = nil) + def new_path(path) # To stay closer with original name and reduce risk of duplicates # we rename suffix instead of removing it path = path.sub(/\.git\z/, '_git') - counter = 0 - base = path + check_routes(path.dup, 0, path) + end + + def check_routes(base, counter, path) + Gitlab.config.repositories.storages.each_with_index do |(_key, storage), index| + if route_exists?(path) || path_exists?(path, storage) + counter += 1 + path = "#{base}#{counter}" - while route_exists?(path) || path_exists?(path, repository_storage_path) - counter += 1 - path = "#{base}#{counter}" + # Start again unless this is the first storage, + # to make sure no other storages contain the new path already. + return check_route(base, counter, path) unless index.zero? + end end path end - def move_namespace(namespace_id, path_was) + 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']] end.compact - path = nil - # Move the namespace directory in all storages paths used by member projects repository_storage_paths.each do |repository_storage_path| # Ensure old directory exists before moving it gitlab_shell.add_namespace(repository_storage_path, path_was) - path = quote_string(new_path(path_was, repository_storage_path)) - unless gitlab_shell.mv_namespace(repository_storage_path, path_was, path) Rails.logger.error "Exception moving path #{repository_storage_path} from #{path_was} to #{path}" -- cgit v1.2.1 From 40fe772279e28b70115ad3d150d84cac579004f5 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 16 Jan 2017 13:18:54 +0100 Subject: fix bug in loop --- db/migrate/20161226122833_remove_dot_git_from_usernames.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb index 479cb544088..4e59cbd8a7f 100644 --- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb +++ b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb @@ -68,14 +68,12 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration end def check_routes(base, counter, path) - Gitlab.config.repositories.storages.each_with_index do |(_key, storage), index| + Gitlab.config.repositories.storages.each_value do |storage| if route_exists?(path) || path_exists?(path, storage) counter += 1 path = "#{base}#{counter}" - # Start again unless this is the first storage, - # to make sure no other storages contain the new path already. - return check_route(base, counter, path) unless index.zero? + return check_route(base, counter, path) end end @@ -110,7 +108,5 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration raise e end end - - path end end -- cgit v1.2.1 From 320cd7f3648b0e30bf48d0fff461d1f79090a589 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 16 Jan 2017 13:37:04 +0100 Subject: fix var inside loop --- db/migrate/20161226122833_remove_dot_git_from_usernames.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb index 4e59cbd8a7f..c3a8f2fd91c 100644 --- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb +++ b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb @@ -68,8 +68,10 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration end def check_routes(base, counter, path) + route_exists = route_exists?(path) + Gitlab.config.repositories.storages.each_value do |storage| - if route_exists?(path) || path_exists?(path, storage) + if route_exists && path_exists?(path, storage) counter += 1 path = "#{base}#{counter}" -- cgit v1.2.1 From efea5e70966ccd656577c794e767ac00154b5c83 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 16 Jan 2017 13:39:57 +0100 Subject: fix typo --- db/migrate/20161226122833_remove_dot_git_from_usernames.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb index c3a8f2fd91c..a01f7e4ed52 100644 --- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb +++ b/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) counter += 1 path = "#{base}#{counter}" -- cgit v1.2.1