Fix short circuit logic between rsync with and without ionice for

storage migrations

3 files changed, 41 insertions, 1 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 7f5ca75..5021721 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,6 @@
+v3.6.4
+  - Fix short circuit logic between rsync with and without ionice for storage migrations
+
 v3.6.3
   - Re-exposing GL_ID to custom hooks
 
diff --git a/lib/gitlab_projects.rb b/lib/gitlab_projects.rb
index 3d92cf2..d3d45ae 100644
--- a/lib/gitlab_projects.rb
+++ b/lib/gitlab_projects.rb
@@ -321,7 +321,9 @@ class GitlabProjects
       rsync_path = 'ionice -c2 -n7 rsync'
       result = system(*%W(#{rsync_path} -a --delete --rsync-path="#{rsync_path}" #{source_path} #{new_full_path}))
 
-      unless result
+      if result
+        true
+      else
         # If the command fails with `ionice` (maybe because we're on a OS X
         # development machine), try again without `ionice`.
         rsync_path = 'rsync'
diff --git a/spec/gitlab_projects_spec.rb b/spec/gitlab_projects_spec.rb
index bc615d4..149d53f 100644
--- a/spec/gitlab_projects_spec.rb
+++ b/spec/gitlab_projects_spec.rb
@@ -227,6 +227,41 @@ describe GitlabProjects do
       FileUtils.cd(new_repo_path) { Dir['**/*'].length.should_not be(0) }
     end
 
+    it "should attempt rsync with ionice first" do
+      expect(gl_projects).to receive(:system).with(
+        'ionice -c2 -n7 rsync', '-a', '--delete', '--rsync-path="ionice -c2 -n7 rsync"',
+        "#{tmp_repo_path}/", new_repo_path
+      ).and_return(true)
+
+      gl_projects.exec.should be_true
+    end
+
+    it "should attempt rsync without ionice if with ionice fails" do
+      expect(gl_projects).to receive(:system).with(
+        'ionice -c2 -n7 rsync', '-a', '--delete', '--rsync-path="ionice -c2 -n7 rsync"',
+        "#{tmp_repo_path}/", new_repo_path
+      ).and_return(false)
+
+      expect(gl_projects).to receive(:system).with(
+        'rsync', '-a', '--delete', '--rsync-path="rsync"', "#{tmp_repo_path}/", new_repo_path
+      ).and_return(true)
+
+      gl_projects.exec.should be_true
+    end
+
+    it "should fail if both rsync attempts fail" do
+      expect(gl_projects).to receive(:system).with(
+        'ionice -c2 -n7 rsync', '-a', '--delete', '--rsync-path="ionice -c2 -n7 rsync"',
+        "#{tmp_repo_path}/", new_repo_path
+      ).and_return(false)
+
+      expect(gl_projects).to receive(:system).with(
+        'rsync', '-a', '--delete', '--rsync-path="rsync"', "#{tmp_repo_path}/", new_repo_path
+      ).and_return(false)
+
+      gl_projects.exec.should be_false
+    end
+
     it "should fail if no destination path is provided" do
       incomplete = build_gitlab_projects('mv-storage', tmp_repos_path, repo_name)
       $logger.should_receive(:error).with("mv-storage failed: no destination storage path provided.")