Merge branch 'master' of gitlab.com:gitlab-org/omnibus-gitlab into pitr

files/gitlab-cookbooks/postgresql/attributes/default.rb

@@ -104,6 +104,7 @@ default['postgresql']['log_rotation_size'] = nil
 # Replication settings
 default['postgresql']['sql_replication_user'] = "gitlab_replicator"
 default['postgresql']['wal_level'] = "minimal"
+default['postgresql']['wal_log_hints'] = "off"
 default['postgresql']['max_wal_senders'] = 0
 default['postgresql']['wal_keep_segments'] = 10
 default['postgresql']['hot_standby'] = "off"

files/gitlab-cookbooks/postgresql/resources/query.rb

 resource_name :postgresql_query
+provides :postgresql_query
 property :description, String, name_property: true
 property :query, String

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

@@ -171,6 +171,7 @@ shared_preload_libraries = '<%= @shared_preload_libraries %>'    # (change requi
 # - Settings -
 wal_level = <%= @wal_level %>
+wal_log_hints = <%= @wal_log_hints %>      # also do full page writes of non-critical updates
           # (change requires restart)
 #fsync = on       # turns forced synchronization on or off
 #wal_sync_method = fsync    # the default is the first option

files/gitlab-ctl-commands-ee/lib/geo/promote_to_primary_node.rb

@@ -16,15 +16,11 @@ module Geo
     def execute
       run_preflight_checks
-
       final_confirmation
-
+      toggle_geo_roles
       promote_postgresql_to_primary
-
       reconfigure
-
       promote_to_primary
-
       success_message
     end
@@ -70,6 +66,20 @@ module Geo
       exit 1
     end
+    def toggle_geo_roles
+      puts
+      puts 'Disabling the secondary role and enabling the primary in the cluster configuration file...'.color(:yellow)
+      puts
+
+      cluster_helper = GitlabClusterHelper.new
+      cluster_helper.config['primary'] = true
+      cluster_helper.config['secondary'] = false
+      return if cluster_helper.write_to_file!
+
+      puts "ERROR: Could not write to #{GitlabClusterHelper::JSON_FILE}.".color(:red)
+      exit 1
+    end
+
     def promote_postgresql_to_primary
       return if recovery_to_point_in_time

files/gitlab-ctl-commands-ee/promote_to_primary_node.rb

+require "#{base_path}/embedded/cookbooks/package/libraries/helpers/gitlab_cluster_helper"
 require "#{base_path}/embedded/service/omnibus-ctl-ee/lib/geo/promote_to_primary_node"
 #

files/gitlab-ctl-commands/pg-upgrade.rb

@@ -71,7 +71,7 @@ add_command_under_category 'revert-pg-upgrade', 'database',
     Kernel.exit 1
   end
-  maintenance_mode('enable')
+  maintenance_mode('enable') unless patroni_enabled
   unless Dir.exist?("#{db_worker.tmp_data_dir}.#{revert_version.major}")
     if !geo_pg_enabled || !Dir.exist?("#{geo_db_worker.tmp_data_dir}.#{revert_version.major}")
@@ -81,6 +81,11 @@ add_command_under_category 'revert-pg-upgrade', 'database',
     end
   end
+  if patroni_enabled
+    @db_worker = db_worker
+    patroni_preflight_check(options)
+  end
+
   log "Reverting database to #{revert_version} in 5 seconds"
   log '=== WARNING ==='
   log 'This will revert the database to what it was before you upgraded, including the data.'
@@ -93,7 +98,15 @@ add_command_under_category 'revert-pg-upgrade', 'database',
     Kernel.exit 0
   end
-  if pg_enabled || patroni_enabled
+  if patroni_enabled
+    log '== Reverting =='
+    if @instance_type == :patroni_leader
+      patroni_leader_downgrade(revert_version)
+    else
+      patroni_replica_downgrade(revert_version)
+    end
+    log '== Reverted =='
+  elsif pg_enabled
     @db_worker = db_worker
     revert(revert_version)
   end
@@ -105,7 +118,7 @@ add_command_under_category 'revert-pg-upgrade', 'database',
   end
   clean_revert_version
-  maintenance_mode('disable')
+  maintenance_mode('disable') unless patroni_enabled
 end
 add_command_under_category 'pg-upgrade', 'database',
