+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: [email protected]?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
