Optionally use oslo.log for deprecated opt logging

While we can't add a hard dependency on oslo.log because it uses
oslo.config, in most cases oslo.log will be installed anyway.  In
the interest of being able to make use of features like
fatal_deprecations in oslo.log, let's use it if it's available.

Change-Id: If9499aa6fc28a6b92447b3825d3ca1957cb2255a

5 files changed, 91 insertions, 17 deletions

diff --git a/lower-constraints.txt b/lower-constraints.txt
index e1eff61..317bd66 100644
--- a/lower-constraints.txt
+++ b/lower-constraints.txt
@@ -26,6 +26,7 @@ netaddr==0.7.18
 openstackdocstheme==1.18.1
 os-client-config==1.28.0
 oslo.i18n==3.15.3
+oslo.log==3.36.0
 oslotest==3.2.0
 pbr==2.0.0
 pep8==1.5.7
diff --git a/oslo_config/cfg.py b/oslo_config/cfg.py
index cf9a3e9..978fd51 100644
--- a/oslo_config/cfg.py
+++ b/oslo_config/cfg.py
@@ -498,6 +498,13 @@ import sys
 
 import enum
 import six
+# NOTE(bnemec): oslo.log depends on oslo.config, so we can't
+# have a hard dependency on oslo.log.  However, in most cases
+# oslo.log will be installed so we can use it.
+try:
+    import oslo_log
+except ImportError:
+    oslo_log = None
 
 from oslo_config import iniparser
 from oslo_config import types
@@ -847,6 +854,27 @@ def _normalize_group_name(group_name):
     return group_name.lower()
 
 
+def _report_deprecation(format_str, format_dict):
+    """Report use of a deprecated option
+
+    Uses versionutils from oslo.log if it is available.  If not, logs
+    a simple warning message.
+
+    :param format_str: The message to use for the report
+    :param format_dict: A dict containing keys for any parameters in format_str
+    """
+    if oslo_log:
+        # We can't import versionutils at the module level because of circular
+        # imports.  Importing just oslo_log at the module level and
+        # versionutils locally allows us to unit test this and still avoid the
+        # circular problem.
+        from oslo_log import versionutils
+        versionutils.report_deprecated_feature(LOG, format_str,
+                                               format_dict)
+    else:
+        LOG.warning(format_str, format_dict)
+
+
 @functools.total_ordering
 class Opt(object):
 
@@ -1085,12 +1113,13 @@ class Opt(object):
                 pretty_reason = ' ({})'.format(self.deprecated_reason)
             else:
                 pretty_reason = ''
-            LOG.warning('Option "%(option)s" from group "%(group)s" is '
-                        'deprecated for removal%(reason)s.  Its value may be '
-                        'silently ignored in the future.',
-                        {'option': self.dest,
-                         'group': pretty_group,
-                         'reason': pretty_reason})
+            format_str = ('Option "%(option)s" from group "%(group)s" is '
+                          'deprecated for removal%(reason)s.  Its value may '
+                          'be silently ignored in the future.')
+            format_dict = {'option': self.dest,
+                           'group': pretty_group,
+                           'reason': pretty_reason}
+            _report_deprecation(format_str, format_dict)
         return (value, loc)
 
     def _add_to_cli(self, parser, group=None):
@@ -2213,12 +2242,9 @@ class _Namespace(argparse.Namespace):
         if name in deprecated and name not in self._emitted_deprecations:
             self._emitted_deprecations.add(name)
             current = (current[0] or 'DEFAULT', current[1])
-            # NOTE(bnemec): Not using versionutils for this to avoid a
-            # circular dependency between oslo.config and whatever library
-            # versionutils ends up in.
-            LOG.warning(self._deprecated_opt_message,
-                        {'dep_option': name[1], 'dep_group': name[0],
-                         'option': current[1], 'group': current[0]})
+            format_dict = {'dep_option': name[1], 'dep_group': name[0],
+                           'option': current[1], 'group': current[0]}
+            _report_deprecation(self._deprecated_opt_message, format_dict)
 
     def _get_value(self, names, multi=False, positional=False,
                    current_name=None, normalized=True):
diff --git a/oslo_config/tests/test_cfg.py b/oslo_config/tests/test_cfg.py
index 22483a5..9b2cd4a 100644
--- a/oslo_config/tests/test_cfg.py
+++ b/oslo_config/tests/test_cfg.py
@@ -964,6 +964,15 @@ class PositionalTestCase(BaseTestCase):
         self.assertEqual('arg1', self.conf.arg1)
 
 
+# The real report_deprecated_feature has caching that causes races in our
+# unit tests.  This replicates the relevant functionality.
+def _fake_deprecated_feature(logger, msg, *args, **kwargs):
+    stdmsg = 'Deprecated: %s' % msg
+    logger.warning(stdmsg, *args, **kwargs)
+
+
+@mock.patch('oslo_log.versionutils.report_deprecated_feature',
+            _fake_deprecated_feature)
 class ConfigFileOptsTestCase(BaseTestCase):
 
     def setUp(self):
@@ -4834,6 +4843,8 @@ class DeprecationWarningTestBase(BaseTestCase):
         self._parser_class = cfg.ConfigParser
 
 
