diff --git a/doc/source/admin/upgrade-notes.rst b/doc/source/admin/upgrade-notes.rst index 95088a47d..07c1f9ecb 100644 --- a/doc/source/admin/upgrade-notes.rst +++ b/doc/source/admin/upgrade-notes.rst @@ -72,7 +72,9 @@ data and configuration settings from nova. * The placement server side settings in ``nova.conf`` should be moved to a separate placement configuration file ``placement.conf``. * The default configuration value of ``[placement]/policy_file`` is changed - from ``placement-policy.yaml`` to ``policy.yaml`` + from ``placement-policy.yaml`` to ``policy.yaml``. This config option is + changed to :oslo.config:option:`oslo_policy.policy_file` since Train + release. * Several tables in the ``nova_api`` database need to be migrated to a new ``placement`` database. diff --git a/doc/source/admin/upgrade-to-stein.rst b/doc/source/admin/upgrade-to-stein.rst index b0d6ff49a..0fca5d8aa 100644 --- a/doc/source/admin/upgrade-to-stein.rst +++ b/doc/source/admin/upgrade-to-stein.rst @@ -121,7 +121,7 @@ Initial Steps * If it exists, move ``/etc/nova/placement-policy.yaml`` to ``/etc/placement/policy.yaml``. If you wish to use a different filename - adjust :oslo.config:option:`placement.policy_file`. + adjust config option ``[placement] policy_file``. #. Configure the database migration tool. diff --git a/placement/conf/placement.py b/placement/conf/placement.py index 05f1be0ca..c1861d06e 100644 --- a/placement/conf/placement.py +++ b/placement/conf/placement.py @@ -32,26 +32,6 @@ being equal, two requests for allocation candidates will return the same results in the same order; but no guarantees are made as to how that order is determined. """), - # TODO(mriedem): When placement is split out of nova, this should be - # deprecated since then [oslo_policy]/policy_file can be used. - cfg.StrOpt( - 'policy_file', - # This default matches what is in - # etc/nova/policy-generator.conf - default='policy.yaml', - deprecated_for_removal=True, - deprecated_since='2.0.0', - deprecated_reason=""" -This option was necessary when placement was part of nova but can now be -replaced with the more standard usage of ``[oslo_policy]/policy_file`` which -has the same semantics but a different default value. If you have a custom -policy.yaml file and were not setting this option but just relying on the -default value, you need to configure placement to use -``[oslo_policy]/policy_file`` with policy.yaml specifically since otherwise -that option defaults to policy.json. -""", - help='The file that defines placement policies. This can be an ' - 'absolute path or relative to the configuration file.'), cfg.StrOpt( 'incomplete_consumer_project_id', default=DEFAULT_CONSUMER_MISSING_ID, diff --git a/placement/policy.py b/placement/policy.py index 58645d5c4..03a5040ca 100644 --- a/placement/policy.py +++ b/placement/policy.py @@ -37,43 +37,7 @@ def init(conf): """Init an Enforcer class. Sets the _ENFORCER global.""" global _ENFORCER if not _ENFORCER: - # TODO(mriedem): This compat code can be removed when the - # [placement]/policy_file config option is removed. - # First check to see if [oslo_policy]/policy_file exists since that's - # what we want people using. That option defaults to policy.json while - # [placement]/policy_file defaults to policy.yaml so if - # [oslo_policy]/policy_file does not exist it means either someone with - # custom policy has not migrated or they are using defaults in code. - if conf.find_file(conf.oslo_policy.policy_file): - # [oslo_policy]/policy_file exists so use it. - policy_file = conf.oslo_policy.policy_file - # Do a sanity check to see if [placement]/policy_file exists but - # with a different name because if so we could be loading up the - # wrong file. For example, maybe someone's packaging or deployment - # tooling creates an empty policy.json but placement.conf is - # actually configured to use [placement]/policy_file=policy.yaml - # with custom rules. - if (conf.placement.policy_file != conf.oslo_policy.policy_file and - conf.find_file(conf.placement.policy_file)): - LOG.error('Found [oslo_policy]/policy_file and ' - '[placement]/policy_file and not sure which to use. ' - 'Using [oslo_policy]/policy_file since ' - '[placement]/policy_file is deprecated but you need ' - 'to clean up your configuration file to stop using ' - '[placement]/policy_file.') - else: - # Check to see if a custom [placement]/policy_file is being used - # and if so, log a warning to migrate to [oslo_policy]/policy_file. - if conf.find_file(conf.placement.policy_file): - LOG.warning('[placement]/policy_file is deprecated. Use ' - '[oslo_policy]/policy_file instead.') - # For backward compatibility use [placement]/policy_file. Even if - # the file does not exist we can specify this since we will load up - # default rules from code. Once we remove the compat code we can - # just stop passing the policy_file kwarg to Enforcer. - policy_file = conf.placement.policy_file - - _enforcer = policy.Enforcer(conf, policy_file=policy_file) + _enforcer = policy.Enforcer(conf) _enforcer.register_defaults(policies.list_rules()) _enforcer.load_rules() _ENFORCER = _enforcer diff --git a/placement/tests/unit/test_policy.py b/placement/tests/unit/test_policy.py index adf19f741..da522f4b4 100644 --- a/placement/tests/unit/test_policy.py +++ b/placement/tests/unit/test_policy.py @@ -11,7 +11,6 @@ # under the License. import os -from unittest import mock import fixtures from oslo_config import cfg @@ -54,13 +53,6 @@ class PlacementPolicyTestCase(base.ContextTestCase): self.conf_fixture.config( group='oslo_policy', policy_file=tmpfilename) - # We have to create the file before initializing the policy enforcer - # otherwise it falls back to using CONF.placement.policy_file. This - # can be removed when the deprecated CONF.placement.policy_file option - # is removed. - with open(tmpfilename, "w") as policyfile: - policyfile.write('# The policy file is empty.') - action = 'placement:test' # Load the default action and rule (defaults to "any"). @@ -91,121 +83,16 @@ class PlacementPolicyTestCase(base.ContextTestCase): policy.authorize( self.ctxt, 'placement', self.target, do_raise=False)) - @mock.patch('placement.policy.LOG.warning') - def test_default_fallback_placement_policy_file_no_exist(self, mock_warn): - """Tests that by default the policy enforcer will fallback to the - [placement]/policy_file when [oslo_policy]/policy_file does not - exist. In this case the placement policy file does not exist so no - warning about using it should be logged. + def test_init_pick_policy_file_from_oslo_config_option(self): + """Tests a scenario where the oslo policy enforcer in init pick + the policy file set in [oslo_policy]/policy_file config option. """ - # Make sure oslo_policy and placement use different policy_file - # defaults (the former uses policy.json, the latter uses policy.yaml). - config = self.conf_fixture.conf - self.assertNotEqual(config.oslo_policy.policy_file, - config.placement.policy_file) - enforcer = policy._get_enforcer(config) - self.assertEqual(config.placement.policy_file, enforcer.policy_file) - # There should not be a warning logged since the policy file does not - # actually exist. - mock_warn.assert_not_called() - - @mock.patch('placement.policy.LOG.warning') - def test_default_fallback_placement_policy_file(self, mock_warn): - """Tests that by default the policy enforcer will fallback to the - [placement]/policy_file when [oslo_policy]/policy_file does not - exist. In this case the plcaement policy file exists, like in the case - of using it to define custom rules, so a warning is logged. - """ - tempdir = self.useFixture(fixtures.TempDir()) - tmpfilename = os.path.join(tempdir.path, 'policy.yaml') - self.conf_fixture.config(group='placement', policy_file=tmpfilename) - # We have to create the file before initializing the policy enforcer - # otherwise it falls back to using CONF.placement.policy_file. This - # can be removed when the deprecated CONF.placement.policy_file option - # is removed. - with open(tmpfilename, "w") as policyfile: - policyfile.write('# I would normally have custom rules in here.') - config = self.conf_fixture.conf - enforcer = policy._get_enforcer(config) - self.assertEqual(config.placement.policy_file, enforcer.policy_file) - # There should not be a warning logged since the policy file does not - # actually exist. - mock_warn.assert_called_once_with( - '[placement]/policy_file is deprecated. Use ' - '[oslo_policy]/policy_file instead.') - - @mock.patch('placement.policy.LOG.error') - def test_init_from_oslo_policy_file_exists_same_policy_file_name( - self, mock_log_error): - """Tests a scenario where the [oslo_policy]/policy_file exists and - is the same name as the [placement]/policy_file so no error is logged - since we'll use the file from oslo_policy config. - """ - # Configure [oslo_policy]/policy_file and [placement]/policy_file with - # the same name. tempdir = self.useFixture(fixtures.TempDir()) tmpfilename = os.path.join(tempdir.path, 'policy.yaml') self.conf_fixture.config(group='oslo_policy', policy_file=tmpfilename) - self.conf_fixture.config(group='placement', policy_file=tmpfilename) - # Create the [oslo_policy]/policy_file. - with open(tmpfilename, "w") as policyfile: - policyfile.write('# Assume upgrade with existing custom policy.') - config = self.conf_fixture.conf - policy._get_enforcer(config) - # Checking what the Enforcer is using for a policy file does not really - # matter too much since they are pointing at the same file, just make - # sure we did not log an error. - mock_log_error.assert_not_called() - - @mock.patch('placement.policy.LOG.error') - def test_init_from_oslo_file_exists_different_name_no_placement_file( - self, mock_log_error): - """Tests a scenario where the [oslo_policy]/policy_file exists and - has a different name from the [placement]/policy_file but the - [placement]/policy_file does not exist so no error is logged. - """ - # Configure [oslo_policy]/policy_file and [placement]/policy_file with - # different names. - tempdir = self.useFixture(fixtures.TempDir()) - tmpfilename = os.path.join(tempdir.path, 'policy.yaml') - self.conf_fixture.config(group='oslo_policy', policy_file=tmpfilename) - self.conf_fixture.config(group='placement', policy_file='policy.json') # Create the [oslo_policy]/policy_file. with open(tmpfilename, "w") as policyfile: policyfile.write('# Assume upgrade with existing custom policy.') config = self.conf_fixture.conf enforcer = policy._get_enforcer(config) self.assertEqual(config.oslo_policy.policy_file, enforcer.policy_file) - # Though the policy file names are different, the placement version - # does not exist while the oslo policy one does so no error is logged. - mock_log_error.assert_not_called() - - @mock.patch('placement.policy.LOG.error') - def test_init_from_oslo_file_exists_different_name_placement_file_exists( - self, mock_log_error): - """Tests a scenario where the [oslo_policy]/policy_file exists and - has a different name from the [placement]/policy_file and the - [placement]/policy_file exists so an error is logged. - """ - # Configure [oslo_policy]/policy_file and [placement]/policy_file with - # different names. - tempdir = self.useFixture(fixtures.TempDir()) - oslo_name = os.path.join(tempdir.path, 'policy.yaml') - self.conf_fixture.config(group='oslo_policy', policy_file=oslo_name) - placement_name = os.path.join(tempdir.path, 'placement-policy.yaml') - self.conf_fixture.config(group='placement', policy_file=placement_name) - # Create the [oslo_policy]/policy_file. - with open(oslo_name, "w") as oslo_policy_file: - oslo_policy_file.write('# New oslo policy config.') - # Create the [placement]/policy_file. - with open(placement_name, "w") as placement_policy_file: - placement_policy_file.write('# Old placement policy file.') - config = self.conf_fixture.conf - enforcer = policy._get_enforcer(config) - self.assertEqual(config.oslo_policy.policy_file, enforcer.policy_file) - # An error should be logged since we're going to use the oslo policy - # file but there is a placement policy file with a different name that - # also exists. - mock_log_error.assert_called_once() - self.assertIn('you need to clean up your configuration file', - mock_log_error.call_args[0][0]) diff --git a/releasenotes/notes/remove-placement-policy-file-config-bb9bb26332413a77.yaml b/releasenotes/notes/remove-placement-policy-file-config-bb9bb26332413a77.yaml new file mode 100644 index 000000000..faf2d1123 --- /dev/null +++ b/releasenotes/notes/remove-placement-policy-file-config-bb9bb26332413a77.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + The deprecated ``[placement]/policy_file`` configuration option is removed + Use the more standard ``[oslo_policy]/policy_file`` config option. If you + do not override policy with custom rules you will have nothing to do. If + you do override the placement default policy then you will need to update + your configuration to use the ``[oslo_policy]/policy_file`` config option.