]> review.fuel-infra Code Review - puppet-modules/puppet-ceilometer.git/commitdiff
Update ceilometer::db class to match other module pattern
authorYanis Guenane <yguenane@redhat.com>
Thu, 24 Sep 2015 08:35:23 +0000 (10:35 +0200)
committerYanis Guenane <yguenane@redhat.com>
Thu, 24 Sep 2015 08:39:14 +0000 (10:39 +0200)
This review aims to update the ceilometer::db class to standardize it with the
other modules db classes[1][2]. It now offers the same parameters to customize database
connection behavior.

[1] https://github.com/openstack/puppet-neutron/blob/master/manifests/db.pp
[2]
https://github.com/openstack/puppet-sahara/blob/master/manifests/db.pp

Change-Id: Ic5ca33f862da300f4f429524ba488568d0608cf3

manifests/db.pp
spec/classes/ceilometer_db_spec.rb

index a6acd77d1501fee522f0a5907d5f6adb1504db83..99fb1d6727e41d8ffedceafebf0709e4a955e7c0 100644 (file)
@@ -1,10 +1,39 @@
-# Configures the ceilometer database
-# This class will install the required libraries depending on the driver
-# specified in the connection_string parameter
+# == Class: ceilometer::db
+#
+#  Configures the ceilometer database
+#  This class will install the required libraries depending on the driver
+#  specified in the connection_string parameter
 #
 # == Parameters
