Merge branch 'postgres-upgrade-failure' into 'master'

Update symlinks of postgres on both upgrade and reconfigure

Builds on top and superseds https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/1120 and https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/1114 

Fixes  #1726

See merge request !1125

docker/assets/gitlab-rb-location.patch

@@ -9,18 +9,6 @@
 
 
  ## Note: configuration settings below are optional.
---- embedded/cookbooks/gitlab.bak/recipes/remove_accounts.rb	2015-09-22 20:50:46.964202003 +0000
-+++ embedded/cookbooks/gitlab/recipes/remove_accounts.rb	2015-09-22 20:51:49.256202003 +0000
-@@ -16,8 +16,8 @@
- #
-
- Gitlab[:node] = node
--if File.exists?("/etc/gitlab/gitlab.rb")
--  Gitlab.from_file("/etc/gitlab/gitlab.rb")
-+if File.exists?("/assets/gitlab.rb")
-+  Gitlab.from_file("/assets/gitlab.rb")
- end
- node.consume_attributes(Gitlab.generate_config(node['fqdn']))
 
 --- embedded/cookbooks/gitlab.bak/recipes/show_config.rb	2015-09-22 20:50:46.964202003 +0000
 +++ embedded/cookbooks/gitlab/recipes/show_config.rb	2015-09-22 20:52:02.716202003 +0000

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

@@ -83,6 +83,15 @@ class PgHelper
   def version
     VersionHelper.version('/opt/gitlab/embedded/bin/psql --version').split.last
   end
+
+  def database_version
+    version_file = "#{@node['gitlab']['postgresql']['data_dir']}/PG_VERSION"
+    if File.exist?(version_file)
+      File.read(version_file).chomp
+    else
+      nil
+    end
+  end
 end
 
 class OmnibusHelper

files/gitlab-cookbooks/gitlab/recipes/postgresql-bin.rb

@@ -0,0 +1,47 @@
+#
+# Copyright:: Copyright (c) 2016 GitLab Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+pg_helper = PgHelper.new(node)
+postgresql_install_dir = File.join(node['package']['install-dir'], 'embedded/postgresql')
+postgresql_data_dir = node['gitlab']['postgresql']['data_dir']
+
+# This recipe will also be called standalone so the resource
+# won't exist for resource collection.
+# We only have ourselves to blame here, we want DRY code this is what we get.
+# The block below is cleanest solution and
+# was found at https://gist.github.com/scalp42/7606857#gistcomment-1618630
+resource_exists = proc do |name|
+  begin
+    resources name
+    true
+  rescue Chef::Exceptions::ResourceNotFound
+    false
+  end
+end
+
+ruby_block "Link postgresql bin files to the correct version" do
+  block do
+    pg_version = pg_helper.database_version || pg_helper.version
+    pg_path = Dir.glob("#{postgresql_install_dir}/#{pg_version}*").first
+    Dir.glob("#{pg_path}/bin/*").each do |pg_bin|
+      FileUtils.ln_sf(pg_bin, "#{node['package']['install-dir']}/embedded/bin/#{File.basename(pg_bin)}")
+    end
+  end
+  only_if do
+    !File.exists?(File.join(postgresql_data_dir, "PG_VERSION")) || pg_helper.version !~ /^#{pg_helper.database_version}/
+  end
+  notifies :restart, 'service[postgresql]', :immediately if OmnibusHelper.should_notify?("postgresql") && resource_exists['service[postgresql]']
+end

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

@@ -89,15 +89,15 @@ execute "/opt/gitlab/embedded/bin/initdb -D #{postgresql_data_dir} -E UTF8" do
 end
 
 postgresql_config = File.join(postgresql_data_dir, "postgresql.conf")
-
-node.default['gitlab']['postgresql']['version'] = pg_helper.version
+should_notify = OmnibusHelper.should_notify?("postgresql")
 
 template postgresql_config do
   source "postgresql.conf.erb"
   owner postgresql_user
   mode "0644"
