From 1ccf7c2b251819c518f9a88de4f1e8541a0216fc Mon Sep 17 00:00:00 2001
From: James Page <james.page@ubuntu.com>
Date: Tue, 9 Jan 2018 13:50:15 +0000
Subject: [PATCH] tests: improve idempotency and isolation

Use sort_keys when serializing in json format for idempotency under
Python 3; Mock out calls to get_osds in broker test to ensure that
a local install of ceph-common does not cause unit test failures.

Change-Id: I6ce4075fd38e8431372a359cc88332d2da7a3974
---
 ceph/broker.py            |  4 ++--
 ceph/utils.py             | 10 +++++-----
 unit_tests/test_broker.py | 17 +++++++++++------
 unit_tests/test_utils.py  |  7 +++++--
 4 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/ceph/broker.py b/ceph/broker.py
index 1c0b928..8ba2e7a 100644
--- a/ceph/broker.py
+++ b/ceph/broker.py
@@ -338,7 +338,7 @@ def save_service(service_name, service):
     service['groups'] = {}
     return monitor_key_set(service='admin',
                            key="cephx.services.{}".format(service_name),
-                           value=json.dumps(service))
+                           value=json.dumps(service, sort_keys=True))
 
 
 def save_group(group, group_name):
@@ -346,7 +346,7 @@ def save_group(group, group_name):
     group_key = get_group_key(group_name=group_name)
     return monitor_key_set(service='admin',
                            key=group_key,
-                           value=json.dumps(group))
+                           value=json.dumps(group, sort_keys=True))
 
 
 def get_group_key(group_name):
diff --git a/ceph/utils.py b/ceph/utils.py
index 7665630..9d6cd3c 100644
--- a/ceph/utils.py
+++ b/ceph/utils.py
@@ -2148,11 +2148,11 @@ def dirs_need_ownership_update(service):
     return False
 
 # A dict of valid ceph upgrade paths. Mapping is old -> new
-UPGRADE_PATHS = {
-    'firefly': 'hammer',
-    'hammer': 'jewel',
-    'jewel': 'luminous',
-}
+UPGRADE_PATHS = collections.OrderedDict([
+    ('firefly', 'hammer'),
+    ('hammer', 'jewel'),
+    ('jewel', 'luminous'),
+])
 
 # Map UCA codenames to ceph codenames
 UCA_CODENAME_MAP = {
diff --git a/unit_tests/test_broker.py b/unit_tests/test_broker.py
index 864a2e3..cd59a67 100644
--- a/unit_tests/test_broker.py
+++ b/unit_tests/test_broker.py
@@ -64,7 +64,7 @@ class CephBrokerTestCase(unittest.TestCase):
             key='cephx.groups.images',
             service='admin',
             value=json.dumps({"pools": ["glance", "cinder"],
-                              "services": ["nova"]}))
+                              "services": ["nova"]}, sort_keys=True))
         _check_call.assert_called_with([
             'ceph', 'auth', 'caps',
             'client.nova', 'mon', 'allow r', 'osd',
@@ -83,7 +83,8 @@ class CephBrokerTestCase(unittest.TestCase):
         _monitor_key_set.assert_called_with(
             key='cephx.groups.images',
             service='admin',
-            value=json.dumps({"pools": ["glance", "cinder"], "services": []}))
+            value=json.dumps({"pools": ["glance", "cinder"], "services": []},
+                             sort_keys=True))
 
     @patch.object(ceph.broker, 'monitor_key_set')
     @patch.object(ceph.broker, 'monitor_key_get')
@@ -98,7 +99,8 @@ class CephBrokerTestCase(unittest.TestCase):
         _monitor_key_set.assert_called_with(
             key='cephx.groups.images',
             service='admin',
-            value=json.dumps({"pools": ["glance"], "services": []}))
+            value=json.dumps({"pools": ["glance"], "services": []},
+                             sort_keys=True))
 
     def test_pool_permission_list_for_service(self):
         service = {
@@ -142,7 +144,7 @@ class CephBrokerTestCase(unittest.TestCase):
         }
         ceph.broker.save_service(service=service, service_name='nova')
         _monitor_key_set.assert_called_with(
-            value='{"groups": {}, "group_names": {"rwx": "images"}}',
+            value=json.dumps(service, sort_keys=True),
             key='cephx.services.nova',
             service='admin')
 
@@ -195,7 +197,7 @@ class CephBrokerTestCase(unittest.TestCase):
         _monitor_key_set.assert_called_with(
             key='cephx.groups.images',
             service='admin',
-            value=json.dumps(group))
+            value=json.dumps(group, sort_keys=True))
 
     @patch.object(ceph.broker, 'monitor_key_get')
     def test_get_group_empty_str(self, _monitor_key_get):
@@ -254,13 +256,16 @@ class CephBrokerTestCase(unittest.TestCase):
                          {'exit-code': 1,
                           'stderr': "Unknown operation 'invalid_op'"})
 
+    @patch.object(ceph.broker, 'get_osds')
     @patch.object(ceph.broker, 'ReplicatedPool')
     @patch.object(ceph.broker, 'pool_exists')
     @patch.object(ceph.broker, 'log')
     def test_process_requests_create_pool_w_pg_num(self, mock_log,
                                                    mock_pool_exists,
-                                                   mock_replicated_pool):
+                                                   mock_replicated_pool,
+                                                   mock_get_osds):
         mock_pool_exists.return_value = False
+        mock_get_osds.return_value = [0, 1, 2]
         reqs = json.dumps({'api-version': 1,
                            'ops': [{
                                'op': 'create-pool',
diff --git a/unit_tests/test_utils.py b/unit_tests/test_utils.py
index 2490d47..1bea1d4 100644
--- a/unit_tests/test_utils.py
+++ b/unit_tests/test_utils.py
@@ -507,8 +507,11 @@ class CephTestCase(unittest.TestCase):
             utils.osd_noout(True)
 
     def test_pretty_print_upgrade_paths(self):
-        expected = (['firefly -> hammer', 'jewel -> luminous',
-                     'hammer -> jewel'])
+        expected = ([
+            'firefly -> hammer',
+            'hammer -> jewel',
+            'jewel -> luminous',
+        ])
         self.assertEqual(utils.pretty_print_upgrade_paths(), expected)