Open Stack

Hi Doug,

On Mon, 2014-12-08 at 15:58 -0500, Doug Hellmann wrote:
> As we’ve discussed a few times, we want to isolate applications from
> the configuration options defined by libraries. One way we have of
> doing that is the ConfigFilter class in oslo.config. When a regular
> ConfigOpts instance is wrapped with a filter, a library can register
> new options on the filter that are not visible to anything that
> doesn’t have the filter object.

Or to put it more simply, the configuration options registered by the
library should not be part of the public API of the library.

>  Unfortunately, the Neutron team has identified an issue with this
> approach. We have a bug report [1] from them about the way we’re using
> config filters in oslo.concurrency specifically, but the issue applies
> to their use everywhere. 
> 
> The neutron tests set the default for oslo.concurrency’s lock_path
> variable to “$state_path/lock”, and the state_path option is defined
> in their application. With the filter in place, interpolation of
> $state_path to generate the lock_path value fails because state_path
> is not known to the ConfigFilter instance.

It seems that Neutron sets this default in its etc/neutron.conf file in
its git tree:

  lock_path = $state_path/lock

I think we should be aiming for defaults like this to be set in code,
and for the sample config files to contain nothing but comments. So,
neutron should do:

  lockutils.set_defaults(lock_path="$state_path/lock")

That's a side detail, however.

> The reverse would also happen (if the value of state_path was somehow
> defined to depend on lock_path),

This dependency wouldn't/shouldn't be code - because Neutron *code*
shouldn't know about the existence of library config options.
Neutron deployers absolutely will be aware of lock_path however.

>  and that’s actually a bigger concern to me. A deployer should be able
> to use interpolation anywhere, and not worry about whether the options
> are in parts of the code that can see each other. The values are all
> in one file, as far as they know, and so interpolation should “just
> work”.

Yes, if a deployer looks at a sample configuration file, all options
listed in there seem like they're in-play for substitution use within
the value of another option. For string substitution only, I'd say there
should be a global namespace where all options are registered.

Now ... one caveat on all of this ... I do think the string substitution
feature is pretty obscure and mostly just used in default values.

> I see a few solutions:
> 
> 1. Don’t use the config filter at all.
> 2. Make the config filter able to add new options and still see
> everything else that is already defined (only filter in one
> direction).
> 3. Leave things as they are, and make the error message better.

4. Just tackle this specific case by making lock_path implicitly
relative to a base path the application can set via an API, so Neutron
would do:

  lockutils.set_base_path(CONF.state_path)

at startup.

5. Make the toplevel ConfigOpts aware of all filters hanging off it, and
somehow cycle through all of those filters just when doing string
substitution.

> Because of the deployment implications of using the filter, I’m
> inclined to go with choice 1 or 2. However, choice 2 leaves open the
> possibility of a deployer wanting to use the value of an option
> defined by one filtered set of code when defining another. I don’t
> know how frequently that might come up, but it seems like the error
> would be very confusing, especially if both options are set in the
> same config file.
> 
> I think that leaves option 1, which means our plans for hiding options
> from applications need to be rethought.
> 
> Does anyone else see another solution that I’m missing?

I'd do something like (3) and (4), then wait to see if it crops up
multiple times in the future before tackling a more general solution.

With option (1), the basic thing to think about is how to maintain API
compatibility - if we expose the options through the API, how do we deal
with future moves, removals, renames, and changing semantics of those
config options.

Mark.