@@ -220,21 +233,7 @@ add_command_under_category 'pg-upgrade', 'database',
     end
   end
-  if patroni_enabled
-    log 'Detected a Patroni cluster.'
-
-    @instance_type = (:patroni_leader if options[:leader]) || (:patroni_replica if options[:replica])
-    guess_patroni_node_role unless @instance_type
-
-    check_patroni_cluster_status
-
-    if @instance_type == :patroni_leader
-      log "Using #{Rainbow('leader').yellow} node upgrade procedure."
-    else
-      log "Using #{Rainbow('replica').yellow} node upgrade procedure."
-      log Rainbow('This procedure REMOVES DATA directory.').yellow
-    end
-  end
+  patroni_preflight_check(options) if patroni_enabled
   if options[:wait]
     # Wait for processes to settle, and give use one last chance to change their
@@ -244,7 +243,7 @@ add_command_under_category 'pg-upgrade', 'database',
     log "If you do not want to upgrade the PostgreSQL server at this time, enter Ctrl-C and see the documentation for details"
     status = GitlabCtl::Util.delay_for(30)
     unless status
-      maintenance_mode('disable')
+      maintenance_mode('disable') unless patroni_enabled
       Kernel.exit(0)
     end
   end
@@ -363,6 +362,25 @@ def patroni_replica_upgrade
   common_post_upgrade(false)
 end
+def patroni_leader_downgrade(revert_version)
+  stop_database
+  create_links(revert_version)
+  revert_data_dir(revert_version)
+  remove_patroni_cluster_state
+  start_database
+  configure_postgresql
+  restart_patroni_node
+end
+
+def patroni_replica_downgrade(revert_version)
+  stop_database
+  create_links(revert_version)
+  revert_data_dir(revert_version)
+  start_database
+  configure_postgresql
+  restart_patroni_node
+end
+
 def configure_postgresql
   log 'Configuring PostgreSQL'
   status = GitlabCtl::Util.chef_run('solo.rb', "#{@db_service_name}-config.json")
@@ -564,6 +582,22 @@ def analyze_cluster
   end
 end
+def patroni_preflight_check(options)
+  log 'Detected a Patroni cluster.'
+
+  @instance_type = (:patroni_leader if options[:leader]) || (:patroni_replica if options[:replica])
+  guess_patroni_node_role unless @instance_type
+
+  check_patroni_cluster_status
+
+  if @instance_type == :patroni_leader
+    log "Using #{Rainbow('leader').yellow} node upgrade procedure."
+  else
+    log "Using #{Rainbow('replica').yellow} node upgrade procedure."
+    log Rainbow('This procedure REMOVES DATA directory.').yellow
+  end
+end
+
 def guess_patroni_node_role
   failure_cause = :none
   unless GitlabCtl::Util.progress_message('Attempting to detect the role of this Patroni node') do
@@ -701,17 +735,27 @@ end
 def revert(version)
   log '== Reverting =='
   run_sv_command_for_service('stop', @db_service_name)
-  if Dir.exist?("#{@db_worker.tmp_data_dir}.#{version.major}")
-    run_command("rm -rf #{@db_worker.data_dir}")
-    run_command(
-      "mv #{@db_worker.tmp_data_dir}.#{version.major} #{@db_worker.data_dir}"
-    )
-  end
+  revert_data_dir(version)
   create_links(version)
   run_sv_command_for_service('start', @db_service_name)
   log'== Reverted =='
 end
+def revert_data_dir(version)
+  if @instance_type == :patroni_replica
+    run_command("rm -rf #{@db_worker.data_dir}")
+    run_command("mkdir #{@db_worker.data_dir}")
+    return
+  end
+
+  return unless Dir.exist?("#{@db_worker.tmp_data_dir}.#{version.major}")
+
+  run_command("rm -rf #{@db_worker.data_dir}")
+  run_command(
+    "mv #{@db_worker.tmp_data_dir}.#{version.major} #{@db_worker.data_dir}"
+  )
+end
+
 def maintenance_mode(command)
   # In order for the deploy page to work, we need nginx, unicorn, redis, and
   # gitlab-workhorse running
@@ -741,7 +785,7 @@ def die(message)
   $stderr.puts message
   revert(@db_worker.initial_version)
   $stderr.puts "== Reverted to #{@db_worker.initial_version}. Please check output for what went wrong =="
