Merge "Add upgrade check for presence of policy.json file"
This commit is contained in:
Zuul
Gerrit Code Review
commit
305eb607f6
@ -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…
x
Reference in New Issue
Block a user