Merge "Fix location where backend_host is configured" into stable/queens

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}";
+      }
     }
   }
 

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 <awoodward@mirantis.com>
 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;
+          }
+        }
+      }
+    }
   }
 }

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

releasenotes/notes/fix-backend_host-usage-316f4fb0f2ef6dae.yaml

@@ -0,0 +1,14 @@
+---
+fixes:
+  - |
+    Fixes `bug 1753596
+    <https://bugs.launchpad.net/tripleo/+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.

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

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 => '<SERVICE DEFAULT>')
       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 => '<SERVICE DEFAULT>')
       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