openstack/designate
Code Issues Proposed changes

Cleaned up coordinator and locking byte handling

Browse Source 
Change-Id: Ia112623c82b4b2e945048037844ea21f854e4399
This commit is contained in:
Erik Olof Gunnar Andersson 2024-01-21 10:10:26 -08:00
parent 097ffc6df1
commit 808f0c9198
5 changed files with 45 additions and 41 deletions
designate
common/decorators
lock.py
coordination.py
producer
service.py
tests/unit
central
test_lock_decorator.py
test_coordination.py

4
designate/common/decorators/lock.py

@ -82,11 +82,11 @@ def synchronized_zone(new_zone=False):
@functools.wraps(f)
def sync_wrapper(cls, *args, **kwargs):
if new_zone is True:
lock_name = 'create-new-zone'
lock_name = b'create-new-zone'
else:
zone_id = extract_zone_id(args, kwargs)
if zone_id:
lock_name = 'zone-%s' % zone_id
lock_name = f'zone-{zone_id}'.encode('ascii')
else:
raise Exception('Failed to determine zone id for '
'synchronized operation')

16
designate/coordination.py

@ -16,6 +16,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import math
from oslo_concurrency import lockutils
@ -26,6 +27,7 @@ import tooz.coordination
import designate.conf
from designate import utils
CONF = designate.conf.CONF
LOG = log.getLogger(__name__)
@ -37,10 +39,7 @@ def _retry_if_tooz_error(exception):
class Coordination:
def __init__(self, name, tg, grouping_enabled=False):
# NOTE(eandersson): Workaround until tooz handles the conversion.
if not isinstance(name, bytes):
name = name.encode('ascii')
self.name = name
self.name = name.encode('ascii')
self.tg = tg
self.coordination_id = None
self._grouping_enabled = grouping_enabled
@ -57,14 +56,13 @@ class Coordination:
def get_lock(self, name):
if self._coordinator:
# NOTE(eandersson): Workaround until tooz handles the conversion.
if not isinstance(name, bytes):
name = name.encode('ascii')
return self._coordinator.get_lock(name)
return lockutils.lock(name)
def start(self):
self.coordination_id = ":".join([CONF.host, utils.generate_uuid()])
self.coordination_id = (
':'.join([CONF.host, utils.generate_uuid()]).encode()
)
self._started = False
backend_url = CONF.coordination.backend_url
@ -75,7 +73,7 @@ class Coordination:
return
self._coordinator = tooz.coordination.get_coordinator(
backend_url, self.coordination_id.encode()
backend_url, self.coordination_id
)
while not self._coordinator.is_started:
self._coordinator.start(start_heart=True)

2
designate/producer/service.py

@ -64,7 +64,7 @@ class Service(service.RPCService):
self._partitioner = coordination.Partitioner(
self.coordination.coordinator, self.service_name,
self.coordination.coordination_id.encode(), range(0, 4096)
self.coordination.coordination_id, range(0, 4096)
)
self._partitioner.start()

18
designate/tests/unit/central/test_lock_decorator.py

