From 15fb464e5d0a4359116cf708a4c929c0e30403f5 Mon Sep 17 00:00:00 2001 From: Igor Malinovskiy Date: Wed, 26 Aug 2015 18:48:16 +0300 Subject: [PATCH] Add availability zones support Rework availability zones support which was inherited from Cinder: - Add public API extension - Preserve AZ if creating a share from a snapshot - Always set AZ in Share API or Share Manager - Update db schema and create db migration - Update appropriate unit tests APIImpact Partially-Implements: blueprint availability-zones Change-Id: Iea9fbc3fea5c0128772115c028989121f397e0c5 --- contrib/tempest/tempest/config_share.py | 2 +- etc/manila/policy.json | 1 + manila/api/contrib/availability_zones.py | 43 ++++++ manila/api/contrib/services.py | 2 +- manila/api/openstack/api_version_request.py | 3 +- .../openstack/rest_api_version_history.rst | 5 + manila/api/v1/shares.py | 11 +- manila/db/api.py | 12 ++ ...0bd302c1a6_add_availability_zones_table.py | 128 ++++++++++++++++++ manila/db/sqlalchemy/api.py | 118 +++++++++++++--- manila/db/sqlalchemy/models.py | 46 ++++++- manila/exception.py | 4 + .../filters/availability_zone_filter.py | 7 +- manila/scheduler/filter_scheduler.py | 5 +- manila/scheduler/simple.py | 25 +--- manila/service.py | 2 +- manila/share/api.py | 22 ++- manila/share/manager.py | 8 ++ .../api/contrib/test_availability_zones.py | 38 ++++++ manila/tests/api/contrib/test_services.py | 8 +- manila/tests/api/v1/test_shares.py | 16 +++ manila/tests/db/sqlalchemy/test_api.py | 79 ++++++++++- manila/tests/policy.json | 1 + .../tests/scheduler/test_filter_scheduler.py | 6 + manila/tests/scheduler/test_scheduler.py | 42 ++---- manila/tests/share/test_api.py | 24 +++- manila/tests/share/test_manager.py | 12 ++ manila/tests/test_service.py | 2 +- 28 files changed, 574 insertions(+), 98 deletions(-) create mode 100644 manila/api/contrib/availability_zones.py create mode 100644 manila/db/migrations/alembic/versions/1f0bd302c1a6_add_availability_zones_table.py create mode 100644 manila/tests/api/contrib/test_availability_zones.py diff --git a/contrib/tempest/tempest/config_share.py b/contrib/tempest/tempest/config_share.py index d9d53535e1..4d59b6bf09 100644 --- a/contrib/tempest/tempest/config_share.py +++ b/contrib/tempest/tempest/config_share.py @@ -36,7 +36,7 @@ ShareGroup = [ help="The minimum api microversion is configured to be the " "value of the minimum microversion supported by Manila."), cfg.StrOpt("max_api_microversion", - default="1.1", + default="1.2", help="The maximum api microversion is configured to be the " "value of the latest microversion supported by Manila."), cfg.StrOpt("region", diff --git a/etc/manila/policy.json b/etc/manila/policy.json index 7061726889..f2b8a4981e 100644 --- a/etc/manila/policy.json +++ b/etc/manila/policy.json @@ -37,6 +37,7 @@ "share_extension:snapshot_admin_actions:reset_status": "rule:admin_api", "share_extension:services": "rule:admin_api", + "share_extension:availability_zones": "", "share_extension:types_manage": "rule:admin_api", "share_extension:types_extra_specs": "rule:admin_api", diff --git a/manila/api/contrib/availability_zones.py b/manila/api/contrib/availability_zones.py new file mode 100644 index 0000000000..d5b5cf348e --- /dev/null +++ b/manila/api/contrib/availability_zones.py @@ -0,0 +1,43 @@ +# Copyright (c) 2013 OpenStack Foundation +# Copyright (c) 2015 Mirantis inc. +# +# 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. + +from manila.api import extensions +from manila.api.openstack import wsgi +from manila import db + +authorize = extensions.extension_authorizer('share', 'availability_zones') + + +class Controller(wsgi.Controller): + def index(self, req): + """Describe all known availability zones.""" + context = req.environ['manila.context'] + authorize(context) + azs = db.availability_zone_get_all(context) + return {'availability_zones': azs} + + +class Availability_zones(extensions.ExtensionDescriptor): + """Describe Availability Zones.""" + + name = 'AvailabilityZones' + alias = 'os-availability-zone' + updated = '2015-07-28T00:00:00+00:00' + + def get_resources(self): + controller = Controller() + res = extensions.ResourceExtension(Availability_zones.alias, + controller) + return [res] diff --git a/manila/api/contrib/services.py b/manila/api/contrib/services.py index 4df0980054..607903fef8 100644 --- a/manila/api/contrib/services.py +++ b/manila/api/contrib/services.py @@ -38,7 +38,7 @@ class ServiceController(object): 'id': service['id'], 'binary': service['binary'], 'host': service['host'], - 'zone': service['availability_zone'], + 'zone': service['availability_zone']['name'], 'status': 'disabled' if service['disabled'] else 'enabled', 'state': 'up' if utils.service_is_up(service) else 'down', 'updated_at': service['updated_at'], diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index d5d703c726..a70623dc33 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -46,6 +46,7 @@ REST_API_VERSION_HISTORY = """ * 1.0 - Initial version. Includes all V1 APIs and extensions in Kilo. * 1.1 - Versions API updated to reflect beginning of microversions epoch. + * 1.2 - Share create() doesn't ignore availability_zone field of share. """ @@ -53,7 +54,7 @@ REST_API_VERSION_HISTORY = """ # The default api version request is defined to be the # the minimum version of the API supported. _MIN_API_VERSION = "1.0" -_MAX_API_VERSION = "1.1" +_MAX_API_VERSION = "1.2" DEFAULT_API_VERSION = _MIN_API_VERSION diff --git a/manila/api/openstack/rest_api_version_history.rst b/manila/api/openstack/rest_api_version_history.rst index 2b79b690cd..b5e9502e2a 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -30,3 +30,8 @@ user documentation. The only API change in version 1.1 is versions, i.e. GET http://localhost:8786/, which now returns the minimum and current microversion values. + +1.2 +--- + Share create() method doesn't ignore availability_zone field of provided + share. diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py index 7e8076555f..dffb6e7e45 100644 --- a/manila/api/v1/shares.py +++ b/manila/api/v1/shares.py @@ -26,6 +26,7 @@ from webob import exc from manila.api import common from manila.api.openstack import wsgi from manila.api.views import shares as share_views +from manila import db from manila import exception from manila.i18n import _ from manila.i18n import _LI @@ -188,8 +189,16 @@ class ShareController(wsgi.Controller): {'share_proto': share_proto, 'size': size}) LOG.info(msg, context=context) + availability_zone = share.get('availability_zone') + + if availability_zone: + try: + db.availability_zone_get(context, availability_zone) + except exception.AvailabilityZoneNotFound as e: + raise exc.HTTPNotFound(explanation=six.text_type(e)) + kwargs = { - 'availability_zone': share.get('availability_zone'), + 'availability_zone': availability_zone, 'metadata': share.get('metadata'), 'is_public': share.get('is_public', False), } diff --git a/manila/db/api.py b/manila/db/api.py index 73bb7594d2..0eeb8663d2 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -830,3 +830,15 @@ def driver_private_data_update(context, host, entity_id, details, def driver_private_data_delete(context, host, entity_id, key=None): """Remove one, list or all key-value pairs for given host and entity_id.""" return IMPL.driver_private_data_delete(context, host, entity_id, key) + + +#################### + +def availability_zone_get(context, id_or_name): + """Get availability zone by name or id.""" + return IMPL.availability_zone_get(context, id_or_name) + + +def availability_zone_get_all(context): + """Get all active availability zones.""" + return IMPL.availability_zone_get_all(context) diff --git a/manila/db/migrations/alembic/versions/1f0bd302c1a6_add_availability_zones_table.py b/manila/db/migrations/alembic/versions/1f0bd302c1a6_add_availability_zones_table.py new file mode 100644 index 0000000000..afc93960dc --- /dev/null +++ b/manila/db/migrations/alembic/versions/1f0bd302c1a6_add_availability_zones_table.py @@ -0,0 +1,128 @@ +# 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_availability_zones_table + +Revision ID: 1f0bd302c1a6 +Revises: 579c267fbb4d +Create Date: 2015-07-24 12:09:36.008570 + +""" + +# revision identifiers, used by Alembic. +revision = '1f0bd302c1a6' +down_revision = '579c267fbb4d' + +from alembic import op +from oslo_utils import timeutils +from oslo_utils import uuidutils +from sqlalchemy import Column, DateTime, ForeignKey, String, UniqueConstraint + +from manila.db.migrations import utils + + +def collect_existing_az_from_services_table(connection, services_table, + az_table): + az_name_to_id_mapping = dict() + existing_services = [] + for service in connection.execute(services_table.select()): + service_id = uuidutils.generate_uuid() + az_name_to_id_mapping[service.availability_zone] = service_id + existing_services.append({ + 'created_at': timeutils.utcnow(), + 'id': service_id, + 'name': service.availability_zone + }) + + op.bulk_insert(az_table, existing_services) + + return az_name_to_id_mapping + + +def upgrade(): + connection = op.get_bind() + + # Create new AZ table and columns + availability_zones_table = op.create_table( + 'availability_zones', + Column('created_at', DateTime), + Column('updated_at', DateTime), + Column('deleted_at', DateTime), + Column('deleted', String(length=36), default='False'), + Column('id', String(length=36), primary_key=True, nullable=False), + Column('name', String(length=255)), + UniqueConstraint('name', 'deleted', name='az_name_uc'), + mysql_engine='InnoDB', + mysql_charset='utf8') + + for table_name, fk_name in (('services', 'service_az_id_fk'), + ('share_instances', 'si_az_id_fk')): + op.add_column( + table_name, + Column('availability_zone_id', String(36), + ForeignKey('availability_zones.id', name=fk_name)) + ) + + # Collect existing AZs from services table + services_table = utils.load_table('services', connection) + az_name_to_id_mapping = collect_existing_az_from_services_table( + connection, services_table, availability_zones_table) + + # Map string AZ names to ID's in target tables + set_az_id_in_table = lambda table, id, name: ( + op.execute( + table.update().where(table.c.availability_zone == name).values( + {'availability_zone_id': id}) + ) + ) + + share_instances_table = utils.load_table('share_instances', connection) + for name, id in az_name_to_id_mapping.items(): + for table_name in [services_table, share_instances_table]: + set_az_id_in_table(table_name, id, name) + + # Remove old AZ columns from tables + op.drop_column('services', 'availability_zone') + op.drop_column('share_instances', 'availability_zone') + + +def downgrade(): + connection = op.get_bind() + + # Create old AZ fields + op.add_column('services', Column('availability_zone', String(length=255))) + op.add_column('share_instances', + Column('availability_zone', String(length=255))) + + # Migrate data + az_table = utils.load_table('availability_zones', connection) + share_instances_table = utils.load_table('share_instances', connection) + services_table = utils.load_table('services', connection) + + for az in connection.execute(az_table.select()): + op.execute( + share_instances_table.update().where( + share_instances_table.c.availability_zone_id == az.id + ).values({'availability_zone': az.name}) + ) + op.execute( + services_table.update().where( + services_table.c.availability_zone_id == az.id + ).values({'availability_zone': az.name}) + ) + + # Remove AZ_id columns and AZ table + op.drop_constraint('service_az_id_fk', 'services', type_='foreignkey') + op.drop_column('services', 'availability_zone_id') + op.drop_constraint('si_az_id_fk', 'share_instances', type_='foreignkey') + op.drop_column('share_instances', 'availability_zone_id') + op.drop_table('availability_zones') diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index bf206f6a58..ccc24e28dc 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -410,12 +410,15 @@ def service_get_by_args(context, host, binary): @require_admin_context def service_create(context, values): + session = get_session() + + ensure_availability_zone_exists(context, values, session) + service_ref = models.Service() service_ref.update(values) if not CONF.enable_new_services: service_ref.disabled = True - session = get_session() with session.begin(): service_ref.save(session) return service_ref @@ -425,6 +428,9 @@ def service_create(context, values): @oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True) def service_update(context, service_id, values): session = get_session() + + 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) @@ -1138,6 +1144,7 @@ def _share_instance_create(context, share_id, values, session): def share_instance_update(context, share_instance_id, values, with_share_data=False): session = get_session() + ensure_availability_zone_exists(context, values, session, strict=False) with session.begin(): instance_ref = _share_instance_update( context, share_instance_id, values, session @@ -1273,10 +1280,13 @@ def share_create(context, values, create_share_instance=True): values = ensure_model_dict_has_id(values) values['share_metadata'] = _metadata_refs(values.get('metadata'), models.ShareMetadata) + session = get_session() share_ref = models.Share() share_instance_values = extract_share_instance_values(values) + ensure_availability_zone_exists(context, share_instance_values, session, + strict=False) share_ref.update(values) - session = get_session() + with session.begin(): share_ref.save(session=session) @@ -1308,10 +1318,14 @@ def share_data_get_for_project(context, project_id, user_id, session=None): @oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True) def share_update(context, share_id, values): session = get_session() + + share_instance_values = extract_share_instance_values(values) + ensure_availability_zone_exists(context, share_instance_values, session, + strict=False) + with session.begin(): share_ref = share_get(context, share_id, session=session) - share_instance_values = extract_share_instance_values(values) _share_instance_update(context, share_ref.instance['id'], share_instance_values, session=session) @@ -1384,24 +1398,26 @@ def _share_get_all_with_filters(context, project_id=None, share_server_id=None, models.ShareTypeExtraSpecs.value == v)) # Apply sorting - try: - attr = getattr(models.Share, sort_key) - except AttributeError: - try: - attr = getattr(models.ShareInstance, sort_key) - except AttributeError: - msg = _("Wrong sorting key provided - '%s'.") % sort_key - raise exception.InvalidInput(reason=msg) - if sort_dir.lower() == 'desc': - query = query.order_by(attr.desc()) - elif sort_dir.lower() == 'asc': - query = query.order_by(attr.asc()) - else: + if sort_dir.lower() not in ('desc', 'asc'): msg = _("Wrong sorting data provided: sort key is '%(sort_key)s' " "and sort direction is '%(sort_dir)s'.") % { "sort_key": sort_key, "sort_dir": sort_dir} raise exception.InvalidInput(reason=msg) + def apply_sorting(model, query): + sort_attr = getattr(model, sort_key) + sort_method = getattr(sort_attr, sort_dir.lower()) + return query.order_by(sort_method()) + + try: + query = apply_sorting(models.Share, query) + except AttributeError: + try: + query = apply_sorting(models.ShareInstance, query) + except AttributeError: + msg = _("Wrong sorting key provided - '%s'.") % sort_key + raise exception.InvalidInput(reason=msg) + # Returns list of shares that satisfy filters. query = query.all() return query @@ -2838,3 +2854,73 @@ def share_type_extra_specs_update_or_create(context, share_type_id, specs): spec_ref.save(session=session) return specs + + +def ensure_availability_zone_exists(context, values, session, strict=True): + az_name = values.pop('availability_zone', None) + + if strict and not az_name: + msg = _("Values dict should have 'availability_zone' field.") + raise ValueError(msg) + elif not az_name: + return + + if uuidutils.is_uuid_like(az_name): + az_ref = availability_zone_get(context, az_name, session=session) + else: + az_ref = availability_zone_create_if_not_exist( + context, az_name, session=session) + + values.update({'availability_zone_id': az_ref['id']}) + + +def availability_zone_get(context, id_or_name, session=None): + if session is None: + session = get_session() + + query = model_query(context, models.AvailabilityZone, session=session) + + if uuidutils.is_uuid_like(id_or_name): + query = query.filter_by(id=id_or_name) + else: + query = query.filter_by(name=id_or_name) + + result = query.first() + + if not result: + raise exception.AvailabilityZoneNotFound(id=id_or_name) + + return result + + +@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}) + 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 az + + +def availability_zone_get_all(context): + session = get_session() + + enabled_services = model_query( + context, models.Service, + models.Service.availability_zone_id, + session=session, + read_deleted="no" + ).filter_by(disabled=0).distinct() + + return model_query(context, models.AvailabilityZone, session=session, + read_deleted="no").filter( + models.AvailabilityZone.id.in_(enabled_services) + ).all() diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index 839eeb345e..4e391980c3 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -69,7 +69,20 @@ class Service(BASE, ManilaBase): topic = Column(String(255)) report_count = Column(Integer, nullable=False, default=0) disabled = Column(Boolean, default=False) - availability_zone = Column(String(255), default='manila') + availability_zone_id = Column(String(36), + ForeignKey('availability_zones.id'), + nullable=True) + + availability_zone = orm.relationship( + "AvailabilityZone", + lazy='immediate', + primaryjoin=( + 'and_(' + 'Service.availability_zone_id == ' + 'AvailabilityZone.id, ' + 'AvailabilityZone.deleted == \'False\')' + ) + ) class ManilaNode(BASE, ManilaBase): @@ -253,6 +266,7 @@ class Share(BASE, ManilaBase): class ShareInstance(BASE, ManilaBase): __tablename__ = 'share_instances' + _extra_keys = ['export_location', 'availability_zone'] _proxified_properties = ('name', 'user_id', 'project_id', 'size', 'display_name', 'display_description', 'snapshot_id', 'share_proto', 'share_type_id', @@ -267,6 +281,11 @@ class ShareInstance(BASE, ManilaBase): if len(self.export_locations) > 0: return self.export_locations[0]['path'] + @property + def availability_zone(self): + if self._availability_zone: + return self._availability_zone['name'] + id = Column(String(36), primary_key=True) share_id = Column(String(36), ForeignKey('shares.id')) deleted = Column(String(36), default='False') @@ -275,7 +294,21 @@ class ShareInstance(BASE, ManilaBase): scheduled_at = Column(DateTime) launched_at = Column(DateTime) terminated_at = Column(DateTime) - availability_zone = Column(String(255)) + + availability_zone_id = Column(String(36), + ForeignKey('availability_zones.id'), + nullable=True) + _availability_zone = orm.relationship( + "AvailabilityZone", + lazy='immediate', + foreign_keys=availability_zone_id, + primaryjoin=( + 'and_(' + 'ShareInstance.availability_zone_id == ' + 'AvailabilityZone.id, ' + 'AvailabilityZone.deleted == \'False\')' + ) + ) export_locations = orm.relationship( "ShareInstanceExportLocations", @@ -522,7 +555,6 @@ class ShareSnapshotInstance(BASE, ManilaBase): progress = Column(String(255)) share_instance = orm.relationship( ShareInstance, backref="snapshot_instances", - foreign_keys=share_instance_id, primaryjoin=( 'and_(' 'ShareSnapshotInstance.share_instance_id == ShareInstance.id,' @@ -674,6 +706,14 @@ class DriverPrivateData(BASE, ManilaBase): value = Column(String(1023), nullable=False) +class AvailabilityZone(BASE, ManilaBase): + """Represents a private data as key-value pairs for a driver.""" + __tablename__ = 'availability_zones' + id = Column(String(36), primary_key=True, nullable=False) + deleted = Column(String(36), default='False') + name = Column(String(255), nullable=False) + + def register_models(): """Register Models and create metadata. diff --git a/manila/exception.py b/manila/exception.py index 7ccdb8d399..82d142fc1c 100644 --- a/manila/exception.py +++ b/manila/exception.py @@ -194,6 +194,10 @@ class InUse(ManilaException): message = _("Resource is in use.") +class AvailabilityZoneNotFound(NotFound): + message = _("Availability zone %(id)s could not be found.") + + class ShareNetworkNotFound(NotFound): message = _("Share network %(share_network_id)s could not be found.") diff --git a/manila/openstack/common/scheduler/filters/availability_zone_filter.py b/manila/openstack/common/scheduler/filters/availability_zone_filter.py index b654310890..1b8d5f3b4a 100644 --- a/manila/openstack/common/scheduler/filters/availability_zone_filter.py +++ b/manila/openstack/common/scheduler/filters/availability_zone_filter.py @@ -25,8 +25,9 @@ class AvailabilityZoneFilter(filters.BaseHostFilter): def host_passes(self, host_state, filter_properties): spec = filter_properties.get('request_spec', {}) props = spec.get('resource_properties', {}) - availability_zone = props.get('availability_zone') + availability_zone_id = props.get('availability_zone_id') - if availability_zone: - return availability_zone == host_state.service['availability_zone'] + if availability_zone_id: + return (availability_zone_id == + host_state.service['availability_zone_id']) return True diff --git a/manila/scheduler/filter_scheduler.py b/manila/scheduler/filter_scheduler.py index e5bc73e691..0fde31fe91 100644 --- a/manila/scheduler/filter_scheduler.py +++ b/manila/scheduler/filter_scheduler.py @@ -240,7 +240,10 @@ class FilterScheduler(driver.Scheduler): Can be overridden in a subclass to add more data. """ shr = request_spec['share_properties'] + inst = request_spec['share_instance_properties'] filter_properties['size'] = shr['size'] - filter_properties['availability_zone'] = shr.get('availability_zone') + filter_properties['availability_zone_id'] = ( + inst.get('availability_zone_id') + ) filter_properties['user_id'] = shr.get('user_id') filter_properties['metadata'] = shr.get('metadata') diff --git a/manila/scheduler/simple.py b/manila/scheduler/simple.py index 0e7edf0579..51abc4d835 100644 --- a/manila/scheduler/simple.py +++ b/manila/scheduler/simple.py @@ -49,30 +49,15 @@ class SimpleScheduler(chance.ChanceScheduler): snapshot_id = request_spec.get('snapshot_id') share_properties = request_spec.get('share_properties') share_size = share_properties.get('size') - availability_zone = share_properties.get('availability_zone') - zone, host = None, None - if availability_zone: - zone, _x, host = availability_zone.partition(':') - if host and context.is_admin: - service = db.service_get_by_args(elevated, host, CONF.share_topic) - if not utils.service_is_up(service): - raise exception.WillNotSchedule(host=host) - updated_share = driver.share_update_db(context, share_id, host) - self.share_rpcapi.create_share_instance( - context, - updated_share.instance, - host, - request_spec, - None, - snapshot_id=snapshot_id - ) - return None + instance_properties = request_spec.get('share_instance_properties', {}) + availability_zone_id = instance_properties.get('availability_zone_id') results = db.service_get_all_share_sorted(elevated) - if zone: + if availability_zone_id: results = [(service_g, gigs) for (service_g, gigs) in results - if service_g['availability_zone'] == zone] + if (service_g['availability_zone_id'] + == availability_zone_id)] for result in results: (service, share_gigabytes) = result if share_gigabytes + share_size > CONF.max_gigabytes: diff --git a/manila/service.py b/manila/service.py index caa132e614..5e7ed5288c 100644 --- a/manila/service.py +++ b/manila/service.py @@ -239,7 +239,7 @@ class Service(service.Service): service_ref = db.service_get(ctxt, self.service_id) state_catalog['report_count'] = service_ref['report_count'] + 1 - if zone != service_ref['availability_zone']: + if zone != service_ref['availability_zone']['name']: state_catalog['availability_zone'] = zone db.service_update(ctxt, diff --git a/manila/share/api.py b/manila/share/api.py index a12f0c31a7..978b5a2d78 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -106,6 +106,7 @@ class API(base.Base): share_type_id = share_type['id'] if share_type else None else: source_share = self.db.share_get(context, snapshot['share_id']) + availability_zone = source_share['availability_zone'] if share_type is None: share_type_id = source_share['share_type_id'] else: @@ -157,9 +158,6 @@ class API(base.Base): 'd_consumed': _consumed('shares')}) raise exception.ShareLimitExceeded(allowed=quotas['shares']) - if availability_zone is None: - availability_zone = CONF.storage_availability_zone - try: is_public = strutils.bool_from_string(is_public, strict=True) except ValueError as e: @@ -169,7 +167,6 @@ class API(base.Base): 'user_id': context.user_id, 'project_id': context.project_id, 'snapshot_id': snapshot_id, - 'availability_zone': availability_zone, 'metadata': metadata, 'display_name': name, 'display_description': description, @@ -196,21 +193,31 @@ class API(base.Base): host = snapshot['share']['host'] self.create_instance(context, share, share_network_id=share_network_id, - host=host) + host=host, availability_zone=availability_zone) return share def create_instance(self, context, share, share_network_id=None, - host=None): + host=None, availability_zone=None): policy.check_policy(context, 'share', 'create') + availability_zone_id = None + if availability_zone: + availability_zone_id = self.db.availability_zone_get( + context, availability_zone).id + + # TODO(u_glide): Add here validation that provided share network + # doesn't conflict with provided availability_zone when Neutron + # will have AZ support. + share_instance = self.db.share_instance_create( context, share['id'], { 'share_network_id': share_network_id, 'status': constants.STATUS_CREATING, 'scheduled_at': timeutils.utcnow(), - 'host': host or '' + 'host': host if host else '', + 'availability_zone_id': availability_zone_id, } ) @@ -226,6 +233,7 @@ class API(base.Base): request_spec = { 'share_properties': share_dict, + 'share_instance_properties': share_instance.to_dict(), 'share_proto': share['share_proto'], 'share_id': share['id'], 'snapshot_id': share['snapshot_id'], diff --git a/manila/share/manager.py b/manila/share/manager.py index 51ed85685c..e568c3b8ca 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -366,6 +366,13 @@ class ShareManager(manager.SchedulerDependentManager): share_instance = self._get_share_instance(context, share_instance_id) share_network_id = share_instance.get('share_network_id', None) + if not share_instance['availability_zone']: + share_instance = self.db.share_instance_update( + context, share_instance_id, + {'availability_zone': CONF.storage_availability_zone}, + with_share_data=True + ) + if share_network_id and not self.driver.driver_handles_share_servers: self.db.share_instance_update( context, share_instance_id, {'status': constants.STATUS_ERROR}) @@ -474,6 +481,7 @@ class ShareManager(manager.SchedulerDependentManager): share_update.update({ 'status': constants.STATUS_AVAILABLE, 'launched_at': timeutils.utcnow(), + 'availability_zone': CONF.storage_availability_zone, }) # NOTE(vponomaryov): we should keep only those export locations diff --git a/manila/tests/api/contrib/test_availability_zones.py b/manila/tests/api/contrib/test_availability_zones.py new file mode 100644 index 0000000000..d4ad24edf4 --- /dev/null +++ b/manila/tests/api/contrib/test_availability_zones.py @@ -0,0 +1,38 @@ +# Copyright 2015 Mirantis Inc. +# All Rights Reserved. +# +# 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. + +import mock + +from manila.api.contrib import availability_zones +from manila import db +from manila import test +from manila.tests.api import fakes + + +class AvailabilityZonesApiTest(test.TestCase): + def setUp(self): + super(AvailabilityZonesApiTest, self).setUp() + self.controller = availability_zones.Controller() + + def test_index(self): + fake_az = [{'test': 'test'}] + self.mock_object(db, 'availability_zone_get_all', + mock.Mock(return_value=fake_az)) + req = fakes.HTTPRequest.blank('/v2/fake/types/1') + + actual_result = self.controller.index(req) + + self.assertDictMatch({'availability_zones': fake_az}, actual_result) + db.availability_zone_get_all.assert_called_once_with(mock.ANY) diff --git a/manila/tests/api/contrib/test_services.py b/manila/tests/api/contrib/test_services.py index a81dde1059..110cd633aa 100644 --- a/manila/tests/api/contrib/test_services.py +++ b/manila/tests/api/contrib/test_services.py @@ -32,7 +32,7 @@ fake_services_list = [ { 'binary': 'manila-scheduler', 'host': 'host1', - 'availability_zone': 'manila1', + 'availability_zone': {'name': 'manila1'}, 'id': 1, 'disabled': True, 'updated_at': datetime.datetime(2012, 10, 29, 13, 42, 2), @@ -41,7 +41,7 @@ fake_services_list = [ { 'binary': 'manila-share', 'host': 'host1', - 'availability_zone': 'manila1', + 'availability_zone': {'name': 'manila1'}, 'id': 2, 'disabled': True, 'updated_at': datetime.datetime(2012, 10, 29, 13, 42, 5), @@ -49,7 +49,7 @@ fake_services_list = [ { 'binary': 'manila-scheduler', 'host': 'host2', - 'availability_zone': 'manila2', + 'availability_zone': {'name': 'manila2'}, 'id': 3, 'disabled': False, 'updated_at': datetime.datetime(2012, 9, 19, 6, 55, 34), @@ -57,7 +57,7 @@ fake_services_list = [ { 'binary': 'manila-share', 'host': 'host2', - 'availability_zone': 'manila2', + 'availability_zone': {'name': 'manila2'}, 'id': 4, 'disabled': True, 'updated_at': datetime.datetime(2012, 9, 18, 8, 3, 38), diff --git a/manila/tests/api/v1/test_shares.py b/manila/tests/api/v1/test_shares.py index 11937b4c35..2a4069af54 100644 --- a/manila/tests/api/v1/test_shares.py +++ b/manila/tests/api/v1/test_shares.py @@ -24,6 +24,7 @@ from manila.api import common from manila.api.v1 import shares from manila.common import constants from manila import context +from manila import db from manila import exception from manila.share import api as share_api from manila.share import share_types @@ -40,6 +41,7 @@ class ShareApiTest(test.TestCase): def setUp(self): super(ShareApiTest, self).setUp() self.controller = shares.ShareController() + self.mock_object(db, 'availability_zone_get') self.mock_object(share_api.API, 'get_all', stubs.stub_get_all_shares) self.mock_object(share_api.API, 'get', @@ -315,6 +317,20 @@ class ShareApiTest(test.TestCase): req, body) + def test_share_create_invalid_availability_zone(self): + self.mock_object( + db, + 'availability_zone_get', + mock.Mock(side_effect=exception.AvailabilityZoneNotFound(id='id')) + ) + body = {"share": copy.deepcopy(self.share)} + + req = fakes.HTTPRequest.blank('/shares') + self.assertRaises(webob.exc.HTTPNotFound, + self.controller.create, + req, + body) + def test_share_show(self): req = fakes.HTTPRequest.blank('/shares/1') res_dict = self.controller.show(req, '1') diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index 3ab591385b..59736a692c 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -186,7 +186,7 @@ class ShareDatabaseAPITestCase(test.TestCase): self.assertRaises(exception.NotFound, db_api.share_get, self.ctxt, share['id']) - @ddt.data('host', 'availability_zone') + @ddt.data('host') def test_share_get_all_sort_by_share_instance_fields(self, sort_key): shares = [db_utils.create_share(**{sort_key: n, 'size': 1}) for n in ('test1', 'test2')] @@ -1038,3 +1038,80 @@ class ShareServerDatabaseAPITestCase(test.TestCase): db_api.share_server_delete(self.ctxt, server['id']) self.assertEqual(len(db_api.share_server_get_all(self.ctxt)), num_records - 1) + + +class ServiceDatabaseAPITestCase(test.TestCase): + + def setUp(self): + super(ServiceDatabaseAPITestCase, self).setUp() + self.ctxt = context.RequestContext(user_id='user_id', + project_id='project_id', + is_admin=True) + + self.service_data = {'host': "fake_host", + 'binary': "fake_binary", + 'topic': "fake_topic", + 'report_count': 0, + 'availability_zone': "fake_zone"} + + def test_create(self): + service = db_api.service_create(self.ctxt, self.service_data) + az = db_api.availability_zone_get(self.ctxt, "fake_zone") + + 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} + + service = db_api.service_create(self.ctxt, self.service_data) + db_api.service_update(self.ctxt, service['id'], update_data) + service = db_api.service_get(self.ctxt, service['id']) + + az = db_api.availability_zone_get(self.ctxt, az_name) + self.assertEqual(az.id, service.availability_zone_id) + valid_values = self.service_data + valid_values.update(update_data) + self.assertSubDictMatch(valid_values, service.to_dict()) + + +@ddt.ddt +class AvailabilityZonesDatabaseAPITestCase(test.TestCase): + + def setUp(self): + super(AvailabilityZonesDatabaseAPITestCase, self).setUp() + self.ctxt = context.RequestContext(user_id='user_id', + project_id='project_id', + is_admin=True) + + @ddt.data({'fake': 'fake'}, {}, {'fakeavailability_zone': 'fake'}, + {'availability_zone': None}, {'availability_zone': ''}) + def test_ensure_availability_zone_exists_invalid(self, test_values): + session = db_api.get_session() + + self.assertRaises(ValueError, db_api.ensure_availability_zone_exists, + self.ctxt, test_values, session) + + def test_az_get(self): + az_name = 'test_az' + 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) + + self.assertEqual(az_name, az_by_id['name']) + self.assertEqual(az_name, az_by_name['name']) + self.assertEqual(az['id'], az_by_id['id']) + 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') + db_api.service_create(self.ctxt, {'availability_zone': 'test2'}) + + actual_result = db_api.availability_zone_get_all(self.ctxt) + + self.assertEqual(1, len(actual_result)) + self.assertEqual('test2', actual_result[0]['name']) diff --git a/manila/tests/policy.json b/manila/tests/policy.json index a33e75f441..9a450fbc29 100644 --- a/manila/tests/policy.json +++ b/manila/tests/policy.json @@ -45,6 +45,7 @@ "share_extension:share_type_access:removeProjectAccess": "rule:admin_api", "share_extension:manage": "rule:admin_api", "share_extension:unmanage": "rule:admin_api", + "share_extension:availability_zones": "", "security_service:index": "", "security_service:get_all_security_services": "rule:admin_api", diff --git a/manila/tests/scheduler/test_filter_scheduler.py b/manila/tests/scheduler/test_filter_scheduler.py index 1dcb9f5274..dc396725c2 100644 --- a/manila/tests/scheduler/test_filter_scheduler.py +++ b/manila/tests/scheduler/test_filter_scheduler.py @@ -36,6 +36,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): fake_context = context.RequestContext('user', 'project') request_spec = { 'share_properties': {'project_id': 1, 'size': 1}, + 'share_instance_properties': {}, 'share_type': {'name': 'NFS'}, 'share_id': ['fake-id1'], } @@ -60,6 +61,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): fake_context = context.RequestContext('user', 'project') request_spec = { 'share_properties': {'project_id': 1, 'size': 1}, + 'share_instance_properties': {}, 'share_type': {'name': 'NFS'}, 'share_id': ['fake-id1'], } @@ -79,6 +81,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): request_spec = { 'share_type': {'name': 'NFS'}, 'share_properties': {'project_id': 1, 'size': 1}, + 'share_instance_properties': {}, } weighed_host = sched._schedule_share(fake_context, request_spec, {}) self.assertIsNotNone(weighed_host.obj) @@ -101,6 +104,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): request_spec = { 'share_type': {'name': 'iSCSI'}, 'share_properties': {'project_id': 1, 'size': 1}, + 'share_instance_properties': {}, } filter_properties = {} sched._schedule_share(self.context, request_spec, @@ -115,6 +119,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): request_spec = { 'share_type': {'name': 'iSCSI'}, 'share_properties': {'project_id': 1, 'size': 1}, + 'share_instance_properties': {}, } filter_properties = {} sched._schedule_share(self.context, request_spec, @@ -129,6 +134,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): request_spec = { 'share_type': {'name': 'iSCSI'}, 'share_properties': {'project_id': 1, 'size': 1}, + 'share_instance_properties': {}, } retry = dict(num_attempts=1) filter_properties = dict(retry=retry) diff --git a/manila/tests/scheduler/test_scheduler.py b/manila/tests/scheduler/test_scheduler.py index 21a4cef963..e7f1ccfb82 100644 --- a/manila/tests/scheduler/test_scheduler.py +++ b/manila/tests/scheduler/test_scheduler.py @@ -260,21 +260,24 @@ class SimpleSchedulerSharesTestCase(test.TestCase): share_id = 'fake' fake_share = { 'id': share_id, - 'availability_zone': 'fake:fake', 'size': 1, } + fake_instance = { + 'availability_zone_id': 'fake', + } fake_service_1 = { 'disabled': False, 'host': 'fake_host1', - 'availability_zone': 'fake', + 'availability_zone_id': 'fake', } fake_service_2 = { 'disabled': False, 'host': 'fake_host2', - 'availability_zone': 'super_fake', + 'availability_zone_id': 'super_fake', } fake_result = [(fake_service_1, 0), (fake_service_2, 1)] fake_request_spec = { 'share_id': share_id, 'share_properties': fake_share, + 'share_instance_properties': fake_instance, } self.mock_object(db, 'service_get_all_share_sorted', mock.Mock(return_value=fake_result)) @@ -298,41 +301,20 @@ class SimpleSchedulerSharesTestCase(test.TestCase): 'availability_zone': 'fake:fake', 'size': 1, } + fake_service = {'disabled': False, 'host': 'fake'} fake_request_spec = { 'share_id': share_id, 'share_properties': fake_share, } - self.mock_object(db, 'service_get_by_args', - mock.Mock(return_value='fake_service')) + self.mock_object(db, 'service_get_all_share_sorted', + mock.Mock(return_value=[(fake_service, 1)])) self.mock_object(driver, 'share_update_db', mock.Mock(return_value=db_utils.create_share())) self.driver.schedule_create_share(self.admin_context, fake_request_spec, {}) - utils.service_is_up.assert_called_once_with('fake_service') - db.service_get_by_args.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), 'fake', 'manila-share') + utils.service_is_up.assert_called_once_with(fake_service) + db.service_get_all_share_sorted.assert_called_once_with( + utils.IsAMatcher(context.RequestContext)) driver.share_update_db.assert_called_once_with( utils.IsAMatcher(context.RequestContext), share_id, 'fake') - - @mock.patch.object(utils, 'service_is_up', mock.Mock(return_value=False)) - def test_create_share_availability_zone_if_service_down(self): - share_id = 'fake' - fake_share = { - 'id': share_id, - 'availability_zone': 'fake:fake', - 'size': 1, - } - fake_request_spec = { - 'share_id': share_id, - 'share_properties': fake_share, - } - with mock.patch.object(db, 'service_get_by_args', - mock.Mock(return_value='fake_service')): - self.assertRaises(exception.WillNotSchedule, - self.driver.schedule_create_share, - self.admin_context, fake_request_spec, {}) - utils.service_is_up.assert_called_once_with('fake_service') - db.service_get_by_args.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), - 'fake', 'manila-share') diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 6c625b68ba..18577277bf 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -153,6 +153,10 @@ class ShareAPITestCase(test.TestCase): mock.Mock(return_value=share_metadata)) self.mock_object(db_api, 'share_type_get', mock.Mock(return_value=share_type)) + az_mock = mock.Mock() + type(az_mock.return_value).id = mock.PropertyMock( + return_value='fake_id') + self.mock_object(db_api, 'availability_zone_get', az_mock) self.mock_object(self.api.share_rpcapi, 'create_share_instance') self.mock_object(self.api.scheduler_rpcapi, 'create_share_instance') @@ -500,6 +504,7 @@ class ShareAPITestCase(test.TestCase): @ddt.data(True, False) def test_create_public_and_private_share(self, is_public): share, share_data = self._setup_create_mocks(is_public=is_public) + az = share_data.pop('availability_zone') self.api.create( self.context, @@ -507,7 +512,7 @@ class ShareAPITestCase(test.TestCase): share_data['size'], share_data['display_name'], share_data['display_description'], - availability_zone=share_data['availability_zone'] + availability_zone=az ) self.assertSubDictMatch(share_data, @@ -522,6 +527,7 @@ class ShareAPITestCase(test.TestCase): @ddt.data(*constants.SUPPORTED_SHARE_PROTOCOLS) def test_create_share_valid_protocol(self, proto): share, share_data = self._setup_create_mocks(protocol=proto) + az = share_data.pop('availability_zone') all_protos = ','.join( proto for proto in constants.SUPPORTED_SHARE_PROTOCOLS) @@ -531,7 +537,7 @@ class ShareAPITestCase(test.TestCase): self.context, proto, share_data['size'], share_data['display_name'], share_data['display_description'], - availability_zone=share_data['availability_zone']) + availability_zone=az) self.assertSubDictMatch(share_data, db_api.share_create.call_args[0][1]) @@ -603,10 +609,11 @@ class ShareAPITestCase(test.TestCase): reservation) db_api.share_delete.assert_called_once_with(self.context, share['id']) - def test_create_share_instance_with_host(self): + def test_create_share_instance_with_host_and_az(self): host, share, share_instance = self._setup_create_instance_mocks() - self.api.create_instance(self.context, share, host=host) + self.api.create_instance(self.context, share, host=host, + availability_zone='fake') db_api.share_instance_create.assert_called_once_with( self.context, share['id'], @@ -615,6 +622,7 @@ class ShareAPITestCase(test.TestCase): 'status': constants.STATUS_CREATING, 'scheduled_at': self.dt_utc, 'host': host, + 'availability_zone_id': 'fake_id', } ) db_api.share_metadata_get.assert_called_once_with(self.context, @@ -627,7 +635,7 @@ class ShareAPITestCase(test.TestCase): host, request_spec=mock.ANY, filter_properties={}, - snapshot_id=share['snapshot_id'] + snapshot_id=share['snapshot_id'], ) self.assertFalse( self.api.scheduler_rpcapi.create_share_instance.called) @@ -857,6 +865,7 @@ class ShareAPITestCase(test.TestCase): self._setup_create_from_snapshot_mocks( use_scheduler=use_scheduler, host=valid_host) ) + az = share_data.pop('availability_zone') self.api.create( self.context, @@ -865,7 +874,7 @@ class ShareAPITestCase(test.TestCase): share_data['display_name'], share_data['display_description'], snapshot=snapshot, - availability_zone=share_data['availability_zone'] + availability_zone=az ) self.assertEqual(0, share_types.get_share_type.call_count) @@ -873,7 +882,8 @@ class ShareAPITestCase(test.TestCase): db_api.share_create.call_args[0][1]) self.api.create_instance.assert_called_once_with( self.context, share, share_network_id=share['share_network_id'], - host=valid_host) + host=valid_host, + availability_zone=snapshot['share']['availability_zone']) share_api.policy.check_policy.assert_called_once_with( self.context, 'share', 'create') quota.QUOTAS.reserve.assert_called_once_with( diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index 6935158186..7494af2586 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -674,6 +674,18 @@ class ShareManagerTestCase(test.TestCase): utils.IsAMatcher(models.ShareInstance), share_server=None) + def test_create_share_instance_update_availability_zone(self): + share = db_utils.create_share(availability_zone=None) + share_id = share['id'] + + self.share_manager.create_share_instance( + self.context, share.instance['id']) + + actual_share = db.share_get(context.get_admin_context(), share_id) + self.assertIsNotNone(actual_share.availability_zone) + self.assertEqual(manager.CONF.storage_availability_zone, + actual_share.availability_zone) + def test_provide_share_server_for_share_incompatible_servers(self): fake_exception = exception.ManilaException("fake") fake_share_server = {'id': 'fake'} diff --git a/manila/tests/test_service.py b/manila/tests/test_service.py index df827fe954..c680aa91af 100644 --- a/manila/tests/test_service.py +++ b/manila/tests/test_service.py @@ -128,7 +128,7 @@ service_ref = { 'binary': binary, 'topic': topic, 'report_count': 0, - 'availability_zone': 'nova', + 'availability_zone': {'name': 'nova'}, 'id': 1, }