-  maintenance_mode('disable')
+  maintenance_mode('disable') unless service_enabled?('patroni')
   exit 1
 end

spec/chef/gitlab-ee/recipes/geo_spec.rb

@@ -48,6 +48,10 @@ RSpec.describe 'gitlab-ee::geo' do
         expect(config_attrs['wal_level']).to eq('hot_standby')
       end
+      it 'defines wal_log_hints' do
+        expect(config_attrs['wal_log_hints']).to eq('off')
+      end
+
       it 'defines max_wal_senders' do
         expect(config_attrs['max_wal_senders']).to eq(10)
       end
@@ -95,6 +99,10 @@ RSpec.describe 'gitlab-ee::geo' do
         expect(config_attrs['wal_level']).to eq('hot_standby')
       end
+      it 'defines wal_log_hints' do
+        expect(config_attrs['wal_log_hints']).to eq('off')
+      end
+
       it 'defines max_wal_senders' do
         expect(config_attrs['max_wal_senders']).to eq(10)
       end

spec/chef/recipes/gitlab-rails_spec.rb

@@ -734,6 +734,37 @@ RSpec.describe 'gitlab::gitlab-rails' do
       end
     end
+    context 'for settings regarding object storage for pages' do
+      it 'allows not setting any values' do
+        expect(chef_run).to create_templatesymlink('Create a gitlab.yml and create a symlink to Rails root').with_variables(
+          hash_including(
+            pages_object_store_enabled: false,
+            pages_object_store_remote_directory: 'pages'
+          )
+        )
+      end
+
+      context 'with values' do
+        before do
+          stub_gitlab_rb(gitlab_rails: {
+                           pages_object_store_enabled: true,
+                           pages_object_store_remote_directory: 'pagescustomdir',
+                           pages_object_store_connection: aws_connection_hash
+                         })
+        end
+
+        it "sets the object storage values" do
+          expect(chef_run).to create_templatesymlink('Create a gitlab.yml and create a symlink to Rails root').with_variables(
+            hash_including(
+              pages_object_store_enabled: true,
+              pages_object_store_remote_directory: 'pagescustomdir',
+              pages_object_store_connection: aws_connection_hash
+            )
+          )
+        end
+      end
+    end
+
     describe 'pseudonymizer settings' do
       it 'allows not setting any values' do
         expect(chef_run).to create_templatesymlink('Create a gitlab.yml and create a symlink to Rails root').with_variables(

spec/chef/recipes/postgresql_spec.rb

@@ -377,6 +377,14 @@ RSpec.describe 'postgresql 9.6' do
         ).with_content(/synchronous_standby_names = ''/)
       end
+      it 'disables wal_log_hints setting' do
+        expect(chef_run.node['postgresql']['wal_log_hints']).to eq('off')
+
+        expect(chef_run).to render_file(
+          postgresql_conf
+        ).with_content(/wal_log_hints = off/)
+      end
+
       it 'does not set dynamic_shared_memory_type by default' do
         expect(chef_run).not_to render_file(
           postgresql_conf
@@ -467,6 +475,22 @@ RSpec.describe 'postgresql 9.6' do
           ).with_content(/^dynamic_shared_memory_type = none/)
         end
       end
+
+      context 'when wal_log_hints is on' do
+        before do
+          stub_gitlab_rb({
+                           postgresql: {
+                             wal_log_hints: 'on'
+                           }
+                         })
+        end
+
+        it 'enables wal_log_hints' do
+          expect(chef_run).to render_file(
+            postgresql_conf
+          ).with_content(/^wal_log_hints = on/)
+        end
+      end
     end
     context 'renders runtime.conf' do

spec/chef/recipes/puma_spec.rb

@@ -119,6 +119,10 @@ RSpec.describe 'gitlab::puma with Ubuntu 16.04' do
         max_threads: 10,
         per_worker_max_memory_mb: 1000
       )
+      expect(chef_run).to render_file("/opt/gitlab/sv/gitlab-workhorse/run").with_content { |content|
+        expect(content).to match(%r(-authSocket /tmp/puma.socket))
+      }
+      expect(Gitlab['gitlab_workhorse']['auth_socket']).to eq('/tmp/puma.socket')
     end
   end

spec/gitlab-ctl-commands-ee/lib/geo/promote_to_primary_node_spec.rb

@@ -11,8 +11,8 @@ RSpec.describe Geo::PromoteToPrimaryNode, '#execute' do
   subject(:command) { described_class.new(instance, options) }
-  let(:temp_directory) { Dir.mktmpdir }
-  let(:gitlab_config_path) { File.join(temp_directory, 'gitlab.rb') }
+  let(:config_path) { Dir.mktmpdir }
+  let(:gitlab_config_path) { File.join(config_path, 'gitlab.rb') }
   before do
     allow($stdout).to receive(:puts)
@@ -22,7 +22,7 @@ RSpec.describe Geo::PromoteToPrimaryNode, '#execute' do
   end
   after do
-    FileUtils.rm_rf(temp_directory)
+    FileUtils.rm_rf(config_path)
   end
   describe '#promote_postgresql_to_primary' do
@@ -85,6 +85,7 @@ RSpec.describe Geo::PromoteToPrimaryNode, '#execute' do
     before do
       allow(STDIN).to receive(:gets).and_return('y')
+      allow(command).to receive(:toggle_geo_roles).and_return(true)
       allow(command).to receive(:promote_postgresql_to_primary).and_return(true)
       allow(command).to receive(:reconfigure).and_return(true)
       allow(command).to receive(:promote_to_primary).and_return(true)
@@ -122,6 +123,53 @@ RSpec.describe Geo::PromoteToPrimaryNode, '#execute' do
     end
   end
+  describe '#toggle_geo_roles' do
+    let(:gitlab_cluster_config_path) { File.join(config_path, 'gitlab-cluster.json') }
+
+    before do
+      stub_const('GitlabClusterHelper::CONFIG_PATH', config_path)
+      stub_const('GitlabClusterHelper::JSON_FILE', gitlab_cluster_config_path)
+
+      allow(STDIN).to receive(:gets).and_return('y')
+
+      allow(command).to receive(:run_preflight_checks).and_return(true)
+      allow(command).to receive(:promote_postgresql_to_primary).and_return(true)
+      allow(command).to receive(:reconfigure).and_return(true)
+      allow(command).to receive(:promote_to_primary).and_return(true)
+      allow(command).to receive(:success_message).and_return(true)
+    end
+
+    context 'when the cluster configuration file does not exist' do
+      it 'creates the file with the Geo primary role enabled and secondary role disabled' do
+        command.execute
+
+        expect(File.exist?(gitlab_cluster_config_path)).to eq(true)
+        expect(read_file_content(gitlab_cluster_config_path)).to eq("primary" => true, "secondary" => false)
+      end
+    end
+
+    context 'when the cluster configuration file exists' do
+      it 'disables the Geo secondary role' do
+        write_file_content(gitlab_cluster_config_path, primary: false, secondary: true)
+
+        command.execute
+
+        expect(read_file_content(gitlab_cluster_config_path)).to eq("primary" => true, "secondary" => false)
+      end
+    end
+
+    def read_file_content(fullpath)
+      JSON.parse(File.read(fullpath))
+    end
+
+    def write_file_content(fullpath, content)
+      File.open(fullpath, 'w') do |f|
+        f.write(content.to_json)
+        f.chmod(0600)
+      end
+    end
+  end
+
   context 'when preflight checks pass' do
     before do
       allow(STDIN).to receive(:gets).and_return('y')
@@ -129,6 +177,7 @@ RSpec.describe Geo::PromoteToPrimaryNode, '#execute' do
       allow_any_instance_of(Geo::PromotionPreflightChecks).to receive(
         :execute).and_return(true)
+      allow(command).to receive(:toggle_geo_roles).and_return(true)
       allow(command).to receive(:promote_postgresql_to_primary).and_return(true)
       allow(command).to receive(:reconfigure).and_return(true)
       allow(command).to receive(:promote_to_primary).and_return(true)
@@ -153,8 +202,12 @@ RSpec.describe Geo::PromoteToPrimaryNode, '#execute' do
       end
       context 'when final confirmation is given' do
-        it 'calls the next subcommand' do
+        it 'calls all the subcommands' do
+          expect(command).to receive(:toggle_geo_roles)
           expect(command).to receive(:promote_postgresql_to_primary)
