Merge pull request #1504 from SimonHoenscheid/unique_service_title

service name should be unique to allow instances

Author: SimonHoenscheid

lib/puppet/type/postgresql_psql.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-Puppet::Type.newtype(:postgresql_psql) do
+Puppet::Type.newtype(:postgresql_psql) do # rubocop:disable Metrics/BlockLength
   newparam(:name) do
     desc 'An arbitrary tag for your own reference; the name of the message.'
     isnamevar
@@ -122,15 +122,25 @@ def matches(value)
     newvalues(:true, :false)
   end
 
+  newparam(:instance) do
+    desc 'The postgresql instance under which the psql command should be executed.'
+    defaultto('main')
+  end
+
   newparam(:sensitive, boolean: true) do
     desc "If 'true', then the executed command will not be echoed into the log. Use this to protect sensitive information passing through."
 
     defaultto(:false)
     newvalues(:true, :false)
   end
 
-  autorequire(:anchor) { ['postgresql::server::service::begin'] }
-  autorequire(:service) { ['postgresqld'] }
+  autorequire(:anchor) do
+    ["postgresql::server::service::begin::#{self[:instance]}"]
+  end
+
+  autorequire(:service) do
+    ["postgresqld_instance_#{self[:instance]}"]
+  end
 
   def should_run_sql(refreshing = false)
     onlyif_param = @parameters[:onlyif]
@@ -148,7 +158,7 @@ def refresh
 
   private
 
-  def set_sensitive_parameters(sensitive_parameters) # rubocop:disable Style/AccessorMethodName
+  def set_sensitive_parameters(sensitive_parameters) # rubocop:disable Naming/AccessorMethodName
     # Respect sensitive commands
     if sensitive_parameters.include?(:unless)
       sensitive_parameters.delete(:unless)

manifests/server/instance/service.pp

