Restore: treat SKIP_REPOSITORIES_PATHS like REPOSITORIES_PATHS when deciding whether to clear existing repositories

Summary

When restoring a gitlab-backup archive, Backup::Targets::Repositories#remove_all_repositories decides whether to wipe existing repositories from disk/Gitaly storage before restoring, based solely on whether REPOSITORIES_PATHS was used to create the backup:

def remove_all_repositories
  return if paths.present?

  storages.presence || Gitlab.config.repositories.storages.keys
end

(lib/backup/targets/repositories.rb)

If REPOSITORIES_PATHS is set, the clear step is skipped, which allows multiple backups of different repository subsets to be restored sequentially without each restore wiping out repositories restored by a previous one.

However, if a backup was instead created using only SKIP_REPOSITORIES_PATHS (leaving REPOSITORIES_PATHS unset/null) to exclude a subset of repositories, paths is empty on restore, so remove_all_repositories returns all configured storages and every existing repository is removed before the restore runs. This makes it impossible to sequentially restore multiple SKIP_REPOSITORIES_PATHS-based subset backups the same way it works for REPOSITORIES_PATHS-based ones, since each restore destroys the repositories left by the previous restore.

Why the "clear existing repositories" step exists

The -remove-all-repositories step passed to gitaly-backup on restore exists to remove stale/dangling repositories, i.e. repositories that exist on disk/in Gitaly storage but are not part of the backup being restored (for example, repos for projects created or deleted after the backup was taken, or otherwise orphaned repo data). Without this step, a restore could leave behind repository data that doesn't correspond to any project/snippet in the restored database, resulting in an inconsistent final state. This is exercised by the "removes stale data" test coverage in spec/tasks/gitlab/backup_rake_spec.rb.

The guard on repositories_paths (return if paths.present?) was added specifically to support the workflow of restoring several partial backups (each created with REPOSITORIES_PATHS for a different subset of repos) into the same instance in sequence, since wiping all repositories on each restore would defeat that workflow. This same reasoning was apparently never extended to skip_repositories_paths, even though a backup created with SKIP_REPOSITORIES_PATHS is conceptually the same kind of "partial backup" (just defined by exclusion rather than inclusion).

Proposal

  1. Update Backup::Targets::Repositories#remove_all_repositories (and any related restore-metadata handling in lib/backup/manager.rb / lib/backup/metadata.rb) so that a backup created with SKIP_REPOSITORIES_PATHS set (and REPOSITORIES_PATHS empty) is also treated as a partial backup for restore purposes, i.e. the repository-clearing step is skipped in that case too. This would allow sequential restores of different SKIP_REPOSITORIES_PATHS-based subset backups, consistent with current REPOSITORIES_PATHS behavior.
  2. Consider additionally exposing this as an explicit restore-time option/flag (e.g. something like REMOVE_ALL_REPOSITORIES=true|false), rather than having the script infer the behavior purely from which paths options were used to create the backup. This would let operators explicitly control whether stale repository data is cleared on restore, independent of how the backup happened to be created, and would cover any future cases the path-based heuristic doesn't anticipate.

Relevant code

  • lib/backup/targets/repositories.rb - remove_all_repositories method
  • lib/backup/gitaly_backup.rb - passes -remove-all-repositories to the gitaly-backup binary
  • lib/backup/metadata.rb - persists repositories_paths / skip_repositories_paths in backup metadata
  • lib/backup/options.rb - repositories_paths / skip_repositories_paths accessors
  • Docs: doc/administration/backup_restore/restore_gitlab.md (Restore specific repositories section)
Edited by 🤖 GitLab Bot 🤖