From fa045ca9fd0abfff738a18755e0914b2f438e072 Mon Sep 17 00:00:00 2001
From: Matt Riedemann <mriedem@us.ibm.com>
Date: Fri, 4 Nov 2016 10:39:57 -0400
Subject: [PATCH 05/10] libvirt: prefer cinder rbd auth values over nova.conf

In the case that the ceph storage backing volumes is different
from the one backing ephemeral storage in nova, the auth values
in the rbd connection_info could be different and not work if
we are using the nova.conf values for ephemeral storage.

This change makes the volume connection config code for rbd
prefer the cinder connection_info values if they exist, and
only falls back to nova config values if cinder doesn't have
anything set.

Depends-On: I4655cae3212d589177d2570403b563a83aad529a

Change-Id: Idcbada705c1d38ac5fd7c600141c2de7020eae25
Closes-Bug: #1635008
---
 nova/tests/unit/virt/libvirt/volume/test_net.py    | 10 +++---
 nova/virt/libvirt/volume/net.py                    | 38 +++++++++++++++-------
 ...-bug-1635008-rbd-vol-auth-83277b02ea87e16e.yaml | 11 +++++++
 3 files changed, 43 insertions(+), 16 deletions(-)
 create mode 100644 releasenotes/notes/ocata-bug-1635008-rbd-vol-auth-83277b02ea87e16e.yaml