+  helper(:pg_helper) { pg_helper }
   variables(node['gitlab']['postgresql'].to_hash)
-  notifies :restart, 'service[postgresql]', :immediately if OmnibusHelper.should_notify?("postgresql")
+  notifies :restart, 'service[postgresql]', :immediately if should_notify
 end
 
 pg_hba_config = File.join(postgresql_data_dir, "pg_hba.conf")
@@ -107,18 +107,16 @@ template pg_hba_config do
   owner postgresql_user
   mode "0644"
   variables(node['gitlab']['postgresql'].to_hash)
-  notifies :restart, 'service[postgresql]', :immediately if OmnibusHelper.should_notify?("postgresql")
+  notifies :restart, 'service[postgresql]', :immediately if should_notify
 end
 
 template File.join(postgresql_data_dir, "pg_ident.conf") do
   owner postgresql_user
   mode "0644"
   variables(node['gitlab']['postgresql'].to_hash)
-  notifies :restart, 'service[postgresql]' if OmnibusHelper.should_notify?("postgresql")
+  notifies :restart, 'service[postgresql]', :immediately  if should_notify
 end
 
-should_notify = OmnibusHelper.should_notify?("postgresql")
-
 runit_service "postgresql" do
   down node['gitlab']['postgresql']['ha']
   control(['t'])
@@ -128,6 +126,13 @@ runit_service "postgresql" do
   log_options node['gitlab']['logging'].to_hash.merge(node['gitlab']['postgresql'].to_hash)
 end
 
+# This recipe must be ran BEFORE any calls to the binaries are made
+# and AFTER the service has been defined
+# to ensure the correct running version of PostgreSQL
+# Only exception to this rule is "initdb" call few lines up because this should
+# run only on new installation at which point we expect to have correct binaries.
+include_recipe 'gitlab::postgresql-bin'
+
 if node['gitlab']['bootstrap']['enable']
   execute "/opt/gitlab/bin/gitlab-ctl start postgresql" do
     retries 20

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

@@ -15,11 +15,7 @@
 # limitations under the License.
 #
 
-Gitlab[:node] = node
-if File.exists?("/etc/gitlab/gitlab.rb")
-  Gitlab.from_file("/etc/gitlab/gitlab.rb")
-end
-node.consume_attributes(Gitlab.generate_config(node['fqdn']))
+include_recipe 'gitlab::config'
 
 account_helper = AccountHelper.new(node)
 
@@ -42,4 +38,3 @@ groups.each do |group|
     manage node['gitlab']['manage-accounts']['enable']
   end
 end
-

files/gitlab-cookbooks/gitlab/templates/default/postgresql.conf.erb

@@ -69,7 +69,7 @@ max_connections = <%= node['gitlab']['postgresql']['max_connections'] %>      #
 # Note:  Increasing max_connections costs ~400 bytes of shared memory per
 # connection slot, plus lock space (see max_locks_per_transaction).
 #superuser_reserved_connections = 3 # (change requires restart)
-<% if node['gitlab']['postgresql']['version'] == "9.2.18" %>
+<% if pg_helper.database_version == "9.2" %>
 unix_socket_directory = '<%= node['gitlab']['postgresql']['unix_socket_directory'] %>'   # (change requires restart)
 <% else %>
 unix_socket_directories = '<%= node['gitlab']['postgresql']['unix_socket_directory'] %>'   # (change requires restart)
@@ -178,7 +178,7 @@ wal_buffers = <%= node['gitlab']['postgresql']['wal_buffers'] %> # -1     # min
 #commit_siblings = 5      # range 1-1000
 
 # - Checkpoints -
-<% if node['gitlab']['postgresql']['version'] == "9.2.18" %>
+<% if pg_helper.database_version == "9.2" %>
 checkpoint_segments = <%= node['gitlab']['postgresql']['checkpoint_segments'] %>		# in logfile segments, min 1, 16MB each, default 3
 <% else %>
 min_wal_size = <%= node['gitlab']['postgresql']['min_wal_size'] %>