+@mock.patch('oslo_log.versionutils.report_deprecated_feature',
+            _fake_deprecated_feature)
 class DeprecationWarningTestScenarios(DeprecationWarningTestBase):
     scenarios = [('default-deprecated', dict(deprecated=True,
                                              group='DEFAULT')),
@@ -4867,7 +4878,8 @@ class DeprecationWarningTestScenarios(DeprecationWarningTestBase):
             self.assertEqual('baz', self.conf.other.foo)
             self.assertEqual('baz', self.conf.other.foo)
         if self.deprecated:
-            expected = (cfg._Namespace._deprecated_opt_message %
+            expected = ('Deprecated: ' +
+                        cfg._Namespace._deprecated_opt_message %
                         {'dep_option': 'bar',
                          'dep_group': self.group,
                          'option': 'foo',
@@ -4877,7 +4889,11 @@ class DeprecationWarningTestScenarios(DeprecationWarningTestBase):
         self.assertEqual(expected, self.log_fixture.output)
 
 
+@mock.patch('oslo_log.versionutils.report_deprecated_feature',
+            _fake_deprecated_feature)
 class DeprecationWarningTests(DeprecationWarningTestBase):
+    log_prefix = 'Deprecated: '
+
     def test_DeprecatedOpt(self):
         default_deprecated = [cfg.DeprecatedOpt('bar')]
         other_deprecated = [cfg.DeprecatedOpt('baz', group='other')]
@@ -4915,7 +4931,8 @@ class DeprecationWarningTests(DeprecationWarningTestBase):
                      'option': current_name,
                      'group': current_group}
                     )
-        self.assertEqual(expected + '\n', self.log_fixture.output)
+        self.assertEqual(self.log_prefix + expected + '\n',
+                         self.log_fixture.output)
 
     def test_deprecated_for_removal(self):
         self.conf.register_opt(cfg.StrOpt('foo',
@@ -4934,7 +4951,7 @@ class DeprecationWarningTests(DeprecationWarningTestBase):
         expected = ('Option "foo" from group "DEFAULT" is deprecated for '
                     'removal.  Its value may be silently ignored in the '
                     'future.\n')
-        self.assertEqual(expected, self.log_fixture.output)
+        self.assertEqual(self.log_prefix + expected, self.log_fixture.output)
 
     def test_deprecated_for_removal_with_group(self):
         self.conf.register_group(cfg.OptGroup('other'))
@@ -4956,7 +4973,7 @@ class DeprecationWarningTests(DeprecationWarningTestBase):
         expected = ('Option "foo" from group "other" is deprecated for '
                     'removal.  Its value may be silently ignored in the '
                     'future.\n')
-        self.assertEqual(expected, self.log_fixture.output)
+        self.assertEqual(self.log_prefix + expected, self.log_fixture.output)
 
     def test_deprecated_with_dest(self):
         self.conf.register_group(cfg.OptGroup('other'))
@@ -4975,4 +4992,14 @@ class DeprecationWarningTests(DeprecationWarningTestBase):
                      'dep_group': 'other',
                      'option': 'foo-bar',
                      'group': 'other'} + '\n')
-        self.assertEqual(expected, self.log_fixture.output)
+        self.assertEqual(self.log_prefix + expected, self.log_fixture.output)
+
+
+class DeprecationWarningTestsNoOsloLog(DeprecationWarningTests):
+    log_prefix = ''
+
+    def setUp(self):
+        super(DeprecationWarningTestsNoOsloLog, self).setUp()
+        # NOTE(bnemec): For some reason if I apply this as a class decorator
+        # it ends up applying to the parent class too and breaks those tests.
+        self.useFixture(fixtures.MockPatchObject(cfg, 'oslo_log', None))
diff --git a/releasenotes/notes/support-fatal-deprecations-ea0513aa58a395ca.yaml b/releasenotes/notes/support-fatal-deprecations-ea0513aa58a395ca.yaml
new file mode 100644
index 0000000..0073f8a
--- /dev/null
+++ b/releasenotes/notes/support-fatal-deprecations-ea0513aa58a395ca.yaml
@@ -0,0 +1,13 @@
+---
+features:
+  - |
+    oslo.config now supports the fatal-deprecations option from oslo.log.  This
+    behavior is only enabled if oslo.log is installed, but oslo.log is still
+    not a hard requirement to avoid a circular dependency.
+upgrade:
+  - |
+    Because support for fatal-deprecations was added in this release, users who
+    have fatal-deprecations enabled and have deprecated config opts in use
+    (which previously was not a problem because oslo.config didn't respect the
+    fatal-deprecations option) will need to resolve that before upgrading or
+    services may fail to start.
diff --git a/test-requirements.txt b/test-requirements.txt
index 94e75fa..69ec3ee 100644
--- a/test-requirements.txt
+++ b/test-requirements.txt
@@ -9,6 +9,11 @@ testscenarios>=0.4 # Apache-2.0/BSD
 testtools>=2.2.0 # MIT
 oslotest>=3.2.0 # Apache-2.0
 
+# oslo.log can't be a runtime dep because it would cause a circular dependency,
+# but we can optionally make use of it so we want to have it installed in our
+# test environment.
+oslo.log>=3.36.0  # Apache-2.0
+
 # when we can require tox>= 1.4, this can go into tox.ini:
 #  [testenv:cover]
 #  deps = {[testenv]deps} coverage
@@ -22,3 +27,5 @@ mock>=2.0.0 # BSD
 
 # Bandit security code scanner
 bandit>=1.1.0 # Apache-2.0
+
+reno>=2.5.0 # Apache-2.0