-#  [*database_connection*]
-#    the connection string. format: [driver]://[user]:[password]@[host]/[database]
+
+# [*database_connection*]
+#   Url used to connect to database.
+#   (Optional) Defaults to 'mysql://ceilometer:ceilometer@localhost/ceilometer'.
+#
+# [*database_idle_timeout*]
+#   Timeout when db connections should be reaped.
+#   (Optional) Defaults to 3600.
+#
+# [*database_min_pool_size*]
+#   Minimum number of SQL connections to keep open in a pool.
+#   (Optional) Defaults to 1.
+#
+# [*database_max_pool_size*]
+#   Maximum number of SQL connections to keep open in a pool.
+#   (Optional) Defaults to 10.
+#
+# [*database_max_retries*]
+#   Maximum db connection retries during startup.
+#   Setting -1 implies an infinite retry count.
+#   (Optional) Defaults to 10.
+#
+# [*database_retry_interval*]
+#   Interval between retries of opening a sql connection.
+#   (Optional) Defaults to 10.
+#
+# [*database_max_overflow*]
+#   If set, use this value for max_overflow with sqlalchemy.
+#   (Optional) Defaults to 20.
 #
 #  [*sync_db*]
 #    enable dbsync.
 #    (optional) Deprecated. Does nothing.
 #
 class ceilometer::db (
-  $database_connection = 'mysql://ceilometer:ceilometer@localhost/ceilometer',
-  $sync_db             = true,
-  $mysql_module        = undef,
+  $database_connection     = 'mysql://ceilometer:ceilometer@localhost/ceilometer',
+  $database_idle_timeout   = 3600,
+  $database_min_pool_size  = 1,
+  $database_max_pool_size  = 10,
+  $database_max_retries    = 10,
+  $database_retry_interval = 10,
+  $database_max_overflow   = 20,
+  $sync_db                 = true,
+  $mysql_module            = undef,
 ) {
 
   include ::ceilometer::params
@@ -32,9 +67,8 @@ class ceilometer::db (
   case $database_connection {
     /^mysql:\/\//: {
       $backend_package = false
-
-      include ::mysql::bindings::python
-      Package<| title == 'python-mysqldb' |> -> Class['ceilometer::db']
+      require 'mysql::bindings'
+      require 'mysql::bindings::python'
     }
     /^postgresql:\/\//: {
       $backend_package = $::ceilometer::params::psycopg_package_name
@@ -59,7 +93,13 @@ class ceilometer::db (
   }
 
   ceilometer_config {
-    'database/connection': value => $database_connection, secret => true;
+    'database/connection':     value => $database_connection, secret => true;
+    'database/idle_timeout':   value => $database_idle_timeout;
+    'database/min_pool_size':  value => $database_min_pool_size;
+    'database/max_retries':    value => $database_max_retries;
+    'database/retry_interval': value => $database_retry_interval;
+    'database/max_pool_size':  value => $database_max_pool_size;
+    'database/max_overflow':   value => $database_max_overflow;
   }
 
   if $sync_db {
index da580ecd83c565ff3b601b6036b4a09c4b89ea82..a52ff0ab5c31da05b1ed905eccd188e6aa556c07 100644 (file)
@@ -2,137 +2,95 @@ require 'spec_helper'
 
 describe 'ceilometer::db' do
 
-  # debian has "python-pymongo"
-  context 'on Debian platforms' do
-    let :facts do
-      { :osfamily => 'Debian' }
-    end
+  shared_examples 'ceilometer::db' do
 
-    let :params do
-      { :database_connection => 'mongodb://localhost:1234/ceilometer',
-        :sync_db             => true }
-    end
+    context 'with default parameters' do
 
-    it { is_expected.to contain_class('ceilometer::params') }
+      it { is_expected.to contain_class('ceilometer::params') }
+      it { is_expected.to contain_class('ceilometer::db::sync') }
+      it { is_expected.to contain_ceilometer_config('database/connection').with_value('mysql://ceilometer:ceilometer@localhost/ceilometer').with_secret(true) }
+      it { is_expected.to contain_ceilometer_config('database/idle_timeout').with_value('3600') }
+      it { is_expected.to contain_ceilometer_config('database/min_pool_size').with_value('1') }
+      it { is_expected.to contain_ceilometer_config('database/max_retries').with_value('10') }
+      it { is_expected.to contain_ceilometer_config('database/retry_interval').with_value('10') }
 
-    it 'installs python-mongodb package' do
-      is_expected.to contain_package('ceilometer-backend-package').with(
-        :ensure => 'present',
-        :name   => 'python-pymongo',
-        :tag    => 'openstack'
-      )
-      is_expected.to contain_ceilometer_config('database/connection').with_value('mongodb://localhost:1234/ceilometer')
-      is_expected.to contain_ceilometer_config('database/connection').with_value( params[:database_connection] ).with_secret(true)
     end
 
-    it 'includes ceilometer::db::sync' do
-      is_expected.to contain_class('ceilometer::db::sync')
-    end
-  end
+    context 'with specific parameters' do
+      let :params do
+        { :database_connection     => 'mongodb://localhost:1234/ceilometer',
+          :database_idle_timeout   => '3601',
+          :database_min_pool_size  => '2',
+          :database_max_retries    => '11',
+          :database_retry_interval => '11',
+          :sync_db                 => false }
+      end
 
-  context 'on Redhat platforms' do
-    let :facts do
-      { :osfamily => 'Redhat',
-        :operatingsystem => 'Fedora',
-        :operatingsystemrelease => 21
-      }
-    end
+      it { is_expected.to contain_class('ceilometer::params') }
+      it { is_expected.not_to contain_class('ceilometer::db::sync') }
+      it { is_expected.to contain_ceilometer_config('database/connection').with_value('mongodb://localhost:1234/ceilometer').with_secret(true) }
+      it { is_expected.to contain_ceilometer_config('database/idle_timeout').with_value('3601') }
+      it { is_expected.to contain_ceilometer_config('database/min_pool_size').with_value('2') }
+      it { is_expected.to contain_ceilometer_config('database/max_retries').with_value('11') }
+      it { is_expected.to contain_ceilometer_config('database/retry_interval').with_value('11') }
 
-    let :params do
-      { :database_connection => 'mongodb://localhost:1234/ceilometer',
-        :sync_db             => false }
     end
 
-    it { is_expected.to contain_class('ceilometer::params') }
-
-    it 'installs pymongo package' do
-      is_expected.to contain_package('ceilometer-backend-package').with(
-        :ensure => 'present',
-        :name => 'python-pymongo')
-      is_expected.to contain_ceilometer_config('database/connection').with_value('mongodb://localhost:1234/ceilometer')
-      is_expected.to contain_ceilometer_config('database/connection').with_value( params[:database_connection] ).with_secret(true)
-    end
+    context 'with mongodb backend' do
+      let :params do
+        { :database_connection     => 'mongodb://localhost:1234/ceilometer', }
+      end
 
-    it 'does not include ceilometer::db::sync' do
-      is_expected.not_to contain_class('ceilometer::db::sync')
-    end
-  end
+      it 'install the proper backend package' do
+        is_expected.to contain_package('ceilometer-backend-package').with(
+          :ensure => 'present',
+          :name   => 'python-pymongo',
+          :tag    => 'openstack'
+        )
+      end
 
-  # RHEL has python-pymongo too
-  context 'on Redhat platforms' do
-    let :facts do
-      { :osfamily => 'Redhat',
-        :operatingsystem => 'CentOS',
-        :operatingsystemrelease => 6.4
-      }
     end
 
-    let :params do
-      { :database_connection => 'mongodb://localhost:1234/ceilometer',
-        :sync_db             => true }
-    end
 
-    it { is_expected.to contain_class('ceilometer::params') }
+    context 'with incorrect database_connection string' do
+      let :params do
+        { :database_connection     => 'redis://ceilometer:ceilometer@localhost/ceilometer', }
+      end
 
-    it 'installs pymongo package' do
-      is_expected.to contain_package('ceilometer-backend-package').with(
-        :ensure => 'present',
-        :name => 'python-pymongo')
+      it_raises 'a Puppet::Error', /validate_re/
     end
 
-    it 'includes ceilometer::db::sync' do
-      is_expected.to contain_class('ceilometer::db::sync')
-    end
   end
 
-  # RHEL has python-sqlite2
-  context 'on Redhat platforms' do
+  context 'on Debian platforms' do
     let :facts do
-      { :osfamily => 'Redhat',
-        :operatingsystem => 'CentOS',
-        :operatingsystemrelease => 6.4
-      }
+      { :osfamily => 'Debian' }
     end
 
-    let :params do
-      { :database_connection => 'sqlite:///var/lib/ceilometer.db',
-        :sync_db             => false }
-    end
+    it_configures 'ceilometer::db'
 
-    it { is_expected.to contain_class('ceilometer::params') }
+    context 'with sqlite backend' do
+      let :params do
+        { :database_connection     => 'sqlite:///var/lib/ceilometer.db', }
+      end
 
-    it 'installs pymongo package' do
-      is_expected.to contain_ceilometer_config('database/connection').with_value('sqlite:///var/lib/ceilometer.db')
-      is_expected.to contain_ceilometer_config('database/connection').with_value( params[:database_connection] ).with_secret(true)
-    end
+      it 'install the proper backend package' do
+        is_expected.to contain_package('ceilometer-backend-package').with(
+          :ensure => 'present',
+          :name   => 'python-pysqlite2',
+          :tag    => 'openstack'
+        )
+      end
 
-    it 'does not include ceilomter::db::sync' do
-      is_expected.not_to contain_class('ceilometer::db::sync')
     end
   end
 
-  # debian has "python-pysqlite2"
-  context 'on Debian platforms' do
+  context 'on Redhat platforms' do
     let :facts do
-      { :osfamily => 'Debian' }
-    end
-
-    let :params do
-      { :database_connection => 'sqlite:///var/lib/ceilometer.db',
-        :sync_db             => true }
+      { :osfamily => 'RedHat' }
     end
 
-    it { is_expected.to contain_class('ceilometer::params') }
-
-    it 'installs python-mongodb package' do
-      is_expected.to contain_package('ceilometer-backend-package').with(
-        :ensure => 'present',
-        :name => 'python-pysqlite2')
-    end
-
-    it 'includes ceilometer::db::sync' do
-      is_expected.to contain_class('ceilometer::db::sync')
-    end
+    it_configures 'ceilometer::db'
   end
 
 end