Merge lp:~james-page/charm-helpers/lp-1480893 into lp:charm-helpers

[James Page]

Fixup problem with out-of-context migration of peer data to leader settings.

[Liam Young (gnuoy)]

LGTM

review: Approve

[Edward Hope-Morley (hopem)]

review: Needs Fixing

=== modified file 'charmhelpers/contrib/peerstorage/__init__.py'
--- charmhelpers/contrib/peerstorage/__init__.py	2015-06-03 13:09:25 +0000
+++ charmhelpers/contrib/peerstorage/__init__.py	2015-08-03 13:24:44 +0000
@@ -59,7 +59,7 @@
 """
 
 
-def leader_get(attribute=None):
+def leader_get(attribute=None, rid=None):
     """Wrapper to ensure that settings are migrated from the peer relation.
 
     This is to support upgrading an environment that does not support
@@ -94,7 +94,8 @@
         # If attribute not present in leader db, check if this unit has set
         # the attribute in the peer relation
         if not leader_settings:
-            peer_setting = relation_get(attribute=attribute, unit=local_unit())
+            peer_setting = _relation_get(attribute=attribute, unit=local_unit(),
+                                         rid=rid)
             if peer_setting:
                 leader_set(settings={attribute: peer_setting})
                 leader_settings = peer_setting
@@ -103,7 +104,7 @@
             settings_migrated = True
             migrated.add(attribute)
     else:
-        r_settings = relation_get(unit=local_unit())
+        r_settings = _relation_get(unit=local_unit(), rid=rid)
         if r_settings:
             for key in set(r_settings.keys()).difference(migrated):
                 # Leader setting wins
@@ -151,7 +152,7 @@
     """
     try:
         if rid in relation_ids('cluster'):
-            return leader_get(attribute)
+            return leader_get(attribute, rid)
         else:
             raise NotImplementedError
     except NotImplementedError:

=== modified file 'tests/contrib/peerstorage/test_peerstorage.py'
--- tests/contrib/peerstorage/test_peerstorage.py	2015-06-03 14:46:50 +0000
+++ tests/contrib/peerstorage/test_peerstorage.py	2015-08-03 13:24:44 +0000
@@ -202,7 +202,7 @@
         l_settings = {'s3': 3}
         r_settings = {'s1': 1, 's2': 2}
 
-        def mock_relation_get(attribute=None, unit=None):
+        def mock_relation_get(attribute=None, unit=None, rid=None):
             if attribute:
                 if attribute in r_settings:
                     return r_settings.get(attribute)
@@ -237,11 +237,11 @@
             self.assertEqual(_dicta, _dictb)
 
         migration_key = '__leader_get_migrated_settings__'
-        self.relation_get.side_effect = mock_relation_get
+        self._relation_get.side_effect = mock_relation_get
         self._leader_get.side_effect = mock_leader_get
         self.leader_set.side_effect = mock_leader_set
 
-        self.assertEqual({'s1': 1, 's2': 2}, peerstorage.relation_get())
+        self.assertEqual({'s1': 1, 's2': 2}, peerstorage._relation_get())
         self.assertEqual({'s3': 3}, peerstorage._leader_get())
         self.assertEqual({'s1': 1, 's2': 2, 's3': 3}, peerstorage.leader_get())
         check_leader_db({'s1': 1, 's2': 2, 's3': 3,
@@ -274,7 +274,7 @@
 
         peerstorage.leader_set.reset_mock()
         self.assertEqual({'s1': 1, 's2': 2, 's3': 2, 's4': 3},
-                         peerstorage.relation_get())
+                         peerstorage._relation_get())
         check_leader_db({'s1': 1, 's3': 3, 's4': 4,
                          migration_key: '["s1", "s4"]'},
                         peerstorage._leader_get())
@@ -290,7 +290,7 @@
         l_settings = {'s3': 3}
         r_settings = {'s1': 1, 's2': 2}
 
-        def mock_relation_get(attribute=None, unit=None):
+        def mock_relation_get(attribute=None, unit=None, rid=None):
             if attribute:
                 if attribute in r_settings:
                     return r_settings.get(attribute)
@@ -314,10 +314,10 @@
 
             l_settings.update(kwargs)
 
-        self.relation_get.side_effect = mock_relation_get
+        self._relation_get.side_effect = mock_relation_get
         self._leader_get.side_effect = mock_leader_get
         self.leader_set.side_effect = mock_leader_set
-        self.assertEqual({'s1': 1, 's2': 2}, peerstorage.relation_get())
+        self.assertEqual({'s1': 1, 's2': 2}, peerstorage._relation_get())
         self.assertEqual({'s3': 3}, peerstorage._leader_get())
         self.assertEqual({'s3': 3}, peerstorage.leader_get())
         self.assertEqual({'s3': 3}, l_settings)