Open Stack

On Fri, Jul 24, 2015 at 09:56:41AM -0400, Doug Hellmann wrote:
> Excerpts from Daniel P. Berrange's message of 2015-07-24 09:48:15 +0100:
> > On Thu, Jul 23, 2015 at 05:55:36PM +0300, mhorban wrote:
> > > Hi all,
> > > 
> > > During development process in nova I faced with an issue related with config
> > > options. Now we have lists of config options and registering options mixed
> > > with source code in regular files.
> > > From one side it can be convenient: to have module-encapsulated config
> > > options. But problems appear when we need to use some config option in
> > > different modules/packages.
> > > 
> > > If some option is registered in module X and module X imports module Y for
> > > some reasons...
> > > and in one day we need to import this option in module Y we will get
> > > exception
> > > NoSuchOptError on import_opt in module Y.
> > > Because of circular dependency.
> > > To resolve it we can move registering of this option in Y module(in the
> > > inappropriate place) or use other tricks.
> > > 
> > > I offer to create file options.py in each package and move all package's
> > > config options and registration code there.
> > > Such approach allows us to import any option in any place of nova without
> > > problems.
> > > 
> > > Implementations of this refactoring can be done piece by piece where piece
> > > is
> > > one package.
> > > 
> > > What is your opinion about this idea?
> > 
> > I tend to think that focusing on problems with dependancy ordering when
> > modules import each others config options is merely attacking a symptom
> > of the real root cause problem.
> > 
> > The way we use config options is really entirely wrong. We have gone
> > to the trouble of creating (or trying to create) structured code with
> > isolated functional areas, files and object classes, and then we throw
> > in these config options which are essentially global variables which are
> > allowed to be accessed by any code anywhere. This destroys the isolation
> > of the various classes we've created, and means their behaviour often
> > based on side effects of config options from unrelated pieces of code.
> > It is total madness in terms of good design practices to have such use
> > of global variables.
> > 
> > So IMHO, if we want to fix the real big problem with config options, we
> > need to be looking to solution where we stop using config options as
> > global variables. We should change our various classes so that the
> > neccessary configurable options as passed into object constructors
> > and/or methods as parameters.
> 
> We've tried to do this in a lot of places throughout Oslo. It mostly
> works, until you hit a driver that uses options that the other drivers
> don't (oslo.messaging has this problem especially).
> 
> We had a ConfigFilter class in oslo.config to enforce the isolation (an
> option registered on a filter object is only visible to the code that
> owns the filter). However, that causes challenges for value
> interpolation. A deployer doesn't know which options are visible to each
> other, and has a flat file where they can clearly see all of the
> values together, so they expect %(foo)s to work no matter where "foo" is
> defined. Until we solve those issues, the ConfigFilter isn't really
> supported.
> 
> The best solution we've come up with so far is to isolate the options
> into groups based on the driver name, and then not allow code outside of
> the driver to access those options for any reason.
> 
> To deal with import ordering issues, we pass ConfigObj instance
> around, and register options at runtime instead of import time,
> usually in an __init__ method.  Registering an option more than
> once is fine.

Yep, I would imagine that the nova/cmd/*.py command line entry
points would trigger loading of the config file and pass the
object into the service they run. The service may pass that
config object on down to some classes, eg I'd expect the nova
ComputeDriver to accept a config object in its constructor.
Likewise the various manager classes like nova/compute/manager.py
These would read the various config option values they care about
and pass those into the various methods / objects that need them

> > As an example in the libvirt driver.
> > 
> > I would set it up so that /only/ the LibvirtDriver class in driver.py
> > was allowed to access the CONF config options. In its constructor it
> > would load all the various config options it needs, and either set
> > class attributes for them, or pass them into other methods it calls.
> > So in the driver.py, instead of calling CONF.libvirt.libvirt_migration_uri
> > everywhere in the code,  in the constructor we'd save that config param
> > value to an attribute 'self.mig_uri = CONF.libvirt.libvirt_migration_uri'
> > and then where needed, we'd just call "self.mig_uri".
> 
> There are, from time to time, specs proposed to provide ways for
> applications to reload their configuration settings without restarting.
> So far we've said that was an application issue, because oslo.config
> can reload the files, but it doesn't know when (that's an app signal
> handling thing) and it doesn't know where values might have been
> saved like you describe.  I don't know if that's something the nova
> team wants to support.

I don't think you can magically reload stuff on the fly regardless
of how we store or access the configuration settings, and in fact
I think our current global variable approach makes it even harder
to deal with that too.

It is not uncommon to read a config parameter and use that to
configure some state somewhere. The CONF object is magically
updated to have the new config value when the user editted the
file. Now we have some code reading CONF.foo directly and acting
on it, and other code using the previously setup state based on
the old CONF.foo value. This will certainly crash & burn in a
horrible way.

To be able to support live updates of config parameters, we need
to make the code using those parameters aware of it and ensure the
change takes effect everywhere that's needed at the same time.
If the config options are merely used to set attributes on classes
or passed in as method parameters, we can clearly understand the
implications of changing the config value on the fly, and call a
suitable method to change it. This is really all just standard
good design practice for object oriented programming and why mixing
in use of global variables with OO design is a horrible idea in
general.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|