Config filter used for lockutils makes it impossible to use string substitution

Bug #1399897 reported by Ihar Hrachyshka
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
oslo.concurrency
Fix Released
High
Davanum Srinivas (DIMS)

Bug Description

For lockutils config options, we use ConfigFilter from oslo.config that forbids access to config options that were not explicitly registered on it. This makes it impossible to use string substitution ($something) in those config option values.

In Neutron, we generally specify lock_path as '$state_path/lock'. When we try to migrate to oslo.concurrency, it no longer works. Changing lockutils config object back to cfg.CONF fixes the problem.

It seems that the whole idea of ConfigFilter is not mature, and the implementation hasn't attempted to support string substitutions. Users should not be forced into knowing whether a specific option is defined thru ConfigFilter or not, and hence whether they are able to use substitutions for the option. Since substitution is a general feature of config files in Openstack, I doubt that ConfigFilter is applicable anywhere at all.

The traceback we see in Neutron L2 (openvswitch) agent is:

2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent Traceback (most recent call last):
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/opt/stack/new/neutron/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py", line 1425, in rpc_loop
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent ovs_restarted)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/opt/stack/new/neutron/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py", line 1221, in process_network_ports
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent port_info.get('updated', set()))
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/opt/stack/new/neutron/neutron/agent/securitygroups_rpc.py", line 341, in setup_port_filters
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent self.prepare_devices_filter(new_devices)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/opt/stack/new/neutron/neutron/agent/securitygroups_rpc.py", line 200, in decorated_function
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent return func(self, *args, **kwargs)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/opt/stack/new/neutron/neutron/agent/securitygroups_rpc.py", line 225, in prepare_devices_filter
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent security_groups, security_group_member_ips)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/lib/python2.7/contextlib.py", line 24, in __exit__
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent self.gen.next()
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/opt/stack/new/neutron/neutron/agent/firewall.py", line 106, in defer_apply
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent self.filter_defer_apply_off()
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/opt/stack/new/neutron/neutron/agent/linux/iptables_firewall.py", line 593, in filter_defer_apply_off
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent self.iptables.defer_apply_off()
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/opt/stack/new/neutron/neutron/agent/linux/iptables_manager.py", line 377, in defer_apply_off
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent self._apply()
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/opt/stack/new/neutron/neutron/agent/linux/iptables_manager.py", line 391, in _apply
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent with lockutils.lock(lock_name, utils.SYNCHRONIZED_PREFIX, True):
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/lib/python2.7/contextlib.py", line 17, in __enter__
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent return self.gen.next()
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo_concurrency/lockutils.py", line 380, in lock
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent ext_lock = external_lock(name, lock_file_prefix, lock_path)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo_concurrency/lockutils.py", line 317, in external_lock
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent lock_file_path = _get_lock_path(name, lock_file_prefix, lock_path)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo_concurrency/lockutils.py", line 308, in _get_lock_path
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent local_lock_path = lock_path or CONF.oslo_concurrency.lock_path
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo/config/cfg.py", line 2361, in __getattr__
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent return self._conf._get(name, self._group)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo/config/cfg.py", line 2091, in _get
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent value = self._do_get(name, group, namespace)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo/config/cfg.py", line 2128, in _do_get
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent return convert(opt._get_from_namespace(namespace, group_name))
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo/config/cfg.py", line 2123, in convert
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent self._substitute(value, group, namespace), opt)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo/config/cfg.py", line 2174, in _substitute
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent self.StrSubWrapper(self, group=group, namespace=namespace))
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/lib/python2.7/string.py", line 205, in safe_substitute
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent return self.pattern.sub(convert, self.template)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/lib/python2.7/string.py", line 190, in convert
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent return '%s' % (mapping[named],)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo/config/cfg.py", line 2441, in __getitem__
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent value = self.conf._get(key, namespace=self.namespace)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo/config/cfg.py", line 2091, in _get
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent value = self._do_get(name, group, namespace)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo/config/cfg.py", line 2109, in _do_get
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent info = self._get_opt_info(name, group)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent File "/usr/local/lib/python2.7/dist-packages/oslo/config/cfg.py", line 2237, in _get_opt_info
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent raise NoSuchOptError(opt_name, group)
2014-12-05 20:34:18.081 1009 TRACE neutron.plugins.openvswitch.agent.ovs_neutron_agent NoSuchOptError: no such option: state_path

Changed in oslo.concurrency:
assignee: nobody → Davanum Srinivas (DIMS) (dims-v)
Revision history for this message
Davanum Srinivas (DIMS) (dims-v) wrote :

Here's a test case to recreate the problem

http://paste.openstack.org/show/147086/

Revision history for this message
Doug Hellmann (doug-hellmann) wrote :

We may need to make lookup of a value for interpolation a special case, since presumably the user would have used the name and not a developer specifying a default.

In the mean time, oslo_concurrency.lockutils.set_defaults() can be used to set a default value for the lock path. Can neutron's tests use that API and pass a full path?

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to oslo.config (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/140143

Revision history for this message
Doug Hellmann (doug-hellmann) wrote :
Changed in oslo.concurrency:
importance: Undecided → High
status: New → Confirmed
no longer affects: oslo.config
Revision history for this message
Ihar Hrachyshka (ihar-hrachyshka) wrote :

@Doug, it's not just about unit tests. The same is true for general neutron.conf used in production. It should be solved on oslo.concurrency (or oslo.config) side, neutron's way of using the library is correct.

Ben Nemec (bnemec)
Changed in oslo.concurrency:
milestone: none → next-kilo
Revision history for this message
Doug Hellmann (doug-hellmann) wrote :
Changed in oslo.concurrency:
status: Confirmed → Fix Committed
Changed in oslo.concurrency:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.