Add rspec tests and remove orphan socket file

- Adds a suite of rspec tests for the gitlab workhorse helper
- Checks for and removes orphan workhorse socket files if present

Signed-off-by: Robert Marshall <rmarshall@gitlab.com>

files/gitlab-cookbooks/gitlab/libraries/helpers/gitlab_workhorse.rb

@@ -22,6 +22,27 @@ class GitlabWorkhorseHelper < BaseHelper
     node['gitlab']['gitlab-workhorse']['listen_network'] == "unix"
   end
+  def deprecated_socket
+    '/var/opt/gitlab/gitlab-workhorse/socket'
+  end
+
+  def orphan_socket
+    default_path = node.default['gitlab']['gitlab-workhorse']['listen_addr']
+    configured_path = node['gitlab']['gitlab-workhorse']['listen_addr']
+
+    return deprecated_socket if default_path == configured_path
+
+    configured_path
+  end
+
+  def orphan_socket?
+    return false unless unix_socket?
+
+    cleanup_needed = orphan_socket != listen_address
+
+    File.exist?(orphan_socket) && cleanup_needed
+  end
+
   def listen_address
     return File.join(sockets_directory, socket_file_name) if unix_socket?

files/gitlab-cookbooks/gitlab/recipes/gitlab-workhorse.rb

@@ -60,7 +60,9 @@ end
 runit_service 'gitlab-workhorse' do
   start_down node['gitlab']['gitlab-workhorse']['ha']
   options({
-    log_directory: log_directory
+    log_directory: log_directory,
+    found_orphan_socket: workhorse_helper.orphan_socket?,
+    orphan_socket_path: workhorse_helper.orphan_socket
   }.merge(params))
   log_options node['gitlab']['logging'].to_hash.merge(node['gitlab']['gitlab-workhorse'].to_hash)
 end

files/gitlab-cookbooks/gitlab/templates/default/sv-gitlab-workhorse-run.erb

@@ -8,6 +8,13 @@ exec 2>&1
 cd <%= node['gitlab']['gitlab-workhorse']['dir'] %>
+<% if @options[:found_orphan_socket] %>
+if [ -e "<%= @options[:orphan_socket_path] %>" ]; then
+  echo "Removing orphaned workhorse socket at <%= @options[:orphan_socket_path] %>"
+  rm "<%= @options[:orphan_socket_path] %>"
+fi
+<% end %>
+
 exec chpst -e /opt/gitlab/etc/gitlab-workhorse/env -P \
   -U <%= node['gitlab']['user']['username'] %>:<%= node['gitlab']['user']['group'] %> \
   -u <%= node['gitlab']['user']['username'] %>:<%= node['gitlab']['user']['group'] %> \

spec/chef/recipes/gitlab-workhorse_spec.rb

@@ -39,6 +39,16 @@ RSpec.describe 'gitlab::gitlab-workhorse' do
     end
   end
+  context 'when the deprecated socket file exists' do
+    it 'includes a cleanup for the orphan socket' do
+      allow(File).to receive(:exist?).and_call_original
+      allow(File).to receive(:exist?).with('/var/opt/gitlab/gitlab-workhorse/socket').and_return(true)
+      expect(chef_run).to render_file("/opt/gitlab/sv/gitlab-workhorse/run").with_content { |content|
+        expect(content).to match(%r(Removing orphaned workhorse socket at))
+      }
+    end
+  end
+
   context 'user and group' do
     context 'default values' do
       it_behaves_like "enabled runit service", "gitlab-workhorse", "root", "root"

spec/libraries/helpers/gitlab_workhorse_helper_spec.rb