files/gitlab-ctl-commands/upgrade.rb

@@ -35,6 +35,20 @@ add_command 'upgrade', 'Run migrations after a package upgrade', 1 do |cmd_name|
     run_sv_command_for_service('stop', sv_name)
   end
 
+  unless progress_message('Checking PostgreSQL executables') do
+    command = %W( chef-client
+                  -z
+                  -c #{base_path}/embedded/cookbooks/solo.rb
+                  -o recipe[gitlab::config]
+                  -o recipe[gitlab::postgresql-bin]
+               )
+
+    status = run_command(command.join(" "))
+    status.success?
+  end
+    log 'Could not update PostgreSQL executables.'
+  end
+
   MIGRATION_SERVICES = %w{postgresql redis}
   MIGRATION_SERVICES.each do |sv_name|
     # If the service is disabled, e.g. because we are using an external

spec/chef/recipes/postgresql_spec.rb

@@ -5,7 +5,12 @@ describe 'postgresql 9.2' do
 
   before do
     allow(Gitlab).to receive(:[]).and_call_original
-    allow_any_instance_of(PgHelper).to receive(:version).and_return("9.2.18")
+    allow_any_instance_of(PgHelper).to receive(:version).and_return('9.2.18')
+    allow_any_instance_of(PgHelper).to receive(:database_version).and_return('9.2')
+  end
+
+  it 'includes the postgresql-bin recipe' do
+    expect(chef_run).to include_recipe('gitlab::postgresql-bin')
   end
 
   context 'with default settings' do
@@ -51,57 +56,123 @@ describe 'postgresql 9.2' do
     end
   end
 
-  it 'sets unix_socket_directory' do
-    expect(chef_run.node['gitlab']['postgresql']['unix_socket_directory'])
-      .to eq('/var/opt/gitlab/postgresql')
-    expect(chef_run.node['gitlab']['postgresql']['unix_socket_directories'])
-      .to eq(nil)
-    expect(chef_run).to render_file(
-      '/var/opt/gitlab/postgresql/data/postgresql.conf'
-    ).with_content { |content|
-      expect(content).to match(
-        /unix_socket_directory = '\/var\/opt\/gitlab\/postgresql'/
-      )
-      expect(content).not_to match(
-        /unix_socket_directories = '\/var\/opt\/gitlab\/postgresql'/
-      )
-    }
-  end
+  context 'version specific settings' do
+    it 'sets unix_socket_directory' do
+      expect(chef_run.node['gitlab']['postgresql']['unix_socket_directory'])
+        .to eq('/var/opt/gitlab/postgresql')
+      expect(chef_run.node['gitlab']['postgresql']['unix_socket_directories'])
+        .to eq(nil)
+      expect(chef_run).to render_file(
+        '/var/opt/gitlab/postgresql/data/postgresql.conf'
+      ).with_content { |content|
+        expect(content).to match(
+          /unix_socket_directory = '\/var\/opt\/gitlab\/postgresql'/
+        )
+        expect(content).not_to match(
+          /unix_socket_directories = '\/var\/opt\/gitlab\/postgresql'/
+        )
+      }
+    end
 
