Skip to content

Use backup path when taking backups during a restore

What does this MR do?

Places backups taken during a restore in the configured backup path rather than the parent directory of the files to be backed up.

Are there points in the code the reviewer needs to double check?

I did not see a logical home for a test of this code change in backup_rake_spec.rb. If the reviewer feels a test is warranted, I'd appreciate some advice in this area.

Why was this MR needed?

We have an HA cluster configured with custom shared data locations as specified in https://docs.gitlab.com/ee/administration/high_availability/nfs.html Attempts to restore a backup fail with a Permission Denied error. This is because the restore process attempts to move the existing artifacts (e.g., uploads) into a new timestamped directory that is in the parent directory of the existing artifacts. In our case, this parent directory is owned by root rather than the gitlab user. I imagine many users with custom data locations will get burned here. Rather than requiring and documenting that the parent folder be owned by the gitlab user, it makes more sense to me to put these backups in the already configured backup path.

Failure: Restoring uploads ... rake aborted! Errno::EACCES: Permission denied @ rb_file_s_rename - (/gitlab-data/uploads, /gitlab-data/uploads.1492548489) /opt/gitlab/embedded/service/gitlab-rails/lib/backup/files.rb:46:in 'backup_existing_files_dir' /opt/gitlab/embedded/service/gitlab-rails/lib/backup/files.rb:37:in 'restore' /opt/gitlab/embedded/service/gitlab-rails/lib/tasks/gitlab/backup.rake:140:in 'block (4 levels) in <top (required)>' /opt/gitlab/embedded/service/gitlab-rails/lib/tasks/gitlab/backup.rake:57:in 'block (3 levels) in <top (required)>'

Screenshots (if relevant)

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Merge request reports