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 <[email protected]>

Author: kbarber

Diff for: 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 @@
 
   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 for: 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 @@
     tablespace  => $tablespace,
     #provider   => 'postgresql',
     require     => Class['postgresql::server'],
+    locale      => $locale,
   }
 
   if ! defined(Postgresql::Database_user[$user]) {

Diff for: manifests/init.pp

@@ -23,20 +23,28 @@
 #                 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:
 #
 # 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 for: 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 {
+  # 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}'"
+  }
 
-  $initdb_command = "${initdb_path} --encoding '${encoding}' --pgdata '${datadir}'"
-  
-  exec { $initdb_command:
-    creates => "${datadir}/PG_VERSION",
-    user    => $user,
-    group   => $group
+  # 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 defined(Package["$postgresql::params::server_package_name"]) {
-    Package["$postgresql::params::server_package_name"] ->
-        Exec[$initdb_command]
+  # 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 for: 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 for: spec/support/shared_examples/non_default_postgres.rb

@@ -6,28 +6,65 @@
   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
-
-  describe 'postgresql::db' do
-    it 'should idempotently create a db that we can connect to' do
+  def install_postgres; end
 
-      # 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"')
       ensure
         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 for: spec/support/shared_examples/system_default_postgres.rb

@@ -49,6 +49,25 @@ def install_postgres
         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