From fdee7801248b5605112e5e2ce0f06f8a5e9ecc6b Mon Sep 17 00:00:00 2001 From: armando-migliaccio Date: Thu, 17 Jul 2014 17:22:49 -0700 Subject: [PATCH] Encapsulate some port properties in the PortContext Bindings to host or status may need further encapsulation to avoid exposing mechanism drivers to underlying DB model details. This was particularly true in the case of the l2pop mech driver. As a result, some docstrings were reworded, and the newly introduced properties used directly in the mech drivers. Partially-implements: blueprint neutron-ovs-dvr Supports blueprint: ml2-hierarchical-port-binding Change-Id: If2a373ef04d19b164585fb4bde4fe6e0cfaf43be --- neutron/plugins/ml2/driver_api.py | 82 +++++++++++++------ neutron/plugins/ml2/driver_context.py | 17 ++++ .../ml2/drivers/arista/mechanism_arista.py | 11 ++- .../ml2/drivers/cisco/apic/mechanism_apic.py | 2 +- .../drivers/cisco/nexus/mech_cisco_nexus.py | 3 +- .../plugins/ml2/drivers/l2pop/mech_driver.py | 49 ++++++----- neutron/tests/unit/ml2/_test_mech_agent.py | 16 ++++ .../arista/test_arista_mechanism_driver.py | 9 ++ .../apic/test_cisco_apic_mechanism_driver.py | 9 ++ 9 files changed, 143 insertions(+), 55 deletions(-) diff --git a/neutron/plugins/ml2/driver_api.py b/neutron/plugins/ml2/driver_api.py index 8b0484d13..779ebdf92 100644 --- a/neutron/plugins/ml2/driver_api.py +++ b/neutron/plugins/ml2/driver_api.py @@ -149,21 +149,23 @@ class NetworkContext(object): @abc.abstractproperty def current(self): - """Return the current state of the network. + """Return the network in its current configuration. - Return the current state of the network, as defined by - NeutronPluginBaseV2.create_network and all extensions in the - ml2 plugin. + Return the network, as defined by NeutronPluginBaseV2. + create_network and all extensions in the ml2 plugin, with + all its properties 'current' at the time the context was + established. """ pass @abc.abstractproperty def original(self): - """Return the original state of the network. + """Return the network in its original configuration. - Return the original state of the network, prior to a call to - update_network. Method is only valid within calls to - update_network_precommit and update_network_postcommit. + Return the network, with all its properties set to their + original values prior to a call to update_network. Method is + only valid within calls to update_network_precommit and + update_network_postcommit. """ pass @@ -185,21 +187,23 @@ class SubnetContext(object): @abc.abstractproperty def current(self): - """Return the current state of the subnet. + """Return the subnet in its current configuration. - Return the current state of the subnet, as defined by - NeutronPluginBaseV2.create_subnet and all extensions in the - ml2 plugin. + Return the subnet, as defined by NeutronPluginBaseV2. + create_subnet and all extensions in the ml2 plugin, with + all its properties 'current' at the time the context was + established. """ pass @abc.abstractproperty def original(self): - """Return the original state of the subnet. + """Return the subnet in its original configuration. - Return the original state of the subnet, prior to a call to - update_subnet. Method is only valid within calls to - update_subnet_precommit and update_subnet_postcommit. + Return the subnet, with all its properties set to their + original values prior to a call to update_subnet. Method is + only valid within calls to update_subnet_precommit and + update_subnet_postcommit. """ pass @@ -216,21 +220,37 @@ class PortContext(object): @abc.abstractproperty def current(self): - """Return the current state of the port. + """Return the port in its current configuration. - Return the current state of the port, as defined by - NeutronPluginBaseV2.create_port and all extensions in the ml2 - plugin. + Return the port, as defined by NeutronPluginBaseV2. + create_port and all extensions in the ml2 plugin, with + all its properties 'current' at the time the context was + established. """ pass @abc.abstractproperty def original(self): - """Return the original state of the port. + """Return the port in its original configuration. - Return the original state of the port, prior to a call to - update_port. Method is only valid within calls to - update_port_precommit and update_port_postcommit. + Return the port, with all its properties set to their + original values prior to a call to update_port. Method is + only valid within calls to update_port_precommit and + update_port_postcommit. + """ + pass + + @abc.abstractproperty + def status(self): + """Return the status of the current port.""" + pass + + @abc.abstractproperty + def original_status(self): + """Return the status of the original port. + + The method is only valid within calls to update_port_precommit and + update_port_postcommit. """ pass @@ -254,6 +274,20 @@ class PortContext(object): """ pass + @abc.abstractproperty + def host(self): + """Return the host associated with the 'current' port.""" + pass + + @abc.abstractproperty + def original_host(self): + """Return the host associated with the 'original' port. + + Method is only valid within calls to update_port_precommit + and update_port_postcommit. + """ + pass + @abc.abstractproperty def bound_driver(self): """Return the currently bound mechanism driver name.""" diff --git a/neutron/plugins/ml2/driver_context.py b/neutron/plugins/ml2/driver_context.py index 08f9b12c9..62b578217 100644 --- a/neutron/plugins/ml2/driver_context.py +++ b/neutron/plugins/ml2/driver_context.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +from neutron.extensions import portbindings from neutron.openstack.common import jsonutils from neutron.plugins.ml2 import db from neutron.plugins.ml2 import driver_api as api @@ -93,6 +94,14 @@ class PortContext(MechanismDriverContext, api.PortContext): def original(self): return self._original_port + @property + def status(self): + return self._port['status'] + + @property + def original_status(self): + return self._original_port['status'] + @property def network(self): return self._network_context @@ -112,6 +121,14 @@ class PortContext(MechanismDriverContext, api.PortContext): if segment[api.ID] == self._original_bound_segment_id: return segment + @property + def host(self): + return self._port.get(portbindings.HOST_ID) + + @property + def original_host(self): + return self._original_port.get(portbindings.HOST_ID) + @property def bound_driver(self): return self._binding.driver diff --git a/neutron/plugins/ml2/drivers/arista/mechanism_arista.py b/neutron/plugins/ml2/drivers/arista/mechanism_arista.py index a86d542d9..08a1696b1 100644 --- a/neutron/plugins/ml2/drivers/arista/mechanism_arista.py +++ b/neutron/plugins/ml2/drivers/arista/mechanism_arista.py @@ -19,7 +19,6 @@ import jsonrpclib from oslo.config import cfg from neutron.common import constants as n_const -from neutron.extensions import portbindings from neutron.openstack.common import log as logging from neutron.plugins.ml2.common import exceptions as ml2_exc from neutron.plugins.ml2 import driver_api @@ -801,7 +800,7 @@ class AristaDriver(driver_api.MechanismDriver): port = context.current device_id = port['device_id'] device_owner = port['device_owner'] - host = port[portbindings.HOST_ID] + host = context.host # device_id and device_owner are set on VM boot is_vm_boot = device_id and device_owner @@ -822,7 +821,7 @@ class AristaDriver(driver_api.MechanismDriver): port = context.current device_id = port['device_id'] device_owner = port['device_owner'] - host = port[portbindings.HOST_ID] + host = context.host # device_id and device_owner are set on VM boot is_vm_boot = device_id and device_owner @@ -885,7 +884,7 @@ class AristaDriver(driver_api.MechanismDriver): device_id = port['device_id'] device_owner = port['device_owner'] - host = port[portbindings.HOST_ID] + host = context.host is_vm_boot = device_id and device_owner if host and is_vm_boot: @@ -926,7 +925,7 @@ class AristaDriver(driver_api.MechanismDriver): """Delete information about a VM and host from the DB.""" port = context.current - host_id = port[portbindings.HOST_ID] + host_id = context.host device_id = port['device_id'] tenant_id = port['tenant_id'] network_id = port['network_id'] @@ -947,7 +946,7 @@ class AristaDriver(driver_api.MechanismDriver): """ port = context.current device_id = port['device_id'] - host = port[portbindings.HOST_ID] + host = context.host port_id = port['id'] network_id = port['network_id'] tenant_id = port['tenant_id'] diff --git a/neutron/plugins/ml2/drivers/cisco/apic/mechanism_apic.py b/neutron/plugins/ml2/drivers/cisco/apic/mechanism_apic.py index d5297df68..f526fd746 100644 --- a/neutron/plugins/ml2/drivers/cisco/apic/mechanism_apic.py +++ b/neutron/plugins/ml2/drivers/cisco/apic/mechanism_apic.py @@ -93,7 +93,7 @@ class APICMechanismDriver(api.MechanismDriver): # Not a compute port, return return - host = port.get(portbindings.HOST_ID) + host = context.host # Check host that the dhcp agent is running on filters = {'device_owner': 'network:dhcp', 'network_id': network} diff --git a/neutron/plugins/ml2/drivers/cisco/nexus/mech_cisco_nexus.py b/neutron/plugins/ml2/drivers/cisco/nexus/mech_cisco_nexus.py index 8db752829..e77212ffb 100644 --- a/neutron/plugins/ml2/drivers/cisco/nexus/mech_cisco_nexus.py +++ b/neutron/plugins/ml2/drivers/cisco/nexus/mech_cisco_nexus.py @@ -155,8 +155,7 @@ class CiscoNexusMechanismDriver(api.MechanismDriver): def _is_vm_migration(self, context): if not context.bound_segment and context.original_bound_segment: - return (context.current.get(portbindings.HOST_ID) != - context.original.get(portbindings.HOST_ID)) + return context.host != context.original_host def _port_action(self, port, segment, func): """Verify configuration and then process event.""" diff --git a/neutron/plugins/ml2/drivers/l2pop/mech_driver.py b/neutron/plugins/ml2/drivers/l2pop/mech_driver.py index 531dd5ee3..ede0f8df4 100644 --- a/neutron/plugins/ml2/drivers/l2pop/mech_driver.py +++ b/neutron/plugins/ml2/drivers/l2pop/mech_driver.py @@ -48,7 +48,8 @@ class L2populationMechanismDriver(api.MechanismDriver, ip['ip_address']] for ip in port['fixed_ips']] def delete_port_postcommit(self, context): - fanout_msg = self._update_port_down(context, context.current) + fanout_msg = self._update_port_down( + context, context.current, context.host) if fanout_msg: self.L2populationAgentNotify.remove_fdb_entries( self.rpc_ctx, fanout_msg) @@ -67,7 +68,8 @@ class L2populationMechanismDriver(api.MechanismDriver, def _fixed_ips_changed(self, context, orig, port, diff_ips): orig_ips, port_ips = diff_ips - port_infos = self._get_port_infos(context, orig) + port_infos = self._get_port_infos( + context, orig, context.original_host) if not port_infos: return agent, agent_ip, segment, port_fdb_entries = port_infos @@ -96,30 +98,34 @@ class L2populationMechanismDriver(api.MechanismDriver, diff_ips = self._get_diff_ips(orig, port) if diff_ips: self._fixed_ips_changed(context, orig, port, diff_ips) - if (port['binding:host_id'] != orig['binding:host_id'] - and port['status'] == const.PORT_STATUS_ACTIVE + if (context.host != context.original_host + and context.status == const.PORT_STATUS_ACTIVE and not self.migrated_ports.get(orig['id'])): # The port has been migrated. We have to store the original # binding to send appropriate fdb once the port will be set # on the destination host - self.migrated_ports[orig['id']] = orig - elif port['status'] != orig['status']: - if port['status'] == const.PORT_STATUS_ACTIVE: + self.migrated_ports[orig['id']] = ( + (orig, context.original_host)) + elif context.status != context.original_status: + if context.status == const.PORT_STATUS_ACTIVE: self._update_port_up(context) - elif port['status'] == const.PORT_STATUS_DOWN: - fdb_entries = self._update_port_down(context, port) + elif context.status == const.PORT_STATUS_DOWN: + fdb_entries = self._update_port_down( + context, port, context.host) self.L2populationAgentNotify.remove_fdb_entries( self.rpc_ctx, fdb_entries) - elif port['status'] == const.PORT_STATUS_BUILD: + elif context.status == const.PORT_STATUS_BUILD: orig = self.migrated_ports.pop(port['id'], None) if orig: - # this port has been migrated : remove its entries from fdb - fdb_entries = self._update_port_down(context, orig) + original_port = orig[0] + original_host = orig[1] + # this port has been migrated: remove its entries from fdb + fdb_entries = self._update_port_down( + context, original_port, original_host) self.L2populationAgentNotify.remove_fdb_entries( self.rpc_ctx, fdb_entries) - def _get_port_infos(self, context, port): - agent_host = port['binding:host_id'] + def _get_port_infos(self, context, port, agent_host): if not agent_host: return @@ -150,14 +156,14 @@ class L2populationMechanismDriver(api.MechanismDriver, return agent, agent_ip, segment, fdb_entries def _update_port_up(self, context): - port_context = context.current - port_infos = self._get_port_infos(context, port_context) + port = context.current + agent_host = context.host + port_infos = self._get_port_infos(context, port, agent_host) if not port_infos: return agent, agent_ip, segment, port_fdb_entries = port_infos - agent_host = port_context['binding:host_id'] - network_id = port_context['network_id'] + network_id = port['network_id'] session = db_api.get_session() agent_active_ports = self.get_agent_network_active_port_count( @@ -209,14 +215,13 @@ class L2populationMechanismDriver(api.MechanismDriver, self.L2populationAgentNotify.add_fdb_entries(self.rpc_ctx, other_fdb_entries) - def _update_port_down(self, context, port_context): - port_infos = self._get_port_infos(context, port_context) + def _update_port_down(self, context, port, agent_host): + port_infos = self._get_port_infos(context, port, agent_host) if not port_infos: return agent, agent_ip, segment, port_fdb_entries = port_infos - agent_host = port_context['binding:host_id'] - network_id = port_context['network_id'] + network_id = port['network_id'] session = db_api.get_session() agent_active_ports = self.get_agent_network_active_port_count( diff --git a/neutron/tests/unit/ml2/_test_mech_agent.py b/neutron/tests/unit/ml2/_test_mech_agent.py index 4fbdc10e5..79f108e3a 100644 --- a/neutron/tests/unit/ml2/_test_mech_agent.py +++ b/neutron/tests/unit/ml2/_test_mech_agent.py @@ -59,6 +59,14 @@ class FakePortContext(api.PortContext): def original(self): return None + @property + def status(self): + return 'DOWN' + + @property + def original_status(self): + return None + @property def network(self): return self._network_context @@ -74,6 +82,14 @@ class FakePortContext(api.PortContext): def original_bound_segment(self): return None + @property + def host(self): + return '' + + @property + def original_host(self): + return None + @property def bound_driver(self): return None diff --git a/neutron/tests/unit/ml2/drivers/arista/test_arista_mechanism_driver.py b/neutron/tests/unit/ml2/drivers/arista/test_arista_mechanism_driver.py index 42028a8fe..8749d5073 100644 --- a/neutron/tests/unit/ml2/drivers/arista/test_arista_mechanism_driver.py +++ b/neutron/tests/unit/ml2/drivers/arista/test_arista_mechanism_driver.py @@ -18,6 +18,7 @@ from oslo.config import cfg from neutron.common import constants as n_const import neutron.db.api as ndb +from neutron.extensions import portbindings from neutron.plugins.ml2.drivers.arista import db from neutron.plugins.ml2.drivers.arista import exceptions as arista_exc from neutron.plugins.ml2.drivers.arista import mechanism_arista as arista @@ -723,3 +724,11 @@ class FakePortContext(object): @property def network(self): return self._network_context + + @property + def host(self): + return self._port.get(portbindings.HOST_ID) + + @property + def original_host(self): + return self._original_port.get(portbindings.HOST_ID) diff --git a/neutron/tests/unit/ml2/drivers/cisco/apic/test_cisco_apic_mechanism_driver.py b/neutron/tests/unit/ml2/drivers/cisco/apic/test_cisco_apic_mechanism_driver.py index 6addd4382..fbd30e67d 100644 --- a/neutron/tests/unit/ml2/drivers/cisco/apic/test_cisco_apic_mechanism_driver.py +++ b/neutron/tests/unit/ml2/drivers/cisco/apic/test_cisco_apic_mechanism_driver.py @@ -19,6 +19,7 @@ import mock from oslo.config import cfg +from neutron.extensions import portbindings from neutron.plugins.ml2.drivers.cisco.apic import mechanism_apic as md from neutron.plugins.ml2.drivers import type_vlan # noqa from neutron.tests import base @@ -224,3 +225,11 @@ class FakePortContext(object): def set_binding(self, segment_id, vif_type, cap_port_filter): pass + + @property + def host(self): + return self._port.get(portbindings.HOST_ID) + + @property + def original_host(self): + return self._original_port.get(portbindings.HOST_ID)