-  it 'sets checkpoint_segments' do
-    expect(chef_run.node['gitlab']['postgresql']['checkpoint_segments'])
-      .to eq(10)
-    expect(chef_run).to render_file(
-      '/var/opt/gitlab/postgresql/data/postgresql.conf'
-    ).with_content(/checkpoint_segments = 10/)
+    it 'sets checkpoint_segments' do
+      expect(chef_run.node['gitlab']['postgresql']['checkpoint_segments'])
+        .to eq(10)
+      expect(chef_run).to render_file(
+        '/var/opt/gitlab/postgresql/data/postgresql.conf'
+      ).with_content(/checkpoint_segments = 10/)
+    end
+
+    context 'running version differs from data version' do
+      before do
+        allow_any_instance_of(PgHelper).to receive(:version).and_return('9.6.1')
+        allow(File).to receive(:exists?).and_call_original
+        allow(File).to receive(:exists?).with("/var/opt/gitlab/postgresql/data/PG_VERSION").and_return(true)
+        allow(Dir).to receive(:glob).and_call_original
+        allow(Dir).to receive(:glob).with("/opt/gitlab/embedded/postgresql/9.2*").and_return(
+          ['/opt/gitlab/embedded/postgresql/9.2.18']
+        )
+        allow(Dir).to receive(:glob).with("/opt/gitlab/embedded/postgresql/9.2.18/bin/*").and_return(
+          %w(
+            /opt/gitlab/embedded/postgresql/9.2.18/bin/foo_one
+            /opt/gitlab/embedded/postgresql/9.2.18/bin/foo_two
+            /opt/gitlab/embedded/postgresql/9.2.18/bin/foo_three
+          )
+        )
+      end
+
+      it 'corrects symlinks to the correct location' do
+        allow(FileUtils).to receive(:ln_sf).and_return(true)
+        %w(foo_one foo_two foo_three).each do |pg_bin|
+          expect(FileUtils).to receive(:ln_sf).with(
+            "/opt/gitlab/embedded/postgresql/9.2.18/bin/#{pg_bin}",
+            "/opt/gitlab/embedded/bin/#{pg_bin}"
+          )
+        end
+        chef_run.ruby_block('Link postgresql bin files to the correct version').old_run_action(:run)
+      end
+    end
   end
 end
 
-describe 'postgresl 9.6' do
+describe 'postgresql 9.6' do
   let(:chef_run) { ChefSpec::SoloRunner.converge('gitlab::default') }
 
   before do
     allow(Gitlab).to receive(:[]).and_call_original
+    allow_any_instance_of(PgHelper).to receive(:version).and_return('9.6.1')
+    allow_any_instance_of(PgHelper).to receive(:database_version).and_return('9.6')
   end
 
-  it 'sets unix_socket_directories' do
-    expect(chef_run.node['gitlab']['postgresql']['unix_socket_directory'])
-      .to eq('/var/opt/gitlab/postgresql')
-    expect(chef_run).to render_file(
-      '/var/opt/gitlab/postgresql/data/postgresql.conf'
-    ).with_content { |content|
-      expect(content).to match(
-        /unix_socket_directories = '\/var\/opt\/gitlab\/postgresql'/
-      )
-      expect(content).not_to match(
-        /unix_socket_directory = '\/var\/opt\/gitlab\/postgresql'/
-      )
-    }
-  end
+  context 'version specific settings' do
+    it 'sets unix_socket_directories' do
+      expect(chef_run.node['gitlab']['postgresql']['unix_socket_directory'])
+        .to eq('/var/opt/gitlab/postgresql')
+      expect(chef_run).to render_file(
+        '/var/opt/gitlab/postgresql/data/postgresql.conf'
+      ).with_content { |content|
+        expect(content).to match(
+          /unix_socket_directories = '\/var\/opt\/gitlab\/postgresql'/
+        )
+        expect(content).not_to match(
+          /unix_socket_directory = '\/var\/opt\/gitlab\/postgresql'/
+        )
+      }
+    end
 
