From cef38b5e849bad294374adaf213efabfc26b0d5c Mon Sep 17 00:00:00 2001
From: Sean McGinnis <sean.mcginnis@gmail.com>
Date: Tue, 26 Mar 2019 16:06:39 -0500
Subject: [PATCH] Add upgrade checker for backup driver path

This adds an upgrade check to make sure the backup_driver setting is
using the full path to the driver and not just specifying the module.
Use of the module path for this setting was deprecated several releases
ago and removed in Stein.

Change-Id: I6b9722ab0d7a8bdf808bd937b0c237e3715ccaab
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
---
 cinder/cmd/status.py                          | 29 ++++++++++--
 cinder/tests/unit/cmd/__init__.py             |  0
 cinder/tests/unit/cmd/test_status.py          | 45 +++++++++++++++++++
 doc/source/cli/cinder-status.rst              |  3 +-
 ...-check-backup_driver-fe009985df2bc32f.yaml |  6 +++
 5 files changed, 78 insertions(+), 5 deletions(-)
 create mode 100644 cinder/tests/unit/cmd/__init__.py
 create mode 100644 cinder/tests/unit/cmd/test_status.py
 create mode 100644 releasenotes/notes/cinder-status-check-backup_driver-fe009985df2bc32f.yaml

diff --git a/cinder/cmd/status.py b/cinder/cmd/status.py
index 3f0a8c7aa52..d0c3b43fd3a 100644
--- a/cinder/cmd/status.py
+++ b/cinder/cmd/status.py
@@ -20,6 +20,9 @@ import sys
 from oslo_config import cfg
 from oslo_upgradecheck import upgradecheck as uc
 
+import cinder.service  # noqa
+
+
 CONF = cfg.CONF
 
 SUCCESS = uc.Code.SUCCESS
@@ -30,12 +33,30 @@ WARNING = uc.Code.WARNING
 class Checks(uc.UpgradeCommands):
     """Upgrade checks to run."""
 
-    def _check_placeholder(self):
-        """This is just a placeholder to test the test framework."""
-        return uc.Result(SUCCESS, 'Some details')
+    def _check_backup_module(self):
+        """Checks for the use of backup driver module paths.
+
+        The use of backup modules for setting backup_driver was deprecated and
+        we now only allow the full driver path. This checks that there are not
+        any remaining settings using the old method.
+        """
+        # We import here to avoid conf loading order issues with cinder.service
+        # above.
+        import cinder.backup.manager  # noqa
+
+        backup_driver = CONF.backup_driver
+
+        # Easy check in that a class name will have mixed casing
+        if backup_driver == backup_driver.lower():
+            return uc.Result(
+                FAILURE,
+                'Backup driver configuration requires the full path to the '
+                'driver, but current setting is using only the module path.')
+
+        return uc.Result(SUCCESS)
 
     _upgrade_checks = (
-        ('Placeholder', _check_placeholder),
+        ('Backup Driver Path', _check_backup_module),
     )
 
 
diff --git a/cinder/tests/unit/cmd/__init__.py b/cinder/tests/unit/cmd/__init__.py
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/cinder/tests/unit/cmd/test_status.py b/cinder/tests/unit/cmd/test_status.py
new file mode 100644
index 00000000000..c5759cff979
--- /dev/null
+++ b/cinder/tests/unit/cmd/test_status.py
@@ -0,0 +1,45 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""Unit tests for the cinder-status CLI interfaces."""
+
+from oslo_config import cfg
+from oslo_upgradecheck import upgradecheck as uc
+import testtools
+
+from cinder.cmd import status
+
+CONF = cfg.CONF
+
+
+class TestCinderStatus(testtools.TestCase):
+    """Test cases for the cinder-status upgrade check command."""
+
+    def setUp(self):
+        super(TestCinderStatus, self).setUp()
+        self.checks = status.Checks()
+
+    def _set_backup_driver(self, driver_path):
+        CONF.set_override('backup_driver', driver_path)
+        self.addCleanup(CONF.clear_override, 'backup_driver')
+
+    def test_check_backup_module(self):
+        self._set_backup_driver(
+            'cinder.backup.drivers.swift.SwiftBackupDriver')
+        result = self.checks._check_backup_module()
+        self.assertEqual(uc.Code.SUCCESS, result.code)
+
+    def test_check_backup_module_not_class(self):
+        self._set_backup_driver('cinder.backup.drivers.swift')
+        result = self.checks._check_backup_module()
+        self.assertEqual(uc.Code.FAILURE, result.code)
+        self.assertIn('requires the full path', result.details)
diff --git a/doc/source/cli/cinder-status.rst b/doc/source/cli/cinder-status.rst
index 313c61c609a..9da190290c4 100644
--- a/doc/source/cli/cinder-status.rst
+++ b/doc/source/cli/cinder-status.rst
@@ -85,7 +85,8 @@ Upgrade
 
   **14.0.0 (Stein)**
 
-  * Placeholder to be filled in with checks as they are added in Stein.
+  * Check added to ensure the backup_driver setting is using the full driver
+    class path and not just the module path.
 
 See Also
 ========
diff --git a/releasenotes/notes/cinder-status-check-backup_driver-fe009985df2bc32f.yaml b/releasenotes/notes/cinder-status-check-backup_driver-fe009985df2bc32f.yaml
new file mode 100644
index 00000000000..7533f5f7c71
--- /dev/null
+++ b/releasenotes/notes/cinder-status-check-backup_driver-fe009985df2bc32f.yaml
@@ -0,0 +1,6 @@
+---
+upgrade:
+  - |
+    A new check is added to the ``cinder-status upgrade check`` CLI to check
+    for the use of backup driver module path instead of full driver class path
+    in the ``backup_driver`` configuration setting.