From 2114333539c67c7d53464d4b9f74946478c9f2df Mon Sep 17 00:00:00 2001 From: Ken Barber Date: Sat, 2 Feb 2013 02:54:33 +0100 Subject: [PATCH] Add locale parameter support This adds the parameter 'locale' to the 'postgresql' class so we have a global default, and adds it two the defined resources 'postgresql::db' and 'postgresql::database'. This allows users to either: * Defined a global default for the cluster * Define a per-database default As a side-effect I had to make sure 'charset' was also exposed in a similar manner as some locales need a particular charset to work. Tests were added to test both the 'createdb' case and 'initdb' case for Redhat, and some refactoring was done to make the existing non_default test area use heredocs so my manifests and test code was kept close together. As apposed to entirely different files and places in the directory structure. I cleaned up the related execs a little bit, adding logoutput => on_failure where needed so we can debug failures. Beforehand execs just 'failed', but now we should be able to get better feedback from failed execs helping support. I also add intention comments in parts of the Puppet code that I touched where it made sense. Signed-off-by: Ken Barber --- manifests/database.pp | 21 ++++--- manifests/db.pp | 10 +++- manifests/init.pp | 14 ++++- manifests/initdb.pp | 35 ++++++++---- manifests/params.pp | 8 ++- .../shared_examples/non_default_postgres.rb | 57 +++++++++++++++---- .../system_default_postgres.rb | 19 +++++++ .../manifests/non_default/test_db.pp | 33 ----------- .../manifests/non_default/test_install.pp | 10 ---- 9 files changed, 127 insertions(+), 80 deletions(-) delete mode 100644 spec/system/test_module/manifests/non_default/test_db.pp delete mode 100644 spec/system/test_module/manifests/non_default/test_install.pp diff --git a/manifests/database.pp b/manifests/database.pp index 588f961..ab9a0cf 100644 --- a/manifests/database.pp +++ b/manifests/database.pp @@ -20,14 +20,20 @@ # needs to be moved over to ruby, and add support for ensurable. define postgresql::database( - $dbname = $title, - $charset = 'UTF8', - $tablespace = undef) -{ + $dbname = $title, + $tablespace = undef, + $charset = $postgresql::params::charset, + $locale = $postgresql::params::locale +) { include postgresql::params + # Optionally set the locale switch. Older versions of createdb may not accept + # --locale, so if the parameter is undefined its safer not to pass it. if ($postgresql::params::version != '8.1') { - $locale_option = '--locale=C' + $locale_option = $locale ? { + undef => '', + default => "--locale=${locale}", + } $public_revoke_privilege = "CONNECT" } else { $locale_option = "" @@ -52,8 +58,9 @@ define postgresql::database( exec { $createdb_command : refreshonly => true, - user => 'postgres', - cwd => $postgresql::params::datadir, + user => 'postgres', + cwd => $postgresql::params::datadir, + logoutput => on_failure, } ~> # This will prevent users from connecting to the database unless they've been diff --git a/manifests/db.pp b/manifests/db.pp index 56372c3..f6531c5 100644 --- a/manifests/db.pp +++ b/manifests/db.pp @@ -17,6 +17,7 @@ # [*charset*] - database charset. defaults to 'utf8' # [*grant*] - privilege to grant user. defaults to 'all'. # [*tablespace*] - database tablespace. default to use the template database's tablespace. +# [*locale*] - locale for database. defaults to 'undef' (effectively 'C'). # # Actions: # @@ -35,10 +36,12 @@ define postgresql::db ( $user, $password, - $charset = 'utf8', + $charset = $postgresql::params::charset, + $locale = $postgresql::params::locale, $grant = 'ALL', - $tablespace = undef) -{ + $tablespace = undef +) { + include postgresql::params postgresql::database { $name: # TODO: ensure is not yet supported @@ -47,6 +50,7 @@ define postgresql::db ( tablespace => $tablespace, #provider => 'postgresql', require => Class['postgresql::server'], + locale => $locale, } if ! defined(Postgresql::Database_user[$user]) { diff --git a/manifests/init.pp b/manifests/init.pp index abb1c12..7748c2f 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -23,6 +23,10 @@ # set to `true`. It determines which package repository should # be used to install the postgres packages. Currently supported # values include `yum.postgresql.org`. +# [*locale*] - This setting defines the default locale for initdb and createdb +# commands. This default to 'undef' which is effectively 'C'. +# [*charset*] - Sets the default charset to be used for initdb and createdb. +# Defaults to 'UTF8'. # Actions: # # Requires: @@ -30,13 +34,17 @@ # Sample Usage: # class postgresql ( - $version = $::postgres_default_version, + $version = $::postgres_default_version, $manage_package_repo = false, - $package_source = undef + $package_source = undef, + $locale = undef, + $charset = 'UTF8' ) { class { 'postgresql::params': version => $version, manage_package_repo => $manage_package_repo, - package_source => $package_source + package_source => $package_source, + locale => $locale, + charset => $charset, } } diff --git a/manifests/initdb.pp b/manifests/initdb.pp index 5925c62..e879bde 100644 --- a/manifests/initdb.pp +++ b/manifests/initdb.pp @@ -18,22 +18,35 @@ class postgresql::initdb( $datadir = $postgresql::params::datadir, - $encoding = 'UTF8', + $encoding = $postgresql::params::charset, $group = 'postgres', $initdb_path = $postgresql::params::initdb_path, $user = 'postgres' ) inherits postgresql::params { - - $initdb_command = "${initdb_path} --encoding '${encoding}' --pgdata '${datadir}'" - - exec { $initdb_command: - creates => "${datadir}/PG_VERSION", - user => $user, - group => $group + # Build up the initdb command. + # + # We optionally add the locale switch if specified. Older versions of the + # initdb command don't accept this switch. So if the user didn't pass the + # parameter, lets not pass the switch at all. + $initdb_command = $postgresql::params::locale ? { + undef => "${initdb_path} --encoding '${encoding}' --pgdata '${datadir}'", + default => "${initdb_path} --encoding '${encoding}' --pgdata '${datadir}' --locale '${postgresql::params::locale}'" } - if defined(Package["$postgresql::params::server_package_name"]) { - Package["$postgresql::params::server_package_name"] -> - Exec[$initdb_command] + # This runs the initdb command, we use the existance of the PG_VERSION file to + # ensure we don't keep running this command. + exec { 'postgresql_initdb': + command => $initdb_command, + creates => "${datadir}/PG_VERSION", + user => $user, + group => $group, + logoutput => on_failure, + } + + # If we manage the package (which is user configurable) make sure the + # package exists first. + if defined(Package[$postgresql::params::server_package_name]) { + Package[$postgresql::params::server_package_name]-> + Exec['postgresql_initdb'] } } diff --git a/manifests/params.pp b/manifests/params.pp index 96bdf10..2936226 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -28,9 +28,11 @@ # correct paths to the postgres dirs. class postgresql::params( - $version = $::postgres_default_version, - $manage_package_repo = false, - $package_source = undef + $version = $::postgres_default_version, + $manage_package_repo = false, + $package_source = undef, + $locale = undef, + $charset = 'UTF8' ) { $user = 'postgres' $group = 'postgres' diff --git a/spec/support/shared_examples/non_default_postgres.rb b/spec/support/shared_examples/non_default_postgres.rb index 67cbacb..ef931fc 100644 --- a/spec/support/shared_examples/non_default_postgres.rb +++ b/spec/support/shared_examples/non_default_postgres.rb @@ -6,22 +6,31 @@ shared_examples :non_default_postgres do include_context :pg_vm_context # this method is required by the pg_vm shared context - def install_postgres - sudo_and_log(vm, 'puppet apply -e "include postgresql_tests::non_default::test_install"') - end + def install_postgres; end - describe 'postgresql::db' do - it 'should idempotently create a db that we can connect to' do - - # A bare-minimum class to add a DB to postgres, which will be running due to ubuntu - test_class = 'class {"postgresql_tests::non_default::test_db": db => "postgresql_test_db" }' + describe 'postgresql global config' do + it 'with version and manage_package_repo set, we should be able to idempotently create a db that we can connect to' do + manifest = <<-EOS + # Configure version and manage_package_repo globally, install postgres + # and then try to install a new database. + class { "postgresql": + version => "9.2", + manage_package_repo => true, + package_source => "yum.postgresql.org", + }-> + class { "postgresql::server": }-> + postgresql::db { "postgresql_test_db": + user => "foo1", + password => "foo1", + } + EOS begin # Run once to check for crashes - sudo_and_log(vm, "puppet apply --detailed-exitcodes -e '#{test_class}'; [ $? == 2 ]") + sudo_and_log(vm, "puppet apply --detailed-exitcodes -e '#{manifest}'; [ $? = 2 ]") # Run again to check for idempotence - sudo_and_log(vm, "puppet apply --detailed-exitcodes -e '#{test_class}'") + sudo_and_log(vm, "puppet apply --detailed-exitcodes -e '#{manifest}'") # Check that the database name is present sudo_psql_and_log(vm, 'postgresql_test_db --command="select datname from pg_database limit 1"') @@ -29,5 +38,33 @@ shared_examples :non_default_postgres do sudo_psql_and_log(vm, '--command="drop database postgresql_test_db" postgres') end end + + it 'with locale and charset, the postgres database should reflect that locale' do + pending('no support for initdb with lucid', :if => vm == :lucid) + + manifest = <<-EOS + # Set global locale and charset option, and try installing postgres + class { 'postgresql': + locale => 'en_NG', + charset => 'UTF8', + }-> + class { 'postgresql::server': } + EOS + + # Run once to check for crashes + sudo_and_log(vm, "puppet apply --detailed-exitcodes -e '#{manifest}'; [ $? = 2 ]") + # Run again to check for idempotence + sudo_and_log(vm, "puppet apply --detailed-exitcodes -e '#{manifest}'") + + # Use the command line to create the database, instead of the puppet way + # to bypass any puppet effects and make sure the default works _always_. + sudo_and_log(vm, "su postgres -c 'createdb test1'") + + # Check locale of database 'postgres' to ensure initdb did the correct + # thing. + sudo_and_log(vm, 'su postgres -c \'psql -c "show lc_ctype" test1\'') + sudo_and_log(vm, 'su postgres -c \'psql -c "show lc_ctype" test1\' | grep en_NG') + sudo_and_log(vm, 'su postgres -c \'psql -c "show lc_collate" test1\' | grep en_NG') + end end end diff --git a/spec/support/shared_examples/system_default_postgres.rb b/spec/support/shared_examples/system_default_postgres.rb index b9be48a..fc6a3e2 100644 --- a/spec/support/shared_examples/system_default_postgres.rb +++ b/spec/support/shared_examples/system_default_postgres.rb @@ -49,6 +49,25 @@ shared_examples :system_default_postgres do sudo_psql_and_log(vm, '--command="drop database postgresql_test_db" postgres') end end + + it 'should take a locale parameter' do + manifest = <<-EOS + include postgresql::server + postgresql::db { 'test1': + user => 'test1', + password => 'test1', + charset => 'UTF8', + locale => 'en_NG', + } + EOS + sudo_and_log(vm, "puppet apply -e '#{manifest}'") + + # Some basic tests here to check if the db indeed was created with the + #rr correct locale. + sudo_and_log(vm, 'su postgres -c \'psql -c "show lc_ctype" test1\'') + sudo_and_log(vm, 'su postgres -c \'psql -c "show lc_ctype" test1\' | grep en_NG') + sudo_and_log(vm, 'su postgres -c \'psql -c "show lc_collate" test1\' | grep en_NG') + end end describe 'postgresql::psql' do diff --git a/spec/system/test_module/manifests/non_default/test_db.pp b/spec/system/test_module/manifests/non_default/test_db.pp deleted file mode 100644 index 7dc029c..0000000 --- a/spec/system/test_module/manifests/non_default/test_db.pp +++ /dev/null @@ -1,33 +0,0 @@ -# puppet-postgresql -# For all details and documentation: -# http://github.com/inkling/puppet-postgresql -# -# Copyright 2012- Inkling Systems, Inc. -# -# 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. - -class postgresql_tests::non_default::test_db($db) { - - class { "postgresql": - version => '9.2', - manage_package_repo => true, - package_source => 'yum.postgresql.org', - } -> - - class { "postgresql::server": } -> - - postgresql::db { $db: - user => $db, - password => $db, - } -} diff --git a/spec/system/test_module/manifests/non_default/test_install.pp b/spec/system/test_module/manifests/non_default/test_install.pp deleted file mode 100644 index 7cba92c..0000000 --- a/spec/system/test_module/manifests/non_default/test_install.pp +++ /dev/null @@ -1,10 +0,0 @@ -class postgresql_tests::non_default::test_install { - - class { "postgresql": - version => '9.2', - manage_package_repo => true, - package_source => 'yum.postgresql.org', - } -> - - class { "postgresql::server": } -}