From e0c09499a68bb21fbcd27200dba49de6de336291 Mon Sep 17 00:00:00 2001 From: Manpreet Kaur Date: Sun, 13 Jun 2021 08:50:05 +0530 Subject: [PATCH] Fix migration for SQLAlchemy 1.4 This patch fixes a database migration for SQLAlchemy 1.4. In tacker repository following issues are encounter: 1. Replace deprecated SQLAlchemy "with_lockmode" method. The method was deprecated in SQLAlchemy 0.9 [1][2] and finally removed in version 1.4. This patch replaces "with_lockmode" with new method "with_for_update", which has no end-user impact. 2. The clause column IN, in the new version it requires parameters passed to operator either be a list of literal value or a tuple, or an empty list [3]. 3. Observe an argument error in 'update' query. Error: sqlalchemy.exc.ArgumentError: subject table for an INSERT, UPDATE or DELETE expected, got Column('id', Uuid(length=36), table=, primary_key=True, nullable=False, default=ColumnDefault()) 4. The INSERT statement requires values to add in table as a list, tuple or dictionary [4]. Error: sqlalchemy.exc.ArgumentError: mapping or sequence expected for parameters This patch creates a dictionary of fields to be insert in VnfLcmOpOccs table. 5. Query fails on applying filters. When applying query to list vnf package or vnf instance using apply_filters() method exported from sqlalchemy_filters, it failed. AttributeError: 'Query' object has no attribute '_join_entities' In SQLAlchemy 1.4 unification of "query" and "select" construct took place [5][6]. The issue observe in tacker is an open bug of SQLAlchemy [7]. This patch drops "sqlalchemy_filters" and add customize method to apply filters in query. Note: This patch address sqlalchemy errors, SAWarnings will be resolve in future patches. [1] https://docs.sqlalchemy.org/en/13/changelog/migration_09.html?highlight=with_lockmode#new-for-update-support-on-select-query [2] https://docs.sqlalchemy.org/en/13/orm/query.html#sqlalchemy.orm.query.Query.with_lockmode [3] https://docs.sqlalchemy.org/en/14/core/sqlelement.html#sqlalchemy.sql.expression.ColumnElement.in_ [4] https://docs.sqlalchemy.org/en/14/core/dml.html#dml-class-documentation-constructors [5] https://github.com/sqlalchemy/sqlalchemy/issues/5159 [6] https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/1599/ [7] https://github.com/juliotrigo/sqlalchemy-filters/issues/61 Co-Authored-By: Ayumu Ueha Change-Id: Ifd1ae2753c639f22fc1afa020222416fe79469ef --- lower-constraints.txt | 1 - requirements.txt | 1 - tacker/db/db_base.py | 2 +- tacker/db/nfvo/nfvo_db_plugin.py | 5 +- tacker/db/nfvo/ns_db.py | 2 +- tacker/db/nfvo/vnffg_db.py | 8 +-- tacker/db/vnfm/vnfm_db.py | 4 +- tacker/objects/common.py | 65 +++++++++++++++++++++++++ tacker/objects/vnf_instance.py | 4 +- tacker/objects/vnf_lcm_op_occs.py | 26 ++++++++-- tacker/objects/vnf_lcm_subscriptions.py | 4 +- tacker/objects/vnf_package.py | 12 +++-- 12 files changed, 111 insertions(+), 23 deletions(-) create mode 100644 tacker/objects/common.py diff --git a/lower-constraints.txt b/lower-constraints.txt index dfedd5f83..c3f51b7d0 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -129,7 +129,6 @@ rsa==3.4.2 setuptools==21.0.0 simplejson==3.13.2 snowballstemmer==1.2.1 -sqlalchemy-filters==0.10.0 sqlalchemy-migrate==0.11.0 SQLAlchemy==1.3.11 sqlparse==0.2.4 diff --git a/requirements.txt b/requirements.txt index b72a45715..735cad5bc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,7 +15,6 @@ keystonemiddleware>=4.17.0 # Apache-2.0 kombu>=4.3.0 # BSD netaddr>=0.7.18 # BSD SQLAlchemy>=1.3.11 # MIT -sqlalchemy-filters>=0.10.0 WebOb>=1.7.1 # MIT python-heatclient>=1.10.0 # Apache-2.0 python-keystoneclient>=3.8.0 # Apache-2.0 diff --git a/tacker/db/db_base.py b/tacker/db/db_base.py index ad2a84b95..4eaef2f09 100644 --- a/tacker/db/db_base.py +++ b/tacker/db/db_base.py @@ -141,7 +141,7 @@ class CommonDbMixin(object): for key, value in filters.items(): column = getattr(model, key, None) if column: - query = query.filter(column.in_(value)) + query = query.filter(column.in_([value])) model_hooks = self._model_query_hooks.get(model, {}) for _name, hooks in model_hooks.items(): result_filter = hooks.get('result_filters', None) diff --git a/tacker/db/nfvo/nfvo_db_plugin.py b/tacker/db/nfvo/nfvo_db_plugin.py index e95c90c63..3d1c65811 100644 --- a/tacker/db/nfvo/nfvo_db_plugin.py +++ b/tacker/db/nfvo/nfvo_db_plugin.py @@ -160,8 +160,7 @@ class NfvoPluginDb(nfvo.NFVOPluginBase, db_base.CommonDbMixin): {'placement_attr': vim.get('placement_attr')}) vim_auth_db = (self._model_query( context, nfvo_db.VimAuth).filter( - nfvo_db.VimAuth.vim_id == vim_id).with_lockmode( - 'update').one()) + nfvo_db.VimAuth.vim_id == vim_id).with_for_update().one()) except orm_exc.NoResultFound: raise nfvo.VimNotFoundException(vim_id=vim_id) vim_auth_db.update({'auth_cred': vim_cred, 'password': @@ -181,7 +180,7 @@ class NfvoPluginDb(nfvo.NFVOPluginBase, db_base.CommonDbMixin): with context.session.begin(subtransactions=True): try: vim_db = (self._model_query(context, nfvo_db.Vim).filter( - nfvo_db.Vim.id == vim_id).with_lockmode('update').one()) + nfvo_db.Vim.id == vim_id).with_for_update().one()) except orm_exc.NoResultFound: raise nfvo.VimNotFoundException(vim_id=vim_id) vim_db.update({'status': status, diff --git a/tacker/db/nfvo/ns_db.py b/tacker/db/nfvo/ns_db.py index 8eaf12c9b..9cc8a06dc 100644 --- a/tacker/db/nfvo/ns_db.py +++ b/tacker/db/nfvo/ns_db.py @@ -138,7 +138,7 @@ class NSPluginDb(network_service.NSPluginBase, db_base.CommonDbMixin): self._model_query(context, NS). filter(NS.id == ns_id). filter(NS.status.in_(current_statuses)). - with_lockmode('update').one()) + with_for_update().one()) except orm_exc.NoResultFound: raise network_service.NSNotFound(ns_id=ns_id) return ns_db diff --git a/tacker/db/nfvo/vnffg_db.py b/tacker/db/nfvo/vnffg_db.py index 476a4cee7..705cac435 100644 --- a/tacker/db/nfvo/vnffg_db.py +++ b/tacker/db/nfvo/vnffg_db.py @@ -1207,7 +1207,7 @@ class VnffgPluginDbMixin(vnffg.VNFFGPluginBase, db_base.CommonDbMixin): self._model_query(context, Vnffg). filter(Vnffg.id == vnffg_id). filter(Vnffg.status.in_(current_statuses)). - with_lockmode('update').one()) + with_for_update().one()) except orm_exc.NoResultFound: raise nfvo.VnffgNotFoundException(vnffg_id=vnffg_id) if vnffg_db.status == constants.PENDING_UPDATE: @@ -1221,7 +1221,7 @@ class VnffgPluginDbMixin(vnffg.VNFFGPluginBase, db_base.CommonDbMixin): self._model_query(context, VnffgNfp). filter(VnffgNfp.id == nfp_id). filter(VnffgNfp.status.in_(current_statuses)). - with_lockmode('update').one()) + with_for_update().one()) except orm_exc.NoResultFound: raise nfvo.NfpNotFoundException(nfp_id=nfp_id) if nfp_db.status == constants.PENDING_UPDATE: @@ -1235,7 +1235,7 @@ class VnffgPluginDbMixin(vnffg.VNFFGPluginBase, db_base.CommonDbMixin): self._model_query(context, VnffgChain). filter(VnffgChain.id == sfc_id). filter(VnffgChain.status.in_(current_statuses)). - with_lockmode('update').one()) + with_for_update().one()) except orm_exc.NoResultFound: raise nfvo.SfcNotFoundException(sfc_id=sfc_id) if sfc_db.status == constants.PENDING_UPDATE: @@ -1249,7 +1249,7 @@ class VnffgPluginDbMixin(vnffg.VNFFGPluginBase, db_base.CommonDbMixin): self._model_query(context, VnffgClassifier). filter(VnffgClassifier.id == fc_id). filter(VnffgClassifier.status.in_(current_statuses)). - with_lockmode('update').one()) + with_for_update().one()) except orm_exc.NoResultFound: raise nfvo.ClassifierNotFoundException(fc_id=fc_id) if fc_db.status == constants.PENDING_UPDATE: diff --git a/tacker/db/vnfm/vnfm_db.py b/tacker/db/vnfm/vnfm_db.py index f7acb9348..f7e6acc9f 100644 --- a/tacker/db/vnfm/vnfm_db.py +++ b/tacker/db/vnfm/vnfm_db.py @@ -491,7 +491,7 @@ class VNFMPluginDb(vnfm.VNFMPluginBase, db_base.CommonDbMixin): self._model_query(context, VNF). filter(VNF.id == vnf_id). filter(VNF.status.in_(current_statuses)). - with_lockmode('update').one()) + with_for_update().one()) except orm_exc.NoResultFound: raise vnfm.VNFNotFound(vnf_id=vnf_id) return vnf_db @@ -799,7 +799,7 @@ class VNFMPluginDb(vnfm.VNFMPluginBase, db_base.CommonDbMixin): self._model_query(context, VNF). filter(VNF.id == vnf_id). filter(~VNF.status.in_(exclude_status)). - with_lockmode('update').one()) + with_for_update().one()) except orm_exc.NoResultFound: LOG.warning('no vnf found %s', vnf_id) return False diff --git a/tacker/objects/common.py b/tacker/objects/common.py new file mode 100644 index 000000000..f5a05ce2c --- /dev/null +++ b/tacker/objects/common.py @@ -0,0 +1,65 @@ +# Copyright (C) 2021 NEC Corp +# 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. + +from tacker.db.db_sqlalchemy import models + + +def apply_filters(query, filters): + """Apply filters to a SQLAlchemy query. + + :param query: The query object to which we apply filters. + :param filters: A dict or an iterable of dicts, where each one includes + the necesary information to create a filter to be applied + to the query. There are single query filters, such as + filters = {'model': 'Foo', 'field': 'name', 'op': '==', + 'value': 'foo'}. And multiple query filters, such as + filters = {'and': [ + {'field': 'name', 'model': 'Foo', 'value': 'foo', + 'op': '=='}, + {'field': 'id', 'model': 'Bar', 'value': 'bar', + 'op': '=='} + ]} + """ + + def apply_filter(query, filter): + value = filter.get('value') + op = filter.get('op') + model = getattr(models, filter.get('model')) + column_attr = getattr(model, filter.get('field')) + + if 'in' == op: + query = query.filter(column_attr.in_(value)) + elif 'not_in' == op: + query = query.filter(~column_attr.in_(value)) + elif '!=' == op: + query = query.filter(column_attr != value) + elif '>' == op: + query = query.filter(column_attr > value) + elif '>=' == op: + query = query.filter(column_attr >= value) + elif '<' == op: + query = query.filter(column_attr < value) + elif '<=' == op: + query = query.filter(column_attr <= value) + elif '==' == op: + query = query.filter(column_attr == value) + return query + + if 'and' in filters: + for filter in filters.get('and'): + query = apply_filter(query, filter) + else: + query = apply_filter(query, filters) + return query diff --git a/tacker/objects/vnf_instance.py b/tacker/objects/vnf_instance.py index a496c1a77..4fccc5dc2 100644 --- a/tacker/objects/vnf_instance.py +++ b/tacker/objects/vnf_instance.py @@ -20,7 +20,6 @@ from oslo_utils import uuidutils from oslo_versionedobjects import base as ovoo_base from sqlalchemy import exc from sqlalchemy.orm import joinedload -from sqlalchemy_filters import apply_filters from tacker._i18n import _ from tacker.common import exceptions @@ -31,6 +30,7 @@ from tacker.db.db_sqlalchemy import models from tacker.db.vnfm import vnfm_db from tacker import objects from tacker.objects import base +from tacker.objects import common from tacker.objects import fields from tacker.objects import vnf_instantiated_info from tacker.objects import vnf_package as vnf_package_obj @@ -120,7 +120,7 @@ def _vnf_instance_list_by_filter(context, columns_to_join=None, query = query.options(joinedload(column)) if filters: - query = apply_filters(query, filters) + query = common.apply_filters(query, filters) return query.all() diff --git a/tacker/objects/vnf_lcm_op_occs.py b/tacker/objects/vnf_lcm_op_occs.py index bb566b859..19db410fb 100644 --- a/tacker/objects/vnf_lcm_op_occs.py +++ b/tacker/objects/vnf_lcm_op_occs.py @@ -18,7 +18,6 @@ from oslo_utils import timeutils from oslo_versionedobjects import base as ovoo_base from sqlalchemy import exc from sqlalchemy.orm import joinedload -from sqlalchemy_filters import apply_filters from tacker.common import exceptions from tacker.common import utils @@ -27,6 +26,7 @@ from tacker.db.db_sqlalchemy import api from tacker.db.db_sqlalchemy import models from tacker import objects from tacker.objects import base +from tacker.objects import common from tacker.objects import fields @@ -35,9 +35,29 @@ LOG = logging.getLogger(__name__) @db_api.context_manager.writer def _vnf_lcm_op_occ_create(context, values): + fields = { + 'id': values.id, + 'operation_state': values.operation_state, + 'state_entered_time': values.state_entered_time, + 'start_time': values.start_time, + 'vnf_instance_id': values.vnf_instance_id, + 'operation': values.operation, + 'is_automatic_invocation': values.is_automatic_invocation, + 'is_cancel_pending': values.is_cancel_pending, + 'error': values.error, + 'resource_changes': values.resource_changes, + 'changed_info': values.changed_info, + 'changed_ext_connectivity': values.changed_ext_connectivity, + 'error_point': values.error_point, + } + if 'grant_id' in values: + fields['grant_id'] = values.grant_id + if 'operation_params' in values: + fields['operation_params'] = values.operation_params + context.session.execute( models.VnfLcmOpOccs.__table__.insert(None), - values) + fields) @db_api.context_manager.writer @@ -125,7 +145,7 @@ def _vnf_lcm_op_occs_get_by_filters(context, read_deleted=None, read_deleted=read_deleted, project_only=True) if filters: - query = apply_filters(query, filters) + query = common.apply_filters(query, filters) return query.all() diff --git a/tacker/objects/vnf_lcm_subscriptions.py b/tacker/objects/vnf_lcm_subscriptions.py index 51f7fa7f2..8e4043be1 100644 --- a/tacker/objects/vnf_lcm_subscriptions.py +++ b/tacker/objects/vnf_lcm_subscriptions.py @@ -16,7 +16,6 @@ from oslo_utils import timeutils from oslo_versionedobjects import base as ovoo_base from sqlalchemy.orm import joinedload from sqlalchemy.sql import text -from sqlalchemy_filters import apply_filters from tacker.common import exceptions from tacker.common import utils @@ -27,6 +26,7 @@ from tacker.db.db_sqlalchemy import api from tacker.db.db_sqlalchemy import models from tacker import objects from tacker.objects import base +from tacker.objects import common from tacker.objects import fields _NO_DATA_SENTINEL = object() @@ -341,7 +341,7 @@ def _vnf_lcm_subscription_list_by_filters(context, converted_value = utils.str_to_bytes(filters['value']) filters.update({'value': converted_value}) - query = apply_filters(query, filters) + query = common.apply_filters(query, filters) if nextpage_opaque_marker: start_offset = CONF.vnf_lcm.subscription_num * nextpage_opaque_marker diff --git a/tacker/objects/vnf_package.py b/tacker/objects/vnf_package.py index 4f8002e50..32662c8a0 100644 --- a/tacker/objects/vnf_package.py +++ b/tacker/objects/vnf_package.py @@ -22,7 +22,6 @@ from oslo_utils import versionutils from oslo_versionedobjects import base as ovoo_base from sqlalchemy.orm import joinedload from sqlalchemy.sql import func -from sqlalchemy_filters import apply_filters from tacker._i18n import _ from tacker.common import exceptions @@ -32,6 +31,7 @@ from tacker.db.db_sqlalchemy import api from tacker.db.db_sqlalchemy import models from tacker import objects from tacker.objects import base +from tacker.objects import common from tacker.objects import fields from tacker.objects import vnf_artifact from tacker.objects import vnf_software_image @@ -185,7 +185,10 @@ def _vnf_package_list_by_filters(context, read_deleted=None, filters=None): if 'VnfPackageArtifactInfo' in filter_data: query = query.join(models.VnfPackageArtifactInfo) - query = apply_filters(query, filters) + if 'VnfPackageVnfd' in filter_data: + query = query.join(models.VnfPackageVnfd) + + query = common.apply_filters(query, filters) return query.all() @@ -232,7 +235,10 @@ def _destroy_vnf_package(context, package_uuid): software_images_query.subquery())).update( updated_values, synchronize_session=False) - software_images_query.update(updated_values, synchronize_session=False) + api.model_query(context, models.VnfSoftwareImage). \ + filter(models.VnfSoftwareImage.id.in_( + flavour_query.subquery())).update( + updated_values, synchronize_session=False) api.model_query(context, models.VnfPackageUserData). \ filter_by(package_uuid=package_uuid). \