Merge branch 'bvl-warn-if-move-fails-on-mountpoint' into 'master'

Warning when restore fails when data-dir is a moint point

Closes #19309

See merge request gitlab-org/gitlab-ce!18533

7 files changed, 108 insertions, 37 deletions

diff --git a/doc/administration/high_availability/nfs.md b/doc/administration/high_availability/nfs.md
index d8928a7fe4c..ad8ffc46559 100644
--- a/doc/administration/high_availability/nfs.md
+++ b/doc/administration/high_availability/nfs.md
@@ -75,43 +75,33 @@ Notice several options that you should consider using:
 | `nobootwait` | Don't halt boot process waiting for this mount to become available
 | `lookupcache=positive` | Tells the NFS client to honor `positive` cache results but invalidates any `negative` cache results. Negative cache results cause problems with Git. Specifically, a `git push` can fail to register uniformly across all NFS clients. The negative cache causes the clients to 'remember' that the files did not exist previously.
 
-## Mount locations
+## A single NFS mount
 
-When using default Omnibus configuration you will need to share 5 data locations
-between all GitLab cluster nodes. No other locations should be shared. The
-following are the 5 locations you need to mount:
-
-| Location | Description | Default configuration |
-| -------- | ----------- | --------------------- |
-| `/var/opt/gitlab/git-data` | Git repository data. This will account for a large portion of your data | `git_data_dirs({"default" => "/var/opt/gitlab/git-data"})`
-| `/var/opt/gitlab/.ssh` | SSH `authorized_keys` file and keys used to import repositories from some other Git services | `user['home'] = '/var/opt/gitlab/'`
-| `/var/opt/gitlab/gitlab-rails/uploads` | User uploaded attachments | `gitlab_rails['uploads_directory'] = '/var/opt/gitlab/gitlab-rails/uploads'`
-| `/var/opt/gitlab/gitlab-rails/shared` | Build artifacts, GitLab Pages, LFS objects, temp files, etc. If you're using LFS this may also account for a large portion of your data | `gitlab_rails['shared_path'] = '/var/opt/gitlab/gitlab-rails/shared'`
-| `/var/opt/gitlab/gitlab-ci/builds` | GitLab CI build traces | `gitlab_ci['builds_directory'] = '/var/opt/gitlab/gitlab-ci/builds'`
+It's recommended to nest all gitlab data dirs within a mount, that allows automatic
+restore of backups without manually moving existing data.
 
-Other GitLab directories should not be shared between nodes. They contain
-node-specific files and GitLab code that does not need to be shared. To ship
-logs to a central location consider using remote syslog. GitLab Omnibus packages
-provide configuration for [UDP log shipping][udp-log-shipping].
-
-### Consolidating mount points
-
-If you don't want to configure 5-6 different NFS mount points, you have a few
-alternative options.
+```
+mountpoint
+└── gitlab-data
+    ├── builds
+    ├── git-data
+    ├── home-git
+    ├── shared
+    └── uploads
+```
 
-#### Change default file locations
+To do so, we'll need to configure Omnibus with the paths to each directory nested
+in the mount point as follows:
 
-Omnibus allows you to configure the file locations. With custom configuration
-you can specify just one main mountpoint and have all of these locations
-as subdirectories. Mount `/gitlab-data` then use the following Omnibus
+Mount `/gitlab-nfs` then use the following Omnibus
 configuration to move each data location to a subdirectory:
 
 ```ruby
-git_data_dirs({"default" => "/gitlab-data/git-data"})
-user['home'] = '/gitlab-data/home'
-gitlab_rails['uploads_directory'] = '/gitlab-data/uploads'
-gitlab_rails['shared_path'] = '/gitlab-data/shared'
-gitlab_ci['builds_directory'] = '/gitlab-data/builds'
+git_data_dirs({"default" => "/gitlab-nfs/gitlab-data/git-data"})
+user['home'] = '/gitlab-nfs/gitlab-data/home'
+gitlab_rails['uploads_directory'] = '/gitlab-nfs/gitlab-data/uploads'
+gitlab_rails['shared_path'] = '/gitlab-nfs/gitlab-data/shared'
+gitlab_ci['builds_directory'] = '/gitlab-nfs/gitlab-data/builds'
 ```
 
 To move the `git` home directory, all GitLab services must be stopped. Run
