openstack
/
placement
Code Issues Proposed changes

Remove deprecated [placement]/policy_file config option 

[placement]/policy_file config option is deprecated since Train
release in favor of ``[oslo_policy]/policy_file`` config option.

In wallaby cycle, default value of ``[oslo_policy]/policy_file``
is going to change to 'policy.yaml' so it is better to remove
the old deprecated config option to avoid confusion for operator.

Change-Id: I427f1f5a82dc1b2e27fa29b68db9ab549df92289
This commit is contained in:
Ghanshyam Mann 2020-11-27 00:00:02 -06:00 committed by Ghanshyam
parent a0acd21937
commit 0c8cc6eef9
6 changed files with 16 additions and 175 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/source/admin/upgrade-notes.rst
View File

@ -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.

2
doc/source/admin/upgrade-to-stein.rst
View File

@ -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.

20
placement/conf/placement.py
View File

@ -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,

38
placement/policy.py
View File

@ -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

119
placement/tests/unit/test_policy.py
View File

@ -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])

8
releasenotes/notes/remove-placement-policy-file-config-bb9bb26332413a77.yaml Normal file
View File

@ -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.