From 57f420801c9e064c9ff8f1a4ab779e0b67260486 Mon Sep 17 00:00:00 2001 From: Erik Olof Gunnar Andersson Date: Tue, 21 May 2019 12:55:13 -0700 Subject: [PATCH] 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 --- designate/objects/recordset.py | 16 +++-- .../tests/unit/objects/test_recordset.py | 58 +++++++++++++++++-- ...fix-recordset-status-204e2747ef47d5ad.yaml | 13 +++++ 3 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/fix-recordset-status-204e2747ef47d5ad.yaml diff --git a/designate/objects/recordset.py b/designate/objects/recordset.py index 42b067e44..2011c735b 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 ab1c9ddbb..ddb005533 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 000000000..a40e704c6 --- /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