From f9729fb388b06672bdd05388d1d8047fb45abfaa Mon Sep 17 00:00:00 2001 From: Sripriya Date: Tue, 14 Jun 2016 21:36:54 -0700 Subject: [PATCH] Introduce uniqueness constraint on resource names Resource names are handled to be unique per tenant for all 3 resources vnfd, vnf and vim. Also existing duplicate names are updated to name-<12char. uudid> based on resource id's last 12 char. to handle the uniqueness constraint on 'name' column for all 3 resources. Change-Id: I689d7219db67892b855e1dc5c3698f9e1a67b408 Closes-Bug: #1475047 Closes-Bug: #1474993 --- ...e-names-vnf-vnfd-vim-832a23d77893ca01.yaml | 4 ++ tacker/common/exceptions.py | 4 ++ tacker/db/db_base.py | 12 ++++ .../0ae5b1ce3024_unique_constraint_name.py | 58 +++++++++++++++++++ .../alembic_migrations/versions/HEAD | 2 +- tacker/db/nfvo/nfvo_db.py | 12 +--- tacker/extensions/nfvo.py | 8 +-- tacker/nfvo/nfvo_plugin.py | 5 ++ tacker/tests/functional/vnfm/test_vnf.py | 12 ++-- .../tests/functional/vnfm/test_vnfm_param.py | 12 ++-- tacker/tests/unit/db/utils.py | 10 ++++ tacker/tests/unit/vm/nfvo/test_nfvo_plugin.py | 18 +++--- tacker/tests/unit/vm/test_plugin.py | 18 ++++++ tacker/vnfm/plugin.py | 6 ++ tacker/vnfm/vim_client.py | 14 ++--- 15 files changed, 154 insertions(+), 41 deletions(-) create mode 100644 releasenotes/notes/unique-names-vnf-vnfd-vim-832a23d77893ca01.yaml create mode 100644 tacker/db/migration/alembic_migrations/versions/0ae5b1ce3024_unique_constraint_name.py diff --git a/releasenotes/notes/unique-names-vnf-vnfd-vim-832a23d77893ca01.yaml b/releasenotes/notes/unique-names-vnf-vnfd-vim-832a23d77893ca01.yaml new file mode 100644 index 000000000..05611962c --- /dev/null +++ b/releasenotes/notes/unique-names-vnf-vnfd-vim-832a23d77893ca01.yaml @@ -0,0 +1,4 @@ +--- +features: + - Introduce uniqueness constraint on the name of Tacker resources such as + VNFD, VNF, VIM, etc. diff --git a/tacker/common/exceptions.py b/tacker/common/exceptions.py index bc9fa38f8..8d11c9820 100644 --- a/tacker/common/exceptions.py +++ b/tacker/common/exceptions.py @@ -257,3 +257,7 @@ class VnfPolicyActionInvalid(BadRequest): class VnfPolicyTypeInvalid(BadRequest): message = _("Invalid type %(type)s for policy %(policy)s, " "should be one of %(valid_types)s") + + +class DuplicateResourceName(TackerException): + message = _("%(resource)s with name %(name)s already exists") diff --git a/tacker/db/db_base.py b/tacker/db/db_base.py index 266985486..3cb20306a 100644 --- a/tacker/db/db_base.py +++ b/tacker/db/db_base.py @@ -15,12 +15,16 @@ import weakref +from oslo_log import log as logging from six import iteritems +from sqlalchemy.orm import exc as orm_exc from sqlalchemy import sql from tacker.common import exceptions as n_exc from tacker.db import sqlalchemyutils +LOG = logging.getLogger(__name__) + class CommonDbMixin(object): """Common methods used in core and service plugins.""" @@ -201,3 +205,11 @@ class CommonDbMixin(object): columns = [c.name for c in model.__table__.columns] return dict((k, v) for (k, v) in iteritems(data) if k in columns) + + def _get_by_name(self, context, model, name): + try: + query = self._model_query(context, model) + return query.filter(model.name == name).one() + except orm_exc.NoResultFound: + LOG.info(_("No result found for %(name)s in %(model)s table"), + {'name': name, 'model': model}) diff --git a/tacker/db/migration/alembic_migrations/versions/0ae5b1ce3024_unique_constraint_name.py b/tacker/db/migration/alembic_migrations/versions/0ae5b1ce3024_unique_constraint_name.py new file mode 100644 index 000000000..d648ff3d7 --- /dev/null +++ b/tacker/db/migration/alembic_migrations/versions/0ae5b1ce3024_unique_constraint_name.py @@ -0,0 +1,58 @@ +# Copyright 2016 OpenStack Foundation +# +# 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. +# + +"""unique_constraint_name + +Revision ID: 0ae5b1ce3024 +Revises: 507122918800 +Create Date: 2016-09-15 16:27:08.736673 + +""" + +# revision identifiers, used by Alembic. +revision = '0ae5b1ce3024' +down_revision = '507122918800' + +from alembic import op +import sqlalchemy as sa + + +def _migrate_duplicate_names(table): + + meta = sa.MetaData(bind=op.get_bind()) + t = sa.Table(table, meta, autoload=True) + + session = sa.orm.Session(bind=op.get_bind()) + with session.begin(subtransactions=True): + dup_names = session.query(t.c.name).group_by( + t.c.name).having(sa.func.count() > 1).all() + if dup_names: + for name in dup_names: + duplicate_obj_query = session.query(t).filter(t.c.name == name[ + 0]).all() + for dup_obj in duplicate_obj_query: + name = dup_obj.name[:242] if dup_obj.name > 242 else \ + dup_obj.name + new_name = '{0}-{1}'.format(name, dup_obj.id[-12:]) + session.execute(t.update().where( + t.c.id == dup_obj.id).values(name=new_name)) + session.commit() + + +def upgrade(active_plugins=None, options=None): + + _migrate_duplicate_names('vnf') + _migrate_duplicate_names('vnfd') + _migrate_duplicate_names('vims') diff --git a/tacker/db/migration/alembic_migrations/versions/HEAD b/tacker/db/migration/alembic_migrations/versions/HEAD index 1005058d1..6823db079 100644 --- a/tacker/db/migration/alembic_migrations/versions/HEAD +++ b/tacker/db/migration/alembic_migrations/versions/HEAD @@ -1 +1 @@ -507122918800 +0ae5b1ce3024 \ No newline at end of file diff --git a/tacker/db/nfvo/nfvo_db.py b/tacker/db/nfvo/nfvo_db.py index ceeccb765..967b754cc 100644 --- a/tacker/db/nfvo/nfvo_db.py +++ b/tacker/db/nfvo/nfvo_db.py @@ -223,16 +223,8 @@ class NfvoPluginDb(nfvo.NFVOPluginBase, db_base.CommonDbMixin): def get_vim_by_name(self, context, vim_name, fields=None, mask_password=True): vim_db = self._get_by_name(context, Vim, vim_name) - return self._make_vim_dict(vim_db, mask_password=mask_password) - - # Deprecated. Will be removed in Ocata release - def _get_by_name(self, context, model, name): - try: - query = self._model_query(context, model) - return query.filter(model.name == name).one() - except orm_exc.NoResultFound: - if issubclass(model, Vim): - raise + return self._make_vim_dict(vim_db, mask_password=mask_password + )if vim_db else None def _validate_default_vim(self, context, vim, vim_id=None): if not vim.get('is_default'): diff --git a/tacker/extensions/nfvo.py b/tacker/extensions/nfvo.py index e1e3601ed..1af5b556b 100644 --- a/tacker/extensions/nfvo.py +++ b/tacker/extensions/nfvo.py @@ -45,11 +45,9 @@ class VimDefaultNameNotDefined(exceptions.TackerException): " in tacker.conf") -# Deprecated. Will be removed in Ocata release -class VimDefaultIdException(exceptions.TackerException): - message = _("Default VIM name %(vim_name)s is invalid or there are " - "multiple VIM matches found. Please specify a valid default " - "VIM in tacker.conf") +class VimDefaultNameNotFound(exceptions.TackerException): + message = _("Default VIM name %(vim_name)s is invalid. Please specify a " + "valid default VIM name in tacker.conf") class VimDefaultDuplicateException(exceptions.TackerException): diff --git a/tacker/nfvo/nfvo_plugin.py b/tacker/nfvo/nfvo_plugin.py index 6dd16527b..cf78ba318 100644 --- a/tacker/nfvo/nfvo_plugin.py +++ b/tacker/nfvo/nfvo_plugin.py @@ -25,6 +25,7 @@ from oslo_utils import strutils from tacker._i18n import _ from tacker.common import driver_manager +from tacker.common import exceptions from tacker.common import log from tacker.common import utils from tacker import context as t_context @@ -36,6 +37,7 @@ from tacker.plugins.common import constants from tacker.vnfm.tosca import utils as toscautils from toscaparser import tosca_template + LOG = logging.getLogger(__name__) @@ -88,6 +90,9 @@ class NfvoPlugin(nfvo_db.NfvoPluginDb, vnffg_db.VnffgPluginDbMixin): LOG.debug(_('Create vim called with parameters %s'), strutils.mask_password(vim)) vim_obj = vim['vim'] + name = vim_obj['name'] + if self._get_by_name(context, nfvo_db.Vim, name): + raise exceptions.DuplicateResourceName(resource='VIM', name=name) vim_type = vim_obj['type'] vim_obj['id'] = str(uuid.uuid4()) vim_obj['status'] = 'PENDING' diff --git a/tacker/tests/functional/vnfm/test_vnf.py b/tacker/tests/functional/vnfm/test_vnf.py index 9501e9284..6e9fc793b 100644 --- a/tacker/tests/functional/vnfm/test_vnf.py +++ b/tacker/tests/functional/vnfm/test_vnf.py @@ -24,10 +24,9 @@ VNF_CIRROS_CREATE_TIMEOUT = 120 class VnfTestCreate(base.BaseTackerTest): - def _test_create_delete_vnf(self, vnf_name, vim_id=None): + def _test_create_delete_vnf(self, vnf_name, vnfd_name, vim_id=None): data = dict() data['tosca'] = read_file('sample_cirros_vnf_no_monitoring.yaml') - vnfd_name = 'sample_cirros_vnf_no_monitoring' toscal = data['tosca'] tosca_arg = {'vnfd': {'name': vnfd_name, 'attributes': {'vnfd': toscal}}} @@ -81,10 +80,13 @@ class VnfTestCreate(base.BaseTackerTest): def test_create_delete_vnf_with_default_vim(self): self._test_create_delete_vnf( - vnf_name='test_vnf_with_cirros_no_monitoring') + vnf_name='test_vnf_with_cirros_no_monitoring_default_vim', + vnfd_name='sample_cirros_vnf_no_monitoring_default_vim') def test_create_delete_vnf_with_vim_id(self): vim_list = self.client.list_vims() vim0_id = self.get_vim(vim_list, 'VIM0')['id'] - self._test_create_delete_vnf(vim_id=vim0_id, - vnf_name='test_vnf_with_cirros_with_default_vim_id') + self._test_create_delete_vnf( + vim_id=vim0_id, + vnf_name='test_vnf_with_cirros_vim_id', + vnfd_name='sample_cirros_vnf_no_monitoring_vim_id') diff --git a/tacker/tests/functional/vnfm/test_vnfm_param.py b/tacker/tests/functional/vnfm/test_vnfm_param.py index 8b6d887ec..e42144a54 100644 --- a/tacker/tests/functional/vnfm/test_vnfm_param.py +++ b/tacker/tests/functional/vnfm/test_vnfm_param.py @@ -21,9 +21,8 @@ from tacker.tests.utils import read_file class VnfmTestParam(base.BaseTackerTest): - def _test_vnfd_create(self, vnfd_file): + def _test_vnfd_create(self, vnfd_file, vnfd_name): yaml_input = read_file(vnfd_file) - vnfd_name = 'sample_cirros_vnf' # TODO(anyone) remove this condition check once old templates # are deprecated if "tosca_definitions_version" in yaml_input: @@ -104,8 +103,9 @@ class VnfmTestParam(base.BaseTackerTest): assert True, "Vnf Delete success" + str(vfn_d) + str(Exception) def test_vnf_param(self): + vnfd_name = 'sample_cirros_vnfd_old_template' vnfd_instance = self._test_vnfd_create( - 'sample_cirros_vnf_param.yaml') + 'sample_cirros_vnf_param.yaml', vnfd_name) values_str = read_file('sample_cirros_vnf_values.yaml') vnf_instance, param_values_dict = self._test_vnf_create(vnfd_instance, 'test_vnf_with_parameters', @@ -127,13 +127,15 @@ class VnfmTestParam(base.BaseTackerTest): self.addCleanup(self.client.delete_vnfd, vnfd_instance['vnfd']['id']) def test_vnfd_param_tosca_template(self): + vnfd_name = 'sample_cirros_vnfd_tosca' vnfd_instance = self._test_vnfd_create( - 'sample-tosca-vnfd-param.yaml') + 'sample-tosca-vnfd-param.yaml', vnfd_name) self._test_vnfd_delete(vnfd_instance) def test_vnf_param_tosca_template(self): + vnfd_name = 'cirros_vnfd_tosca_param' vnfd_instance = self._test_vnfd_create( - 'sample-tosca-vnfd-param.yaml') + 'sample-tosca-vnfd-param.yaml', vnfd_name) values_str = read_file('sample-tosca-vnf-values.yaml') values_dict = yaml.safe_load(values_str) vnf_instance, param_values_dict = self._test_vnf_create(vnfd_instance, diff --git a/tacker/tests/unit/db/utils.py b/tacker/tests/unit/db/utils.py index 1d42b7496..5bcc02b1e 100644 --- a/tacker/tests/unit/db/utils.py +++ b/tacker/tests/unit/db/utils.py @@ -156,6 +156,16 @@ def get_dummy_device_obj_userdata_attr(): 'description': u"Parameterized VNF descriptor"} +def get_vim_obj(): + return {'vim': {'type': 'openstack', 'auth_url': + 'http://localhost:5000', 'vim_project': {'name': + 'test_project'}, 'auth_cred': {'username': 'test_user', + 'password': + 'test_password'}, + 'name': 'VIM0', + 'tenant_id': 'test-project'}} + + def get_vim_auth_obj(): return {'username': 'test_user', 'password': 'test_password', diff --git a/tacker/tests/unit/vm/nfvo/test_nfvo_plugin.py b/tacker/tests/unit/vm/nfvo/test_nfvo_plugin.py index b6aedfff1..25481c1a6 100644 --- a/tacker/tests/unit/vm/nfvo/test_nfvo_plugin.py +++ b/tacker/tests/unit/vm/nfvo/test_nfvo_plugin.py @@ -18,6 +18,8 @@ import uuid from mock import patch + +from tacker.common import exceptions from tacker import context from tacker.db.common_services import common_services_db from tacker.db.nfvo import nfvo_db @@ -173,13 +175,7 @@ class TestNfvoPlugin(db_base.SqlTestCase): session.flush() def test_create_vim(self): - vim_dict = {'vim': {'type': 'openstack', 'auth_url': - 'http://localhost:5000', 'vim_project': {'name': - 'test_project'}, 'auth_cred': {'username': 'test_user', - 'password': - 'test_password'}, - 'name': 'VIM0', - 'tenant_id': 'test-project'}} + vim_dict = utils.get_vim_obj() vim_type = 'openstack' res = self.nfvo_plugin.create_vim(self.context, vim_dict) self._cos_db_plugin.create_event.assert_any_call( @@ -201,6 +197,14 @@ class TestNfvoPlugin(db_base.SqlTestCase): self.assertIn('created_at', res) self.assertIn('updated_at', res) + def test_create_vim_duplicate_name(self): + self._insert_dummy_vim() + vim_dict = utils.get_vim_obj() + vim_dict['vim']['name'] = 'fake_vim' + self.assertRaises(exceptions.DuplicateResourceName, + self.nfvo_plugin.create_vim, + self.context, vim_dict) + def test_delete_vim(self): self._insert_dummy_vim() vim_type = 'openstack' diff --git a/tacker/tests/unit/vm/test_plugin.py b/tacker/tests/unit/vm/test_plugin.py index 64c7ed7f6..0b930f72f 100644 --- a/tacker/tests/unit/vm/test_plugin.py +++ b/tacker/tests/unit/vm/test_plugin.py @@ -18,6 +18,7 @@ import uuid import mock import yaml +from tacker.common import exceptions from tacker import context from tacker.db.common_services import common_services_db from tacker.db.nfvo import nfvo_db @@ -192,6 +193,14 @@ class TestVNFMPlugin(db_base.SqlTestCase): self.vnfm_plugin.create_vnfd, self.context, vnfd_obj) + def test_create_vnfd_duplicate_name(self): + self._insert_dummy_device_template() + vnfd_obj = utils.get_dummy_vnfd_obj() + vnfd_obj['vnfd']['name'] = 'fake_template' + self.assertRaises(exceptions.DuplicateResourceName, + self.vnfm_plugin.create_vnfd, + self.context, vnfd_obj) + def test_create_vnf(self): self._insert_dummy_device_template() vnf_obj = utils.get_dummy_vnf_obj() @@ -232,6 +241,15 @@ class TestVNFMPlugin(db_base.SqlTestCase): self.assertIn('type', resources) self.assertIn('id', resources) + def test_create_vnf_duplicate_name(self): + self._insert_dummy_device_template() + self._insert_dummy_device() + vnf_obj = utils.get_dummy_vnf_obj() + vnf_obj['vnf']['name'] = 'fake_device' + self.assertRaises(exceptions.DuplicateResourceName, + self.vnfm_plugin.create_vnf, + self.context, vnf_obj) + def test_delete_vnf(self): self._insert_dummy_device_template() dummy_device_obj = self._insert_dummy_device() diff --git a/tacker/vnfm/plugin.py b/tacker/vnfm/plugin.py index 19a5809fa..8448e429d 100644 --- a/tacker/vnfm/plugin.py +++ b/tacker/vnfm/plugin.py @@ -157,6 +157,9 @@ class VNFMPlugin(vnfm_db.VNFMPluginDb, VNFMMgmtMixin): " will be removed in Ocata. infra_driver will be automatically" " derived from target vim type. mgmt_driver will be derived " "from TOSCA template values.") + name = vnfd_data['name'] + if self._get_by_name(context, vnfm_db.VNFD, name): + raise exceptions.DuplicateResourceName(resource='VNFD', name=name) service_types = vnfd_data.get('service_types') if not attributes.is_attr_set(service_types): @@ -344,6 +347,9 @@ class VNFMPlugin(vnfm_db.VNFMPluginDb, VNFMMgmtMixin): def create_vnf(self, context, vnf): vnf_info = vnf['vnf'] + name = vnf_info['name'] + if self._get_by_name(context, vnfm_db.VNF, name): + raise exceptions.DuplicateResourceName(resource='VNF', name=name) vnf_attributes = vnf_info['attributes'] if vnf_attributes.get('param_values'): param = vnf_attributes['param_values'] diff --git a/tacker/vnfm/vim_client.py b/tacker/vnfm/vim_client.py index 815ce2c9d..7041b0c64 100644 --- a/tacker/vnfm/vim_client.py +++ b/tacker/vnfm/vim_client.py @@ -53,7 +53,7 @@ class VimClient(object): if not vim_id: LOG.debug(_('VIM id not provided. Attempting to find default ' - 'VIM id')) + 'VIM information')) try: vim_info = nfvo_plugin.get_default_vim(context) except Exception: @@ -66,8 +66,7 @@ class VimClient(object): 'default-vim in tacker.conf is deprecated and will be ' 'removed in Newton cycle') vim_info = self._get_default_vim_by_name(context, - nfvo_plugin, - vim_name) + nfvo_plugin, vim_name) else: try: vim_info = nfvo_plugin.get_vim(context, vim_id, @@ -91,11 +90,10 @@ class VimClient(object): # Deprecated. Will be removed in Ocata release def _get_default_vim_by_name(self, context, plugin, vim_name): - try: - vim_info = plugin.get_vim_by_name(context, vim_name, - mask_password=False) - except Exception: - raise nfvo.VimDefaultIdException(vim_name=vim_name) + vim_info = plugin.get_vim_by_name(context, vim_name, + mask_password=False) + if not vim_info: + raise nfvo.VimDefaultNameNotFound(vim_name=vim_name) return vim_info def _build_vim_auth(self, vim_info):