Merge branch '2703-update-pgb-notify-template' into 'master'

Resolve "Update pgb-notify template"

Closes #2703

See merge request !1873

files/gitlab-cookbooks/consul/recipes/watchers.rb

@@ -36,5 +36,6 @@ end
 # Watcher specific settings
 if node['consul']['watchers'].include?('postgresql')
   node.default['gitlab']['pgbouncer']['databases_ini'] = '/var/opt/gitlab/consul/databases.ini'
+  node.default['gitlab']['pgbouncer']['databases_json'] = '/var/opt/gitlab/consul/databases.json'
   node.default['gitlab']['pgbouncer']['databases_ini_user'] = 'gitlab-consul'
 end

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

@@ -156,6 +156,7 @@ default['gitlab']['pgbouncer']['stats_users'] = %w(gitlab-psql postgres pgbounce
 default['gitlab']['pgbouncer']['ignore_startup_parameters'] = 'extra_float_digits'
 default['gitlab']['pgbouncer']['databases_ini'] = '/var/opt/gitlab/pgbouncer/databases.ini'
 default['gitlab']['pgbouncer']['databases_ini_user'] = 'gitlab-psql'
+default['gitlab']['pgbouncer']['databases_json'] = '/var/opt/gitlab/pgbouncer/databases.json'
 default['gitlab']['pgbouncer']['databases'] = {}
 default['gitlab']['pgbouncer']['logfile'] = nil

files/gitlab-cookbooks/gitlab-ee/recipes/pgbouncer.rb

@@ -15,7 +15,6 @@
 #
 account_helper = AccountHelper.new(node)
-pgb_helper = PgbouncerHelper.new(node)
 include_recipe 'gitlab::postgresql_user'
@@ -49,12 +48,27 @@ template "#{node['gitlab']['pgbouncer']['data_directory']}/pgbouncer.ini" do
   notifies :run, 'execute[reload pgbouncer]', :immediately
 end
-template node['gitlab']['pgbouncer']['databases_ini'] do
-  source "#{File.basename(name)}.erb"
-  user node['gitlab']['pgbouncer']['databases_ini_user']
-  helper(:pgb_helper) { pgb_helper }
-  variables lazy { node['gitlab']['pgbouncer'].to_hash }
-  notifies :run, 'execute[reload pgbouncer]', :immediately
+file 'databases.json' do
+  path lazy { node['gitlab']['pgbouncer']['databases_json'] }
+  user lazy { node['gitlab']['pgbouncer']['databases_ini_user'] }
+  group lazy { node['gitlab']['pgbouncer']['databases_ini_user'] }
+  content node['gitlab']['pgbouncer']['databases'].to_json
+  notifies :run, 'execute[generate databases.ini]', :immediately
+end
+
+execute 'generate databases.ini' do
+  command lazy {
+    <<~EOF
+    /opt/gitlab/bin/gitlab-ctl pgb-notify \
+     --databases-json #{node['gitlab']['pgbouncer']['databases_json']} \
+     --databases-ini #{node['gitlab']['pgbouncer']['databases_ini']} \
+     --hostuser #{node['gitlab']['pgbouncer']['databases_ini_user']} \
+     --host #{node['gitlab']['pgbouncer']['listen_addr']} \
+     --port #{node['gitlab']['pgbouncer']['listen_port']} \
+     --user #{node['gitlab']['pgbouncer']['databases_ini_user']}
+    EOF
+  }
+  action :nothing
 end
 execute 'reload pgbouncer' do

files/gitlab-ctl-commands-ee/lib/pgbouncer.rb

@@ -9,7 +9,7 @@ end
 module Pgbouncer
   class Databases
-    attr_accessor :install_path, :databases, :options, :ini_file, :template_file, :attributes
+    attr_accessor :install_path, :databases, :options, :ini_file, :json_file, :template_file, :attributes, :userinfo
     attr_reader :data_path
     def initialize(options, install_path, base_data_path)
@@ -17,32 +17,55 @@ module Pgbouncer
       @attributes = GitlabCtl::Util.get_node_attributes(install_path)
       @install_path = install_path
       @options = options
-      @ini_file = attributes['gitlab']['pgbouncer']['databases_ini']
+      @ini_file = options['databases_ini'] || attributes['gitlab']['pgbouncer']['databases_ini']
+      @json_file = options['databases_json'] || attributes['gitlab']['pgbouncer']['databases_json']
       @template_file = "#{@install_path}/embedded/cookbooks/gitlab-ee/templates/default/databases.ini.erb"
-      @databases = {
-        options['database'] => {
-          'host' => options['newhost'],
-          'port' => options['port'] || '5432',
-          'dbname' => options['database'] || 'gitlabhq_production',
-          'user' => 'pgbouncer'
-        }
-      }
+      @databases = update_databases(JSON.parse(File.read(@json_file)))
+      @userinfo = GitlabCtl::Util.userinfo(options['host_user']) if options['host_user']
+    end
+
+    def update_databases(original)
+      updated = {}
+      original.each do |db, settings|
+        updated[db] = ''
+        settings['host'] = options['newhost'] if options['newhost']
+        settings['port'] = options['port'] if options['port']
+        settings['auth_user'] = options['user'] if options['user']
+        settings['dbname'] =  options['pg_database'] if options['pg_database']
+        settings.each do |setting, value|
+          updated[db] << " #{setting}=#{value}"
+        end
+        updated[db].strip!
+      end
+      updated
+    end
+
+    def database_ini_template
+      <<~EOF
+        [databases]
+        <% @databases.each do |db, settings| %>
+        <%= db %> = <%= settings %>
+        <% end %>
+      EOF
     end
     def data_path=(path)
       full_path = "#{path}/pgbouncer"
       unless Dir.exist?(full_path)
-        raise "The directory #{full_path} does exist. Please ensure pgbouncer is configured on this node"
+        raise "The directory #{full_path} does not exist. Please ensure pgbouncer is configured on this node"
       end
       @data_path = full_path
     end
     def render
-      ERB.new(File.read(@template_file)).result(binding)
+      ERB.new(database_ini_template).result(binding)
     end
     def write
-      File.open(@ini_file, 'w') { |f| f.puts render }
+      File.open(@ini_file, 'w') do |file|
+        file.puts render
+        file.chown(userinfo.uid, userinfo.gid) if options['host_user']
+      end
     end
     def pgbouncer_command(command)

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

@@ -65,6 +65,8 @@ end
 def get_pg_options
   options = {
     'database' => 'gitlabhq_production',
+    'databases_ini' => nil,
+    'databases_json' => nil,
     'host' => nil,
     'port' => nil,
     'user' => 'pgbouncer',
@@ -101,6 +103,14 @@ def get_pg_options
     opts.on('--newhost HOSTNAME', 'The new master when updating pgbouncer') do |h|
       options['newhost'] = h
     end
+
+    opts.on('--databases-json JSON', 'The location of the databases.json file') do |j|
+      options['databases_json'] = j
+    end
+
+    opts.on('--databases-ini JSON', 'The location of the databases.json file') do |j|
+      options['databases_ini'] = j
+    end
   end.parse!(ARGV)
   options
 end

files/gitlab-ctl-commands/lib/gitlab_ctl/util.rb

@@ -58,6 +58,10 @@ module GitlabCtl
         raise GitlabCtl::Errors::PasswordMismatch unless password.eql?(password_confirm)
         password
       end
+
+      def userinfo(username)
+        Etc.getpwnam(username)
+      end
     end
   end
 end

files/gitlab-ctl-commands/lib/postgresql/pgpass.rb

@@ -17,10 +17,12 @@
 require 'erb'
 require 'etc'
+require_relative '../../lib/gitlab_ctl'
+
 module GitlabCtl
   class PostgreSQL
     class Pgpass
-      attr_accessor :hostname, :port, :database, :username, :password, :host_user
+      attr_accessor :hostname, :port, :database, :username, :password, :host_user, :userinfo
       def initialize(options = {})
         @hostname = options[:hostname] || '*'
@@ -29,6 +31,7 @@ module GitlabCtl
         @username = options[:username] || '*'
         @password = options[:password] || '*'
         @host_user = options[:host_user]
+        @userinfo = GitlabCtl::Util.userinfo(host_user)
       end
       def pgpass_template
@@ -47,10 +50,6 @@ module GitlabCtl
         "#{userinfo.dir}/.pgpass"
       end
-      def userinfo
-        Etc.getpwnam(host_user)
-      end
-
       def write
         File.open(filename, 'w') do |file|
           file.puts render

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

@@ -18,7 +18,7 @@ require 'chef_helper'
 describe 'gitlab-ee::pgbouncer' do
   let(:chef_run) { ChefSpec::SoloRunner.converge('gitlab-ee::default') }
   let(:pgbouncer_ini) { '/var/opt/gitlab/pgbouncer/pgbouncer.ini' }
-  let(:databases_ini) { '/var/opt/gitlab/pgbouncer/databases.ini' }
+  let(:databases_json) { '/var/opt/gitlab/pgbouncer/databases.json' }
   before do
     allow(Gitlab).to receive(:[]).and_call_original
@@ -96,22 +96,38 @@ describe 'gitlab-ee::pgbouncer' do
       end
     end
-    context 'databases.ini' do
-      let(:fake_databases) { '/fake/databases.ini' }
-      before do
+    context 'databases.json' do
+      it 'creates databases.json' do
+        expect(chef_run).to create_file(databases_json)
+          .with_content("{\"gitlabhq_production\":{\"host\":\"1.2.3.4\"}}")
+          .with(user: 'gitlab-psql', group: 'gitlab-psql')
+      end
+
+      it 'notifies pgb-notify to generate databases.ini' do
+        json_resource = chef_run.file(databases_json)
+        expect(json_resource).to notify('execute[generate databases.ini]').to(:run).immediately
+      end
+      it 'stores in a different location when attribute is set' do
         stub_gitlab_rb(
           pgbouncer: {
             enable: true,
-            databases_ini: fake_databases,
-            databases_ini_user: 'fakeuser'
+            databases_json: '/fakepath/fakedatabases.json'
           }
         )
+        expect(chef_run).to create_file('databases.json')
+          .with(path: '/fakepath/fakedatabases.json')
       end
-      it 'can be set to a non-standard path' do
-        expect(chef_run).to render_file(pgbouncer_ini).with_content(%r{^%include /fake/databases.ini})
-        expect(chef_run).to create_template(fake_databases).with(user: 'fakeuser')
+      it 'changes the user when the attribute is changed' do
+        stub_gitlab_rb(
+          pgbouncer: {
+            enable: true,
+            databases_ini_user: 'fakeuser'
+          }
+        )
+        expect(chef_run).to create_file('databases.json')
+          .with(user: 'fakeuser', group: 'fakeuser')
       end
     end
   end
@@ -132,32 +148,6 @@ describe 'gitlab-ee::pgbouncer' do
     }
   end
-  it 'allows you to specify multiple databases' do
-    stub_gitlab_rb(
-      {
-        pgbouncer: {
-          enable: true,
-          databases: {
-            first: {
-              host: '1.2.3.4',
-              port: '6432',
-              user: 'first_user'
-            },
-            second: {
-              host: '5.6.7.8',
-              port: '7432',
-              user: 'second_user'
-            }
-          }
-        }
-      }
-    )
-    expect(chef_run).to render_file(databases_ini).with_content { |content|
-      expect(content).to match(%r{^first = host=1\.2\.3\.4 port=6432 auth_user=first_user$})
-      expect(content).to match(%r{^second = host=5\.6\.7\.8 port=7432 auth_user=second_user$})
-    }
-  end
-
   it 'does not create the user file by default' do
     expect(chef_run).not_to render_file('/var/opt/gitlab/pgbouncer/pg_auth')
   end
@@ -178,30 +168,10 @@ describe 'gitlab-ee::pgbouncer' do
         }
       }
     )
