Re: [openstack-dev] [neutron][oslo] Mitaka neutron-*aas are broken when --config-dir is passed

2016-05-26 Thread Ihar Hrachyshka

> On 25 May 2016, at 18:47, Armando M.  wrote:
> 
> 
> 
> On 25 May 2016 at 09:02, Brandon Logan  wrote:
> +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.
> 
> If my memory doesn't fail me, that code is still around for a couple of 
> reasons, and one being documented in [1] (the other one I think was because 
> of the devstack's way of configuring *-aas). My suggestion would be to 
> thoroughly think what the implications are before we let it up in flames. 
> Things might have changed since the last time this code was touched, but one 
> never knows.
> 
> [1] https://bugs.launchpad.net/neutron/+bug/1492069

Thanks for the link. So as far as I understand the use case, it’s to allow an 
external service plugin to drop neutron_.conf file into 
/etc/neutron and expect it to load service providers definition from there, 
without the need to change neutron-server CLI. Right?

If that’s the case, it may be achieved with oslo.config --config-dir option, 
where you would create a directory somewhere under /etc/neutron and drop all 
your additional configuration files there (or symlink them from there), and 
then pass the directory name thru --config-dir, so that oslo.config would load 
all *.conf files from the directory. In that way, you have your CLI stable for 
neutron-server process, while the list of configuration files to load 
service_providers sections from is dynamic and depends on subprojects installed.

Actually, that’s exactly how we handle additional configuration files in RDO 
where we create /etc/neutron/conf.d/ directories per service and 
make sure that each service loads configuration files from there. F.e. that’s 
CLI we use for neutron-server:

https://github.com/openstack-packages/neutron/blob/rpm-master/neutron-server.service#L8

/usr/bin/neutron-server […]  --config-dir /etc/neutron/conf.d/neutron-server […]

So when you want to make neutron-server to load an additional config file for 
you, you just drop your .conf file there.

I would prefer neutron upstream comes up with a standard directory layout for 
exactly that need, and document it as a deployment requirement, so that all 
subprojects may rely on the directory being created and loaded by the 
controller. Of course, I would prefer if it reflects what RDO already does, so 
that we don’t need to make the change. :)

As for devstack, yes indeed we load neutron_*aas.conf files there relying on 
the magic code. This should not stop us from deprecating and eventually 
removing the code though. I am happy to push the needed devstack patches if we 
agree that’s the end goal to have pure oslo.config configuration loading.

Ihar
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [neutron][oslo] Mitaka neutron-*aas are broken when --config-dir is passed

2016-05-25 Thread Armando M.
On 25 May 2016 at 09:02, Brandon Logan  wrote:

> +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.
>

If my memory doesn't fail me, that code is still around for a couple of
reasons, and one being documented in [1] (the other one I think was because
of the devstack's way of configuring *-aas). My suggestion would be to
thoroughly think what the implications are before we let it up in flames.
Things might have changed since the last time this code was touched, but
one never knows.

[1] https://bugs.launchpad.net/neutron/+bug/1492069


>
> 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.
> > >
> 

Re: [openstack-dev] [neutron][oslo] Mitaka neutron-*aas are broken when --config-dir is passed

2016-05-25 Thread Doug Hellmann
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 

[openstack-dev] [neutron][oslo] Mitaka neutron-*aas are broken when --config-dir is passed

2016-05-25 Thread Ihar Hrachyshka
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.]

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. (?)

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.

Thoughts?

Ihar