Fix location where backend_host is configured

When the backend_host option is specified, configure it in each
cinder-volume backend driver's section and not in the DEFAULT section.
Backend drivers only look for the option in their backend section, so
setting in the DEFAULT section had no effect.

Clarify the fact that the DEFAULT host option is *not* deprecated, and
is only deprecated in backend sections (where the backed_host should be
used).

Partial-Bug: #1753596
Change-Id: I11a55571f4bed630967242c797e08e11c47eab11
(cherry picked from commit bec1827866)

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