ignore location when comparing options for duplicate registration

Before this change, two options that were identical except for the
location in the code where the Opt class was instantiated or how they
were initialized (the default, set_override, etc.) would compare as
not being equal. The result is that consumers of oslo.config that
include multiple definitions of the same option but in different files
would report an error for a duplicate option. The error may also be
triggered by using set_override() and re-registering an option under
some circumstances.

This patch ensures that the location where the option's value is set
does not factor into the equality check for the option.

Change-Id: I0c57f92c28f220009e01cfe641ec14039c51f671
Signed-off-by: Doug Hellmann <doug@doughellmann.com>

oslo_config/cfg.py

@@ -1030,11 +1030,27 @@ class Opt(object):
                                         % {'default': self.default,
                                            'opt': self.type})
 
+    def _vars_for_cmp(self):
+        # NOTE(dhellmann): Get the instance variables of this Opt and
+        # then make a new dictionary so we can modify the contents
+        # before returning it without removing any attributes of the
+        # object.
+        v = dict(vars(self))
+
+        # NOTE(dhellmann): Ignore the location where the option is
+        # defined when comparing them. Ideally we could use this to
+        # detect duplicate settings in code bases, but as long as the
+        # options match otherwise they should be safe.
+        if '_set_location' in v:
+            del v['_set_location']
+
+        return v
+
     def __ne__(self, another):
-        return vars(self) != vars(another)
+        return self._vars_for_cmp() != another._vars_for_cmp()
 
     def __eq__(self, another):
-        return vars(self) == vars(another)
+        return self._vars_for_cmp() == another._vars_for_cmp()
 
     __hash__ = object.__hash__
 

oslo_config/tests/test_get_location.py

@@ -163,3 +163,23 @@ class GetLocationTestCase(base.BaseTestCase):
             filename,
             loc.detail,
         )
+
+    def test_duplicate_registration(self):
+        # NOTE(dhellmann): The ZFS driver in Cinder uses multiple Opt
+        # instances with the same settings but in different
+        # modules. We don't want to break that case with the option
+        # location stuff, so we need to test that it works. To
+        # reproduce that test we have to simulate the option being
+        # created in a different file, so we create a new Opt instance
+        # and replace its _set_location data.
+        dupe_opt = cfg.StrOpt(
+            'normal_opt',
+            default='normal_opt_default',
+        )
+        dupe_opt._set_location = cfg.LocationInfo(
+            cfg.Locations.opt_default,
+            'an alternative file',
+        )
+        # We expect register_opt() to return False to indicate that
+        # the option was already registered.
+        self.assertFalse(self.conf.register_opt(dupe_opt))