Warn about using EC with isa_l_rs_vand and nparity >= 5

We *know* there are combinations that will prevent decoding, and we're
going to be increasingly aggressive about getting it out of clusters.

Partial-Bug: 1639691
Change-Id: I50159c9d19f2385d5f60112e9aaefa1a68098313

swift/common/storage_policy.py

@@ -12,8 +12,10 @@
 # limitations under the License.
 
 
+import logging
 import os
 import string
+import sys
 import textwrap
 import six
 from six.moves.configparser import ConfigParser
@@ -453,6 +455,26 @@ class ECStoragePolicy(BaseStoragePolicy):
             raise PolicyError('Invalid ec_object_segment_size %r' %
                               ec_segment_size, index=self.idx)
 
+        if self._ec_type == 'isa_l_rs_vand' and self._ec_nparity >= 5:
+            logger = logging.getLogger("swift.common.storage_policy")
+            if not logger.handlers:
+                # If nothing else, log to stderr
+                logger.addHandler(logging.StreamHandler(sys.__stderr__))
+            logger.warning(
+                'Storage policy %s uses an EC configuration known to harm '
+                'data durability. Any data in this policy should be migrated. '
+                'See https://bugs.launchpad.net/swift/+bug/1639691 for '
+                'more information.' % self.name)
+            if not is_deprecated:
+                # TODO: To fully close bug 1639691, uncomment the raise and
+                # removing the warning below. This will be in the Pike release
+                # at the earliest.
+                logger.warning(
+                    'In a future release, this will prevent services from '
+                    'starting unless the policy is marked as deprecated.')
+                # raise PolicyError('Storage policy %s MUST be deprecated' %
+                #                   self.name)
+
         # Initialize PyECLib EC backend
         try:
             self.pyeclib_driver = \

test/unit/common/test_storage_policy.py

@@ -12,7 +12,9 @@
 # limitations under the License.
 
 """ Tests for swift.common.storage_policies """
+import contextlib
 import six
+import logging
 import unittest
 import os
 import mock
@@ -20,6 +22,7 @@ from functools import partial
 from six.moves.configparser import ConfigParser
 from tempfile import NamedTemporaryFile
 from test.unit import patch_policies, FakeRing, temptree, DEFAULT_TEST_EC_TYPE
+import swift.common.storage_policy
 from swift.common.storage_policy import (
     StoragePolicyCollection, POLICIES, PolicyError, parse_storage_policies,
     reload_storage_policies, get_policy_string, split_policy_string,
@@ -30,6 +33,26 @@ from swift.common.exceptions import RingLoadError
 from pyeclib.ec_iface import ECDriver
 
 
+class CapturingHandler(logging.Handler):
+    def __init__(self):
+        super(CapturingHandler, self).__init__()
+        self._records = []
+
+    def emit(self, record):
+        self._records.append(record)
+
+
+@contextlib.contextmanager
+def capture_logging(log_name):
+    captured = CapturingHandler()
+    logger = logging.getLogger(log_name)
+    logger.addHandler(captured)
+    try:
+        yield captured._records
+    finally:
+        logger.removeHandler(captured)
+
+
 @BaseStoragePolicy.register('fake')
 class FakeStoragePolicy(BaseStoragePolicy):
     """
@@ -582,6 +605,91 @@ class TestStoragePolicies(unittest.TestCase):
             PolicyError, "must specify a storage policy section "
             "for policy index 0", parse_storage_policies, bad_conf)
 
+    @mock.patch.object(swift.common.storage_policy, 'VALID_EC_TYPES',
+                       ['isa_l_rs_vand', 'isa_l_rs_cauchy'])
+    @mock.patch('swift.common.storage_policy.ECDriver')
+    def test_known_bad_ec_config(self, mock_driver):
+        good_conf = self._conf("""
+        [storage-policy:0]
+        name = bad-policy
+        policy_type = erasure_coding
+        ec_type = isa_l_rs_cauchy
+        ec_num_data_fragments = 10
+        ec_num_parity_fragments = 5
+        """)
+
+        with capture_logging('swift.common.storage_policy') as records:
+            parse_storage_policies(good_conf)
+        mock_driver.assert_called_once()
+        mock_driver.reset_mock()
+        self.assertFalse([(r.levelname, r.message) for r in records])
+
+        good_conf = self._conf("""
+        [storage-policy:0]
+        name = bad-policy
+        policy_type = erasure_coding
+        ec_type = isa_l_rs_vand
+        ec_num_data_fragments = 10
+        ec_num_parity_fragments = 4
+        """)
+
+        with capture_logging('swift.common.storage_policy') as records:
+            parse_storage_policies(good_conf)
+        mock_driver.assert_called_once()
+        mock_driver.reset_mock()
+        self.assertFalse([(r.levelname, r.message) for r in records])
+
+        bad_conf = self._conf("""
+        [storage-policy:0]
+        name = bad-policy
+        policy_type = erasure_coding
+        ec_type = isa_l_rs_vand
+        ec_num_data_fragments = 10
+        ec_num_parity_fragments = 5
+        """)
+
+        with capture_logging('swift.common.storage_policy') as records:
+            parse_storage_policies(bad_conf)
+        mock_driver.assert_called_once()
+        mock_driver.reset_mock()
+        self.assertEqual([r.levelname for r in records],
+                         ['WARNING', 'WARNING'])
+        for msg in ('known to harm data durability',
+                    'Any data in this policy should be migrated',
+                    'https://bugs.launchpad.net/swift/+bug/1639691'):
+            self.assertIn(msg, records[0].message)
+        self.assertIn('In a future release, this will prevent services from '
+                      'starting', records[1].message)
+
+        slightly_less_bad_conf = self._conf("""
+        [storage-policy:0]
+        name = bad-policy
+        policy_type = erasure_coding
+        ec_type = isa_l_rs_vand
+        ec_num_data_fragments = 10
+        ec_num_parity_fragments = 5
+        deprecated = true
+
+        [storage-policy:1]
+        name = good-policy
+        policy_type = erasure_coding
+        ec_type = isa_l_rs_cauchy
+        ec_num_data_fragments = 10
+        ec_num_parity_fragments = 5
+        default = true
+        """)
+
+        with capture_logging('swift.common.storage_policy') as records:
+            parse_storage_policies(slightly_less_bad_conf)
+        self.assertEqual(2, mock_driver.call_count)
+        mock_driver.reset_mock()
+        self.assertEqual([r.levelname for r in records],
+                         ['WARNING'])
+        for msg in ('known to harm data durability',
+                    'Any data in this policy should be migrated',
+                    'https://bugs.launchpad.net/swift/+bug/1639691'):
+            self.assertIn(msg, records[0].message)
+
     def test_no_default(self):
         orig_conf = self._conf("""
         [storage-policy:0]