openstack
/
puppet-oslo
Code Issues Proposed changes

Messaging notifications should be set as a list 

We currently join the list into a comma-separated string.
This does not make a lot of sense because oslo messaging
notification drivers must not be passed in the driver=foo,bar form
but in the driver=foo\ndriver=bar form. See:

- https://docs.openstack.org/oslo.config/queens/configuration/format.html

multi valued (MultiStrOpt)

    A multi-valued option is a string value and can be given more than once, all values will be used.

    # Driver or drivers to handle sending notifications. (multi valued)
    notification_driver = nova.openstack.common.notifier.rpc_notifier
    notification_driver = ceilometer.compute.nova_notifier

By simply passing down the list we let all the puppet backends
that use the openstack_config:ruby backend to work correctly
(https://github.com/openstack/puppet-openstacklib#defined-provider-for-openstack_config-ruby)

This will fix the scenario where multiple notification drivers are
specified which is currently broken and will let the single driver
scenario to keep working as is.

Change-Id: If65946412b42e0919456ed92fdd8e3788ad67872
Related-Bug: #1851629
This commit is contained in:
Michele Baldessari 2019-11-07 12:01:41 +01:00 committed by Takashi Kajinami
parent 0e212e8257
commit c7b0cc82fa
2 changed files with 22 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
manifests/messaging/notifications.pp
View File

@ -10,7 +10,7 @@
# [*driver*]
# (Optional) The Driver(s) to handle sending notifications.
# Possible values are messaging, messagingv2, routing, log, test, noop.
# (list value)
# (list value or string value)
# Defaults to $::os_service_default.
#
# [*transport_url*]
@ -32,10 +32,12 @@ define oslo::messaging::notifications(
$transport_url = $::os_service_default,
$topics = $::os_service_default,
) {
if !is_service_default($driver) {
$driver_orig = join(any2array($driver), ',')
} else {
if is_service_default($driver) or is_string($driver) {
# When we have a string value for driver, we keep passing it as string
# to reduce any chance of breaking things in a backwards incompatible way
$driver_orig = $driver
} else {
$driver_orig = any2array($driver)
}
if !is_service_default($topics) {

19
spec/defines/oslo_messaging_notifications_spec.rb
View File

@ -17,15 +17,28 @@ describe 'oslo::messaging::notifications' do
context 'with overridden parameters' do
let :params do
{ :driver => ['messaging'],
{ :driver => ['messaging', 'messagingv2'],
:transport_url => 'some_protocol://some_url',
:topics => ['foo', 'baa'],
}
end
it 'configure oslo_messaging_notifications with overridden values' do
is_expected.to contain_keystone_config('oslo_messaging_notifications/driver').with_value(['messaging', 'messagingv2'])
is_expected.to contain_keystone_config('oslo_messaging_notifications/transport_url').with_value('some_protocol://some_url').with_secret(true)
is_expected.to contain_keystone_config('oslo_messaging_notifications/topics').with_value('foo,baa')
end
end
context 'with a single item in lists' do
let :params do
{ :driver => ['messaging'],
:topics => ['notifications'],
}
end
it 'configure oslo_messaging_notifications with overridden values' do
is_expected.to contain_keystone_config('oslo_messaging_notifications/driver').with_value('messaging')
is_expected.to contain_keystone_config('oslo_messaging_notifications/transport_url').with_value('some_protocol://some_url').with_secret(true)
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