Refactor array values processing

We have different strategies in handling list
values. This patch is bringing the situation
to the consistent state in this question.
Arrays and csv-strings are allowed.
This allows to put this logic inside oslo module
and clean up code duplication in exterior modules
(which can just pass parameters to oslo).
Also patch bring a few updates to top-file docs
and tests.

Change-Id: I37a37d236e643d3cf39b46243119b8393a3feea7

manifests/cache.pp

@@ -59,7 +59,7 @@
 # [*backend_argument*]
 #   (Optional) Arguments supplied to the backend module. Specify this option
 #   once per argument to be passed to the dogpile.cache backend.
-#   Example format: "<argname>:<value>". (string value)
+#   Example format: "<argname>:<value>". (list value)
 #   Defaults to $::os_service_default
 #
 # [*proxies*]
@@ -129,14 +129,20 @@ define oslo::cache(
   $memcache_pool_connection_get_timeout = $::os_service_default,
 ){
 
-  if !is_service_default($memcache_servers) and is_array($memcache_servers) {
+  if !is_service_default($backend_argument) {
-    $memcache_servers_orig = join($memcache_servers, ',')
+    $backend_argument_orig = join(any2array($backend_argument), ',')
+  } else {
+    $backend_argument_orig = $backend_argument
+  }
+
+  if !is_service_default($memcache_servers) {
+    $memcache_servers_orig = join(any2array($memcache_servers), ',')
   } else {
     $memcache_servers_orig = $memcache_servers
   }
 
-  if !is_service_default($proxies) and is_array($proxies) {
+  if !is_service_default($proxies) {
-    $proxies_orig = join($proxies, ',')
+    $proxies_orig = join(any2array($proxies), ',')
   } else {
     $proxies_orig = $proxies
   }
@@ -145,7 +151,7 @@ define oslo::cache(
     'cache/config_prefix'                        => { value => $config_prefix },
     'cache/expiration_time'                      => { value => $expiration_time },
     'cache/backend'                              => { value => $backend },
-    'cache/backend_argument'                     => { value => $backend_argument },
+    'cache/backend_argument'                     => { value => $backend_argument_orig },
     'cache/proxies'                              => { value => $proxies_orig },
     'cache/enabled'                              => { value => $enabled },
     'cache/debug_cache_backend'                  => { value => $debug_cache_backend },

manifests/messaging/notifications.pp

@@ -8,30 +8,44 @@
 # === Parameters:
 #
 # [*driver*]
-#  (Optional) The Driver(s) to handle sending notifications.
+#   (Optional) The Driver(s) to handle sending notifications.
-#  Possible values are messaging, messagingv2, routing, log, test, noop.
+#   Possible values are messaging, messagingv2, routing, log, test, noop.
-#  Defaults to $::os_service_default.
+#   (list value)
+#   Defaults to $::os_service_default.
 #
 # [*transport_url*]
-#  (Optional) A URL representing the messaging driver to use for
+#   (Optional) A URL representing the messaging driver to use for
-#  notifications. If not set, we fall back to the same
+#   notifications. If not set, we fall back to the same
-#  configuration used for RPC.
+#   configuration used for RPC.
-#  Defaults to $::os_service_default.
+#   (string value)
+#   Defaults to $::os_service_default.
 #
 # [*topics*]
-#  (Optional) AMQP topic(s) used for OpenStack notifications
+#   (Optional) AMQP topic(s) used for OpenStack notifications
-#  Defaults to $::os_service_default.
+#   (list value)
+#   Defaults to $::os_service_default.
 #
 define oslo::messaging::notifications(
   $driver        = $::os_service_default,
   $transport_url = $::os_service_default,
   $topics        = $::os_service_default,
 ) {
+  if !is_service_default($driver) {
+    $driver_orig = join(any2array($driver), ',')
+  } else {
+    $driver_orig = $driver
+  }
+
+  if !is_service_default($topics) {
+    $topics_orig = join(any2array($topics), ',')
+  } else {
+    $topics_orig = $topics
+  }
 
   $notification_options = {
-    'oslo_messaging_notifications/driver' => { value => $driver },
+    'oslo_messaging_notifications/driver'        => { value => $driver_orig },
     'oslo_messaging_notifications/transport_url' => { value => $transport_url },
-    'oslo_messaging_notifications/topics' => { value => $topics },
+    'oslo_messaging_notifications/topics'        => { value => $topics_orig },
   }
 
   create_resources($name, $notification_options)

manifests/messaging/rabbit.pp

@@ -177,12 +177,8 @@ define oslo::messaging::rabbit(
     fail('Unsupported Kombu compression. Possible values are gzip and bz2')
   }
 
-  if !is_service_default($rabbit_hosts) and !is_array($rabbit_hosts) {
-    fail('Rabbit hosts should be an array')
-  }
-
   if !is_service_default($rabbit_hosts) {
-    $rabbit_hosts_orig = join($rabbit_hosts, ',')
+    $rabbit_hosts_orig = join(any2array($rabbit_hosts), ',')
     if size($rabbit_hosts) > 1 and is_service_default($rabbit_ha_queues) {
       $rabbit_ha_queues_orig = true
     } else {

manifests/versionedobjects.pp

@@ -8,7 +8,7 @@
 # === Parameters:
 #
 # [*fatal_exception_format_errors*]
-#  (Optional) Make exception message format errors fatal.
+#  (Optional) Make exception message format errors fatal. (boolean value)
 #  Defaults to $::os_service_default.
 #
 define oslo::versionedobjects(

spec/defines/oslo_cache_spec.rb

@@ -30,7 +30,7 @@ describe 'oslo::cache' do
           :config_prefix                        => 'cache.oslo',
           :expiration_time                      => '600',
           :backend                              => 'dogpile.cache.null',
-          :backend_argument                     => 'foo:bar',
+          :backend_argument                     => ['foo:bar'],
           :proxies                              => ['proxy1', 'proxy2'],
           :enabled                              => true,
           :debug_cache_backend                  => true,
@@ -43,7 +43,7 @@ describe 'oslo::cache' do
         }
       end
 
-      it 'configures cache setion' do
+      it 'configures cache section' do
         is_expected.to contain_keystone_config('cache/config_prefix').with_value('cache.oslo')
         is_expected.to contain_keystone_config('cache/expiration_time').with_value('600')
         is_expected.to contain_keystone_config('cache/backend').with_value('dogpile.cache.null')
@@ -59,6 +59,22 @@ describe 'oslo::cache' do
         is_expected.to contain_keystone_config('cache/memcache_pool_connection_get_timeout').with_value('10')
       end
     end
+
+    context 'with string in list parameters' do
+      let :params do
+        {
+          :backend_argument => 'foo:bar',
+          :memcache_servers => 'host1:11211,host2:11211',
+          :proxies          => 'proxy1,proxy2',
+        }
+      end
+
+      it 'configures oslo_policy section with overriden list values as strings' do
+        is_expected.to contain_keystone_config('cache/backend_argument').with_value('foo:bar')
+        is_expected.to contain_keystone_config('cache/memcache_servers').with_value('host1:11211,host2:11211')
+        is_expected.to contain_keystone_config('cache/proxies').with_value('proxy1,proxy2')
+      end
+    end
   end
 
   context 'on a Debian osfamily' do

spec/defines/oslo_messaging_notifications_spec.rb

@@ -17,9 +17,9 @@ describe 'oslo::messaging::notifications' do
 
     context 'with overridden parameters' do
       let :params do
-          { :driver        => 'messaging',
+          { :driver        => ['messaging'],
             :transport_url => 'some_protocol://some_url',
-            :topics        => 'notifications',
+            :topics        => ['notifications'],
           }
       end
 
@@ -29,6 +29,20 @@ describe 'oslo::messaging::notifications' do
         is_expected.to contain_keystone_config('oslo_messaging_notifications/topics').with_value('notifications')
       end
     end
+
+    context 'with string in list parameters' do
+      let :params do
+        {
+          :driver => 'messaging',
+          :topics => 'notifications',
+        }
+      end
+
+      it 'configures oslo_messaging_notifications section with overriden list values as strings' do
+        is_expected.to contain_keystone_config('oslo_messaging_notifications/driver').with_value('messaging')
+        is_expected.to contain_keystone_config('oslo_messaging_notifications/topics').with_value('notifications')
+      end
+    end
   end
 
   on_supported_os({

spec/defines/oslo_messaging_rabbit_spec.rb

@@ -156,14 +156,6 @@ describe 'oslo::messaging::rabbit' do
       end
     end
 
-    context 'with incorrect rabbit hosts' do
-      let :params do
-        { :rabbit_hosts => 'rabbit1:5672,rabbit2:5673' }
-      end
-
-      it { is_expected.to raise_error Puppet::Error, /Rabbit hosts should be an array/ }
-    end
-
     context 'with incorrect kombu compression' do
       let :params do
         { :kombu_compression => 'foo' }
@@ -195,6 +187,16 @@ describe 'oslo::messaging::rabbit' do
 
       it { is_expected.to raise_error Puppet::Error, /The kombu_ssl_version parameter requires rabbit_use_ssl to be set to true/ }
     end
+
+    context 'with string in list parameters' do
+      let :params do
+        { :rabbit_hosts => 'rabbit1:5672,rabbit2:5673' }
+      end
+
+      it 'configures rabbit with overriden list values as strings' do
+        is_expected.to contain_keystone_config('oslo_messaging_rabbit/rabbit_hosts').with_value('rabbit1:5672,rabbit2:5673')
+      end
+    end
   end
 
   context 'on a Debian osfamily' do

spec/defines/oslo_policy_spec.rb

@@ -37,7 +37,7 @@ describe 'oslo::policy' do
         }
       end
 
-      it 'configures oslo_policy section' do
+      it 'configures oslo_policy section with overriden list values as strings' do
         is_expected.to contain_keystone_config('oslo_policy/policy_dirs').with_value('dir1,/dir/2')
       end
     end