+          expect(command).to receive(:reconfigure)
+          expect(command).to receive(:promote_to_primary)
+          expect(command).to receive(:success_message)
           command.execute
         end
@@ -199,9 +252,11 @@ RSpec.describe Geo::PromoteToPrimaryNode, '#execute' do
       it 'calls all the subcommands if user affirms' do
         allow(STDIN).to receive(:gets).and_return('y')
+        is_expected.to receive(:toggle_geo_roles)
         is_expected.to receive(:promote_postgresql_to_primary)
         is_expected.to receive(:reconfigure)
         is_expected.to receive(:promote_to_primary)
+        is_expected.to receive(:success_message)
         command.execute
       end
@@ -215,4 +270,47 @@ RSpec.describe Geo::PromoteToPrimaryNode, '#execute' do
       end
     end
   end
+
+  context 'when writing to the cluster configuration file fail' do
+    around do |example|
+      example.run
+    rescue SystemExit
+    end
+
+    before do
+      allow(STDIN).to receive(:gets).and_return('y')
+
+      allow(command).to receive(:run_preflight_checks).and_return(true)
+
+      allow_any_instance_of(GitlabClusterHelper)
+        .to receive(:write_to_file!).and_return(false)
+    end
+
+    it 'exits with 1' do
+      expect { command.execute }.to raise_error(SystemExit)
+    end
+  end
+
+  context 'when writing to the cluster configuration file succeed' do
+    before do
+      allow(STDIN).to receive(:gets).and_return('y')
+
+      allow(command).to receive(:promote_postgresql_to_primary).and_return(true)
+      allow(command).to receive(:reconfigure).and_return(true)
+      allow(command).to receive(:promote_to_primary).and_return(true)
+      allow(command).to receive(:success_message).and_return(true)
+
+      allow_any_instance_of(GitlabClusterHelper)
+        .to receive(:write_to_file!).and_return(true)
+    end
+
+    it 'calls all the subcommands' do
+      expect(command).to receive(:promote_postgresql_to_primary)
+      expect(command).to receive(:reconfigure)
+      expect(command).to receive(:promote_to_primary)
+      expect(command).to receive(:success_message)
+
+      command.execute
+    end
+  end
 end

spec/libraries/helpers/gitlab_cluster_helper_spec.rb

