diff --git a/manifests/backend/rbd.pp b/manifests/backend/rbd.pp index e6ff2b03..11d8d5df 100644 --- a/manifests/backend/rbd.pp +++ b/manifests/backend/rbd.pp @@ -120,13 +120,16 @@ define cinder::backend::rbd ( } } - if $backend_host { - cinder_config { - "${name}/backend_host": value => $backend_host; - } - } else { - cinder_config { - "${name}/backend_host": value => "rbd:${rbd_pool}"; + # Avoid colliding with code in backends.pp + unless defined(Cinder_config["${name}/backend_host"]) { + if $backend_host { + cinder_config { + "${name}/backend_host": value => $backend_host; + } + } else { + cinder_config { + "${name}/backend_host": value => "rbd:${rbd_pool}"; + } } } diff --git a/manifests/backends.pp b/manifests/backends.pp index ac26f808..4a84c5c8 100644 --- a/manifests/backends.pp +++ b/manifests/backends.pp @@ -10,9 +10,14 @@ # Example: ['volume1', 'volume2', 'sata3'] # Defaults to undef # +# [*backend_host*] +# (optional) Backend override of host value. +# Defaults to hiera('cinder::backend_host', undef) +# # Author: Andrew Woodward class cinder::backends ( $enabled_backends = undef, + $backend_host = hiera('cinder::backend_host', undef) ) { include ::cinder::deps @@ -26,5 +31,15 @@ set up backends. No volume service(s) started successfully otherwise.") cinder_config { 'DEFAULT/enabled_backends': value => join($enabled_backends, ','); } + if $backend_host { + $enabled_backends.each |$backend| { + # Avoid colliding with code in backend/rbd.pp + unless defined(Cinder_config["${backend}/backend_host"]) { + cinder_config { + "${backend}/backend_host": value => $backend_host; + } + } + } + } } } diff --git a/manifests/init.pp b/manifests/init.pp index 0e640a96..98d66977 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -243,6 +243,11 @@ # 'cinder::backend::rdb::volume_tmp_dir' parameter. # Defaults to $::os_service_default # +# [*host*] +# (optional) Name of this node. This can be an opaque identifier. It is +# not necessarily a host name, FQDN, or IP address. +# Defaults to $::os_service_default. +# # [*purge_config*] # (optional) Whether to set only the specified config options # in the cinder config. @@ -298,11 +303,6 @@ # (Optional) DEPRECATED. Virtual_host to use. # Defaults to $::os_service_default # -# [*host*] -# (optional) DEPRECATED. Name of this node. This can be an opaque identifier. It is -# not necessarily a host name, FQDN, or IP address. -# Defaults to $::os_service_default. -# # [*rpc_backend*] # (Optional) DEPRECATED. Use these options to configure the RabbitMQ message system. # Defaults to undef @@ -361,6 +361,7 @@ class cinder ( $enable_v3_api = true, $lock_path = $::cinder::params::lock_path, $image_conversion_dir = $::os_service_default, + $host = $::os_service_default, $purge_config = false, $backend_host = $::os_service_default, # DEPRECATED PARAMETERS @@ -374,7 +375,6 @@ class cinder ( $rabbit_port = $::os_service_default, $rabbit_userid = $::os_service_default, $rabbit_virtual_host = $::os_service_default, - $host = $::os_service_default, $rpc_backend = undef, ) inherits cinder::params { @@ -382,10 +382,6 @@ class cinder ( include ::cinder::db include ::cinder::logging - if !is_service_default($host) { - warning('host is deprecated, has no effect and will be removed in a future release, use backend_host instead') - } - if !is_service_default($rabbit_host) or !is_service_default($rabbit_hosts) or !is_service_default($rabbit_password) or @@ -470,7 +466,10 @@ instead.") 'DEFAULT/allow_availability_zone_fallback': value => $allow_availability_zone_fallback; 'DEFAULT/image_conversion_dir': value => $image_conversion_dir; 'DEFAULT/host': value => $host; - 'DEFAULT/backend_host': value => $backend_host; + + # NOTE(abishop): $backend_host is not written here because it is not a valid + # DEFAULT option. It is only recognized in the backend sections. Instead, + # for backward compatibility, backends.pp references this parameter. } # V3 APIs diff --git a/releasenotes/notes/fix-backend_host-usage-316f4fb0f2ef6dae.yaml b/releasenotes/notes/fix-backend_host-usage-316f4fb0f2ef6dae.yaml new file mode 100644 index 00000000..1a6c275e --- /dev/null +++ b/releasenotes/notes/fix-backend_host-usage-316f4fb0f2ef6dae.yaml @@ -0,0 +1,14 @@ +--- +fixes: + - | + Fixes `bug 1753596 + `__ so the backend_host + option is set in each cinder-volume backend driver's section, and not in + the DEFAULT section where the option isn't relevant. +other: + - | + A previous deprecation notice stated the host option was deprecated in + favor of the backend_host option. This was misleading because the note + failed to clarify this only applied to options in the cinder-volume + backend driver sections. In the DEFAULT section, the host option is not + deprecated, and the backend_host option isn't relevant. diff --git a/spec/classes/cinder_backends_spec.rb b/spec/classes/cinder_backends_spec.rb index c6006150..52bd8586 100644 --- a/spec/classes/cinder_backends_spec.rb +++ b/spec/classes/cinder_backends_spec.rb @@ -45,6 +45,29 @@ describe 'cinder::backends' do it 'configures cinder.conf with default params' do is_expected.to contain_cinder_config('DEFAULT/enabled_backends').with_value(p[:enabled_backends].join(',')) + is_expected.to_not contain_cinder_config('lowcost/backend_host') + is_expected.to_not contain_cinder_config('regular/backend_host') + is_expected.to_not contain_cinder_config('premium/backend_host') + end + end + + context 'configure cinder with backend_host' do + before :each do + params.merge!( + :enabled_backends => ['lowcost', 'regular', 'premium'], + :backend_host => 'somehost', + ) + end + + let(:pre_condition) do + # Verify there are no collisions with any previously defined value. + "cinder_config { 'regular/backend_host': value => 'anotherhost' }" + end + + it 'configures backend_host in each backend' do + is_expected.to contain_cinder_config('lowcost/backend_host').with_value('somehost') + is_expected.to contain_cinder_config('regular/backend_host').with_value('anotherhost') + is_expected.to contain_cinder_config('premium/backend_host').with_value('somehost') end end end diff --git a/spec/classes/cinder_init_spec.rb b/spec/classes/cinder_init_spec.rb index 09912ceb..668c2160 100644 --- a/spec/classes/cinder_init_spec.rb +++ b/spec/classes/cinder_init_spec.rb @@ -53,8 +53,10 @@ describe 'cinder' do is_expected.to contain_cinder_config('DEFAULT/default_availability_zone').with(:value => 'nova') is_expected.to contain_cinder_config('DEFAULT/allow_availability_zone_fallback').with(:value => '') is_expected.to contain_cinder_config('DEFAULT/api_paste_config').with(:value => '/etc/cinder/api-paste.ini') - is_expected.to contain_cinder_config('DEFAULT/backend_host').with(:value => '') is_expected.to contain_cinder_config('oslo_concurrency/lock_path').with(:value => '/var/lock/cinder') + + # backend_host should not be written to DEFAULT section + is_expected.to_not contain_cinder_config('DEFAULT/backend_host') end end @@ -288,14 +290,4 @@ describe 'cinder' do it { is_expected.to contain_cinder_config('DEFAULT/transport_url').with_value('rabbit://rabbit_user:password@localhost:5673') } end - describe 'with backend_host' do - let :params do - req_params.merge({ - :backend_host => 'cinder_backend', - }) - end - - it { is_expected.to contain_cinder_config('DEFAULT/backend_host').with_value('cinder_backend') } - end - end