Add comprehensive checks for container restarts
When adding a dashboard to grafana the containers aren't restarted when they should be. This is due to a bug in Kolla where the logic to determine whether or the container needs to be restarted fails in the case where the file does not exist in the container. This patch adds more comprehensive checks for container restarts in the set_configs.py file. This patch also adds a test to ensure that the functions work as expected. Closes-Bug: #1997984 Co-Authored-By: Will Szumski <will@stackhpc.com> Change-Id: I67f5f12700d7b55f26bff81e9b54559303da6d83
This commit is contained in:
parent
c1cea309a4
commit
d9a6c5f390
@ -162,9 +162,11 @@ class ConfigFile(object):
|
|||||||
def _cmp_file(self, source, dest):
|
def _cmp_file(self, source, dest):
|
||||||
# check exsit
|
# check exsit
|
||||||
if (os.path.exists(source) and
|
if (os.path.exists(source) and
|
||||||
not self.optional and
|
|
||||||
not os.path.exists(dest)):
|
not os.path.exists(dest)):
|
||||||
return False
|
return False
|
||||||
|
if (os.path.exists(dest) and
|
||||||
|
not os.path.exists(source)):
|
||||||
|
return False
|
||||||
# check content
|
# check content
|
||||||
with open(source, 'rb') as f1, open(dest, 'rb') as f2:
|
with open(source, 'rb') as f1, open(dest, 'rb') as f2:
|
||||||
if f1.read() != f2.read():
|
if f1.read() != f2.read():
|
||||||
|
@ -0,0 +1,6 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Fix container restart conditions to be tolerant of missing optional source
|
||||||
|
and/or destination files. For more details, see the following `bug report
|
||||||
|
<https://bugs.launchpad.net/kolla/+bug/1997984>`_
|
@ -344,3 +344,74 @@ class ConfigFileTest(base.BaseTestCase):
|
|||||||
self.assertEqual([mock.call('/fake/file1', 'rb'),
|
self.assertEqual([mock.call('/fake/file1', 'rb'),
|
||||||
mock.call('/fake/file2', 'rb')],
|
mock.call('/fake/file2', 'rb')],
|
||||||
mock_open.call_args_list)
|
mock_open.call_args_list)
|
||||||
|
|
||||||
|
@mock.patch('glob.glob')
|
||||||
|
def test_check_non_optional_src_file_not_exists(self,
|
||||||
|
mock_glob):
|
||||||
|
config_file = set_configs.ConfigFile(
|
||||||
|
'/var/lib/kolla/config_files/bar', '/foo', 'user1', '0644')
|
||||||
|
|
||||||
|
mock_glob.return_value = []
|
||||||
|
|
||||||
|
self.assertRaises(set_configs.MissingRequiredSource,
|
||||||
|
config_file.check)
|
||||||
|
|
||||||
|
@mock.patch('glob.glob')
|
||||||
|
def test_check_optional_src_file_not_exists(self,
|
||||||
|
mock_glob):
|
||||||
|
config_file = set_configs.ConfigFile(
|
||||||
|
'/var/lib/kolla/config_files/bar', '/foo', 'user1', '0644',
|
||||||
|
optional=True)
|
||||||
|
mock_glob.return_value = []
|
||||||
|
|
||||||
|
self.assertIsNone(config_file.check())
|
||||||
|
|
||||||
|
@mock.patch('glob.glob')
|
||||||
|
@mock.patch('os.path.isdir')
|
||||||
|
@mock.patch.object(set_configs.ConfigFile, '_cmp_file')
|
||||||
|
def test_check_raises_config_bad_state(self,
|
||||||
|
mock_cmp_file,
|
||||||
|
mock_isdir,
|
||||||
|
mock_glob):
|
||||||
|
config_file = set_configs.ConfigFile(
|
||||||
|
'/var/lib/kolla/config_files/bar', '/foo', 'user1', '0644',
|
||||||
|
optional=True)
|
||||||
|
mock_cmp_file.return_value = False
|
||||||
|
mock_isdir.return_value = False
|
||||||
|
mock_glob.return_value = ['/var/lib/kolla/config_files/bar']
|
||||||
|
|
||||||
|
self.assertRaises(set_configs.ConfigFileBadState, config_file.check)
|
||||||
|
|
||||||
|
@mock.patch('os.path.exists', autospec=True)
|
||||||
|
def test_cmp_file_optional_src_exists_dest_no_exists(self, mock_os_exists):
|
||||||
|
config_file = set_configs.ConfigFile(
|
||||||
|
'/var/lib/kolla/config_files/bar', '/foo', 'user1', '0644',
|
||||||
|
optional=True)
|
||||||
|
|
||||||
|
def fake_exists(path):
|
||||||
|
if path == '/var/lib/kolla/config_files/bar':
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
mock_os_exists.side_effect = fake_exists
|
||||||
|
|
||||||
|
self.assertIs(False,
|
||||||
|
config_file._cmp_file('/var/lib/kolla/config_files/bar',
|
||||||
|
'/foo'))
|
||||||
|
|
||||||
|
@mock.patch('os.path.exists', autospec=True)
|
||||||
|
def test_cmp_file_optional_src_no_exists_dest_exists(self, mock_os_exists):
|
||||||
|
config_file = set_configs.ConfigFile(
|
||||||
|
'/var/lib/kolla/config_files/bar', '/foo', 'user1', '0644',
|
||||||
|
optional=True)
|
||||||
|
|
||||||
|
def fake_exists(path):
|
||||||
|
if path == '/var/lib/kolla/config_files/bar':
|
||||||
|
return False
|
||||||
|
return True
|
||||||
|
|
||||||
|
mock_os_exists.side_effect = fake_exists
|
||||||
|
|
||||||
|
self.assertIs(False,
|
||||||
|
config_file._cmp_file('/var/lib/kolla/config_files/bar',
|
||||||
|
'/foo'))
|
||||||
|
Loading…
Reference in New Issue
Block a user