Refactor workhorse helper

- Remove the workhorse helper methods that are refactored out
- Fix workhorse helper tests

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

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

@@ -19,6 +19,16 @@ module GitlabWorkhorse
   class << self
     def parse_variables
       Gitlab['gitlab_workhorse']['auth_socket'] = nil if !auth_socket_specified? && auth_backend_specified?
+
+      default_network = Gitlab['node']['gitlab']['gitlab-workhorse']['listen_network']
+      user_network = Gitlab['gitlab_workhorse']['listen_network']
+      network = user_network || default_network
+
+      default_sockets_dir = Gitlab['node']['gitlab']['gitlab-workhorse']['sockets_directory']
+      user_sockets_dir = Gitlab['gitlab_workhorse']['sockets_directory']
+      sockets_dir = user_sockets_dir || default_sockets_dir
+
+      Gitlab['gitlab_workhorse']['listen_addr'] ||= File.join(sockets_dir, 'socket') if network == "unix"
     end
     def parse_secrets

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

@@ -3,16 +3,6 @@ require_relative 'base_helper'
 class GitlabWorkhorseHelper < BaseHelper
   attr_reader :node
-  def socket_file_name
-    return unless unix_socket?
-
-    file_path = node['gitlab']['gitlab-workhorse']['listen_addr']
-
-    return File.basename(file_path) unless file_path.nil?
-
-    'socket'
-  end
-
   def sockets_directory
     return unless unix_socket?
@@ -23,47 +13,10 @@ class GitlabWorkhorseHelper < BaseHelper
     node['gitlab']['gitlab-workhorse']['listen_network'] == "unix"
   end
-  def deprecated_socket
-    '/var/opt/gitlab/gitlab-workhorse/socket'
-  end
-
   def user_customized_socket?
     default_path = node.default['gitlab']['gitlab-workhorse']['listen_addr']
     configured_path = node['gitlab']['gitlab-workhorse']['listen_addr']
     default_path != configured_path
   end
-
-  def user_customized_sockets_directory?
-    default_directory = node.default['gitlab']['gitlab-workhorse']['sockets_directory']
-    configured_directory = node['gitlab']['gitlab-workhorse']['sockets_directory']
-
-    default_directory != configured_directory
-  end
-
-  def orphan_socket
-    return deprecated_socket unless user_customized_socket?
-
-    node['gitlab']['gitlab-workhorse']['listen_addr']
-  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 node['gitlab']['gitlab-workhorse']['listen_addr'] unless unix_socket?
-
-    selinux_socket = File.join(sockets_directory, socket_file_name)
-
-    return selinux_socket if user_customized_sockets_directory?
-
-    return node['gitlab']['gitlab-workhorse']['listen_addr'] if user_customized_socket?
-
-    selinux_socket
-  end
 end

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

@@ -37,7 +37,7 @@ if workhorse_helper.unix_socket?
     group account_helper.web_server_group
     mode '0750'
     recursive true
-    not_if { workhorse_helper.user_customized_socket? && !workhorse_helper.user_customized_sockets_directory? }
+    not_if { workhorse_helper.user_customized_socket? }
   end
 end
@@ -61,9 +61,7 @@ end
 runit_service 'gitlab-workhorse' do
   start_down node['gitlab']['gitlab-workhorse']['ha']
   options({
-    log_directory: log_directory,
-    found_orphan_socket: workhorse_helper.orphan_socket?,
-    orphan_socket_path: workhorse_helper.orphan_socket
+    log_directory: log_directory
   }.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,10 +8,10 @@ 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] %>"
+<% if File.exist?('/var/opt/gitlab/gitlab-workhorse/socket') %>
+if [ -e "/var/opt/gitlab/gitlab-workhorse/socket" ]; then
+  echo "Removing orphaned workhorse socket at '/var/opt/gitlab/gitlab-workhorse/socket'"
+  rm /var/opt/gitlab/gitlab-workhorse/socket
 fi
 <% end %>

spec/libraries/helpers/gitlab_workhorse_helper_spec.rb

@@ -26,22 +26,12 @@ RSpec.describe GitlabWorkhorseHelper 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 }
@@ -53,14 +43,8 @@ RSpec.describe GitlabWorkhorseHelper do
         )
       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
+        it 'returns the default directory path' do
           expect(subject.sockets_directory).to eq(new_directory)
         end
       end
@@ -70,36 +54,6 @@ RSpec.describe GitlabWorkhorseHelper 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 listen_addr' do
@@ -120,25 +74,6 @@ RSpec.describe GitlabWorkhorseHelper do
             expect(subject.user_customized_socket?).to be true
           end
         end
-
-        describe '#user_customized_sockets_directory?' do
-          it 'should be false when the directory is default' do
-            expect(subject.user_customized_sockets_directory?).to be false
-          end
-        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 'is false when the listen address is customized' do
-            allow(File).to receive(:exist?).with(deprecated_custom).and_return(true)
-            expect(subject.orphan_socket?).to be false
-          end
-        end
       end
       context 'with sockets_directory configured' do
@@ -154,28 +89,9 @@ RSpec.describe GitlabWorkhorseHelper do
           )
         end
-        describe "#user_customized_sockets_directory?" do
-          it 'should be true when the directory has been set' do
-            expect(subject.user_customized_sockets_directory?).to be true
-          end
-        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 'is true when the listen address is customized' do
-            allow(File).to receive(:exist?).with(deprecated_custom).and_return(true)
-            expect(subject.orphan_socket?).to be true
-          end
-        end
-
-        describe '#listen_address' do
-          it 'returns the path the custom directory' do
-            expect(subject.listen_address).to eq(new_custom_path)
+        describe '#sockets_directory' do
+          it 'returns the user configured sockets directory path' do
+            expect(subject.sockets_directory).to eq(new_custom_directory)
           end
         end
       end