@@ -122,22 +112,52 @@ Run `sudo gitlab-ctl reconfigure` to start using the central location. Please
 be aware that if you had existing data you will need to manually copy/rsync it
 to these new locations and then restart GitLab.
 
-#### Bind mounts
+## Bind mounts
+
+Alternatively to changing the configuration in Omnibus, bind mounts can be used
+to store the data on an NFS mount.
 
 Bind mounts provide a way to specify just one NFS mount and then
 bind the default GitLab data locations to the NFS mount. Start by defining your
 single NFS mount point as you normally would in `/etc/fstab`. Let's assume your
-NFS mount point is `/gitlab-data`. Then, add the following bind mounts in
+NFS mount point is `/gitlab-nfs`. Then, add the following bind mounts in
 `/etc/fstab`:
 
 ```bash
-/gitlab-data/git-data /var/opt/gitlab/git-data none bind 0 0
-/gitlab-data/.ssh /var/opt/gitlab/.ssh none bind 0 0
-/gitlab-data/uploads /var/opt/gitlab/gitlab-rails/uploads none bind 0 0
-/gitlab-data/shared /var/opt/gitlab/gitlab-rails/shared none bind 0 0
-/gitlab-data/builds /var/opt/gitlab/gitlab-ci/builds none bind 0 0
+/gitlab-nfs/gitlab-data/git-data /var/opt/gitlab/git-data none bind 0 0
+/gitlab-nfs/gitlab-data/.ssh /var/opt/gitlab/.ssh none bind 0 0
+/gitlab-nfs/gitlab-data/uploads /var/opt/gitlab/gitlab-rails/uploads none bind 0 0
+/gitlab-nfs/gitlab-data/shared /var/opt/gitlab/gitlab-rails/shared none bind 0 0
+/gitlab-nfs/gitlab-data/builds /var/opt/gitlab/gitlab-ci/builds none bind 0 0
 ```
 
+Using bind mounts will require manually making sure the data directories
+are empty before attempting a restore. Read more about the
+[restore prerequisites](../../raketasks/backup_restore.md).
+
+## Multiple NFS mounts
+
+When using default Omnibus configuration you will need to share 5 data locations
+between all GitLab cluster nodes. No other locations should be shared. The
+following are the 5 locations need to be shared:
+
+| Location | Description | Default configuration |
+| -------- | ----------- | --------------------- |
+| `/var/opt/gitlab/git-data` | Git repository data. This will account for a large portion of your data | `git_data_dirs({"default" => "/var/opt/gitlab/git-data"})`
+| `/var/opt/gitlab/.ssh` | SSH `authorized_keys` file and keys used to import repositories from some other Git services | `user['home'] = '/var/opt/gitlab/'`
+| `/var/opt/gitlab/gitlab-rails/uploads` | User uploaded attachments | `gitlab_rails['uploads_directory'] = '/var/opt/gitlab/gitlab-rails/uploads'`
+| `/var/opt/gitlab/gitlab-rails/shared` | Build artifacts, GitLab Pages, LFS objects, temp files, etc. If you're using LFS this may also account for a large portion of your data | `gitlab_rails['shared_path'] = '/var/opt/gitlab/gitlab-rails/shared'`
+| `/var/opt/gitlab/gitlab-ci/builds` | GitLab CI build traces | `gitlab_ci['builds_directory'] = '/var/opt/gitlab/gitlab-ci/builds'`
+
+Other GitLab directories should not be shared between nodes. They contain
+node-specific files and GitLab code that does not need to be shared. To ship
+logs to a central location consider using remote syslog. GitLab Omnibus packages
+provide configuration for [UDP log shipping][udp-log-shipping].
+
+Having multiple NFS mounts will require manually making sure the data directories
+are empty before attempting a restore. Read more about the
+[restore prerequisites](../../raketasks/backup_restore.md).
+
 ---
 
 Read more on high-availability configuration:
diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md
index bbd2d214fe4..785cc32d590 100644
--- a/doc/raketasks/backup_restore.md
+++ b/doc/raketasks/backup_restore.md
@@ -498,6 +498,13 @@ more of the following options:
   Read what the [backup timestamp is about](#backup-timestamp).
 - `force=yes` - Does not ask if the authorized_keys file should get regenerated and assumes 'yes' for warning that database tables will be removed.
 
+If you are restoring into directories that are mountpoints you will need to make
+sure these directories are empty before attempting a restore. Otherwise GitLab
+will attempt to move these directories before restoring the new data and this
+would cause an error.
+
+Read more on [configuring NFS mounts](../administration/high_availability/nfs.md)
+
 ### Restore for installation from source
 
 ```
diff --git a/lib/backup/files.rb b/lib/backup/files.rb
index 88cb7e7b5a4..9895db9e451 100644
--- a/lib/backup/files.rb
+++ b/lib/backup/files.rb
@@ -53,6 +53,8 @@ module Backup
           FileUtils.mv(files, timestamped_files_path)
         rescue Errno::EACCES
           access_denied_error(app_files_dir)
+        rescue Errno::EBUSY
+          resource_busy_error(app_files_dir)
         end
       end
     end
diff --git a/lib/backup/helper.rb b/lib/backup/helper.rb
index a1ee0faefe9..54b9ce10b4d 100644
--- a/lib/backup/helper.rb
+++ b/lib/backup/helper.rb
@@ -13,5 +13,19 @@ module Backup
       EOS
       raise message
     end
+
+    def resource_busy_error(path)
+      message = <<~EOS
+
+      ### NOTICE ###
+      As part of restore, the task tried to rename `#{path}` before restoring.
+      This could not be completed, perhaps `#{path}` is a mountpoint?
+
+      To complete the restore, please move the contents of `#{path}` to a
+      different location and run the restore task again.
+
+      EOS
+      raise message
+    end
   end
 end
diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb
index 89e3f1d9076..65e06fd78c0 100644
--- a/lib/backup/repository.rb
+++ b/lib/backup/repository.rb
@@ -81,6 +81,8 @@ module Backup
           FileUtils.mv(files, bk_repos_path)
         rescue Errno::EACCES
           access_denied_error(path)
+        rescue Errno::EBUSY
+          resource_busy_error(path)
         end
       end
     end
diff --git a/spec/lib/backup/files_spec.rb b/spec/lib/backup/files_spec.rb
index 14d055cbcc1..99872211a4e 100644
--- a/spec/lib/backup/files_spec.rb
+++ b/spec/lib/backup/files_spec.rb
@@ -62,5 +62,19 @@ describe Backup::Files do
         subject.restore
       end
     end
+
+    describe 'folders that are a mountpoint' do
+      before do
+        allow(FileUtils).to receive(:mv).and_raise(Errno::EBUSY)
+        allow(subject).to receive(:run_pipeline!).and_return(true)
+      end
+
+      it 'shows error message' do
+        expect(subject).to receive(:resource_busy_error).with("/var/gitlab-registry")
+                             .and_call_original
+
+        expect { subject.restore }.to raise_error(/is a mountpoint/)
+      end
+    end
   end
 end
diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repository_spec.rb
index e4c1c9bafc0..b3777be312b 100644
--- a/spec/lib/backup/repository_spec.rb
+++ b/spec/lib/backup/repository_spec.rb
@@ -81,6 +81,18 @@ describe Backup::Repository do
         subject.restore
       end
     end
+
+    describe 'folder that is a mountpoint' do
+      before do
+        allow(FileUtils).to receive(:mv).and_raise(Errno::EBUSY)
+      end
+
+      it 'shows error message' do
+        expect(subject).to receive(:resource_busy_error).and_call_original
+
+        expect { subject.restore }.to raise_error(/is a mountpoint/)
+      end
+    end
   end
 
   describe '#empty_repo?' do