Merge branch '2659-reconfigure-always-changes-postgres-runit-files' into 'master'

Resolve "Reconfigure always changes postgres runit files"

Closes #2659

See merge request !1841

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

@@ -43,4 +43,16 @@ class OmnibusHelper # rubocop:disable Style/MultilineIfModifier (disabled so we
   def group_exists?(group)
     success?("getent group #{group}")
   end
+
+  def expected_user?(file, user)
+    File.stat(file).uid == Etc.getpwnam(user).uid
+  end
+
+  def expected_group?(file, group)
+    File.stat(file).gid == Etc.getgrnam(group).gid
+  end
+
+  def expected_owner?(file, user, group)
+    expected_user?(file, user) && expected_group?(file, group)
+  end
 end unless defined?(OmnibusHelper) # Prevent reloading in chefspec: https://github.com/sethvargo/chefspec/issues/562#issuecomment-74120922

files/gitlab-cookbooks/runit/definitions/runit_service.rb

@@ -194,18 +194,22 @@ define :runit_service, directory: nil, only_if: false, finish_script: false, con
       mode 0755
     end
+    supervisor_owner = params[:supervisor_owner] || 'root'
+    supervisor_group = params[:supervisor_group] || 'root'
     %w(ok control).each do |fl|
       file "#{sv_dir_name}/supervise/#{fl}" do
-        owner params[:supervisor_owner] || 'root'
-        group params[:supervisor_group] || 'root'
+        owner supervisor_owner
+        group supervisor_group
         not_if { params[:supervisor_owner].nil? || params[:supervisor_group].nil? }
+        only_if { !omnibus_helper.expected_owner?(name, supervisor_owner, supervisor_group) }
         action :touch
       end
       file "#{sv_dir_name}/log/supervise/#{fl}" do
-        owner params[:supervisor_owner] || 'root'
-        group params[:supervisor_group] || 'root'
+        owner supervisor_owner
+        group supervisor_group
         not_if { params[:supervisor_owner].nil? || params[:supervisor_group].nil? }
+        only_if { !omnibus_helper.expected_owner?(name, supervisor_owner, supervisor_group) }
         action :touch
       end
     end

spec/chef_helper.rb

@@ -59,6 +59,7 @@ RSpec.configure do |config|
     mock_file_load(%r{gitlab/libraries/helper})
     allow_any_instance_of(PgHelper).to receive(:database_version).and_return("9.2")
+    stub_expected_owner?
     # Clear services list before each test
     Services.reset_list
   end

spec/libraries/helper_spec.rb

@@ -205,4 +205,37 @@ describe OmnibusHelper do
       expect(chef_run.template('/var/opt/gitlab/nginx/conf/gitlab-http.conf')).not_to notify('service[nginx]').to(:restart).delayed
     end
   end
+
+  context 'expected_owner?' do
+    let(:oh) { OmnibusHelper.new(chef_run.node) }
+    before do
+      allow_any_instance_of(OmnibusHelper).to receive(:expected_owner?)
+        .and_call_original
+      allow_any_instance_of(OmnibusHelper).to receive(
+        :expected_user?).and_return(false)
+      allow_any_instance_of(OmnibusHelper).to receive(
+        :expected_user?
+      ).with('/tmp/fakefile', 'fakeuser').and_return(true)
+      allow_any_instance_of(OmnibusHelper).to receive(
+        :expected_group?).and_return(false)
+      allow_any_instance_of(OmnibusHelper).to receive(
+        :expected_group?
+      ).with('/tmp/fakefile', 'fakegroup').and_return(true)
+    end
+
+    it 'should return false if the group is wrong' do
+      expect(oh.expected_owner?('/tmp/fakefile', 'fakeuser', 'wronggroup'))
+        .to be false
+    end
+
+    it 'should return false if the user is wrong' do
+      expect(oh.expected_owner?('/tmp/fakefile', 'wronguser', 'fakegroup'))
+        .to be false
+    end
+
+    it 'should return true if user and group is correct' do
+      expect(oh.expected_owner?('/tmp/fakefile', 'fakeuser', 'fakegroup'))
+        .to be true
+    end
+  end
 end

spec/support/macros.rb

@@ -22,6 +22,10 @@ module GitlabSpec
       stub_service_success_status(service, value)
     end
+    def stub_expected_owner?
+      allow_any_instance_of(OmnibusHelper).to receive(:expected_owner?).and_return(true)
+    end
+
     def stub_env_var(var, value)
       allow(ENV).to receive(:[]).with(var).and_return(value)
     end