-    expect(chef_run).to render_file(databases_ini)
-      .with_content(/^gitlabhq_production = host=127\.0\.0\.1 port=5432 auth_user=fakeuser$/)
     expect(chef_run).to render_file('/var/opt/gitlab/pgbouncer/pg_auth')
       .with_content(%r{^"fakeuser" "md5fakemd5password"$})
   end
-  it 'adds arbitrary values to the databases.ini file' do
-    stub_gitlab_rb(
-      {
-        pgbouncer: {
-          enable: true,
-          databases: {
-            gitlab_db: {
-              host: 'fakehost',
-              fakefield: 'fakedata'
-            }
-          }
-        }
-      }
-    )
-    expect(chef_run).to render_file(databases_ini)
-      .with_content(/^gitlab_db = host=fakehost fakefield=fakedata$/)
-  end
-
   it 'creates arbitrary user' do
     stub_gitlab_rb(
       {

spec/gitlab-ctl-commands-ee/lib/pgbouncer_spec.rb

+require 'chef_helper'
+
+$LOAD_PATH << File.join(__dir__, '../../../files/gitlab-ctl-commands-ee/lib')
+
+require 'pgbouncer'
+
+describe Pgbouncer::Databases do
+  let(:fake_ohai) do
+    {
+      normal: {
+        gitlab: {
+          pgbouncer: {
+            databases_ini: '/fakedata/pgbouncer/databases.ini',
+            databases_json: '/fakedata/pgbouncer/databases.json'
+          }
+        }
+      }
+    }.to_json.to_s
+  end
+
+  let(:fake_databases_json) do
+    {
+      fakedb: {
+        host: 'fakehost',
+        user: 'fakeuser',
+        port: 9999,
+        password: 'dslgkjdfgklfsd'
+      }
+    }.to_json.to_s
+  end
+
+  before do
+    allow(GitlabCtl::Util).to receive(:fqdn).and_return('fakehost')
+    allow(Dir).to receive(:exist?).and_call_original
+    allow(Dir).to receive(:exist?).with('/fakedata/pgbouncer').and_return(true)
+    allow(File).to receive(:read).and_call_original
+    allow(File).to receive(:read).with('/fakeinstall/embedded/nodes/fakehost.json').and_return(fake_ohai)
+    allow(File).to receive(:read).with('/fakedata/pgbouncer/databases.json').and_return(fake_databases_json)
+    @obj = Pgbouncer::Databases.new({}, '/fakeinstall', '/fakedata')
+  end
+
+  context 'default behavior' do
+    it 'allows creating a new object' do
+      expect(@obj.class).to be(Pgbouncer::Databases)
+    end
+
+    it 'creates the database object' do
+      results = {
+        "fakedb" => "host=fakehost user=fakeuser port=9999 password=dslgkjdfgklfsd"
+      }
+      expect(@obj.databases).to eq(results)
+    end
+
+    it 'renders the template' do
+      expect(@obj.render).to eq(
+        "[databases]\n\nfakedb = host=fakehost user=fakeuser port=9999 password=dslgkjdfgklfsd\n\n"
+      )
+    end
+  end
+
+  context 'with options' do
+    before do
+      options = {
+        'databases_ini' => '/another/databases.ini',
+        'databases_json' => '/another/databases.json',
+        'port' => 8888,
+        'user' => 'fakeuser'
+      }
+      allow(File).to receive(:read).with('/another/databases.json').and_return(fake_databases_json)
+      @obj = Pgbouncer::Databases.new(options, '/fakeinstall', '/fakedata')
+    end
+
+    it 'sets the custom options' do
+      expect(@obj.ini_file).to eq('/another/databases.ini')
+      expect(@obj.json_file).to eq('/another/databases.json')
+    end
+
+    it 'renders the template' do
+      expect(@obj.render).to eq(
+        "[databases]\n\nfakedb = host=fakehost user=fakeuser port=8888 password=dslgkjdfgklfsd auth_user=fakeuser\n\n"
+      )
+    end
+  end
+end