-  it 'does not set checkpoint_segments' do
-    expect(chef_run).not_to render_file(
-      '/var/opt/gitlab/postgresql/data/postgresql.conf'
-    ).with_content(/checkpoint_segments = 10/)
+    it 'does not set checkpoint_segments' do
+      expect(chef_run).not_to render_file(
+        '/var/opt/gitlab/postgresql/data/postgresql.conf'
+      ).with_content(/checkpoint_segments = 10/)
+    end
+
+    context 'running version differs from data version' do
+      before do
+        allow_any_instance_of(PgHelper).to receive(:version).and_return('9.2.18')
+        allow(File).to receive(:exists?).and_call_original
+        allow(File).to receive(:exists?).with("/var/opt/gitlab/postgresql/data/PG_VERSION").and_return(true)
+        allow(Dir).to receive(:glob).and_call_original
+        allow(Dir).to receive(:glob).with("/opt/gitlab/embedded/postgresql/9.6*").and_return(
+          ['/opt/gitlab/embedded/postgresql/9.6.1']
+        )
+        allow(Dir).to receive(:glob).with("/opt/gitlab/embedded/postgresql/9.6.1/bin/*").and_return(
+          %w(
+            /opt/gitlab/embedded/postgresql/9.6.1/bin/foo_one
+            /opt/gitlab/embedded/postgresql/9.6.1/bin/foo_two
+            /opt/gitlab/embedded/postgresql/9.6.1/bin/foo_three
+          )
+        )
+      end
+
+      it 'corrects symlinks to the correct location' do
+        allow(FileUtils).to receive(:ln_sf).and_return(true)
+        %w(foo_one foo_two foo_three).each do |pg_bin|
+          expect(FileUtils).to receive(:ln_sf).with(
+            "/opt/gitlab/embedded/postgresql/9.6.1/bin/#{pg_bin}",
+            "/opt/gitlab/embedded/bin/#{pg_bin}"
+          )
+        end
+        chef_run.ruby_block('Link postgresql bin files to the correct version').old_run_action(:run)
+      end
+    end
   end
 end

spec/chef_helper.rb

@@ -51,5 +51,6 @@ RSpec.configure do |config|
     # Prevent chef converge from reloading the storage helper library, which would override our helper stub
     mock_file_load(%r{gitlab/libraries/storage_directory_helper})
     mock_file_load(%r{gitlab/libraries/helper})
+    allow_any_instance_of(PgHelper).to receive(:database_version).and_return("9.2")
   end
 end

spec/libraries/helper_spec.rb

@@ -1,15 +1,39 @@
 require 'chef_helper'
 
 describe PgHelper do
-  let(:chef_run) { ChefSpec::SoloRunner.converge('gitlab::default') }
+  let(:chef_run) do
+    ChefSpec::SoloRunner.new do |node|
+      node.set['gitlab']['postgresql']['data_dir'] = '/fakedir'
+      node.set['package']['install-dir'] = '/fake/install/dir'
+    end.converge('gitlab::default')
+  end
   let(:node) { chef_run.node }
 
   before do
-    allow(VersionHelper).to receive(:version).with('/opt/gitlab/embedded/bin/psql --version').and_return("YYYYYYYY XXXXXXX")
+    allow(VersionHelper).to receive(:version).with(
+      '/opt/gitlab/embedded/bin/psql --version'
+    ).and_return('YYYYYYYY XXXXXXX')
+    @helper = PgHelper.new(node)
   end
 
   it 'returns a valid version' do
-    helper = PgHelper.new(node)
-    expect(helper.version).to eq("XXXXXXX")
+    expect(@helper.version).to eq('XXXXXXX')
+  end
+
+  it 'returns a valid database_version' do
+    allow(File).to receive(:exist?).and_call_original
+    allow(File).to receive(:exist?).with(
+      '/fakedir/PG_VERSION'
+    ).and_return(true)
+    allow(File).to receive(:read).and_call_original
+    allow(File).to receive(:read).with(
+      '/fakedir/PG_VERSION'
+    ).and_return('111.222')
+    allow(Dir).to receive(:glob).with(
+      '/fake/install/dir/embedded/postgresql/*'
+    ).and_return(['111.222.18', '222.333.11'])
+    # We mock this in chef_helper.rb. Overide the mock to call the original
+    allow_any_instance_of(PgHelper).to receive(:database_version).and_call_original
+    expect(@helper.database_version).to eq('111.222')
   end
 end