Add upgrade check for presence of policy.json file
Correct file is policy.yaml and only needed if overriding defaults. This only warns if the file is present in case it was left by a previous version and not actually needed or used. Also checks for the policy_file being overridden in config, and if so, warns if the specified file does not exist. Change-Id: Ia46e843ad245cf8263b97b773fac9bc6c6fe6647 Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
This commit is contained in:
Sean McGinnis
parent
06ab31adce
commit
8c6355f3cc
@ -15,14 +15,15 @@
|
||||
|
||||
"""CLI interface for cinder status commands."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
|
||||
from oslo_config import cfg
|
||||
from oslo_upgradecheck import upgradecheck as uc
|
||||
|
||||
from cinder.policy import DEFAULT_POLICY_FILENAME
|
||||
import cinder.service # noqa
|
||||
|
||||
|
||||
CONF = cfg.CONF
|
||||
|
||||
SUCCESS = uc.Code.SUCCESS
|
||||
@ -33,6 +34,10 @@ WARNING = uc.Code.WARNING
|
||||
class Checks(uc.UpgradeCommands):
|
||||
"""Upgrade checks to run."""
|
||||
|
||||
def _file_exists(self, path):
|
||||
"""Helper for mocking check of os.path.exists."""
|
||||
return os.path.exists(path)
|
||||
|
||||
def _check_backup_module(self):
|
||||
"""Checks for the use of backup driver module paths.
|
||||
|
||||
@ -55,14 +60,71 @@ class Checks(uc.UpgradeCommands):
|
||||
|
||||
return uc.Result(SUCCESS)
|
||||
|
||||
def _check_policy_file(self):
|
||||
"""Checks if a policy.json file is present.
|
||||
|
||||
With the switch to policy-in-code, policy files should be policy.yaml
|
||||
and should only be present if overriding default policy. Just checks
|
||||
and warns if the old file is present to make sure they are aware it is
|
||||
not being used.
|
||||
"""
|
||||
# make sure we know where to look for the policy file
|
||||
config_dir = CONF.find_file('cinder.conf')
|
||||
if not config_dir:
|
||||
return uc.Result(
|
||||
WARNING,
|
||||
'Cannot locate your cinder configuration directory. '
|
||||
'Please re-run using the --config-dir <dirname> option.')
|
||||
|
||||
policy_file = CONF.oslo_policy.policy_file
|
||||
json_file = os.path.join(os.path.dirname(config_dir), 'policy.json')
|
||||
|
||||
if policy_file == DEFAULT_POLICY_FILENAME:
|
||||
# Default is being used, check for old json file
|
||||
if self._file_exists(json_file):
|
||||
return uc.Result(
|
||||
WARNING,
|
||||
'policy.json file is present. Make sure any changes from '
|
||||
'the default policies are present in a policy.yaml file '
|
||||
'instead. If you really intend to use a policy.json file, '
|
||||
'make sure that its absolute path is set as the value of '
|
||||
"the 'policy_file' configuration option in the "
|
||||
'[oslo_policy] section of your cinder.conf file.')
|
||||
|
||||
else:
|
||||
# They have configured a custom policy file. It is OK if it does
|
||||
# not exist, but we should check and warn about it while we're
|
||||
# checking.
|
||||
if not policy_file.startswith('/'):
|
||||
# policy_file is relative to config_dir
|
||||
policy_file = os.path.join(os.path.dirname(config_dir),
|
||||
policy_file)
|
||||
if not self._file_exists(policy_file):
|
||||
return uc.Result(
|
||||
WARNING,
|
||||
"Configured policy file '%s' does not exist. This may be "
|
||||
"expected, but default policies will be used until any "
|
||||
"desired overrides are added to the configured file." %
|
||||
policy_file)
|
||||
|
||||
return uc.Result(SUCCESS)
|
||||
|
||||
_upgrade_checks = (
|
||||
('Backup Driver Path', _check_backup_module),
|
||||
('Use of Policy File', _check_policy_file),
|
||||
)
|
||||
|
||||
|
||||
def main():
|
||||
return uc.main(CONF, 'cinder', Checks())
|
||||
|
||||
# TODO(rosmaita): need to do this because we suggest using the
|
||||
# --config-dir option, and if the user gives a bogus value, we
|
||||
# get a stacktrace. Needs to be fixed in oslo_upgradecheck
|
||||
try:
|
||||
return uc.main(CONF, 'cinder', Checks())
|
||||
except cfg.ConfigDirNotFoundError:
|
||||
return('ERROR: cannot read the cinder configuration directory.\n'
|
||||
'Please re-run using the --config-dir <dirname> option '
|
||||
'with a valid cinder configuration directory.')
|
||||
|
||||
if __name__ == '__main__':
|
||||
sys.exit(main())
|
||||
|
||||
@ -12,6 +12,7 @@
|
||||
|
||||
"""Unit tests for the cinder-status CLI interfaces."""
|
||||
|
||||
import mock
|
||||
from oslo_config import cfg
|
||||
from oslo_upgradecheck import upgradecheck as uc
|
||||
import testtools
|
||||
@ -28,18 +29,69 @@ class TestCinderStatus(testtools.TestCase):
|
||||
super(TestCinderStatus, self).setUp()
|
||||
self.checks = status.Checks()
|
||||
|
||||
def _set_backup_driver(self, driver_path):
|
||||
CONF.set_override('backup_driver', driver_path)
|
||||
self.addCleanup(CONF.clear_override, 'backup_driver')
|
||||
# Make sure configuration is initialized
|
||||
try:
|
||||
CONF([], project='cinder')
|
||||
except cfg.RequiredOptError:
|
||||
# Doesn't matter in this situation
|
||||
pass
|
||||
|
||||
# Make sure our expected path is returned
|
||||
patcher = mock.patch.object(CONF, 'find_file')
|
||||
self.addCleanup(patcher.stop)
|
||||
self.find_file = patcher.start()
|
||||
self.find_file.return_value = '/etc/cinder/'
|
||||
|
||||
def _set_config(self, key, value, group=None):
|
||||
CONF.set_override(key, value, group=group)
|
||||
self.addCleanup(CONF.clear_override, key, group=group)
|
||||
|
||||
def test_check_backup_module(self):
|
||||
self._set_backup_driver(
|
||||
self._set_config(
|
||||
'backup_driver',
|
||||
'cinder.backup.drivers.swift.SwiftBackupDriver')
|
||||
result = self.checks._check_backup_module()
|
||||
self.assertEqual(uc.Code.SUCCESS, result.code)
|
||||
|
||||
def test_check_backup_module_not_class(self):
|
||||
self._set_backup_driver('cinder.backup.drivers.swift')
|
||||
self._set_config('backup_driver', 'cinder.backup.drivers.swift')
|
||||
result = self.checks._check_backup_module()
|
||||
self.assertEqual(uc.Code.FAILURE, result.code)
|
||||
self.assertIn('requires the full path', result.details)
|
||||
|
||||
def test_check_policy_file(self):
|
||||
with mock.patch.object(self.checks, '_file_exists') as fe:
|
||||
fe.return_value = False
|
||||
result = self.checks._check_policy_file()
|
||||
|
||||
self.assertEqual(uc.Code.SUCCESS, result.code)
|
||||
|
||||
def test_check_policy_file_exists(self):
|
||||
with mock.patch.object(self.checks, '_file_exists') as fe:
|
||||
fe.return_value = True
|
||||
result = self.checks._check_policy_file()
|
||||
|
||||
self.assertEqual(uc.Code.WARNING, result.code)
|
||||
self.assertIn('policy.json file is present', result.details)
|
||||
|
||||
def test_check_policy_file_custom_path(self):
|
||||
policy_path = '/my/awesome/configs/policy.yaml'
|
||||
self._set_config('policy_file', policy_path, group='oslo_policy')
|
||||
with mock.patch.object(self.checks, '_file_exists') as fe:
|
||||
fe.return_value = False
|
||||
result = self.checks._check_policy_file()
|
||||
fe.assert_called_with(policy_path)
|
||||
|
||||
self.assertEqual(uc.Code.WARNING, result.code)
|
||||
self.assertIn(policy_path, result.details)
|
||||
|
||||
def test_check_policy_file_custom_file(self):
|
||||
policy_path = 'mypolicy.yaml'
|
||||
self._set_config('policy_file', policy_path, group='oslo_policy')
|
||||
with mock.patch.object(self.checks, '_file_exists') as fe:
|
||||
fe.return_value = False
|
||||
result = self.checks._check_policy_file()
|
||||
fe.assert_called_with('/etc/cinder/%s' % policy_path)
|
||||
|
||||
self.assertEqual(uc.Code.WARNING, result.code)
|
||||
self.assertIn(policy_path, result.details)
|
||||
|
||||
@ -87,6 +87,8 @@ Upgrade
|
||||
|
||||
* Check added to ensure the backup_driver setting is using the full driver
|
||||
class path and not just the module path.
|
||||
* Checks for the presence of a **policy.json** file have been added to warn
|
||||
if policy changes should be present in a **policy.yaml** file.
|
||||
|
||||
See Also
|
||||
========
|
||||
|
||||
@ -0,0 +1,7 @@
|
||||
---
|
||||
upgrade:
|
||||
- |
|
||||
A warning has been added to the ``cinder-status upgrade check`` CLI if a
|
||||
``policy.json`` file is present. Documentation has been updated to
|
||||
correct the file as ``policy.yaml`` if any policies need to be changed from
|
||||
their defaults.
|
||||
Loading…
Reference in New Issue
Block a user