@@ -33,7 +33,7 @@
   anchor { "postgresql::server::service::begin::${name}": }
 
   if $service_manage {
-    service { 'postgresqld':
+    service { "postgresqld_instance_${name}":
       ensure    => $service_ensure,
       enable    => $service_enable,
       name      => $service_name,
@@ -48,18 +48,18 @@
       #
       # Without it, we may continue doing more work before the database is
       # prepared leading to a nasty race condition.
-      postgresql_conn_validator { 'validate_service_is_running':
+      postgresql_conn_validator { "validate_service_is_running_instance_${name}":
         run_as           => $user,
         db_name          => $default_database,
         port             => $port,
         connect_settings => $connect_settings,
         sleep            => 1,
         tries            => 60,
         psql_path        => $psql_path,
-        require          => Service['postgresqld'],
+        require          => Service["postgresqld_instance_${name}"],
         before           => Anchor["postgresql::server::service::end::${name}"],
       }
-      Postgresql::Server::Database <| title == $default_database |> -> Postgresql_conn_validator['validate_service_is_running']
+      Postgresql::Server::Database <| title == $default_database |> -> Postgresql_conn_validator["validate_service_is_running_instance_${name}"]
     }
   }
 

spec/classes/server/service_spec.rb

@@ -10,5 +10,5 @@
   end
 
   it { is_expected.to contain_class('postgresql::server::service') }
-  it { is_expected.to contain_service('postgresqld').with_name('postgresql').with_status('systemctl status postgresql') }
+  it { is_expected.to contain_service('postgresqld_instance_main').with_name('postgresql').with_status('systemctl status postgresql') }
 end

spec/classes/server_spec.rb

@@ -15,7 +15,7 @@
     }
 
     it 'validates connection' do
-      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running')
+      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running_instance_main')
     end
   end
 
@@ -61,7 +61,7 @@ class { 'postgresql::globals':
     it { is_expected.to contain_class('postgresql::server::passwd') }
 
     it 'validates connection' do
-      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running')
+      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running_instance_main')
     end
 
     it 'sets postgres password' do
@@ -85,7 +85,7 @@ class { 'postgresql::globals':
     it { is_expected.to contain_class('postgresql::server::passwd') }
 
     it 'validates connection' do
-      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running')
+      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running_instance_main')
     end
 
     it 'sets postgres password' do
@@ -103,7 +103,7 @@ class { 'postgresql::globals':
     it { is_expected.to contain_class('postgresql::server') }
 
     it 'shouldnt validate connection' do
-      expect(subject).not_to contain_postgresql_conn_validator('validate_service_is_running')
+      expect(subject).not_to contain_postgresql_conn_validator('validate_service_is_running_instance_main')
     end
   end
 
@@ -118,7 +118,7 @@ class { 'postgresql::globals':
     }
 
     it 'validates connection' do
-      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running')
+      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running_instance_main')
     end
   end
 
@@ -135,7 +135,7 @@ class { 'postgresql::globals':
     it { is_expected.to contain_postgresql__server__config_entry('data_directory_for_instance_main') }
 
     it 'validates connection' do
-      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running')
+      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running_instance_main')
     end
   end
 
@@ -150,23 +150,23 @@ class { 'postgresql::globals':
     }
 
     it 'validates connection' do
-      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running')
+      expect(subject).to contain_postgresql_conn_validator('validate_service_is_running_instance_main')
     end
   end
 
   describe 'service_manage => true' do
     let(:params) { { service_manage: true } }
 
-    it { is_expected.to contain_service('postgresqld') }
+    it { is_expected.to contain_service('postgresqld_instance_main') }
   end
 
   describe 'service_manage => false' do
     let(:params) { { service_manage: false } }
 
-    it { is_expected.not_to contain_service('postgresqld') }
+    it { is_expected.not_to contain_service('postgresqld_instance_main') }
 
     it 'shouldnt validate connection' do
-      expect(subject).not_to contain_postgresql_conn_validator('validate_service_is_running')
+      expect(subject).not_to contain_postgresql_conn_validator('validate_service_is_running_instance_main')
     end
   end
 
@@ -182,7 +182,7 @@ class { 'postgresql::globals':
     end
 
     it 'stills enable the service' do
-      expect(subject).to contain_service('postgresqld').with(ensure: 'running')
+      expect(subject).to contain_service('postgresqld_instance_main').with(ensure: 'running')
     end
   end
 

spec/defines/server/database_spec.rb

@@ -13,7 +13,7 @@
   end
 
   it { is_expected.to contain_postgresql__server__database('test') }
-  it { is_expected.to contain_postgresql_psql('CREATE DATABASE "test"').that_requires('Service[postgresqld]') }
+  it { is_expected.to contain_postgresql_psql('CREATE DATABASE "test"').that_requires('Service[postgresqld_instance_main]') }
 
   context "with comment set to 'test comment'" do
     let(:params) { { comment: 'test comment' } }

spec/defines/server/default_privileges_spec.rb

@@ -337,7 +337,7 @@ class {'postgresql::server':}
 
     it do
       expect(subject).to contain_postgresql_psql('default_privileges:test') \
-        .that_requires(['Service[postgresqld]', 'Postgresql::Server::Role[test]'])
+        .that_requires(['Service[postgresqld_instance_main]', 'Postgresql::Server::Role[test]'])
     end
   end
 

spec/defines/server/grant_spec.rb

@@ -207,7 +207,7 @@ class {'postgresql::server':}
 
     it do
       expect(subject).to contain_postgresql_psql('grant:test') \
-        .that_requires(['Service[postgresqld]', 'Postgresql::Server::Role[test]'])
+        .that_requires(['Service[postgresqld_instance_main]', 'Postgresql::Server::Role[test]'])
     end
   end
 

spec/defines/server/reassign_owned_by_spec.rb

@@ -31,6 +31,6 @@ class {'postgresql::server':}
     expect(subject).to contain_postgresql_psql('reassign_owned_by:test:REASSIGN OWNED BY "test_old_role" TO "test_new_role"')
       .with_command('REASSIGN OWNED BY "test_old_role" TO "test_new_role"')
       .with_onlyif(%r{SELECT tablename FROM pg_catalog.pg_tables WHERE\s*schemaname NOT IN \('pg_catalog', 'information_schema'\) AND\s*tableowner = 'test_old_role'.*}m)
-      .that_requires('Service[postgresqld]')
+      .that_requires('Service[postgresqld_instance_main]')
   }
 end

spec/defines/server/role_spec.rb

@@ -105,7 +105,7 @@
             'PGUSER' => 'login-user',
             'PGPASSWORD' => 'login-pass'
           },
-        ).that_requires('Service[postgresqld]')
+        ).that_requires('Service[postgresqld_instance_main]')
     end
 
     it 'has alter role for "test" user with password as ****' do
@@ -364,7 +364,7 @@ class { 'postgresql::server':
     end
 
     it 'has drop role for "test" user if ensure absent' do
-      expect(subject).to contain_postgresql_psql('DROP ROLE "test"').that_requires('Service[postgresqld]')
+      expect(subject).to contain_postgresql_psql('DROP ROLE "test"').that_requires('Service[postgresqld_instance_main]')
     end
   end
 

spec/defines/server/tablespace_spec.rb

@@ -21,7 +21,7 @@
 
   it { is_expected.to contain_file('/srv/data/foo').with_ensure('directory') }
   it { is_expected.to contain_postgresql__server__tablespace('test') }
-  it { is_expected.to contain_postgresql_psql('CREATE TABLESPACE "test"').that_requires('Service[postgresqld]') }
+  it { is_expected.to contain_postgresql_psql('CREATE TABLESPACE "test"').that_requires('Service[postgresqld_instance_main]') }
 
   context 'with different owner' do
     let :params do