From 2f0bd74604efe7f21823cba53b0a27f0ccc7ea12 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Tue, 30 Jul 2019 11:18:27 +0200 Subject: [PATCH] Fix get_driver_options Any new Cinder driver we add that doesn't have the "get_driver_options" method defined will break the driver list generation tools. The reason why it breaks them is because this method must be static, yet our base driver class doesn't define it as static. This patch: - Sets the base method as static to prevent new drivers from breaking the tools. - Documents the existence of this method for driver developers. - Adds get_driver_options method for drivers that are missing it. - Fix macrosan_client help message that breaks the doc building process. Closes-Bug: #1838225 Change-Id: I4797724d7b55709f0903d522b0233242b867146d --- cinder/tests/unit/test_interface.py | 59 +++++++++++++++++++ cinder/volume/driver.py | 1 + .../drivers/infortrend/infortrend_fc_cli.py | 5 ++ .../infortrend/infortrend_iscsi_cli.py | 5 ++ cinder/volume/drivers/macrosan/config.py | 2 +- cinder/volume/drivers/macrosan/driver.py | 5 ++ doc/source/contributor/drivers.rst | 17 ++++++ 7 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 cinder/tests/unit/test_interface.py diff --git a/cinder/tests/unit/test_interface.py b/cinder/tests/unit/test_interface.py new file mode 100644 index 00000000000..9f8f124ed33 --- /dev/null +++ b/cinder/tests/unit/test_interface.py @@ -0,0 +1,59 @@ +# Copyright (c) 2019, Red Hat, Inc. +# All Rights Reserved. +# +# 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. + +import mock + +from cinder.interface import util +from cinder import test + + +class GetDriversTestCase(test.TestCase): + def test_get_volume_drivers(self): + # Just ensure that it doesn't raise an exception + drivers = util.get_volume_drivers() + self.assertNotEqual(0, len(drivers)) + for driver in drivers: + self.assertIsInstance(driver, util.DriverInfo) + + @mock.patch('cinder.volume.drivers.lvm.LVMVolumeDriver.get_driver_options') + def test_get_volume_drivers_fail(self, driver_opt): + driver_opt.side_effect = ValueError + self.assertRaises(ValueError, util.get_volume_drivers) + + def test_get_backup_drivers(self): + # Just ensure that it doesn't raise an exception + drivers = util.get_backup_drivers() + self.assertNotEqual(0, len(drivers)) + for driver in drivers: + self.assertIsInstance(driver, util.DriverInfo) + + @mock.patch('cinder.backup.drivers.ceph.CephBackupDriver.' + 'get_driver_options') + def test_get_backup_drivers_fail(self, driver_opt): + driver_opt.side_effect = ValueError + self.assertRaises(ValueError, util.get_backup_drivers) + + def test_get_fczm_drivers(self): + # Just ensure that it doesn't raise an exception + drivers = util.get_fczm_drivers() + self.assertNotEqual(0, len(drivers)) + for driver in drivers: + self.assertIsInstance(driver, util.DriverInfo) + + @mock.patch('cinder.zonemanager.drivers.cisco.cisco_fc_zone_driver.' + 'CiscoFCZoneDriver.get_driver_options') + def test_get_fczm_drivers_fail(self, driver_opt): + driver_opt.side_effect = ValueError + self.assertRaises(ValueError, util.get_fczm_drivers) diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index 6125c53c8e6..e3e25686480 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -562,6 +562,7 @@ class BaseVD(object): def check_for_setup_error(self): return + @staticmethod def get_driver_options(): """Return the oslo_config options specific to the driver.""" return volume_opts diff --git a/cinder/volume/drivers/infortrend/infortrend_fc_cli.py b/cinder/volume/drivers/infortrend/infortrend_fc_cli.py index eb25d1a22d2..997ba56d122 100644 --- a/cinder/volume/drivers/infortrend/infortrend_fc_cli.py +++ b/cinder/volume/drivers/infortrend/infortrend_fc_cli.py @@ -39,6 +39,11 @@ class InfortrendCLIFCDriver(driver.FibreChannelDriver): 'FC', configuration=self.configuration) self.VERSION = self.common.VERSION + @staticmethod + def get_driver_options(): + """Return the oslo_config options specific to the driver.""" + return common_cli.infortrend_opts + def do_setup(self, context): """Any initialization the volume driver does while starting. diff --git a/cinder/volume/drivers/infortrend/infortrend_iscsi_cli.py b/cinder/volume/drivers/infortrend/infortrend_iscsi_cli.py index 30b0b2fc9a2..3cefc937556 100644 --- a/cinder/volume/drivers/infortrend/infortrend_iscsi_cli.py +++ b/cinder/volume/drivers/infortrend/infortrend_iscsi_cli.py @@ -38,6 +38,11 @@ class InfortrendCLIISCSIDriver(driver.ISCSIDriver): 'iSCSI', configuration=self.configuration) self.VERSION = self.common.VERSION + @staticmethod + def get_driver_options(): + """Return the oslo_config options specific to the driver.""" + return common_cli.infortrend_opts + def do_setup(self, context): """Any initialization the volume driver does while starting. diff --git a/cinder/volume/drivers/macrosan/config.py b/cinder/volume/drivers/macrosan/config.py index 69ca770866e..70029b5f70e 100644 --- a/cinder/volume/drivers/macrosan/config.py +++ b/cinder/volume/drivers/macrosan/config.py @@ -87,7 +87,7 @@ macrosan_opts = [ (host; client_name; sp1_iscsi_port; sp2_iscsi_port), (host; client_name; sp1_iscsi_port; sp2_iscsi_port) Important warning, Client_name has the following requirements: - [a-zA-Z0-9.-_:], the maximum number of characters is 31 + [a-zA-Z0-9.-_:], the maximum number of characters is 31 E.g: (controller1; decive1; eth-1:0; eth-2:0), (controller2; decive2; eth-1:0/eth-1:1; eth-2:0/eth-2:1), diff --git a/cinder/volume/drivers/macrosan/driver.py b/cinder/volume/drivers/macrosan/driver.py index 675919f40a1..c722f5418f1 100644 --- a/cinder/volume/drivers/macrosan/driver.py +++ b/cinder/volume/drivers/macrosan/driver.py @@ -182,6 +182,11 @@ class MacroSANBaseDriver(driver.VolumeDriver): self.initialize_iscsi_info() + @staticmethod + def get_driver_options(): + """Return the oslo_config options specific to the driver.""" + return config.macrosan_opts + @property def _self_node_wwns(self): return [] diff --git a/doc/source/contributor/drivers.rst b/doc/source/contributor/drivers.rst index ff85ecc26c3..7b1a2061edf 100644 --- a/doc/source/contributor/drivers.rst +++ b/doc/source/contributor/drivers.rst @@ -35,6 +35,23 @@ There are some basic attributes that all drivers classes should have: The tooling system will also use the name and docstring of the driver class. +Configuration options +--------------------- + +Each driver requires different configuration options set in the cinder.conf +file to operate, and due to the complexities of the Object Oriented programming +mechanisms (inheritance, composition, overwriting, etc.) once your driver +defines its parameters in the code Cinder has no automated way of telling which +configuration options are relevant to your driver. + +In order to assist operators and installation tools we recommend reporting the +relevant options: + +* For operators: In the documentation under + ``doc/source/configuration/block-storage``. +* For operators and installers: Through the ``get_driver_options`` static + method returning that returns a list of all the Oslo Config parameters. + Minimum Features ----------------