Fix bug in recordset status

This patch changes the status handling of record sets
to always report the appropriate status.

Closes-Bug: #1842994
Change-Id: Ic2f4f41a9a87440e3849f380ea5989e15f378619

3 files changed, 77 insertions, 10 deletions

diff --git a/designate/objects/recordset.py b/designate/objects/recordset.py
index 42b067e4..2011c735 100755
--- a/designate/objects/recordset.py
+++ b/designate/objects/recordset.py
@@ -69,14 +69,18 @@ class RecordSet(base.DesignateObject, base.DictObjectMixin,
 
     @property
     def status(self):
-        # Return the worst status in order of ERROR, PENDING, ACTIVE
-        status = 'ACTIVE'
+        # Return the worst status in order of ERROR, PENDING, ACTIVE, DELETED.
+        status = None
+        statuses = {
+            'ERROR': 0,
+            'PENDING': 1,
+            'ACTIVE': 2,
+            'DELETED': 3,
+        }
         for record in self.records:
-            if (record.status == 'ERROR') or \
-                    (record.status == 'PENDING' and status != 'ERROR') or \
-                    (status != 'PENDING'):
+            if not status or statuses[record.status] < statuses[status]:
                 status = record.status
-        return status
+        return status or 'ACTIVE'
 
     fields = {
         'shard': fields.IntegerFields(nullable=True, minimum=0, maximum=4095),
diff --git a/designate/tests/unit/objects/test_recordset.py b/designate/tests/unit/objects/test_recordset.py
index ab1c9ddb..ddb00553 100644
--- a/designate/tests/unit/objects/test_recordset.py
+++ b/designate/tests/unit/objects/test_recordset.py
@@ -14,7 +14,6 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 import itertools
-import unittest
 
 import mock
 import oslotest.base
@@ -111,16 +110,16 @@ class RecordSetTest(oslotest.base.BaseTestCase):
         )
         self.assertEqual('DELETE', record_set.action)
 
-    @unittest.expectedFailure  # bug
     def test_status_error(self):
-        statuses = ('ERROR', 'PENDING', 'ACTIVE')
-        for s1, s2, s3 in itertools.permutations(statuses):
+        statuses = ('ERROR', 'PENDING', 'ACTIVE', 'DELETED')
+        for s1, s2, s3, s4 in itertools.permutations(statuses):
             record_set = objects.RecordSet(
                 name='www.example.org.', type='A',
                 records=objects.RecordList(objects=[
                     objects.Record(data='192.0.2.1', status=s1),
                     objects.Record(data='192.0.2.2', status=s2),
                     objects.Record(data='192.0.2.3', status=s3),
+                    objects.Record(data='192.0.2.4', status=s4),
                 ])
             )
             self.assertEqual(record_set.status, 'ERROR')
@@ -163,6 +162,57 @@ class RecordSetTest(oslotest.base.BaseTestCase):
         )
         self.assertEqual('DELETED', record_set.status)
 
+    def test_status_many_expect_error(self):
+        rs = objects.RecordSet(
+            name='www.example.org.', type='A',
+            records=objects.RecordList(objects=[
+                objects.Record(data='192.0.2.2', status='ACTIVE'),
+                objects.Record(data='192.0.2.3', status='DELETED'),
+                objects.Record(data='192.0.2.4', status='DELETED'),
+                objects.Record(data='192.0.2.5', status='DELETED'),
+                objects.Record(data='192.0.2.6', status='ACTIVE'),
+                objects.Record(data='192.0.2.7', status='ACTIVE'),
+                objects.Record(data='192.0.2.8', status='ERROR'),
+                objects.Record(data='192.0.2.9', status='ACTIVE'),
+                objects.Record(data='192.0.2.10', status='ACTIVE'),
+            ])
+        )
+        self.assertEqual('ERROR', rs.status)
+
+    def test_status_many_expect_pending(self):
+        rs = objects.RecordSet(
+            name='www.example.org.', type='A',
+            records=objects.RecordList(objects=[
+                objects.Record(data='192.0.2.2', status='ACTIVE'),
+                objects.Record(data='192.0.2.3', status='DELETED'),
+                objects.Record(data='192.0.2.4', status='PENDING'),
+                objects.Record(data='192.0.2.5', status='DELETED'),
+                objects.Record(data='192.0.2.6', status='PENDING'),
+                objects.Record(data='192.0.2.7', status='ACTIVE'),
+                objects.Record(data='192.0.2.8', status='DELETED'),
+                objects.Record(data='192.0.2.9', status='PENDING'),
+                objects.Record(data='192.0.2.10', status='ACTIVE'),
+            ])
+        )
+        self.assertEqual('PENDING', rs.status)
+
+    def test_status_many_expect_active(self):
+        rs = objects.RecordSet(
+            name='www.example.org.', type='A',
+            records=objects.RecordList(objects=[
+                objects.Record(data='192.0.2.2', status='ACTIVE'),
+                objects.Record(data='192.0.2.3', status='DELETED'),
+                objects.Record(data='192.0.2.4', status='DELETED'),
+                objects.Record(data='192.0.2.5', status='DELETED'),
+                objects.Record(data='192.0.2.6', status='ACTIVE'),
+                objects.Record(data='192.0.2.7', status='ACTIVE'),
+                objects.Record(data='192.0.2.8', status='DELETED'),
+                objects.Record(data='192.0.2.9', status='ACTIVE'),
+                objects.Record(data='192.0.2.10', status='ACTIVE'),
+            ])
+        )
+        self.assertEqual('ACTIVE', rs.status)
+
     def test_validate(self):
         record_set = create_test_recordset()
         record_set.validate()
diff --git a/releasenotes/notes/fix-recordset-status-204e2747ef47d5ad.yaml b/releasenotes/notes/fix-recordset-status-204e2747ef47d5ad.yaml
new file mode 100644
index 00000000..a40e704c
--- /dev/null
+++ b/releasenotes/notes/fix-recordset-status-204e2747ef47d5ad.yaml
@@ -0,0 +1,13 @@
+---
+fixes:
+  - |
+    Fixed a bug with the recordset status implementation to make it report
+    its status more accurately.
+
+    A recordset will now always report its highest priority state when it
+    contains multiple records. The order of priority is,
+    `ERROR`, `PENDING`, `ACTIVE` and `DELETED`.
+
+    See `bug 1842994`_ for more information.
+
+    .. _bug 1842994: https://bugs.launchpad.net/designate/+bug/1842994