Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add driver_options #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cinderlib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

setup = cinderlib.setup
Backend = cinderlib.Backend
driver_options = cinderlib.Backend.driver_options

get_connector_properties = objects.brick_connector.get_connector_properties
list_supported_drivers = cinderlib.Backend.list_supported_drivers
5 changes: 5 additions & 0 deletions cinderlib/cinderlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ def __init__(self, volume_backend_name, **driver_cfg):

self._pool_names = tuple(pool['pool_name'] for pool in stats['pools'])

@staticmethod
def driver_options(volume_driver):
"""Return the driver specific configuration options."""
return importutils.import_class(volume_driver).get_driver_options()

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the idea of being able to report the configuration options for the drivers!!

I think in most cases the caller will want the information for all drivers, and doing it this way would end up with all the drivers loaded in memory, but I still see value in being able to get the options for a specific driver.

That's why I would like to do 2 things:

  • Add this information to the data returned by list_supported_drivers, which already spawns a different process so we free the memory after we get the info.
  • Make this method also use a different process to retrieve the information

If the Cinder patch adds this information to DriverInfo in cinder/interface/util.py we could get this information directly here!
For the time being I'm going to modify the mapping in list_supported_drivers so it returns a dictionary for each entry instead of a single string. That way it'll be more extensible and allow us to add this information.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working on the next patchset that will add the info to the cinder/interface/util.py

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I pushed up a new patchset that includes dumping the options in generate_driver_list.py

http://paste.openstack.org/show/744690/

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The output looks amazing!!
With that mechanism in place cinderlib will be able to return not only name, description, and type, but also default value and choices for each configuration option!
Which is going to be a game changer for cinderlib, as it will help get seamless integrations in other systems and also assist developers.

So we'll just need to convert driver_options to something we can serialize, so we could replace the code here:

for driver in mapping.values():
    driver.pop('cls', None)

With:

for driver in mapping.values():
    driver.pop('cls', None)
    options = driver.get('driver_options')
    if options:
        driver['driver_options'] = {
            opt.name: {'help': opt.help, 'type': '%s' % opt.type,
                              'default': opt.default, 'choices': opt.type.choices}
            for opt in options
        }

Or something like that, since I don't know if the default and choices are right.

@property
def pool_names(self):
return self._pool_names
Expand Down