diff --git a/designate/central/service.py b/designate/central/service.py index f06d736ba..5b156ea89 100644 --- a/designate/central/service.py +++ b/designate/central/service.py @@ -2894,5 +2894,13 @@ class Service(service.RPCService, service.Service): return self.storage.update_service_status(context, db_status) except exceptions.ServiceStatusNotFound: + LOG.info( + "Creating new service status entry for %(service_name)s " + "at %(hostname)s", + { + 'service_name': service_status.service_name, + 'hostname': service_status.hostname + } + ) return self.storage.create_service_status( context, service_status) diff --git a/designate/storage/impl_sqlalchemy/migrate_repo/versions/100_unique_service_status.py b/designate/storage/impl_sqlalchemy/migrate_repo/versions/100_unique_service_status.py new file mode 100644 index 000000000..fad493257 --- /dev/null +++ b/designate/storage/impl_sqlalchemy/migrate_repo/versions/100_unique_service_status.py @@ -0,0 +1,47 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# +"""Add Unique constraint on ('service_name', 'hostname') in the +service_statuses table for bug #1768824""" + +import sys + +from migrate.changeset.constraint import UniqueConstraint +from oslo_log import log as logging +from sqlalchemy import exc +from sqlalchemy.schema import MetaData +from sqlalchemy.schema import Table + +LOG = logging.getLogger() +EXPLANATION = """ +You need to manually remove duplicate entries from the database. + +The error message was: +%s +""" + + +def upgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + service_statuses_table = Table('service_statuses', meta, autoload=True) + + # Add UniqueConstraint based on service_name and hostname. + constraint = UniqueConstraint('service_name', 'hostname', + table=service_statuses_table, + name="unique_service_status") + try: + constraint.create() + except exc.IntegrityError as e: + LOG.error(EXPLANATION, e) + # Use sys.exit so we don't blow up with a huge trace + sys.exit(1) diff --git a/designate/tests/__init__.py b/designate/tests/__init__.py index 4e27a2eb6..429d90504 100644 --- a/designate/tests/__init__.py +++ b/designate/tests/__init__.py @@ -76,6 +76,13 @@ class TestCase(base.BaseTestCase): 'status': "UP", 'stats': {}, 'capabilities': {}, + }, { + 'id': 'c326f735-eecc-4968-969f-355a43c4ae27', + 'service_name': 'baz', + 'hostname': 'qux', + 'status': "UP", + 'stats': {}, + 'capabilities': {}, }] quota_fixtures = [{ diff --git a/designate/tests/test_central/test_service.py b/designate/tests/test_central/test_service.py index d2675bdfa..2b0e405cb 100644 --- a/designate/tests/test_central/test_service.py +++ b/designate/tests/test_central/test_service.py @@ -32,6 +32,7 @@ from oslo_messaging.notify import notifier from designate import exceptions from designate import objects from designate.mdns import rpcapi as mdns_api +from designate.tests import fixtures from designate.tests.test_central import CentralTestCase from designate.storage.impl_sqlalchemy import tables @@ -39,6 +40,11 @@ LOG = logging.getLogger(__name__) class CentralServiceTest(CentralTestCase): + def setUp(self): + super(CentralServiceTest, self).setUp() + self.stdlog = fixtures.StandardLogging() + self.useFixture(self.stdlog) + def test_stop(self): # Test stopping the service self.central_service.stop() @@ -3119,6 +3125,48 @@ class CentralServiceTest(CentralTestCase): self.assertEqual(zone_serial, new_zone_serial) + def test_create_new_service_status_entry(self): + values = self.get_service_status_fixture() + + service_status = self.central_service.update_service_status( + self.admin_context, objects.ServiceStatus.from_dict(values)) + + self.assertIn("Creating new service status entry for foo at bar", + self.stdlog.logger.output) + + # Make sure this was never updated. + self.assertIsNone(service_status.updated_at) + + def test_update_existing_service_status_entry(self): + values = self.get_service_status_fixture() + + new_service_status = objects.ServiceStatus.from_dict(values) + self.storage.create_service_status( + self.admin_context, new_service_status) + + service_status = self.central_service.update_service_status( + self.admin_context, objects.ServiceStatus.from_dict(values)) + + self.assertEqual(new_service_status.id, service_status.id) + + # Make sure the entry was updated. + self.assertIsNotNone(service_status.updated_at) + + def test_update_existing_service_status_entry_with_id_provided(self): + values = self.get_service_status_fixture(fixture=1) + + self.storage.create_service_status( + self.admin_context, objects.ServiceStatus.from_dict(values)) + + service_status = self.central_service.update_service_status( + self.admin_context, objects.ServiceStatus.from_dict(values)) + + self.assertEqual('c326f735-eecc-4968-969f-355a43c4ae27', + service_status.id) + + # Make sure the entry was updated. + self.assertIsNotNone(service_status.updated_at) + def test_create_zone_transfer_request(self): zone = self.create_zone() zone_transfer_request = self.create_zone_transfer_request(zone) diff --git a/designate/tests/test_storage/__init__.py b/designate/tests/test_storage/__init__.py index 21a2a9574..8f096b304 100644 --- a/designate/tests/test_storage/__init__.py +++ b/designate/tests/test_storage/__init__.py @@ -2882,6 +2882,16 @@ class StorageTestCase(object): uuid = '97f57960-f41b-4e93-8e22-8fd6c7e2c183' self.storage.delete_pool_also_notify(self.admin_context, uuid) + def test_create_service_status_duplicate(self): + values = self.get_service_status_fixture(fixture=0) + + self.storage.create_service_status( + self.admin_context, objects.ServiceStatus.from_dict(values)) + + with testtools.ExpectedException(exceptions.DuplicateServiceStatus): + self.storage.create_service_status( + self.admin_context, objects.ServiceStatus.from_dict(values)) + # Zone Transfer Accept tests def test_create_zone_transfer_request(self): zone = self.create_zone() diff --git a/designate/tests/unit/test_service_status.py b/designate/tests/unit/test_service_status.py index 2a001445b..6ad43942b 100644 --- a/designate/tests/unit/test_service_status.py +++ b/designate/tests/unit/test_service_status.py @@ -83,7 +83,7 @@ class RpcEmitterTest(oslotest.base.BaseTestCase): status_factory = mock.Mock(return_value=(status, stats, capabilities,)) emitter = service_status.RpcEmitter("svc", self.mock_tg, - status_factory=status_factory) + status_factory=status_factory) emitter.start() central = mock.Mock() diff --git a/releasenotes/notes/bug-1768824-service_statuses-constraint-7a30eb78dc63b86f.yaml b/releasenotes/notes/bug-1768824-service_statuses-constraint-7a30eb78dc63b86f.yaml new file mode 100644 index 000000000..5f62385dd --- /dev/null +++ b/releasenotes/notes/bug-1768824-service_statuses-constraint-7a30eb78dc63b86f.yaml @@ -0,0 +1,15 @@ +--- +upgrade: + - | + If there are duplicate service entries in the service_statuses table + and the db sync command fails, you may need to truncate the + service_statuses table. +fixes: + - | + Fixed `bug 1768824`_ which could cause the service_statuses table + to be flooded with duplicate service entries. + + We fixed this by introducing a new unique constraint to the + service_statuses table. + + .. _Bug 1768824: https://bugs.launchpad.net/designate/+bug/1768824