From dee5544446df0e4a6f54dce8597a1e37263d2c1c Mon Sep 17 00:00:00 2001 From: Armando Migliaccio Date: Tue, 2 Aug 2016 15:45:26 -0700 Subject: [PATCH] Introduce state management for trunk resources A trunk is a complex resource. This patch introduces the states and documents the state machine for the trunk resource. Only user initiated state transitions are introduced in this patch. Driver-initiated state transitions will be introduced as follow up. Partially-implements: bp/vlan-aware-vms Change-Id: I0446e26f9ee5193a3270336ac64d0186fe5a6c7b --- neutron/objects/trunk.py | 2 +- neutron/services/trunk/constants.py | 46 +++++++++++++++++++ neutron/services/trunk/exceptions.py | 5 ++ neutron/services/trunk/plugin.py | 42 ++++++++++++++++- .../tests/unit/services/trunk/test_plugin.py | 40 ++++++++++++++++ 5 files changed, 133 insertions(+), 2 deletions(-) diff --git a/neutron/objects/trunk.py b/neutron/objects/trunk.py index e7567adf0fa..96476bc6d69 100644 --- a/neutron/objects/trunk.py +++ b/neutron/objects/trunk.py @@ -96,7 +96,7 @@ class Trunk(base.NeutronDbObject): 'sub_ports': obj_fields.ListOfObjectsField(SubPort.__name__), } - fields_no_update = ['tenant_id', 'port_id', 'status'] + fields_no_update = ['tenant_id', 'port_id'] synthetic_fields = ['sub_ports'] diff --git a/neutron/services/trunk/constants.py b/neutron/services/trunk/constants.py index 3e0f3bd01d3..6ec250468e9 100644 --- a/neutron/services/trunk/constants.py +++ b/neutron/services/trunk/constants.py @@ -12,11 +12,57 @@ # License for the specific language governing permissions and limitations # under the License. + +# Valid trunk statuses + +# The trunk is happy, yay! +# A trunk remains in ACTIVE state when updates like name or admin_status_up +# occur. It goes back to ACTIVE state from other states (e.g. BUILD) when +# logical and physical resource provisioning has completed successfully. The +# attribute ADMIN_STATE_UP is not to be confused with STATUS: the former +# indicates whether a trunk can be managed. If a trunk has admin_state_up +# equal to false, the trunk plugin will reject any user request to manage +# the trunk resources (i.e. adding/removing sub-ports). ACTIVE_STATUS = 'ACTIVE' +# The server has acknowledged the user request: a user has asked to either +# create a trunk or add/remove resources to a trunk, and the plugin has +# created/updated the logical resource. The request has been passed along +# to a backend, and the physical resources associated to the trunk are +# in the process of being provisioned. +PENDING_STATUS = 'PENDING' + +# A driver/backend has acknowledged the server request: once the server +# notifies the driver/backend, a trunk is in BUILD state while the +# backend provisions the trunk resources. +BUILD_STATUS = 'BUILD' + +# Should any temporary system failure occur during the provisioning process, +# a trunk is in DEGRADED state. This means that the trunk was only +# partially provisioned, and only a subset of the subports were added +# successfully to the trunk. The operation of removing/adding the faulty +# subports may be attempted as a recovery measure. +DEGRADED_STATUS = 'DEGRADED' + +# Due to unforeseen circumstances, the user request has led to a conflict, and +# the trunk cannot be provisioned correctly for a subset of subports. For +# instance, a subport belonging to a network might not be compatible with +# the current trunk configuration, or the binding process leads to a persistent +# failure. Removing the 'offending' resource may be attempted as a recovery +# measure, but readding it to the trunk should lead to the same error +# condition. A trunk in ERROR status should be brought back to a sane status +# (i.e. any state except ERROR state) before attempting to add more subports, +# therefore requests of adding more subports must be rejected to avoid +# cascading errors. +ERROR_STATUS = 'ERROR' + + +# String literals for identifying trunk resources PARENT_PORT = 'parent_port' SUBPORTS = 'subports' TRUNK = 'trunk' TRUNK_PLUGIN = 'trunk_plugin' + +# String literals for segmentation types VLAN = 'vlan' diff --git a/neutron/services/trunk/exceptions.py b/neutron/services/trunk/exceptions.py index 475934c8739..f202f2011cb 100644 --- a/neutron/services/trunk/exceptions.py +++ b/neutron/services/trunk/exceptions.py @@ -57,3 +57,8 @@ class TrunkInUse(n_exc.InUse): class TrunkDisabled(n_exc.Conflict): message = _("Trunk %(trunk_id)s is currently disabled.") + + +class TrunkInErrorState(n_exc.Conflict): + message = _("Trunk %(trunk_id)s is in error state. Attempt " + "to resolve the error condition before proceeding.") diff --git a/neutron/services/trunk/plugin.py b/neutron/services/trunk/plugin.py index 69cf8ac095b..89d808c02be 100644 --- a/neutron/services/trunk/plugin.py +++ b/neutron/services/trunk/plugin.py @@ -118,12 +118,19 @@ class TrunkPlugin(service_base.ServicePluginBase, segmentation_type=p['segmentation_type']) for p in trunk['sub_ports']] admin_state_up = trunk.get('admin_state_up', True) + # NOTE(status_police): a trunk is created in PENDING status. Depending + # on the nature of the create request, a driver may set the status + # immediately to ACTIVE if no physical provisioning is required. + # Otherwise a transition to BUILD (or ERROR) should be expected + # depending on how the driver reacts. PRECOMMIT failures prevent the + # trunk from being created altogether. 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'], + status=constants.PENDING_STATUS, sub_ports=sub_ports) with db_api.autonested_transaction(context.session): trunk_obj.create() @@ -143,6 +150,10 @@ class TrunkPlugin(service_base.ServicePluginBase, with db_api.autonested_transaction(context.session): trunk_obj = self._get_trunk(context, trunk_id) original_trunk = copy.deepcopy(trunk_obj) + # NOTE(status_police): a trunk status should not change during an + # update_trunk(), even in face of PRECOMMIT failures. This is + # because only name and admin_state_up are being affected, and + # these are DB properties only. trunk_obj.update_fields(trunk_data, reset_changes=True) trunk_obj.update() payload = callbacks.TrunkPayload(context, trunk_id, @@ -161,6 +172,10 @@ class TrunkPlugin(service_base.ServicePluginBase, rules.trunk_can_be_managed(context, trunk) trunk_port_validator = rules.TrunkPortValidator(trunk.port_id) if not trunk_port_validator.is_bound(context): + # NOTE(status_police): when a trunk is deleted, the logical + # object disappears from the datastore, therefore there is no + # status transition involved. If PRECOMMIT failures occur, + # the trunk remains in the status where it was. trunk.delete() payload = callbacks.TrunkPayload(context, trunk_id, original_trunk=trunk) @@ -184,8 +199,24 @@ class TrunkPlugin(service_base.ServicePluginBase, with db_api.autonested_transaction(context.session): trunk = self._get_trunk(context, trunk_id) - original_trunk = copy.deepcopy(trunk) rules.trunk_can_be_managed(context, trunk) + original_trunk = copy.deepcopy(trunk) + # NOTE(status_police): the trunk status should transition to + # PENDING (and consequently to BUILD and finally in ACTIVE + # or ERROR), only if it is not in ERROR status already. A user + # should attempt to resolve the ERROR condition before adding + # more subports to the trunk. Should a trunk be in PENDING or + # BUILD state (e.g. when dealing with multiple concurrent + # requests), the status is still forced to PENDING and thus + # can potentially overwrite an interleaving state change to + # ACTIVE. Eventually the driver should bring the status back + # to ACTIVE or ERROR. + if trunk.status == constants.ERROR_STATUS: + raise trunk_exc.TrunkInErrorState(trunk_id=trunk_id) + else: + trunk.status = constants.PENDING_STATUS + trunk.update() + for subport in subports: obj = trunk_objects.SubPort( context=context, @@ -240,6 +271,15 @@ class TrunkPlugin(service_base.ServicePluginBase, del trunk.sub_ports[:] trunk.sub_ports.extend(current_subports.values()) + # NOTE(status_police): the trunk status should transition to + # PENDING irrespective of the status in which it is in to allow + # the user to resolve potential conflicts due to prior add_subports + # operations. + # Should a trunk be in PENDING or BUILD state (e.g. when dealing + # with multiple concurrent requests), the status is still forced + # to PENDING. See add_subports() for more details. + trunk.status = constants.PENDING_STATUS + trunk.update() payload = callbacks.TrunkPayload(context, trunk_id, current_trunk=trunk, original_trunk=original_trunk, diff --git a/neutron/tests/unit/services/trunk/test_plugin.py b/neutron/tests/unit/services/trunk/test_plugin.py index 2c5e26755be..6c1b4fcc72d 100644 --- a/neutron/tests/unit/services/trunk/test_plugin.py +++ b/neutron/tests/unit/services/trunk/test_plugin.py @@ -218,3 +218,43 @@ class TrunkPluginTestCase(test_plugin.Ml2PluginV2TestCase): def test_remove_subports_notify_precommit_delete(self): self._test_remove_subports_notify(events.PRECOMMIT_DELETE) + + def test_create_trunk_in_pending_state(self): + with self.port() as port: + trunk = self._create_test_trunk(port) + self.assertEqual( + constants.PENDING_STATUS, trunk['status']) + + def test_add_subports_trunk_in_error_state_raises(self): + with self.port() as port, self.port() as subport: + trunk = self._create_test_trunk(port) + trunk_obj = self._get_trunk_obj(trunk['id']) + trunk_obj.status = constants.ERROR_STATUS + trunk_obj.update() + s = create_subport_dict(subport['port']['id']) + self.assertRaises(trunk_exc.TrunkInErrorState, + self.trunk_plugin.add_subports, + self.context, trunk['id'], {'sub_ports': [s]}) + + def test_add_subports_trunk_goes_to_pending(self): + with self.port() as port, self.port() as subport: + trunk = self._create_test_trunk(port) + trunk_obj = self._get_trunk_obj(trunk['id']) + trunk_obj.status = constants.ACTIVE_STATUS + trunk_obj.update() + s = create_subport_dict(subport['port']['id']) + trunk = self.trunk_plugin.add_subports( + self.context, trunk['id'], {'sub_ports': [s]}) + self.assertEqual(constants.PENDING_STATUS, trunk['status']) + + def test_remove_subports_trunk_goes_to_pending(self): + with self.port() as port, self.port() as subport: + s = create_subport_dict(subport['port']['id']) + trunk = self._create_test_trunk(port, [s]) + trunk_obj = self._get_trunk_obj(trunk['id']) + trunk_obj.status = constants.ACTIVE_STATUS + trunk_obj.update() + trunk = self.trunk_plugin.remove_subports( + self.context, trunk['id'], + {'sub_ports': [{'port_id': subport['port']['id']}]}) + self.assertEqual(constants.PENDING_STATUS, trunk['status'])