diff --git a/ceph/__init__.py b/ceph/__init__.py index 9527f6b..9b2f6e5 100644 --- a/ceph/__init__.py +++ b/ceph/__init__.py @@ -1430,11 +1430,17 @@ def upgrade_monitor(new_version): service_stop('ceph-mon-all') apt_install(packages=PACKAGES, fatal=True) - # Ensure the ownership of Ceph's directories is correct - chownr(path=os.path.join(os.sep, "var", "lib", "ceph"), - owner=ceph_user(), - group=ceph_user(), - follow_links=True) + # Ensure the files and directories under /var/lib/ceph is chowned + # properly as part of the move to the Jewel release, which moved the + # ceph daemons to running as ceph:ceph instead of root:root. + if new_version == 'jewel': + # Ensure the ownership of Ceph's directories is correct + owner = ceph_user() + chownr(path=os.path.join(os.sep, "var", "lib", "ceph"), + owner=owner, + group=owner, + follow_links=True) + if systemd(): for mon_id in get_local_mon_ids(): service_start('ceph-mon@{}'.format(mon_id)) @@ -1609,11 +1615,18 @@ def upgrade_osd(new_version): service_stop('ceph-osd-all') apt_install(packages=PACKAGES, fatal=True) - # Ensure the ownership of Ceph's directories is correct - chownr(path=os.path.join(os.sep, "var", "lib", "ceph"), - owner=ceph_user(), - group=ceph_user(), - follow_links=True) + # Ensure the files and directories under /var/lib/ceph is chowned + # properly as part of the move to the Jewel release, which moved the + # ceph daemons to running as ceph:ceph instead of root:root. Only do + # it when necessary as this is an expensive operation to run. + if new_version == 'jewel': + owner = ceph_user() + status_set('maintenance', 'Updating file ownership for OSDs') + chownr(path=os.path.join(os.sep, "var", "lib", "ceph"), + owner=owner, + group=owner, + follow_links=True) + if systemd(): for osd_id in get_local_osd_ids(): service_start('ceph-osd@{}'.format(osd_id)) diff --git a/unit_tests/test_mon_upgrade_roll.py b/unit_tests/test_mon_upgrade_roll.py index 7ee764e..f8b4c69 100644 --- a/unit_tests/test_mon_upgrade_roll.py +++ b/unit_tests/test_mon_upgrade_roll.py @@ -82,16 +82,15 @@ class UpgradeRollingTestCase(unittest.TestCase): @patch('ceph.add_source') @patch('ceph.get_local_mon_ids') @patch('ceph.systemd') - @patch('ceph.ceph_user') @patch('ceph.get_version') @patch('ceph.config') - def test_upgrade_monitor(self, config, get_version, ceph_user, systemd, - local_mons, add_source, apt_update, status_set, - log, service_start, service_stop, chownr, - apt_install): - get_version.return_value = "0.80" + def test_upgrade_monitor_hammer(self, config, get_version, + systemd, local_mons, add_source, + apt_update, status_set, log, + service_start, service_stop, chownr, + apt_install): + get_version.side_effect = [0.80, 0.94] config.side_effect = config_side_effect - ceph_user.return_value = "ceph" systemd.return_value = False local_mons.return_value = ['a'] @@ -102,13 +101,51 @@ class UpgradeRollingTestCase(unittest.TestCase): log.assert_has_calls( [ - call('Current ceph version is 0.80'), + call('Current ceph version is 0.8'), call('Upgrading to: hammer') ] ) status_set.assert_has_calls([ call('maintenance', 'Upgrading monitor'), ]) + assert not chownr.called + + @patch('ceph.apt_install') + @patch('ceph.chownr') + @patch('ceph.service_stop') + @patch('ceph.service_start') + @patch('ceph.log') + @patch('ceph.status_set') + @patch('ceph.apt_update') + @patch('ceph.add_source') + @patch('ceph.get_local_mon_ids') + @patch('ceph.systemd') + @patch('ceph.get_version') + @patch('ceph.config') + def test_upgrade_monitor_jewel(self, config, get_version, + systemd, local_mons, add_source, + apt_update, status_set, log, + service_start, service_stop, chownr, + apt_install): + get_version.side_effect = [0.94, 10.1] + config.side_effect = config_side_effect + systemd.return_value = False + local_mons.return_value = ['a'] + + ceph.upgrade_monitor('jewel') + service_stop.assert_called_with('ceph-mon-all') + service_start.assert_called_with('ceph-mon-all') + add_source.assert_called_with('cloud:trusty-kilo', 'key') + + log.assert_has_calls( + [ + call('Current ceph version is 0.94'), + call('Upgrading to: jewel') + ] + ) + status_set.assert_has_calls([ + call('maintenance', 'Upgrading monitor'), + ]) chownr.assert_has_calls( [ call(group='ceph', owner='ceph', path='/var/lib/ceph', diff --git a/unit_tests/test_osd_upgrade_roll.py b/unit_tests/test_osd_upgrade_roll.py index 82088c8..4a878ab 100644 --- a/unit_tests/test_osd_upgrade_roll.py +++ b/unit_tests/test_osd_upgrade_roll.py @@ -71,16 +71,14 @@ class UpgradeRollingTestCase(unittest.TestCase): @patch('ceph.add_source') @patch('ceph.get_local_osd_ids') @patch('ceph.systemd') - @patch('ceph.ceph_user') @patch('ceph.get_version') @patch('ceph.config') - def test_upgrade_osd(self, config, get_version, ceph_user, systemd, - local_osds, add_source, apt_update, status_set, - log, service_start, service_stop, chownr, - apt_install): + def test_upgrade_osd_hammer(self, config, get_version, systemd, local_osds, + add_source, apt_update, status_set, log, + service_start, service_stop, chownr, + apt_install): config.side_effect = config_side_effect - get_version.return_value = "0.80" - ceph_user.return_value = "ceph" + get_version.side_effect = [0.80, 0.94] systemd.return_value = False local_osds.return_value = [0, 1, 2] @@ -92,10 +90,47 @@ class UpgradeRollingTestCase(unittest.TestCase): ]) log.assert_has_calls( [ - call('Current ceph version is 0.80'), + call('Current ceph version is 0.8'), call('Upgrading to: hammer') ] ) + # Make sure on an Upgrade to Hammer that chownr was NOT called. + assert not chownr.called + + @patch('ceph.apt_install') + @patch('ceph.chownr') + @patch('ceph.service_stop') + @patch('ceph.service_start') + @patch('ceph.log') + @patch('ceph.status_set') + @patch('ceph.apt_update') + @patch('ceph.add_source') + @patch('ceph.get_local_osd_ids') + @patch('ceph.systemd') + @patch('ceph.get_version') + @patch('ceph.config') + def test_upgrade_osd_jewel(self, config, get_version, systemd, + local_osds, add_source, apt_update, status_set, + log, service_start, service_stop, chownr, + apt_install): + config.side_effect = config_side_effect + get_version.side_effect = [0.94, 10.1] + systemd.return_value = False + local_osds.return_value = [0, 1, 2] + + ceph.upgrade_osd('jewel') + service_stop.assert_called_with('ceph-osd-all') + service_start.assert_called_with('ceph-osd-all') + status_set.assert_has_calls([ + call('maintenance', 'Upgrading osd'), + call('maintenance', 'Updating file ownership for OSDs') + ]) + log.assert_has_calls( + [ + call('Current ceph version is 0.94'), + call('Upgrading to: jewel') + ] + ) chownr.assert_has_calls( [ call(group='ceph', owner='ceph', path='/var/lib/ceph',