+require 'chef_helper'
+
+RSpec.describe GitlabWorkhorseHelper do
+  let(:node) { chef_run.node }
+  subject { described_class.new(node) }
+
+  before do
+    allow(Gitlab).to receive(:[]).and_call_original
+  end
+
+  context 'workhorse is listening on a tcp socket' do
+    cached(:chef_run) { converge_config }
+    let(:tcp_address) { '10.0.1.42' }
+
+    before do
+      stub_gitlab_rb(
+        gitlab_workhorse: {
+          listen_network: 'http',
+          listen_addr: tcp_address
+        }
+      )
+    end
+
+    describe '#unix_socket?' do
+      it 'returns false' do
+        expect(subject.unix_socket?).to be false
+      end
+    end
+
+    describe '#listen_address' do
+      it 'returns the tcp listen address' do
+        expect(subject.listen_address).to eq(tcp_address)
+      end
+    end
+  end
+
+  context 'workhorse is listening on a unix socket' do
+    let(:socket_file_name) { 'socket' }
+    let(:deprecated_path) { '/var/opt/gitlab/gitlab-workhorse/socket' }
+    let(:new_directory) { '/var/opt/gitlab/gitlab-workhorse/sockets' }
+    let(:new_path) { '/var/opt/gitlab/gitlab-workhorse/sockets/socket' }
+    let(:deprecated_custom) { '/where/is/my/ten/mm/socket' }
+    let(:new_custom_directory) { '/where/is/my/ten/mm/sockets' }
+    let(:new_custom_path) { '/where/is/my/ten/mm/sockets/socket' }
+
+    context 'with default workhorse configuration' do
+      cached(:chef_run) { converge_config }
+      before do
+        stub_gitlab_rb(
+          gitlab_workhorse: {
+            listen_network: 'unix'
+          }
+        )
+      end
+
+      describe "#socket_file_name" do
+        it 'returns only the socket file base name' do
+          expect(subject.socket_file_name).to eq(socket_file_name)
+        end
+      end
+
+      describe '#sockets_directory' do
+        it 'returns the expected directory path' do
+          expect(subject.sockets_directory).to eq(new_directory)
+        end
+      end
+
+      describe '#unix_socket?' do
+        it 'returns true' do
+          expect(subject.unix_socket?).to be true
+        end
+      end
+
+      describe '#deprecated_socket' do
+        it 'returns the workhorse socket path not in a directory' do
+          expect(subject.deprecated_socket).to eq(deprecated_path)
+        end
+      end
+
+      describe '#orphan_socket' do
+        it 'returns the deprecated path when using default configuration' do
+          expect(subject.orphan_socket).to eq(deprecated_path)
+        end
+      end
+
+      describe '#orphan_socket?' do
+        it 'true when the orphan socket exists on disk' do
+          allow(File).to receive(:exist?).with(deprecated_path).and_return(true)
+          expect(subject.orphan_socket?).to be true
+        end
+
+        it 'false when the orphan socket does not exist on disk' do
+          allow(File).to receive(:exist?).with(deprecated_path).and_return(true)
+          expect(subject.orphan_socket?).to be true
+        end
+      end
+
+      describe '#listen_address' do
+        it 'returns the adjusted listen address' do
+          expect(subject.listen_address).to eq(new_path)
+        end
+      end
+    end
+
+    context 'with custom workhorse configuration' do
+      cached(:chef_run) { converge_config }
+
+      before do
+        stub_gitlab_rb(
+          gitlab_workhorse: {
+            listen_network: 'unix',
+            listen_addr: deprecated_custom
+          }
+        )
+      end
+
+      describe '#orphan_socket' do
+        it 'returns the configured custom path' do
+          expect(subject.orphan_socket).to eq(deprecated_custom)
+        end
+      end
+
+      describe '#orphan_socket?' do
+        it 'true when the orphan socket exists on disk' do
+          allow(File).to receive(:exist?).with(deprecated_custom).and_return(true)
+          expect(subject.orphan_socket?).to be true
+        end
+
+        it 'false when the orphan socket does not exist on disk' do
+          allow(File).to receive(:exist?).with(deprecated_custom).and_return(true)
+          expect(subject.orphan_socket?).to be true
+        end
+      end
+    end
+  end
+end