diff --git a/nova/tests/unit/virt/libvirt/volume/test_net.py b/nova/tests/unit/virt/libvirt/volume/test_net.py
index 456f3bba81..1d96839fd5 100644
--- a/nova/tests/unit/virt/libvirt/volume/test_net.py
+++ b/nova/tests/unit/virt/libvirt/volume/test_net.py
@@ -59,7 +59,7 @@ class LibvirtNetVolumeDriverTestCase(
             'driver_volume_type': 'rbd',
             'data': {
                 'name': '%s/%s' % ('rbd', volume['name']),
-                'auth_enabled': CONF.libvirt.rbd_secret_uuid is not None,
+                'auth_enabled': CONF.libvirt.rbd_user is not None,
                 'auth_username': CONF.libvirt.rbd_user,
                 'secret_type': 'ceph',
                 'secret_uuid': CONF.libvirt.rbd_secret_uuid,
@@ -114,7 +114,9 @@ class LibvirtNetVolumeDriverTestCase(
         self.assertEqual(self.uuid, tree.find('./auth/secret').get('uuid'))
         libvirt_driver.disconnect_volume(connection_info, "vde")
 
-    def test_libvirt_rbd_driver_auth_enabled_flags_override(self):
+    def test_libvirt_rbd_driver_auth_enabled_flags(self):
+        # The values from the cinder connection_info take precedence over
+        # nova.conf values.
         libvirt_driver = net.LibvirtNetVolumeDriver(self.fake_conn)
         connection_info = self.rbd_connection(self.vol)
         secret_type = 'ceph'
@@ -132,9 +134,9 @@ class LibvirtNetVolumeDriverTestCase(
         conf = libvirt_driver.get_config(connection_info, self.disk_info)
         tree = conf.format_dom()
         self._assertNetworkAndProtocolEquals(tree)
-        self.assertEqual(flags_user, tree.find('./auth').get('username'))
+        self.assertEqual(self.user, tree.find('./auth').get('username'))
         self.assertEqual(secret_type, tree.find('./auth/secret').get('type'))
-        self.assertEqual(flags_uuid, tree.find('./auth/secret').get('uuid'))
+        self.assertEqual(self.uuid, tree.find('./auth/secret').get('uuid'))
         libvirt_driver.disconnect_volume(connection_info, "vde")
 
     def test_libvirt_rbd_driver_auth_disabled(self):
diff --git a/nova/virt/libvirt/volume/net.py b/nova/virt/libvirt/volume/net.py
index 670929c549..e60ce3be88 100644
--- a/nova/virt/libvirt/volume/net.py
+++ b/nova/virt/libvirt/volume/net.py
@@ -10,14 +10,17 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_log import log as logging
+
 import nova.conf
 from nova import exception
-from nova.i18n import _
+from nova.i18n import _, _LW
 from nova import utils
 from nova.virt.libvirt.volume import volume as libvirt_volume
 
 
 CONF = nova.conf.CONF
+LOG = logging.getLogger(__name__)
 
 
 class LibvirtNetVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver):
@@ -51,19 +54,30 @@ class LibvirtNetVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver):
             self.connection._host.delete_secret(usage_type, usage_name)
 
     def _set_auth_config_rbd(self, conf, netdisk_properties):
+        # The rbd volume driver in cinder sets auth_enabled if the rbd_user is
+        # set in cinder. The rbd auth values from the cinder connection take
+        # precedence over any local nova config values in case the cinder ceph
+        # backend is configured differently than the nova rbd ephemeral storage
+        # configuration.
         auth_enabled = netdisk_properties.get('auth_enabled')
-        if CONF.libvirt.rbd_secret_uuid:
-            conf.auth_secret_uuid = CONF.libvirt.rbd_secret_uuid
-            auth_enabled = True  # Force authentication locally
-            if CONF.libvirt.rbd_user:
-                conf.auth_username = CONF.libvirt.rbd_user
         if auth_enabled:
-            conf.auth_username = (conf.auth_username or
-                                  netdisk_properties['auth_username'])
-            conf.auth_secret_type = (conf.auth_secret_type or
-                                     netdisk_properties['secret_type'])
-            conf.auth_secret_uuid = (conf.auth_secret_uuid or
-                                     netdisk_properties['secret_uuid'])
+            conf.auth_username = netdisk_properties['auth_username']
+            conf.auth_secret_uuid = netdisk_properties['secret_uuid']
+            # secret_type is always hard-coded to 'ceph' in cinder
+            conf.auth_secret_type = netdisk_properties['secret_type']
+        elif CONF.libvirt.rbd_secret_uuid:
+            # Anyone relying on falling back to nova config is probably having
+            # this work accidentally and we'll remove that support in the
+            # 16.0.0 Pike release.
+            LOG.warning(_LW('Falling back to Nova configuration values for '
+                            'RBD authentication. Cinder should be configured '
+                            'for auth with Ceph volumes. This fallback will '
+                            'be dropped in the Nova 16.0.0 Pike release.'))
+            # use the nova config values
+            conf.auth_username = CONF.libvirt.rbd_user
+            conf.auth_secret_uuid = CONF.libvirt.rbd_secret_uuid
+            # secret_type is always hard-coded to 'ceph' in cinder
+            conf.auth_secret_type = netdisk_properties['secret_type']
 
     def _set_auth_config_iscsi(self, conf, netdisk_properties):
         if netdisk_properties.get('auth_method') == 'CHAP':
diff --git a/releasenotes/notes/ocata-bug-1635008-rbd-vol-auth-83277b02ea87e16e.yaml b/releasenotes/notes/ocata-bug-1635008-rbd-vol-auth-83277b02ea87e16e.yaml
new file mode 100644
index 0000000000..f7e00a1b78
--- /dev/null
+++ b/releasenotes/notes/ocata-bug-1635008-rbd-vol-auth-83277b02ea87e16e.yaml
@@ -0,0 +1,11 @@
+---
+upgrade:
+  - |
+    When making connections to Ceph-backed volumes via the Libvirt driver, the
+    auth values (rbd_user, rbd_secret_uuid) are now pulled from the backing
+    cinder.conf rather than nova.conf. The nova.conf values are only used if
+    set and the cinder.conf values are not set, but this fallback support is
+    considered accidental and will be removed in the Nova 16.0.0 Pike release.
+    See the Ceph documentation for `configuring Cinder`_ for RBD auth.
+
+    .. _configuring Cinder: http://docs.ceph.com/docs/master/rbd/rbd-openstack/#configuring-cinder
-- 
2.13.7