@ -46,7 +46,8 @@ class CentralDecoratorTests(oslotest.base.BaseTestCase):
@lock.synchronized_zone()
def mock_get_zone(cls, current_index, zone_obj):
self.assertEqual(
{'zone-%s' % zone_obj.id}, cls.zone_lock_local._held
{f'zone-{zone_obj.id}'.encode('ascii')},
cls.zone_lock_local._held
)
if current_index % 3 == 0:
raise exceptions.ZoneNotFound()
@ -62,15 +63,18 @@ class CentralDecoratorTests(oslotest.base.BaseTestCase):
def test_synchronized_new_zone_with_recursion(self):
@lock.synchronized_zone(new_zone=True)
def mock_create_zone(cls, context):
self.assertEqual({'create-new-zone'}, cls.zone_lock_local._held)
self.assertEqual({b'create-new-zone'}, cls.zone_lock_local._held)
mock_create_record(
cls, context, zone.Zone(id=utils.generate_uuid())
)
@lock.synchronized_zone()
def mock_create_record(cls, context, zone_obj):
self.assertIn('zone-%s' % zone_obj.id, cls.zone_lock_local._held)
self.assertIn('create-new-zone', cls.zone_lock_local._held)
self.assertIn(
f'zone-{zone_obj.id}'.encode('ascii'),
cls.zone_lock_local._held
)
self.assertIn(b'create-new-zone', cls.zone_lock_local._held)
mock_create_zone(
self.service, self.context
@ -80,14 +84,16 @@ class CentralDecoratorTests(oslotest.base.BaseTestCase):
@lock.synchronized_zone()
def mock_create_record(cls, context, record_obj):
self.assertEqual(
{'zone-%s' % record_obj.zone_id}, cls.zone_lock_local._held
{f'zone-{record_obj.zone_id}'.encode('ascii')},
cls.zone_lock_local._held
)
mock_get_zone(cls, context, zone.Zone(id=record_obj.zone_id))
@lock.synchronized_zone()
def mock_get_zone(cls, context, zone_obj):
self.assertEqual(
{'zone-%s' % zone_obj.id}, cls.zone_lock_local._held
{f'zone-{zone_obj.id}'.encode('ascii')},
cls.zone_lock_local._held
)
mock_create_record(

46
designate/tests/unit/test_coordination.py

@ -14,6 +14,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import shutil
import tempfile
from unittest import mock
@ -34,7 +35,7 @@ class TestCoordination(oslotest.base.BaseTestCase):
def setUp(self):
super().setUp()
self.useFixture(cfg_fixture.Config(CONF))
self.name = 'coordination'
self.service_name = 'service_name'
self.tg = mock.Mock()
self.tempdir = tempfile.mkdtemp()
CONF.set_override('backend_url', "file://%s" % self.tempdir,
@ -48,33 +49,32 @@ class TestCoordination(oslotest.base.BaseTestCase):
)
def test_start(self):
service = coordination.Coordination(self.name, self.tg)
service = coordination.Coordination(self.service_name, self.tg)
service.start()
self.assertTrue(service.started)
service.stop()
def test_start_with_grouping_enabled(self):
service = coordination.Coordination(
self.name, self.tg, grouping_enabled=True
self.service_name, self.tg, grouping_enabled=True
)
service.start()
self.assertTrue(service.started)
self.assertIn(self.name.encode('utf-8'),
self.assertIn(service.name,
service.coordinator.get_groups().get())
self.assertIn(service.coordination_id.encode('utf-8'),
service.coordinator.get_members(
self.name.encode('utf-8')).get())
self.assertIn(service.coordination_id,
service.coordinator.get_members(service.name).get())
service.stop()
def test_stop(self):
service = coordination.Coordination(self.name, self.tg)
service = coordination.Coordination(self.service_name, self.tg)
service.start()
service.stop()
self.assertFalse(service.started)
def test_stop_with_grouping_enabled(self):
service = coordination.Coordination(
self.name, self.tg, grouping_enabled=True
self.service_name, self.tg, grouping_enabled=True
)
service.start()
service.stop()
@ -83,27 +83,27 @@ class TestCoordination(oslotest.base.BaseTestCase):
def test_start_no_coordination(self):
CONF.set_override('backend_url', None, group='coordination')
# self.config(backend_url=None, group="coordination")
service = coordination.Coordination(self.name, self.tg)
service = coordination.Coordination(self.service_name, self.tg)
service.start()
self.assertIsNone(service.coordinator)
def test_stop_no_coordination(self):
CONF.set_override('backend_url', None, group='coordination')
service = coordination.Coordination(self.name, self.tg)
service = coordination.Coordination(self.service_name, self.tg)
self.assertIsNone(service.coordinator)
service.start()
service.stop()
def test_get_lock(self):
service = coordination.Coordination(self.name, self.tg)
service = coordination.Coordination(self.service_name, self.tg)
service._coordinator = mock.Mock()
service.get_lock('lock')
service.get_lock(b'lock')
service._coordinator.get_lock.assert_called_with(b'lock')
def test_un_watchers(self):
service = coordination.Coordination(self.name, self.tg)
service = coordination.Coordination(self.service_name, self.tg)
service._started = True
service._coordinator = mock.Mock()
@ -112,7 +112,7 @@ class TestCoordination(oslotest.base.BaseTestCase):
service._coordinator.run_watchers.assert_called_with()
def test_run_watchers_not_started(self):
service = coordination.Coordination(self.name, self.tg)
service = coordination.Coordination(self.service_name, self.tg)
service._started = False
service._coordinator = mock.Mock()
@ -121,15 +121,15 @@ class TestCoordination(oslotest.base.BaseTestCase):
service._coordinator.run_watchers.assert_not_called()
def test_create_group(self):
service = coordination.Coordination(self.name, self.tg)
service = coordination.Coordination(self.service_name, self.tg)
service._coordinator = mock.Mock()
service._create_group()
service._coordinator.create_group.assert_called_with(b'coordination')
service._coordinator.create_group.assert_called_with(b'service_name')
def test_create_group_already_exists(self):
service = coordination.Coordination(self.name, self.tg)
service = coordination.Coordination(self.service_name, self.tg)
service._coordinator = mock.Mock()
service._coordinator.create_group.side_effect = (
tooz.coordination.GroupAlreadyExist('')
@ -137,18 +137,18 @@ class TestCoordination(oslotest.base.BaseTestCase):
service._create_group()
service._coordinator.create_group.assert_called_with(b'coordination')
service._coordinator.create_group.assert_called_with(b'service_name')
def test_disable_grouping(self):
service = coordination.Coordination(self.name, self.tg)
service = coordination.Coordination(self.service_name, self.tg)
service._coordinator = mock.Mock()
service._disable_grouping()
service._coordinator.leave_group.assert_called_with(b'coordination')
service._coordinator.leave_group.assert_called_with(b'service_name')
def test_disable_grouping_already_exists(self):
service = coordination.Coordination(self.name, self.tg)
service = coordination.Coordination(self.service_name, self.tg)
service._coordinator = mock.Mock()
service._coordinator.leave_group.side_effect = (
tooz.coordination.GroupNotCreated('')
@ -156,7 +156,7 @@ class TestCoordination(oslotest.base.BaseTestCase):
service._disable_grouping()
service._coordinator.leave_group.assert_called_with(b'coordination')
service._coordinator.leave_group.assert_called_with(b'service_name')
@mock.patch('tenacity.nap.time', mock.Mock())