+require 'spec_helper'
+require_relative '../../../files/gitlab-cookbooks/package/libraries/helpers/gitlab_cluster_helper'
+
+RSpec.describe GitlabClusterHelper do
+  let(:gitlab_cluster_config_path) { described_class::JSON_FILE }
+
+  describe '.config_available?' do
+    context 'when cluster configuration file exists' do
+      it 'returns true' do
+        allow(File).to receive(:exist?).with(gitlab_cluster_config_path).and_return(true)
+
+        expect(described_class.config_available?).to eq(true)
+      end
+    end
+
+    context 'when cluster configuration file does not exist' do
+      it 'returns false' do
+        expect(described_class.config_available?).to eq(false)
+      end
+    end
+  end
+
+  describe '#config' do
+    context 'when the cluster configuration file does not exist' do
+      it 'returns an empty hash' do
+        expect(subject.config).to be_empty
+      end
+    end
+
+    context 'when the cluster configuration file exists' do
+      it 'parses the file content' do
+        stub_file_content(gitlab_cluster_config_path, foo: 'bar')
+
+        expect(subject.config).to eq('foo' => 'bar')
+      end
+    end
+  end
+
+  describe '#load_roles!' do
+    before do
+      stub_gitlab_rb(application_role: { enable: true }, geo_primary_role: { enable: nil }, geo_secondary_role: { enable: true })
+    end
+
+    it 'overrides roles defined in the configuration file' do
+      stub_file_content(gitlab_cluster_config_path, secondary: false)
+
+      subject.load_roles!
+
+      expect(Gitlab['application_role']['enable']).to eq(true)
+      expect(Gitlab['geo_secondary_role']['enable']).to eq(false)
+    end
+
+    it 'does not override roles not defined in the configuration file' do
+      stub_file_content(gitlab_cluster_config_path, {})
+
+      subject.load_roles!
+
+      expect(Gitlab['application_role']['enable']).to eq(true)
+      expect(Gitlab['geo_secondary_role']['enable']).to eq(true)
+    end
+
+    it 'prints a warning message for each enabled role defined in the configuration file' do
+      stub_file_content(gitlab_cluster_config_path, primary: true, secondary: false)
+
+      expect(LoggingHelper)
+        .not_to receive(:warning)
+        .with("The geo_primary_role is defined in #{gitlab_cluster_config_path} as primary and takes priority over the role in the /etc/gitlab/gitlab.rb")
+
+      expect(LoggingHelper)
+        .to receive(:warning)
+        .with("The geo_secondary_role is defined in #{gitlab_cluster_config_path} as secondary and takes priority over the role in the /etc/gitlab/gitlab.rb")
+        .once
+
+      subject.load_roles!
+    end
+  end
+
+  describe '#write_to_file!' do
+    let(:config_path) { Dir.mktmpdir }
+    let(:gitlab_cluster_config_path) { File.join(config_path, 'gitlab-cluster.json') }
+
+    before do
+      stub_const('GitlabClusterHelper::CONFIG_PATH', config_path)
+      stub_const('GitlabClusterHelper::JSON_FILE', gitlab_cluster_config_path)
+    end
+
+    after do
+      FileUtils.rm_rf(config_path)
+    end
+
+    context 'when the config directory does not exist' do
+      it 'does not create the configuration file' do
+        FileUtils.rm_rf(config_path)
+
+        subject.write_to_file!
+
+        expect(File.exist?(gitlab_cluster_config_path)).to eq(false)
+      end
+    end
+
+    context 'when the cluster configuration file does not exist' do
+      it 'creates the configuration file' do
+        FileUtils.rm_rf(gitlab_cluster_config_path)
+
+        subject.write_to_file!
+
+        expect(File.exist?(gitlab_cluster_config_path)).to eq(true)
+        expect(read_file_content(gitlab_cluster_config_path)).to be_empty
+      end
+    end
+
+    context 'when the cluster configuration file exists' do
+      it 'overrides previous settings' do
+        write_file_content(gitlab_cluster_config_path, foo: 'bar', zoo: true)
+        subject.config['zoo'] = false
+
+        subject.write_to_file!
+
+        expect(read_file_content(gitlab_cluster_config_path)).to eq("foo" => "bar", "zoo" => false)
+      end
+    end
+  end
+
+  def stub_file_content(fullpath, content)
+    allow(File).to receive(:exist?).with(fullpath).and_return(true)
+    allow(IO).to receive(:read).with(fullpath).and_return(content.to_json)
+  end
+
+  def read_file_content(fullpath)
+    JSON.parse(File.read(fullpath))
+  end
+
+  def write_file_content(fullpath, content)
+    File.open(fullpath, 'w') do |f|
+      f.write(content.to_json)
+      f.chmod(0600)
+    end
+  end
+end

spec/support/shared_examples/ctl_shared_context.rb

@@ -3,8 +3,16 @@ require 'omnibus-ctl'
 RSpec.shared_context 'ctl' do
   let(:ctl) { Omnibus::Ctl.new('testing-ctl') }
+
   before do
     allow_any_instance_of(Omnibus::Ctl).to receive(:require).and_call_original
+
+    allow_any_instance_of(Omnibus::Ctl).to receive(:require).with(
+      "/opt/testing-ctl/embedded/cookbooks/package/libraries/helpers/gitlab_cluster_helper"
+    ) do
+      require_relative("../../../files/gitlab-cookbooks/package/libraries/helpers/gitlab_cluster_helper")
+    end
+
     allow_any_instance_of(Omnibus::Ctl).to receive(:require).with(
       "/opt/testing-ctl/embedded/service/omnibus-ctl-ee/lib/geo/#{command_script}"
     ) do

spec/support/shared_examples/object_store_shared_context.rb

@@ -10,7 +10,8 @@ RSpec.shared_context 'object storage config' do
       external_diffs: { bucket: 'external_diffs' },
       packages: { bucket: 'packages' },
       terraform_state: { enabled: false, bucket: 'terraform' },
-      uploads: { bucket: 'uploads' }
+      uploads: { bucket: 'uploads' },
+      pages: { bucket: 'pages' }
     }
   end
   let(:aws_connection_hash) do