db: Migrate "service" APIs to enginefacade
Migrate service-related APIs from the legacy enginefacade to the modern context-based enginefacade. This one is a little tricky as it highlights two issues. * The models in 'manila.db.sqlalchemy.models' do not match those created by the migrations. This causes us issues since we apply the former rather than running migrations when creating our database via the DatabaseFixture, which means we're not actually testing against what our customers see. One gap is closed here in order to get newly added test working as expected but there's a larger issue that needs to be addressed at a later date. * We are using a pattern of "try to create and retrieve on failure" rather than "try to get and create on failure" when determining whether to create availability zone records. This works where each operation results in a new transaction but does not when the transaction is shared. We switch this. With these two issues addressed, the rest of the migration is relatively straightforward. Signed-off-by: Stephen Finucane <stephenfin@redhat.com> Change-Id: Icfe4b225e216dfda46366af1eb3820b193befa0f
This commit is contained in:
Stephen Finucane
parent
e1057ae66f
commit
83a7fdf010
@ -500,21 +500,22 @@ def share_resources_host_update(context, current_host, new_host):
|
||||
|
||||
|
||||
@require_admin_context
|
||||
@context_manager.writer
|
||||
def service_destroy(context, service_id):
|
||||
session = get_session()
|
||||
with session.begin():
|
||||
service_ref = service_get(context, service_id, session=session)
|
||||
service_ref.soft_delete(session)
|
||||
service_ref = _service_get(context, service_id)
|
||||
service_ref.soft_delete(context.session)
|
||||
|
||||
|
||||
@require_admin_context
|
||||
def service_get(context, service_id, session=None):
|
||||
result = (model_query(
|
||||
context,
|
||||
models.Service,
|
||||
session=session).
|
||||
filter_by(id=service_id).
|
||||
first())
|
||||
def _service_get(context, service_id):
|
||||
result = (
|
||||
model_query(
|
||||
context,
|
||||
models.Service,
|
||||
).filter_by(
|
||||
id=service_id,
|
||||
).first()
|
||||
)
|
||||
if not result:
|
||||
raise exception.ServiceNotFound(service_id=service_id)
|
||||
|
||||
@ -522,6 +523,13 @@ def service_get(context, service_id, session=None):
|
||||
|
||||
|
||||
@require_admin_context
|
||||
@context_manager.reader
|
||||
def service_get(context, service_id):
|
||||
return _service_get(context, service_id)
|
||||
|
||||
|
||||
@require_admin_context
|
||||
@context_manager.reader
|
||||
def service_get_all(context, disabled=None):
|
||||
query = model_query(context, models.Service)
|
||||
|
||||
@ -532,6 +540,7 @@ def service_get_all(context, disabled=None):
|
||||
|
||||
|
||||
@require_admin_context
|
||||
@context_manager.reader
|
||||
def service_get_all_by_topic(context, topic):
|
||||
return (model_query(
|
||||
context, models.Service, read_deleted="no").
|
||||
@ -541,6 +550,7 @@ def service_get_all_by_topic(context, topic):
|
||||
|
||||
|
||||
@require_admin_context
|
||||
@context_manager.reader
|
||||
def service_get_by_host_and_topic(context, host, topic):
|
||||
result = (model_query(
|
||||
context, models.Service, read_deleted="no").
|
||||
@ -554,39 +564,53 @@ def service_get_by_host_and_topic(context, host, topic):
|
||||
|
||||
|
||||
@require_admin_context
|
||||
def _service_get_all_topic_subquery(context, session, topic, subq, label):
|
||||
def _service_get_all_topic_subquery(context, topic, subq, label):
|
||||
sort_value = getattr(subq.c, label)
|
||||
return (model_query(context, models.Service,
|
||||
func.coalesce(sort_value, 0),
|
||||
session=session, read_deleted="no").
|
||||
filter_by(topic=topic).
|
||||
filter_by(disabled=False).
|
||||
outerjoin((subq, models.Service.host == subq.c.host)).
|
||||
order_by(sort_value).
|
||||
all())
|
||||
return (
|
||||
model_query(
|
||||
context, models.Service,
|
||||
func.coalesce(sort_value, 0),
|
||||
read_deleted="no",
|
||||
).filter_by(
|
||||
topic=topic,
|
||||
).filter_by(
|
||||
disabled=False,
|
||||
).outerjoin(
|
||||
(subq, models.Service.host == subq.c.host)
|
||||
).order_by(
|
||||
sort_value
|
||||
).all()
|
||||
)
|
||||
|
||||
|
||||
@require_admin_context
|
||||
@context_manager.reader
|
||||
def service_get_all_share_sorted(context):
|
||||
session = get_session()
|
||||
with session.begin():
|
||||
topic = CONF.share_topic
|
||||
label = 'share_gigabytes'
|
||||
subq = (model_query(context, models.Share,
|
||||
func.sum(models.Share.size).label(label),
|
||||
session=session, read_deleted="no").
|
||||
join(models.ShareInstance,
|
||||
models.ShareInstance.share_id == models.Share.id).
|
||||
group_by(models.ShareInstance.host).
|
||||
subquery())
|
||||
return _service_get_all_topic_subquery(context,
|
||||
session,
|
||||
topic,
|
||||
subq,
|
||||
label)
|
||||
topic = CONF.share_topic
|
||||
label = 'share_gigabytes'
|
||||
subq = (
|
||||
model_query(
|
||||
context,
|
||||
models.Share,
|
||||
func.sum(models.Share.size).label(label),
|
||||
read_deleted="no",
|
||||
).join(
|
||||
models.ShareInstance,
|
||||
models.ShareInstance.share_id == models.Share.id,
|
||||
).group_by(
|
||||
models.ShareInstance.host
|
||||
).subquery()
|
||||
)
|
||||
return _service_get_all_topic_subquery(
|
||||
context,
|
||||
topic,
|
||||
subq,
|
||||
label,
|
||||
)
|
||||
|
||||
|
||||
@require_admin_context
|
||||
@context_manager.reader
|
||||
def service_get_by_args(context, host, binary):
|
||||
result = (model_query(context, models.Service).
|
||||
filter_by(host=host).
|
||||
@ -600,32 +624,28 @@ def service_get_by_args(context, host, binary):
|
||||
|
||||
|
||||
@require_admin_context
|
||||
@context_manager.writer
|
||||
def service_create(context, values):
|
||||
session = get_session()
|
||||
|
||||
_ensure_availability_zone_exists(context, values, session)
|
||||
_ensure_availability_zone_exists(context, values)
|
||||
|
||||
service_ref = models.Service()
|
||||
service_ref.update(values)
|
||||
if not CONF.enable_new_services:
|
||||
service_ref.disabled = True
|
||||
|
||||
with session.begin():
|
||||
service_ref.save(session)
|
||||
return service_ref
|
||||
service_ref.save(context.session)
|
||||
return service_ref
|
||||
|
||||
|
||||
@require_admin_context
|
||||
@oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True)
|
||||
@context_manager.writer
|
||||
def service_update(context, service_id, values):
|
||||
session = get_session()
|
||||
_ensure_availability_zone_exists(context, values, strict=False)
|
||||
|
||||
_ensure_availability_zone_exists(context, values, session, strict=False)
|
||||
|
||||
with session.begin():
|
||||
service_ref = service_get(context, service_id, session=session)
|
||||
service_ref.update(values)
|
||||
service_ref.save(session=session)
|
||||
service_ref = _service_get(context, service_id)
|
||||
service_ref.update(values)
|
||||
service_ref.save(context.session)
|
||||
|
||||
|
||||
###################
|
||||
@ -5307,7 +5327,9 @@ def share_type_extra_specs_update_or_create(context, share_type_id, specs):
|
||||
return specs
|
||||
|
||||
|
||||
def _ensure_availability_zone_exists(context, values, session, strict=True):
|
||||
def _ensure_availability_zone_exists(
|
||||
context, values, session=None, *, strict=True,
|
||||
):
|
||||
az_name = values.pop('availability_zone', None)
|
||||
|
||||
if strict and not az_name:
|
||||
@ -5317,9 +5339,9 @@ def _ensure_availability_zone_exists(context, values, session, strict=True):
|
||||
return
|
||||
|
||||
if uuidutils.is_uuid_like(az_name):
|
||||
az_ref = availability_zone_get(context, az_name, session=session)
|
||||
az_ref = _availability_zone_get(context, az_name, session=session)
|
||||
else:
|
||||
az_ref = availability_zone_create_if_not_exist(
|
||||
az_ref = _availability_zone_create_if_not_exist(
|
||||
context, az_name, session=session)
|
||||
|
||||
values.update({'availability_zone_id': az_ref['id']})
|
||||
@ -5330,6 +5352,11 @@ def availability_zone_get(context, id_or_name, session=None):
|
||||
if session is None:
|
||||
session = get_session()
|
||||
|
||||
return _availability_zone_get(context, id_or_name, session=session)
|
||||
|
||||
|
||||
@require_context
|
||||
def _availability_zone_get(context, id_or_name, session=None):
|
||||
query = model_query(context, models.AvailabilityZone, session=session)
|
||||
|
||||
if uuidutils.is_uuid_like(id_or_name):
|
||||
@ -5345,21 +5372,23 @@ def availability_zone_get(context, id_or_name, session=None):
|
||||
return result
|
||||
|
||||
|
||||
# TODO(stephenfin): Remove the 'session' argument once all callers have been
|
||||
# converted
|
||||
@require_context
|
||||
@oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True)
|
||||
def availability_zone_create_if_not_exist(context, name, session=None):
|
||||
if session is None:
|
||||
session = get_session()
|
||||
|
||||
az = models.AvailabilityZone()
|
||||
az.update({'id': uuidutils.generate_uuid(), 'name': name})
|
||||
def _availability_zone_create_if_not_exist(context, name, session=None):
|
||||
try:
|
||||
with session.begin():
|
||||
az.save(session)
|
||||
# NOTE(u_glide): Do not catch specific exception here, because it depends
|
||||
# on concrete backend used by SqlAlchemy
|
||||
except Exception:
|
||||
return availability_zone_get(context, name, session=session)
|
||||
return _availability_zone_get(context, name, session=session)
|
||||
except exception.AvailabilityZoneNotFound:
|
||||
az = models.AvailabilityZone()
|
||||
az.update({'id': uuidutils.generate_uuid(), 'name': name})
|
||||
# TODO(stephenfin): Remove this branch once all callers have been
|
||||
# updated not to pass 'session'
|
||||
if session is not None:
|
||||
with session.begin():
|
||||
az.save(session)
|
||||
else:
|
||||
az.save(context.session)
|
||||
return az
|
||||
|
||||
|
||||
|
||||
@ -1202,6 +1202,9 @@ class DriverPrivateData(BASE, ManilaBase):
|
||||
class AvailabilityZone(BASE, ManilaBase):
|
||||
"""Represents a private data as key-value pairs for a driver."""
|
||||
__tablename__ = 'availability_zones'
|
||||
__table_args__ = (
|
||||
schema.UniqueConstraint('name', 'deleted', name='az_name_uc'),
|
||||
)
|
||||
id = Column(String(36), primary_key=True, nullable=False)
|
||||
deleted = Column(String(36), default='False')
|
||||
name = Column(String(255), nullable=False)
|
||||
|
||||
@ -72,6 +72,14 @@ class DatabaseFixture(fixtures.Fixture):
|
||||
self.engine = db_session.get_engine()
|
||||
self.engine.dispose()
|
||||
conn = self.engine.connect()
|
||||
# FIXME(stephenfin): This is an issue. We're not applying our
|
||||
# migrations on SQLite in-memory backends and because the model schemas
|
||||
# and migration schemas don't currently match exactly, we are not
|
||||
# testing against something resembling what our customers would see.
|
||||
# We should (a) start applying the migrations for all backends (which
|
||||
# will require reworking the migrations since SQLite doesn't support
|
||||
# ALTER fully, meaning batch mode must be used) and (b) get the two
|
||||
# different sets of schemas in sync and keep them in sync.
|
||||
if sql_connection == "sqlite://":
|
||||
self.setup_sqlite(db_migrate)
|
||||
else:
|
||||
|
||||
@ -3101,8 +3101,10 @@ class ShareNetworkSubnetDatabaseAPITestCase(BaseDatabaseAPITestCase):
|
||||
self.assertEqual(1, len(subnets))
|
||||
|
||||
def test_get_by_availability_zone_id(self):
|
||||
az = db_api.availability_zone_create_if_not_exist(self.fake_context,
|
||||
'fake_zone_id')
|
||||
with db_api.context_manager.writer.using(self.fake_context):
|
||||
az = db_api._availability_zone_create_if_not_exist(
|
||||
self.fake_context, 'fake_zone_id',
|
||||
)
|
||||
self.subnet_dict['availability_zone_id'] = az['id']
|
||||
db_api.share_network_subnet_create(self.fake_context, self.subnet_dict)
|
||||
|
||||
@ -3112,8 +3114,10 @@ class ShareNetworkSubnetDatabaseAPITestCase(BaseDatabaseAPITestCase):
|
||||
self._check_fields(expected=self.subnet_dict, actual=result[0])
|
||||
|
||||
def test_get_az_subnets(self):
|
||||
az = db_api.availability_zone_create_if_not_exist(self.fake_context,
|
||||
'fake_zone_id')
|
||||
with db_api.context_manager.writer.using(self.fake_context):
|
||||
az = db_api._availability_zone_create_if_not_exist(
|
||||
self.fake_context, 'fake_zone_id',
|
||||
)
|
||||
self.subnet_dict['availability_zone_id'] = az['id']
|
||||
db_api.share_network_subnet_create(self.fake_context, self.subnet_dict)
|
||||
|
||||
@ -3710,6 +3714,21 @@ class ServiceDatabaseAPITestCase(test.TestCase):
|
||||
self.assertEqual(az.id, service.availability_zone_id)
|
||||
self.assertSubDictMatch(self.service_data, service.to_dict())
|
||||
|
||||
def test_create__az_exists(self):
|
||||
|
||||
# there's no public AZ create method so we have to define one ourselves
|
||||
@db_api.context_manager.writer
|
||||
def availability_zone_create(context, name):
|
||||
return db_api._availability_zone_create_if_not_exist(
|
||||
context, name,
|
||||
)
|
||||
|
||||
az = availability_zone_create(self.ctxt, 'fake_zone')
|
||||
service = db_api.service_create(self.ctxt, self.service_data)
|
||||
|
||||
self.assertEqual(az.id, service.availability_zone_id)
|
||||
self.assertSubDictMatch(self.service_data, service.to_dict())
|
||||
|
||||
def test_update(self):
|
||||
az_name = 'fake_zone2'
|
||||
update_data = {"availability_zone": az_name}
|
||||
@ -3744,7 +3763,10 @@ class AvailabilityZonesDatabaseAPITestCase(test.TestCase):
|
||||
|
||||
def test_az_get(self):
|
||||
az_name = 'test_az'
|
||||
az = db_api.availability_zone_create_if_not_exist(self.ctxt, az_name)
|
||||
with db_api.context_manager.writer.using(self.ctxt):
|
||||
az = db_api._availability_zone_create_if_not_exist(
|
||||
self.ctxt, az_name
|
||||
)
|
||||
|
||||
az_by_id = db_api.availability_zone_get(self.ctxt, az['id'])
|
||||
az_by_name = db_api.availability_zone_get(self.ctxt, az_name)
|
||||
@ -3755,9 +3777,11 @@ class AvailabilityZonesDatabaseAPITestCase(test.TestCase):
|
||||
self.assertEqual(az['id'], az_by_name['id'])
|
||||
|
||||
def test_az_get_all(self):
|
||||
db_api.availability_zone_create_if_not_exist(self.ctxt, 'test1')
|
||||
db_api.availability_zone_create_if_not_exist(self.ctxt, 'test2')
|
||||
db_api.availability_zone_create_if_not_exist(self.ctxt, 'test3')
|
||||
with db_api.context_manager.writer.using(self.ctxt):
|
||||
db_api._availability_zone_create_if_not_exist(self.ctxt, 'test1')
|
||||
db_api._availability_zone_create_if_not_exist(self.ctxt, 'test2')
|
||||
db_api._availability_zone_create_if_not_exist(self.ctxt, 'test3')
|
||||
|
||||
db_api.service_create(self.ctxt, {'availability_zone': 'test2'})
|
||||
|
||||
actual_result = db_api.availability_zone_get_all(self.ctxt)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user