chownr /var/lib/ceph only when needed

Infernalis release of Ceph switched to running the ceph daemons
under the ceph:ceph user instead of root:root. The upgrade OSD
logic does not take this into conideration and blindly runs the
recursive chown for whatever the current user is expected to be
when the upgrade is run.

This is problematic in production clusters with large numbers of
objects stored across 10 or more OSDs as this will cause a walk
of all the inodes under /var/lib/ceph unnecessarily. This change
ensures that the chown is only done when the upgrade target is
Jewel (Ubuntu Cloud Archives do not contain Infernalis release).

Change-Id: Id4bd48ebb90928b989df718404263e40f926c201
Partial-Bug: #1657527

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))

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',

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',