Merge "ignore location when comparing options for duplicate registration"

oslo_config/cfg.py

@@ -1031,11 +1031,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

@@ -167,3 +167,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))