Open Stack

+1

This sounds like a sane plane.  That magical config load caused me some
problems in the past when I didn't know about it, would be glad to see
it go.  I thought it being deprecated and removed was planned anyway,
and honestly didn't think it was still in the code base because I hadn't
run into any issues recently.

Thanks,
Brandon

On Wed, 2016-05-25 at 10:56 -0400, Doug Hellmann wrote:
> Excerpts from Ihar Hrachyshka's message of 2016-05-25 14:03:24 +0200:
> > Hi all,
> > 
> > Our internal Mitaka testing revealed that neutron-server fails to start when:
> > - any neutron-*aas service plugin is enabled (in our particular case, it was lbaas);
> > - --config-dir option is passed to the process via CLI.
> > 
> > Since RDO/OSP neutron-server systemd unit files use --config-dir options extensively, it renders all neutron-*aas broken as of Mitaka for us.
> > 
> > The failure is reported as: https://launchpad.net/bugs/1585102 and the traceback can be found in: http://paste.openstack.org/show/498502/
> > 
> > As you can see, it crashes in provider_configuration neutron module, where we have a custom parser for service_providers configuration:
> > 
> > https://github.com/openstack/neutron/blob/master/neutron/services/provider_configuration.py#L83
> > 
> > This code was introduced in Kilo when neutron-*aas were split of the tree. The intent of the code at the time was to allow service plugins to load neutron_*aas.conf files located in /etc/neutron/ that are not passed explicitly to neutron-server via --config-file options. [A decision that was, in my opinion, wrong in the first place: we should not have introduced ‘magic’ in neutron that allowed the controller to load configuration files implicitly, and we would be better off just relying on oslo.config facilities, like using --config-dir to load an ‘unknown’ set of configuration files.]
> 
> +1
> 
> > 
> > The failure was triggered by oslo.config 3.8.0 release that is part of Mitaka series, particularly by the following patch: https://review.openstack.org/#q,Ibd0566f11df62da031afb128c9687c5e8c7b27ae,n,z This patch, among other things, changed the type of ‘config_dir’ option from string to list [of strings]. Since configuration options are not considered part of public API, we can’t claim that oslo.config broke their API guarantees and revert the patch. [Even if that would be the case, we could not do it because we already released several Mitaka and Newton releases of the library with the patch included, so it’s probably late to switch back.]
> > 
> > I have proposed a fix for provider_configuration module that would adopt the new list type for the option: https://review.openstack.org/#/c/320304/ Actually, it does not even rely on the option anymore, instead it pulls values using config_dirs property defined on ConfigOpts objects, which I assume is part of public API.
> > 
> > Since Mitaka supports anything oslo.config >= 3.7.0, we would also need to support the older type in some graceful way, if we backport the fix there.
> > 
> > Doug Hellmann has concerns about the approach taken. In his own words, "This approach may solve the problem in the short term, but it's going to leave you with some headaches later in this cycle when we expand oslo.config.” Specifically, "There are plans under way to expand configuration sources from just files and directories to use URLs. I expect some existing options to be renamed or otherwise deprecated as part of that work, and using the option value here will break neutron when that happens.” (more details in the patch)
> 
> > 
> > First, it’s a surprise to me that config_dirs property (not an option) is not part of public API of the library. I thought that if something is private, we name it with a leading underscore. (?)
> 
> I was conflating "config_dirs" with the --config-dir option value,
> which as you point out is incorrect.  Sorry for the confusion.
> 
> > 
> > If we don’t have public access to the symbol, a question arises on how we tackle that in neutron/mitaka (!). Note that we are not talking about a next release, it’s current neutron/mitaka that is broken and should be fixed to work with oslo.config 3.8.0, so any follow up work in oslo.config itself won’t make it to stable/mitaka for the library. So we need some short term solution here.
> > 
> > Doug suggested that neutron team would work with oslo folks to expose missing bits from oslo.config to consumers: "There are several ways we could address the need here. For example, we could provide a method that returns the source info (file names, directories, etc.). We could add a class method that has the effect of making a new ConfigOpts instance with the same source information as an existing object passed to it. Or we could split the config locating logic out of ConfigOpts and make it a separate object that can be shared. We should discuss those options on the ML, so please start a thread.”
> > 
> > It may be a good idea, but honestly, I don’t want to see neutron following the path we took back in kilo. I would prefer seeing neutron getting rid of implicit loading of specifically named configuration files for service plugins (and just for a single option!)
> > 
> > My plan to get out of those woods would be:
> > - short term, we proceed on the direction I took with the patch, adopting list type in newton, and gracefully handling both in mitaka;
> > - long term, deprecate (Newton) and remove (Ocata) the whole special casing code for service providers from neutron. Any configuration files to load for service plugins or any other plugin would need to be specified on CLI with either --config-file or --config-dir. No more magic.
> 
> +1
> 
> As I pointed out in the review, there is likely to be some work
> happening this cycle to support fetching config files from URLs,
> not just files on the local filesystem (I say "likely" because it
> depends on me finding time or another volunteer to implement it).
> Relying on any magic to find the configuration files separately
> from the arguments given to the main application is going to break
> when deployers start using URLs.
> 
> I thought there was some sort of hard requirement leading to using
> separate files, which is why I was proposing ways to continue
> supporting that. It seems that's not the case, which is good.
> Really, a key aspect of using oslo.config is that application code
> should not be worrying about where the configuration settings come
> from. Stop thinking about files, because that's an implementation
> detail that may go away.
> 
> So I recommend that you adopt Ihar's suggestion and have plugins
> register their options with the same config object that neutron
> uses, since that object will already know where the config should
> come from. Besides eliminating the magic, that approach has the
> added benefit of not forcing deployers to use separate config sources
> for plugins if they don't want to do that.
> 
> Doug
> 
> > 
> > Thoughts?
> > 
> > Ihar
> 
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev