Merge branch 'sh-fix-redis-disable' into 'master'

Fix bug where OmnibusHelper#not_listening? returned true when service was disabled

Closes #2302

See merge request !1527

files/gitlab-cookbooks/gitlab/libraries/omnibus_helper.rb

@@ -14,7 +14,9 @@ class OmnibusHelper
   end
 
   def not_listening?(service_name)
-    File.exists?("/opt/gitlab/service/#{service_name}/down") && service_down?(service_name)
+    return true unless service_enabled?(service_name)
+
+    service_down?(service_name)
   end
 
   def service_enabled?(service_name)
@@ -36,4 +38,4 @@ class OmnibusHelper
   def group_exists?(group)
     success?("getent group #{group}")
   end
-end
+end unless defined?(OmnibusHelper) # Prevent reloading in chefspec: https://github.com/sethvargo/chefspec/issues/562#issuecomment-74120922

spec/libraries/omnibus_helper_spec.rb

@@ -3,8 +3,14 @@ require_relative '../../files/gitlab-cookbooks/gitlab/libraries/omnibus_helper.r
 
 describe OmnibusHelper do
   let(:chef_run) { ChefSpec::SoloRunner.converge('gitlab::config') }
+  let(:node) { chef_run.node }
+
   subject { described_class.new(chef_run.node) }
 
+  before do
+    allow(Gitlab).to receive(:[]).and_call_original
+  end
+
   describe '#user_exists?' do
     it 'returns true when user exists' do
       expect(subject.user_exists?('root')).to be_truthy
@@ -24,4 +30,38 @@ describe OmnibusHelper do
       expect(subject.group_exists?('nonexistentgroup')).to be_falsey
     end
   end
+
+  describe '#not_listening?' do
+    context 'when Redis is disabled' do
+      before do
+        stub_gitlab_rb(
+          redis: { enable: false }
+        )
+      end
+
+      it 'returns true when service is disabled' do
+        expect(subject.not_listening?('redis')).to be_truthy
+      end
+    end
+
+    context 'when Redis is enabled' do
+      before do
+        stub_gitlab_rb(
+          redis: { enable: true }
+        )
+      end
+
+      it 'returns true when service is down' do
+        stub_service_failure_status('redis', true)
+
+        expect(subject.not_listening?('redis')).to be_truthy
+      end
+
+      it 'returns false when service is up' do
+        stub_service_failure_status('redis', false)
+
+        expect(subject.not_listening?('redis')).to be_falsey
+      end
+    end
+  end
 end

spec/support/macros.rb

@@ -9,9 +9,17 @@ module GitlabSpec
       end
     end
 
+    def stub_service_success_status(service, value)
+      allow_any_instance_of(OmnibusHelper).to receive(:success?).with("/opt/gitlab/init/#{service} status").and_return(value)
+    end
+
+    def stub_service_failure_status(service, value)
+      allow_any_instance_of(OmnibusHelper).to receive(:failure?).with("/opt/gitlab/init/#{service} status").and_return(value)
+    end
+
     def stub_should_notify?(service, value)
       allow(File).to receive(:symlink?).with("/opt/gitlab/service/#{service}").and_return(value)
-      allow_any_instance_of(OmnibusHelper).to receive(:success?).with("/opt/gitlab/init/#{service} status").and_return(value)
+      stub_service_success_status(service, value)
     end
 
     def stub_env_var(var, value)