Nexus_Switch config parsing incompatible with newer oslo.config versions

Bug #1196084 reported by Dirk Mueller on 2013-06-29
20
This bug affects 2 people
Affects Status Importance Assigned to Milestone
neutron
High
Dirk Mueller
Grizzly
High
Unassigned
oslo-incubator
Undecided
Unassigned

Bug Description

quantum/plugins/cisco/common/config.py iterates over all config files with config groups starting with [NEXUS_PLUGIN]. This was apparently only possible by using nonstandard API:

        for parsed_file in cfg.CONF._cparser.parsed:
            for parsed_item in parsed_file.keys():
                nexus_name, sep, nexus_ip = parsed_item.partition(':')
                if nexus_name == 'NEXUS_SWITCH':
                    for nexus_key, value in parsed_file[parsed_item].items():

Recently, _cparser has been renamed/removed as the implementation in oslo.config was changed by Mark. since then this code fails.

Is there a public API for iterating over all found groups in configs? I was only able to find a way to iterate over pre-registered config groups, which does not work here (as the code does not know the exact config group name yet. The code is designed to look for all groups in the form of [NEXUS_PLUGIN:<ip>].

Changed in neutron:
status: New → Incomplete
Dirk Mueller (dmllr) wrote :

This was caused by commit f083d7cfcbc222720fd479a1dabe08d7ae7ed044:

Author: Mark McLoughlin <email address hidden>
Date: Fri May 17 00:31:22 2013 +0100

Parse config files in an argparse callback

Part of fixing bug #1176817

where self._cparser was renamed to self._namespace:

- self._cparser = None
+ self._namespace = None
         self._cli_values = {}

Mark, could you explain why the bug was marked as incomplete?

Changed in neutron:
status: Incomplete → New
Mark McLoughlin (markmc) wrote :

I think it's pretty clear Quantum should never have used this internal API, so marking as Invalid in oslo

Changed in oslo:
status: New → Invalid
Mark McLoughlin (markmc) wrote :

Look at the way register_deprecated() works in the nicira plugin to get an idea of how to implement this somewhat sanely

Basically, the only reason you want to parse these files directly is to get the list of [NEXUS_SWITCH:<ip>] sections

Once you've got the list of IPs, you can do e.g.

  nexus_opts = [
     cfg.StrOpt('username'),
    ...
  ]

  def _read_nexus_switch_config(conf, ip, nexus_dict):
       group_name = 'nexus_switch:' + ip
       conf.register_opts(nexus_opts, group=group_name)
       for key, value in conf[group_name].items():
           nexus_dict[(ip, key)] = value

  def _read_nexus_config(conf):
      multi_parser = cfg.MultiConfigParser()
      read_ok = multi_parser.read(conf.config_file)
      if len(read_ok) != len(conf.config_file):
          raise cfg.Error("Some config files were not parsed properly")

     nexus_dict = {}

      for parsed_file in multi_parser.parsed:
          for section in parsed_file.keys():
              if not section.lower().startswith("nexus_switch:"):
                  continue

              ip = section.split(':', 1)[1]

              _read_nexus_switch_config(conf, ip, nexus_dict):

Dirk Mueller (dmllr) wrote :

Thanks for the explanation! I'll work on it.

Changed in neutron:
assignee: nobody → Dirk Mueller (dmllr)
status: New → In Progress
Kyle Mestery (mestery) wrote :

Dirk and Mark, thanks for triaging this and finding the bug in the Nexus plugin here. If there's anything else you guys need from us on the Cisco side, let me know. We can test out your fix with actual Nexus hardware if you want, for example.

Dirk Mueller (dmllr) wrote :

That would be great. I'll ping you once the review is online in gerrit.

tags: added: cisco
Changed in neutron:
status: In Progress → Triaged
importance: Undecided → Medium
Mark McLoughlin (markmc) wrote :

Dirk - any update? We really could do with this backported to stable/grizzly

Changed in neutron:
importance: Medium → High
Kyle Mestery (mestery) wrote :

Dirk, if you are swamped, we at Cisco are happy to take this over and fix it. Please let us know ASAP.

Dirk Mueller (dmllr) wrote :

I was literally just working on it when I got interrupted by debugging another issue. I'll post a patch by tomorrow morning.

Changed in neutron:
status: Triaged → In Progress
Dirk Mueller (dmllr) wrote :

This changeset seems to work for me (tested with a small test wrapper):

https://review.openstack.org/#/c/34249

Reviewed: https://review.openstack.org/34249
Committed: http://github.com/openstack/neutron/commit/f8e3a5e5877f6099acaa9b8a734df5cb78f3b4ec
Submitter: Jenkins
Branch: master

commit f8e3a5e5877f6099acaa9b8a734df5cb78f3b4ec
Author: Dirk Mueller <email address hidden>
Date: Mon Jun 24 20:56:32 2013 +0200

    Be compatible with oslo.config 1.2.0a3+

    The private API of oslo.config changed, and the Cisco
    options parsing code was using it.
    Use an explicit MultiConfigParser instance for parsing
    the config files.

    Fixes LP Bug #1196084

    Change-Id: I7ffcac3c295491fe9ba8abc7e98f33157a48c51b

Changed in neutron:
status: In Progress → Fix Committed
Dirk Mueller (dmllr) wrote :

This code doesn't exist in grizzly, so marking as invalid there. there is however similar code in the Nicira plugin. I'll file a separate bugreport for this.

Thierry Carrez (ttx) on 2013-07-17
Changed in neutron:
milestone: none → havana-2
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-10-17
Changed in neutron:
milestone: havana-2 → 2013.2
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers