From 8607833fddba074823de58f4f4bd2707bf90fc9c Mon Sep 17 00:00:00 2001
From: Daisuke Morita <morita.daisuke@lab.ntt.co.jp>
Date: Fri, 12 Dec 2014 10:37:33 +0900
Subject: [PATCH] Output account-reaper's logs for PolicyError

By some operational mistakes, policy settings can be inconsistent among
servers of swift cluster. Before this patch, if a policy setting of the
server where account-reaper daemon is running is different from other
servers, reap_object process failed without outputing error logs and
count up remaining objects.

Change-Id: Ic84fef7b49c77f0197f0bec4d41401686114d50e
Related-Bug: 1375384
---
 swift/account/reaper.py          | 13 ++++++++++--
 test/unit/account/test_reaper.py | 35 ++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/swift/account/reaper.py b/swift/account/reaper.py
index 0891d6fc59..dc32165e5d 100644
--- a/swift/account/reaper.py
+++ b/swift/account/reaper.py
@@ -31,7 +31,7 @@ from swift.common.ring import Ring
 from swift.common.utils import get_logger, whataremyips, ismount, \
     config_true_value, Timestamp
 from swift.common.daemon import Daemon
-from swift.common.storage_policy import POLICIES
+from swift.common.storage_policy import POLICIES, PolicyError
 
 
 class AccountReaper(Daemon):
@@ -353,6 +353,10 @@ class AccountReaper(Daemon):
                 break
             try:
                 policy_index = headers.get('X-Backend-Storage-Policy-Index', 0)
+                policy = POLICIES.get_by_index(policy_index)
+                if not policy:
+                    self.logger.error('ERROR: invalid storage policy index: %r'
+                                      % policy_index)
                 for obj in objects:
                     if isinstance(obj['name'], unicode):
                         obj['name'] = obj['name'].encode('utf8')
@@ -428,7 +432,12 @@ class AccountReaper(Daemon):
           of the container node dicts.
         """
         container_nodes = list(container_nodes)
-        ring = self.get_object_ring(policy_index)
+        try:
+            ring = self.get_object_ring(policy_index)
+        except PolicyError:
+            self.stats_objects_remaining += 1
+            self.logger.increment('objects_remaining')
+            return
         part, nodes = ring.get_nodes(account, container, obj)
         successes = 0
         failures = 0
diff --git a/test/unit/account/test_reaper.py b/test/unit/account/test_reaper.py
index deec2e3301..3450bff425 100644
--- a/test/unit/account/test_reaper.py
+++ b/test/unit/account/test_reaper.py
@@ -50,6 +50,9 @@ class FakeLogger(object):
     def info(self, msg, *args):
         self.msg = msg
 
+    def error(self, msg, *args):
+        self.msg = msg
+
     def timing_since(*args, **kwargs):
         pass
 
@@ -313,6 +316,13 @@ class TestReaper(unittest.TestCase):
         self.assertEqual(r.stats_objects_remaining, 1)
         self.assertEqual(r.stats_objects_possibly_remaining, 1)
 
+    def test_reap_object_non_exist_policy_index(self):
+        r = self.init_reaper({}, fakelogger=True)
+        r.reap_object('a', 'c', 'partition', cont_nodes, 'o', 2)
+        self.assertEqual(r.stats_objects_deleted, 0)
+        self.assertEqual(r.stats_objects_remaining, 1)
+        self.assertEqual(r.stats_objects_possibly_remaining, 0)
+
     @patch('swift.account.reaper.Ring',
            lambda *args, **kwargs: unit.FakeRing())
     def test_reap_container(self):
@@ -404,6 +414,31 @@ class TestReaper(unittest.TestCase):
         self.assertEqual(r.logger.inc['return_codes.4'], 3)
         self.assertEqual(r.stats_containers_remaining, 1)
 
+    @patch('swift.account.reaper.Ring',
+           lambda *args, **kwargs: unit.FakeRing())
+    def test_reap_container_non_exist_policy_index(self):
+        r = self.init_reaper({}, fakelogger=True)
+        with patch.multiple('swift.account.reaper',
+                            direct_get_container=DEFAULT,
+                            direct_delete_object=DEFAULT,
+                            direct_delete_container=DEFAULT) as mocks:
+            headers = {'X-Backend-Storage-Policy-Index': 2}
+            obj_listing = [{'name': 'o'}]
+
+            def fake_get_container(*args, **kwargs):
+                try:
+                    obj = obj_listing.pop(0)
+                except IndexError:
+                    obj_list = []
+                else:
+                    obj_list = [obj]
+                return headers, obj_list
+
+            mocks['direct_get_container'].side_effect = fake_get_container
+            r.reap_container('a', 'partition', acc_nodes, 'c')
+        self.assertEqual(r.logger.msg,
+                         'ERROR: invalid storage policy index: 2')
+
     def fake_reap_container(self, *args, **kwargs):
         self.called_amount += 1
         self.r.stats_containers_deleted = 1