Add unit tests and fix related bugs
This change adds more unit tests to the rolling OSD upgrade scenario to ensure more complete coverage. Change-Id: Ibd7c8d6c46520957a3298446efc6b5fff210a51a Related-Bug: #1662591
This commit is contained in:
parent
cd75b59b33
commit
80c8256fb6
|
@ -561,7 +561,8 @@ def _get_child_dirs(path):
|
|||
if not os.path.isdir(path):
|
||||
raise ValueError('Specified path "%s" is not a directory' % path)
|
||||
|
||||
return filter(os.path.isdir, os.listdir(path))
|
||||
files_in_dir = [os.path.join(path, f) for f in os.listdir(path)]
|
||||
return list(filter(os.path.isdir, files_in_dir))
|
||||
|
||||
|
||||
def _get_osd_num_from_dirname(dirname):
|
||||
|
@ -1559,6 +1560,7 @@ def lock_and_roll(upgrade_key, service, my_name, version):
|
|||
my_name,
|
||||
version,
|
||||
stop_timestamp))
|
||||
status_set('maintenance', 'Finishing upgrade')
|
||||
monitor_key_set(upgrade_key, "{}_{}_{}_done".format(service,
|
||||
my_name,
|
||||
version),
|
||||
|
@ -1744,7 +1746,7 @@ def _upgrade_single_osd(osd_num, osd_dir):
|
|||
"""
|
||||
stop_osd(osd_num)
|
||||
disable_osd(osd_num)
|
||||
update_owner(os.path.join(OSD_BASE_DIR, osd_dir))
|
||||
update_owner(osd_dir)
|
||||
enable_osd(osd_num)
|
||||
start_osd(osd_num)
|
||||
|
||||
|
@ -1860,8 +1862,8 @@ def update_owner(path, recurse_dirs=True):
|
|||
cmd = ['chown', user_group, path]
|
||||
if os.path.isdir(path) and recurse_dirs:
|
||||
status_set('maintenance', ('Updating ownership of %s to %s' %
|
||||
(path.split('/')[-1], user)))
|
||||
cmd.insert('-R', 1)
|
||||
(path, user)))
|
||||
cmd.insert(1, '-R')
|
||||
|
||||
log('Changing ownership of {path} to {user}'.format(
|
||||
path=path, user=user_group), DEBUG)
|
||||
|
@ -1910,14 +1912,12 @@ def dirs_need_ownership_update(service):
|
|||
expected_owner = expected_group = ceph_user()
|
||||
path = os.path.join(CEPH_BASE_DIR, service)
|
||||
for child in _get_child_dirs(path):
|
||||
child_path = os.path.join(path, child)
|
||||
curr_owner, curr_group = owner(child_path)
|
||||
curr_owner, curr_group = owner(child)
|
||||
|
||||
if (curr_owner == expected_owner) and (curr_group == expected_group):
|
||||
continue
|
||||
|
||||
log('Directory "%s" needs its ownership updated' % child_path,
|
||||
DEBUG)
|
||||
log('Directory "%s" needs its ownership updated' % child, DEBUG)
|
||||
return True
|
||||
|
||||
# All child directories had the expected ownership
|
||||
|
|
|
@ -155,7 +155,7 @@ class UpgradeRollingTestCase(unittest.TestCase):
|
|||
@patch.object(ceph, 'start_osd')
|
||||
def test_upgrade_single_osd(self, start_osd, enable_osd, update_owner,
|
||||
disable_osd, stop_osd):
|
||||
ceph._upgrade_single_osd(1, 'ceph-1')
|
||||
ceph._upgrade_single_osd(1, '/var/lib/ceph/osd/ceph-1')
|
||||
stop_osd.assert_called_with(1)
|
||||
disable_osd.assert_called_with(1)
|
||||
update_owner.assert_called_with('/var/lib/ceph/osd/ceph-1')
|
||||
|
@ -294,6 +294,73 @@ class UpgradeRollingTestCase(unittest.TestCase):
|
|||
upgrade_key='osd-upgrade',
|
||||
version='0.94.1')
|
||||
|
||||
@patch('os.path.exists')
|
||||
@patch('os.listdir')
|
||||
@patch('os.path.isdir')
|
||||
def test__get_child_dirs(self, isdir, listdir, exists):
|
||||
isdir.side_effect = [True, True, True, False, True]
|
||||
listdir.return_value = ['mon', 'bootstrap-osd', 'foo', 'bootstrap-mon']
|
||||
exists.return_value = True
|
||||
|
||||
child_dirs = ceph._get_child_dirs('/var/lib/ceph')
|
||||
isdir.assert_has_calls([call('/var/lib/ceph'),
|
||||
call('/var/lib/ceph/mon'),
|
||||
call('/var/lib/ceph/bootstrap-osd'),
|
||||
call('/var/lib/ceph/foo'),
|
||||
call('/var/lib/ceph/bootstrap-mon')])
|
||||
self.assertListEqual(['/var/lib/ceph/mon',
|
||||
'/var/lib/ceph/bootstrap-osd',
|
||||
'/var/lib/ceph/bootstrap-mon'], child_dirs)
|
||||
|
||||
@patch('os.path.exists')
|
||||
@patch('os.path.isdir')
|
||||
def test__get_child_dirs_not_dir(self, isdir, exists):
|
||||
isdir.return_value = False
|
||||
exists.return_value = False
|
||||
|
||||
with self.assertRaises(ValueError):
|
||||
ceph._get_child_dirs('/var/lib/ceph')
|
||||
|
||||
@patch('os.path.exists')
|
||||
def test__get_child_dirs_no_exist(self, exists):
|
||||
exists.return_value = False
|
||||
|
||||
with self.assertRaises(ValueError):
|
||||
ceph._get_child_dirs('/var/lib/ceph')
|
||||
|
||||
@patch('ceph.ceph_user')
|
||||
@patch('os.path.isdir')
|
||||
@patch('subprocess.check_call')
|
||||
@patch('ceph.status_set')
|
||||
def test_update_owner_no_recurse(self, status_set, check_call,
|
||||
isdir, ceph_user):
|
||||
ceph_user.return_value = 'ceph'
|
||||
isdir.return_value = True
|
||||
ceph.update_owner('/var/lib/ceph', False)
|
||||
check_call.assert_called_with(['chown', 'ceph:ceph', '/var/lib/ceph'])
|
||||
|
||||
@patch('ceph.ceph_user')
|
||||
@patch('os.path.isdir')
|
||||
@patch('subprocess.check_call')
|
||||
@patch('ceph.status_set')
|
||||
def test_update_owner_recurse_file(self, status_set, check_call,
|
||||
isdir, ceph_user):
|
||||
ceph_user.return_value = 'ceph'
|
||||
isdir.return_value = False
|
||||
ceph.update_owner('/var/lib/ceph', True)
|
||||
check_call.assert_called_with(['chown', 'ceph:ceph', '/var/lib/ceph'])
|
||||
|
||||
@patch('ceph.ceph_user')
|
||||
@patch('os.path.isdir')
|
||||
@patch('subprocess.check_call')
|
||||
@patch('ceph.status_set')
|
||||
def test_update_owner_recurse(self, status_set, check_call,
|
||||
isdir, ceph_user):
|
||||
ceph_user.return_value = 'ceph'
|
||||
isdir.return_value = True
|
||||
ceph.update_owner('/var/lib/ceph', True)
|
||||
check_call.assert_called_with(['chown', '-R', 'ceph:ceph',
|
||||
'/var/lib/ceph'])
|
||||
|
||||
"""
|
||||
@patch('ceph.log')
|
||||
|
|
Loading…
Reference in New Issue