diff --git a/neutron/db/migration/alembic_migrations/versions/newton/expand/5abc0278ca73_add_support_for_vlan_trunking.py b/neutron/db/migration/alembic_migrations/versions/newton/expand/5abc0278ca73_add_support_for_vlan_trunking.py index 8ba6254a6d8..cff6a845ce1 100644 --- a/neutron/db/migration/alembic_migrations/versions/newton/expand/5abc0278ca73_add_support_for_vlan_trunking.py +++ b/neutron/db/migration/alembic_migrations/versions/newton/expand/5abc0278ca73_add_support_for_vlan_trunking.py @@ -18,14 +18,20 @@ down_revision = '45f8dd33480b' from alembic import op import sqlalchemy as sa +from sqlalchemy import sql def upgrade(): op.create_table('trunks', + sa.Column('admin_state_up', sa.Boolean(), + nullable=False, server_default=sql.true()), sa.Column('tenant_id', sa.String(length=255), nullable=True, index=True), sa.Column('id', sa.String(length=36), nullable=False), + sa.Column('name', sa.String(length=255), nullable=True), sa.Column('port_id', sa.String(length=36), nullable=False), + sa.Column('status', sa.String(length=16), + nullable=False, server_default='ACTIVE'), sa.Column('standard_attr_id', sa.BigInteger(), nullable=False), sa.ForeignKeyConstraint(['port_id'], ['ports.id'], ondelete='CASCADE'), diff --git a/neutron/extensions/trunk.py b/neutron/extensions/trunk.py index ce35d13bb0e..eaa6fdd8f2c 100755 --- a/neutron/extensions/trunk.py +++ b/neutron/extensions/trunk.py @@ -85,9 +85,16 @@ validators.validators['type:subports'] = validate_subports RESOURCE_ATTRIBUTE_MAP = { 'trunks': { + 'admin_state_up': {'allow_post': True, 'allow_put': True, + 'default': True, + 'convert_to': converters.convert_to_boolean, + 'is_visible': True}, 'id': {'allow_post': False, 'allow_put': False, 'validate': {'type:uuid': None}, 'is_visible': True, 'primary_key': True}, + 'name': {'allow_post': True, 'allow_put': True, + 'validate': {'type:string': attr.NAME_MAX_LEN}, + 'default': '', 'is_visible': True}, 'tenant_id': {'allow_post': True, 'allow_put': False, 'required_by_policy': True, 'validate': @@ -97,6 +104,8 @@ RESOURCE_ATTRIBUTE_MAP = { 'required_by_policy': True, 'validate': {'type:uuid': None}, 'is_visible': True}, + 'status': {'allow_post': False, 'allow_put': False, + 'is_visible': True}, 'sub_ports': {'allow_post': True, 'allow_put': False, 'default': [], 'convert_list_to': converters.convert_kvp_list_to_dict, diff --git a/neutron/objects/trunk.py b/neutron/objects/trunk.py index 32a14256d35..2965b7a55de 100644 --- a/neutron/objects/trunk.py +++ b/neutron/objects/trunk.py @@ -87,13 +87,16 @@ class Trunk(base.NeutronDbObject): db_model = models.Trunk fields = { + 'admin_state_up': obj_fields.BooleanField(), 'id': obj_fields.UUIDField(), 'tenant_id': obj_fields.StringField(), + 'name': obj_fields.StringField(), 'port_id': obj_fields.UUIDField(), + 'status': obj_fields.StringField(), 'sub_ports': obj_fields.ListOfObjectsField(SubPort.__name__), } - fields_no_update = ['tenant_id', 'port_id'] + fields_no_update = ['tenant_id', 'port_id', 'status'] synthetic_fields = ['sub_ports'] diff --git a/neutron/services/trunk/constants.py b/neutron/services/trunk/constants.py index 71b1e443795..3e0f3bd01d3 100644 --- a/neutron/services/trunk/constants.py +++ b/neutron/services/trunk/constants.py @@ -12,6 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. +ACTIVE_STATUS = 'ACTIVE' + PARENT_PORT = 'parent_port' SUBPORTS = 'subports' TRUNK = 'trunk' diff --git a/neutron/services/trunk/exceptions.py b/neutron/services/trunk/exceptions.py index fe6f5087820..3f55d85042d 100644 --- a/neutron/services/trunk/exceptions.py +++ b/neutron/services/trunk/exceptions.py @@ -43,3 +43,7 @@ class ParentPortInUse(n_exc.InUse): class TrunkInUse(n_exc.InUse): message = _("Trunk %(trunk_id)s is currently in use.") + + +class TrunkDisabled(n_exc.Conflict): + message = _("Trunk %(trunk_id)s is currently disabled.") diff --git a/neutron/services/trunk/models.py b/neutron/services/trunk/models.py index 69933b66b54..6148f6ab1b5 100644 --- a/neutron/services/trunk/models.py +++ b/neutron/services/trunk/models.py @@ -14,19 +14,28 @@ # under the License. import sqlalchemy as sa +from sqlalchemy import sql +from neutron.api.v2 import attributes from neutron.db import model_base from neutron.db import models_v2 +from neutron.services.trunk import constants class Trunk(model_base.HasStandardAttributes, model_base.BASEV2, model_base.HasId, model_base.HasTenant): + admin_state_up = sa.Column( + sa.Boolean(), nullable=False, server_default=sql.true()) + name = sa.Column(sa.String(attributes.NAME_MAX_LEN)) port_id = sa.Column(sa.String(36), sa.ForeignKey('ports.id', ondelete='CASCADE'), nullable=False, unique=True) + status = sa.Column( + sa.String(16), nullable=False, server_default=constants.ACTIVE_STATUS) + port = sa.orm.relationship( models_v2.Port, backref=sa.orm.backref('trunk_port', lazy='joined', uselist=False, diff --git a/neutron/services/trunk/plugin.py b/neutron/services/trunk/plugin.py index 2d9ca450001..b4476d8abcc 100644 --- a/neutron/services/trunk/plugin.py +++ b/neutron/services/trunk/plugin.py @@ -51,6 +51,9 @@ class TrunkPlugin(service_base.ServicePluginBase, supported_extension_aliases = ["trunk", "trunk-details"] + __native_pagination_support = True + __native_sorting_support = True + def __init__(self): db_base_plugin_v2.NeutronDbPluginV2.register_dict_extend_funcs( attributes.PORTS, [_extend_port_trunk_details]) @@ -85,11 +88,7 @@ class TrunkPlugin(service_base.ServicePluginBase, @db_base_plugin_common.convert_result_to_dict def get_trunk(self, context, trunk_id, fields=None): """Return information for the specified trunk.""" - obj = trunk_objects.Trunk.get_object(context, id=trunk_id) - if obj is None: - raise trunk_exc.TrunkNotFound(trunk_id=trunk_id) - - return obj + return self._get_trunk(context, trunk_id) @db_base_plugin_common.filter_fields @db_base_plugin_common.convert_result_to_dict @@ -112,33 +111,42 @@ class TrunkPlugin(service_base.ServicePluginBase, segmentation_id=p['segmentation_id'], segmentation_type=p['segmentation_type']) for p in trunk['sub_ports']] + admin_state_up = trunk.get('admin_state_up', True) trunk_obj = trunk_objects.Trunk(context=context, + admin_state_up=admin_state_up, id=uuidutils.generate_uuid(), + name=trunk.get('name', ""), tenant_id=trunk['tenant_id'], port_id=trunk['port_id'], sub_ports=sub_ports) trunk_obj.create() return trunk_obj + @db_base_plugin_common.convert_result_to_dict + def update_trunk(self, context, trunk_id, trunk): + """Update information for the specified trunk.""" + trunk_data = trunk['trunk'] + with db_api.autonested_transaction(context.session): + trunk_obj = self._get_trunk(context, trunk_id) + trunk_obj.update_nonidentifying_fields( + trunk_data, reset_changes=True) + trunk_obj.update() + return trunk_obj + def delete_trunk(self, context, trunk_id): """Delete the specified trunk.""" - trunk = trunk_objects.Trunk.get_object(context, id=trunk_id) - if trunk: - trunk_validator = rules.TrunkPortValidator(trunk.port_id) - if not trunk_validator.is_bound(context): + with db_api.autonested_transaction(context.session): + trunk = self._get_trunk(context, trunk_id) + rules.trunk_can_be_managed(context, trunk) + trunk_port_validator = rules.TrunkPortValidator(trunk.port_id) + if not trunk_port_validator.is_bound(context): trunk.delete() - return - raise trunk_exc.TrunkInUse(trunk_id=trunk_id) - - raise trunk_exc.TrunkNotFound(trunk_id=trunk_id) + else: + raise trunk_exc.TrunkInUse(trunk_id=trunk_id) @db_base_plugin_common.convert_result_to_dict def add_subports(self, context, trunk_id, subports): """Add one or more subports to trunk.""" - trunk = trunk_objects.Trunk.get_object(context, id=trunk_id) - if trunk is None: - raise trunk_exc.TrunkNotFound(trunk_id=trunk_id) - # Check for basic validation since the request body here is not # automatically validated by the API layer. subports_validator = rules.SubPortsValidator( @@ -147,6 +155,8 @@ class TrunkPlugin(service_base.ServicePluginBase, added_subports = [] with db_api.autonested_transaction(context.session): + trunk = self._get_trunk(context, trunk_id) + rules.trunk_can_be_managed(context, trunk) for subport in subports: obj = trunk_objects.SubPort( context=context, @@ -167,9 +177,8 @@ class TrunkPlugin(service_base.ServicePluginBase, def remove_subports(self, context, trunk_id, subports): """Remove one or more subports from trunk.""" with db_api.autonested_transaction(context.session): - trunk = trunk_objects.Trunk.get_object(context, id=trunk_id) - if trunk is None: - raise trunk_exc.TrunkNotFound(trunk_id=trunk_id) + trunk = self._get_trunk(context, trunk_id) + rules.trunk_can_be_managed(context, trunk) subports_validator = rules.SubPortsValidator( self._segmentation_types, subports) @@ -203,3 +212,11 @@ class TrunkPlugin(service_base.ServicePluginBase, """Return subports for the specified trunk.""" trunk = self.get_trunk(context, trunk_id) return {'sub_ports': trunk['sub_ports']} + + def _get_trunk(self, context, trunk_id): + """Return the trunk object or raise if not found.""" + obj = trunk_objects.Trunk.get_object(context, id=trunk_id) + if obj is None: + raise trunk_exc.TrunkNotFound(trunk_id=trunk_id) + + return obj diff --git a/neutron/services/trunk/rules.py b/neutron/services/trunk/rules.py index 98add6dcaf4..80fb80cee31 100644 --- a/neutron/services/trunk/rules.py +++ b/neutron/services/trunk/rules.py @@ -26,6 +26,13 @@ from neutron.services.trunk import exceptions as trunk_exc # This layer is introduced for keeping busines logic and # data persistence decoupled. + +def trunk_can_be_managed(context, trunk): + """Validate that the trunk can be managed.""" + if not trunk.admin_state_up: + raise trunk_exc.TrunkDisabled(trunk_id=trunk.id) + + class TrunkPortValidator(object): def __init__(self, port_id): diff --git a/neutron/tests/tempest/api/test_trunk.py b/neutron/tests/tempest/api/test_trunk.py index d1fa3259913..05e14301ae0 100644 --- a/neutron/tests/tempest/api/test_trunk.py +++ b/neutron/tests/tempest/api/test_trunk.py @@ -55,10 +55,10 @@ class TrunkTestJSONBase(base.BaseAdminNetworkTest): trunks_cleanup(cls.client, cls.trunks) super(TrunkTestJSONBase, cls).resource_cleanup() - def _create_trunk_with_network_and_parent(self, subports): + def _create_trunk_with_network_and_parent(self, subports, **kwargs): network = self.create_network() parent_port = self.create_port(network) - trunk = self.client.create_trunk(parent_port['id'], subports) + trunk = self.client.create_trunk(parent_port['id'], subports, **kwargs) self.trunks.append(trunk['trunk']) return trunk @@ -88,6 +88,20 @@ class TrunkTestJSON(TrunkTestJSONBase): self.client.delete_trunk(trunk_id) self.assertRaises(lib_exc.NotFound, self.client.show_trunk, trunk_id) + @test.idempotent_id('4ce46c22-a2b6-4659-bc5a-0ef2463cab32') + def test_create_update_trunk(self): + trunk = self._create_trunk_with_network_and_parent(None) + trunk_id = trunk['trunk']['id'] + res = self.client.show_trunk(trunk_id) + self.assertTrue(res['trunk']['admin_state_up']) + self.assertEqual("", res['trunk']['name']) + res = self.client.update_trunk( + trunk_id, name='foo', admin_state_up=False) + self.assertFalse(res['trunk']['admin_state_up']) + self.assertEqual("foo", res['trunk']['name']) + # enable the trunk so that it can be managed + self.client.update_trunk(trunk_id, admin_state_up=True) + @test.idempotent_id('73365f73-bed6-42cd-960b-ec04e0c99d85') def test_list_trunks(self): trunk1 = self._create_trunk_with_network_and_parent(None) @@ -162,7 +176,6 @@ class TrunkTestJSON(TrunkTestJSONBase): class TrunksSearchCriteriaTest(base.BaseSearchCriteriaTest): resource = 'trunk' - field = 'id' @classmethod def skip_checks(cls): @@ -178,7 +191,7 @@ class TrunksSearchCriteriaTest(base.BaseSearchCriteriaTest): net = cls.create_network(network_name='trunk-search-test-net') for name in cls.resource_names: parent_port = cls.create_port(net) - trunk = cls.client.create_trunk(parent_port['id'], []) + trunk = cls.client.create_trunk(parent_port['id'], [], name=name) cls.trunks.append(trunk['trunk']) @classmethod diff --git a/neutron/tests/tempest/api/test_trunk_negative.py b/neutron/tests/tempest/api/test_trunk_negative.py index 556c3744ef1..16a9af3deeb 100644 --- a/neutron/tests/tempest/api/test_trunk_negative.py +++ b/neutron/tests/tempest/api/test_trunk_negative.py @@ -127,6 +127,45 @@ class TrunkTestJSON(test_trunk.TrunkTestJSONBase): 'segmentation_type': 'vlan', 'segmentation_id': 2}]) + @test.attr(type='negative') + @test.idempotent_id('7f132ccc-1380-42d8-9c44-50411612bd01') + def test_add_subport_port_id_disabled_trunk(self): + trunk = self._create_trunk_with_network_and_parent( + None, admin_state_up=False) + self.assertRaises(lib_exc.Conflict, + self.client.add_subports, + trunk['trunk']['id'], + [{'port_id': trunk['trunk']['port_id'], + 'segmentation_type': 'vlan', + 'segmentation_id': 2}]) + self.client.update_trunk( + trunk['trunk']['id'], admin_state_up=True) + + @test.attr(type='negative') + @test.idempotent_id('8f132ccc-1380-42d8-9c44-50411612bd01') + def test_remove_subport_port_id_disabled_trunk(self): + trunk = self._create_trunk_with_network_and_parent( + None, admin_state_up=False) + self.assertRaises(lib_exc.Conflict, + self.client.remove_subports, + trunk['trunk']['id'], + [{'port_id': trunk['trunk']['port_id'], + 'segmentation_type': 'vlan', + 'segmentation_id': 2}]) + self.client.update_trunk( + trunk['trunk']['id'], admin_state_up=True) + + @test.attr(type='negative') + @test.idempotent_id('9f132ccc-1380-42d8-9c44-50411612bd01') + def test_delete_trunk_disabled_trunk(self): + trunk = self._create_trunk_with_network_and_parent( + None, admin_state_up=False) + self.assertRaises(lib_exc.Conflict, + self.client.delete_trunk, + trunk['trunk']['id']) + self.client.update_trunk( + trunk['trunk']['id'], admin_state_up=True) + @test.attr(type='negative') @test.idempotent_id('00cb40bb-1593-44c8-808c-72b47e64252f') def test_add_subport_duplicate_segmentation_details(self): diff --git a/neutron/tests/tempest/services/network/json/network_client.py b/neutron/tests/tempest/services/network/json/network_client.py index 32206047b38..36a7f5bd4aa 100644 --- a/neutron/tests/tempest/services/network/json/network_client.py +++ b/neutron/tests/tempest/services/network/json/network_client.py @@ -659,7 +659,8 @@ class NetworkClientJSON(service_client.RestClient): body = jsonutils.loads(body) return service_client.ResponseBody(resp, body) - def create_trunk(self, parent_port_id, subports, tenant_id=None): + def create_trunk(self, parent_port_id, subports, + tenant_id=None, name=None, admin_state_up=None): uri = '%s/trunks' % self.uri_prefix post_data = { 'trunk': { @@ -670,11 +671,24 @@ class NetworkClientJSON(service_client.RestClient): post_data['trunk']['sub_ports'] = subports if tenant_id is not None: post_data['trunk']['tenant_id'] = tenant_id + if name is not None: + post_data['trunk']['name'] = name + if admin_state_up is not None: + post_data['trunk']['admin_state_up'] = admin_state_up resp, body = self.post(uri, self.serialize(post_data)) body = self.deserialize_single(body) self.expected_success(201, resp.status) return service_client.ResponseBody(resp, body) + def update_trunk(self, trunk_id, **kwargs): + put_body = {'trunk': kwargs} + body = jsonutils.dumps(put_body) + uri = '%s/trunks/%s' % (self.uri_prefix, trunk_id) + resp, body = self.put(uri, body) + self.expected_success(200, resp.status) + body = jsonutils.loads(body) + return service_client.ResponseBody(resp, body) + def show_trunk(self, trunk_id): uri = '%s/trunks/%s' % (self.uri_prefix, trunk_id) resp, body = self.get(uri) diff --git a/neutron/tests/unit/objects/test_objects.py b/neutron/tests/unit/objects/test_objects.py index 0c8d94d736d..b0fcfae3861 100644 --- a/neutron/tests/unit/objects/test_objects.py +++ b/neutron/tests/unit/objects/test_objects.py @@ -41,7 +41,7 @@ object_data = { 'SubnetPool': '1.0-e8300bfbc4762cc88a7f6205b52da2f8', 'SubnetPoolPrefix': '1.0-13c15144135eb869faa4a76dc3ee3b6c', 'SubPort': '1.0-72c8471068db1f0491b5480fe49b52bb', - 'Trunk': '1.0-ee3f16ebc40c16bda7be6dcd963895cc', + 'Trunk': '1.0-80ebebb57f2b0dbb510f84d91421ed10', }