Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

service name should be unique to allow instances #1504

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 14 additions & 4 deletions 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
Expand Down Expand Up @@ -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]
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions manifests/server/instance/service.pp
Expand Up @@ -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,
Expand All @@ -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}"]
}
}

Expand Down
2 changes: 1 addition & 1 deletion spec/classes/server/service_spec.rb
Expand Up @@ -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
22 changes: 11 additions & 11 deletions spec/classes/server_spec.rb
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion spec/defines/server/database_spec.rb
Expand Up @@ -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' } }
Expand Down
2 changes: 1 addition & 1 deletion spec/defines/server/default_privileges_spec.rb
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion spec/defines/server/grant_spec.rb
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion spec/defines/server/reassign_owned_by_spec.rb
Expand Up @@ -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
4 changes: 2 additions & 2 deletions spec/defines/server/role_spec.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion spec/defines/server/tablespace_spec.rb
Expand Up @@ -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
Expand Down