netapp, nfs, storwize and rbd plugins register opts only after instantiation

Bug #1179159 reported by Tom Fifield on 2013-05-12
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Medium
John Griffith

Bug Description

Hi,

In most plugins, configuration options are defined prior to the class, then registered directly below the definition, eg:

volume_opts = [
    cfg.StrOpt('volume_group',
               default='cinder-volumes',
               help='Name for the VG that will contain exported volumes'),
....
    cfg.IntOpt('lvm_mirrors',
               default=0,
               help='If set, create lvms with multiple mirrors. Note that '
                    'this requires lvm_mirrors + 2 pvs with available space'),
]

FLAGS = flags.FLAGS
FLAGS.register_opts(volume_opts)

class LVMVolumeDriver(driver.VolumeDriver):
...

This practice of registering opts seems to be common across other OpenStack products using oslo.config

However for netapp, nfs, storwize and rbd, the options are registered on instantiation of the class, using self.configuration.append_config_values().

I am not a cinder developer, and there may be good reason for this different way of doing things. However, I find the options being registered on import to be quite convenient - and am actually relying on this practice to automatically generate configuration documentation ;)

Thoughts?

John Griffith (john-griffith) wrote :

We actually need to switch over to using cfg everywhere properly anyway and at the same time should fix this back up. We started this a while back (using configuration.append in __init__); and I'm not sure our logic on why we needed to do it was actually justified.

Changed in cinder:
status: New → Confirmed
importance: Undecided → Medium
Changed in cinder:
assignee: nobody → John Griffith (john-griffith)

Fix proposed to branch: master
Review: https://review.openstack.org/31134

Changed in cinder:
status: Confirmed → In Progress

Reviewed: https://review.openstack.org/31134
Committed: http://github.com/openstack/cinder/commit/0b553dcaf8b158872c472fc0dbd9d46d2fc27d25
Submitter: Jenkins
Branch: master

commit 0b553dcaf8b158872c472fc0dbd9d46d2fc27d25
Author: John Griffith <email address hidden>
Date: Thu May 30 16:05:45 2013 -0600

    Fix config registration in cinder volume drivers.

    The config documentation relies on options being registered
    on a modules import. Our need to move the drivers to using
    self.configuration for multi-backend support means that options
    wouldn't be loaded until object initialization which breaks
    documentation.

    This patch puts a dummy CONF init/load back in the drivers. While putting
    this change together I came across a number of drivers still using FLAGS,
    and even worse a number of drivers using a mixture of FLAGS and CONF and
    self.configuraiton. So most of those are cleaned up here as well.

    Note there are two drivers that were not updated at all here:
      1. windows.py
      2. zadara.py

    The zadara folks have indicated that they're in the process of updating and
    releasing a new version of their driver so I left that as is.

    The windows driver needs a bit of work to switch over.

    Fixes bug: 1179159

    Change-Id: I90165299bf080da17741d027e36e361540da0ff8

Changed in cinder:
status: In Progress → Fix Committed
Thierry Carrez (ttx) on 2013-07-17
Changed in cinder:
milestone: none → havana-2
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-10-17
Changed in cinder:
milestone: havana-2 → 2013.2
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers