Browse Source

ML2: Remove validate_port_binding() and unbind_port()

The API implemented by ML2 mechanism drivers included three methods
related to port binding that were called within DB transactions, but
that could potentially involve communication with controllers or
devices that should not be done within transactions. A subsequent
patch will move the calls to bind_port() outside of tranactions. This
patch eliminates the other two methods from the MechanismDriver API.

The validate_port_binding() method was previously called on the bound
mechanism driver to check whether an existing binding was still valid,
so that the port could be rebound if something changed. But since nova
has no way to handle changes to binding:vif_type or
binding:vif_details after a port is initially plugged, this turned out
not to be useful, so the method has been removed from the
MechanismDriver API. Now, once a port is successfully bound, the
binding remains until the port is deleted or any of it's
binding:host_id, binding:vnic_type, or binding:profile attribute
values are changed.

The unbind_port() method was previously called on the bound mechanism
driver as an existing binding was removed. This method was not used by
any existing mechanism drivers, and was redundant with the
update_port_precommit() and update_port_postcommit() methods that are
called on all mechanism drivers when an existing binding is removed,
so this method has also been removed from the driver API.

Eliminating the unbind_port() call allows the binding details to be
made available via the PortContext in delete_port_postcommit() calls,
completing the resolution of bug 1276395.

Closes-bug: 1276395
Partial-bug: 1276391
Change-Id: I70fb65b478373c4f07f5273baa097fc50e5ba2ef
tags/2014.1.rc1
Robert Kukura 5 years ago
parent
commit
a57dc2c30a

+ 0
- 23
neutron/plugins/ml2/driver_api.py View File

@@ -594,26 +594,3 @@ class MechanismDriver(object):
594 594
         details.
595 595
         """
596 596
         pass
597
-
598
-    def validate_port_binding(self, context):
599
-        """Check whether existing port binding is still valid.
600
-
601
-        :param context: PortContext instance describing the port
602
-        :returns: True if binding is valid, otherwise False
603
-
604
-        Called inside transaction context on session to validate that
605
-        the MechanismDriver's existing binding for the port is still
606
-        valid.
607
-        """
608
-        return False
609
-
610
-    def unbind_port(self, context):
611
-        """Undo existing port binding.
612
-
613
-        :param context: PortContext instance describing the port
614
-
615
-        Called inside transaction context on session to notify the
616
-        MechanismDriver that its existing binding for the port is no
617
-        longer valid.
618
-        """
619
-        pass

+ 2
- 2
neutron/plugins/ml2/drivers/cisco/nexus/mech_cisco_nexus.py View File

@@ -153,7 +153,7 @@ class CiscoNexusMechanismDriver(api.MechanismDriver):
153 153
         host_id = context.current.get(portbindings.HOST_ID)
154 154
 
155 155
         # Workaround until vlan can be retrieved during delete_port_postcommit
156
-        # (or prehaps unbind_port) event.
156
+        # event.
157 157
         if func == self._delete_switch_entry:
158 158
             vlan_id = self._delete_port_postcommit_vlan
159 159
         else:
@@ -168,7 +168,7 @@ class CiscoNexusMechanismDriver(api.MechanismDriver):
168 168
             raise excep.NexusMissingRequiredFields(fields=fields)
169 169
 
170 170
         # Workaround until vlan can be retrieved during delete_port_postcommit
171
-        # (or prehaps unbind_port) event.
171
+        # event.
172 172
         if func == self._delete_nxos_db:
173 173
             self._delete_port_postcommit_vlan = vlan_id
174 174
         else:

+ 17
- 40
neutron/plugins/ml2/drivers/mech_agent.py View File

@@ -35,8 +35,7 @@ class AgentMechanismDriverBase(api.MechanismDriver):
35 35
     at least one segment of the port's network.
36 36
 
37 37
     MechanismDrivers using this base class must pass the agent type to
38
-    __init__(), and must implement try_to_bind_segment_for_agent() and
39
-    check_segment_for_agent().
38
+    __init__(), and must implement try_to_bind_segment_for_agent().
40 39
     """
41 40
 
42 41
     def __init__(self, agent_type,
@@ -75,26 +74,6 @@ class AgentMechanismDriverBase(api.MechanismDriver):
75 74
                 LOG.warning(_("Attempting to bind with dead agent: %s"),
76 75
                             agent)
77 76
 
78
-    def validate_port_binding(self, context):
79
-        LOG.debug(_("Validating binding for port %(port)s on "
80
-                    "network %(network)s"),
81
-                  {'port': context.current['id'],
82
-                   'network': context.network.current['id']})
83
-        for agent in context.host_agents(self.agent_type):
84
-            LOG.debug(_("Checking agent: %s"), agent)
85
-            if agent['alive'] and self.check_segment_for_agent(
86
-                context.bound_segment, agent):
87
-                LOG.debug(_("Binding valid"))
88
-                return True
89
-        LOG.warning(_("Binding invalid for port: %s"), context.current)
90
-        return False
91
-
92
-    def unbind_port(self, context):
93
-        LOG.debug(_("Unbinding port %(port)s on "
94
-                    "network %(network)s"),
95
-                  {'port': context.current['id'],
96
-                   'network': context.network.current['id']})
97
-
98 77
     @abstractmethod
99 78
     def try_to_bind_segment_for_agent(self, context, segment, agent):
100 79
         """Try to bind with segment for agent.
@@ -114,21 +93,6 @@ class AgentMechanismDriverBase(api.MechanismDriver):
114 93
         return True. Otherwise, it must return False.
115 94
         """
116 95
 
117
-    @abstractmethod
118
-    def check_segment_for_agent(self, segment, agent):
119
-        """Check if segment can be bound for agent.
120
-
121
-        :param segment: segment dictionary describing segment to bind
122
-        :param agent: agents_db entry describing agent to bind
123
-        :returns: True iff segment can be bound for agent
124
-
125
-        Called inside transaction during validate_port_binding() so
126
-        that derived MechanismDrivers can use agent_db data along with
127
-        built-in knowledge of the corresponding agent's capabilities
128
-        to determine whether or not the specified network segment can
129
-        be bound for the agent.
130
-        """
131
-
132 96
 
133 97
 @six.add_metaclass(ABCMeta)
134 98
 class SimpleAgentMechanismDriverBase(AgentMechanismDriverBase):
@@ -144,9 +108,7 @@ class SimpleAgentMechanismDriverBase(AgentMechanismDriverBase):
144 108
 
145 109
     MechanismDrivers using this base class must pass the agent type
146 110
     and the values for binding:vif_type and binding:vif_details to
147
-    __init__(). They must implement check_segment_for_agent() as
148
-    defined in AgentMechanismDriverBase, which will be called during
149
-    both binding establishment and validation.
111
+    __init__(), and must implement check_segment_for_agent().
150 112
     """
151 113
 
152 114
     def __init__(self, agent_type, vif_type, vif_details,
@@ -171,3 +133,18 @@ class SimpleAgentMechanismDriverBase(AgentMechanismDriverBase):
171 133
             return True
172 134
         else:
173 135
             return False
136
+
137
+    @abstractmethod
138
+    def check_segment_for_agent(self, segment, agent):
139
+        """Check if segment can be bound for agent.
140
+
141
+        :param segment: segment dictionary describing segment to bind
142
+        :param agent: agents_db entry describing agent to bind
143
+        :returns: True iff segment can be bound for agent
144
+
145
+        Called inside transaction during bind_port so that derived
146
+        MechanismDrivers can use agent_db data along with built-in
147
+        knowledge of the corresponding agent's capabilities to
148
+        determine whether or not the specified network segment can be
149
+        bound for the agent.
150
+        """

+ 0
- 12
neutron/plugins/ml2/drivers/mechanism_odl.py View File

@@ -344,18 +344,6 @@ class OpenDaylightMechanismDriver(api.MechanismDriver):
344 344
                            'physnet': segment[api.PHYSICAL_NETWORK],
345 345
                            'nettype': segment[api.NETWORK_TYPE]})
346 346
 
347
-    def validate_port_binding(self, context):
348
-        if self.check_segment(context.bound_segment):
349
-            LOG.debug(_('Binding valid.'))
350
-            return True
351
-        LOG.warning(_("Binding invalid for port: %s"), context.current)
352
-
353
-    def unbind_port(self, context):
354
-        LOG.debug(_("Unbinding port %(port)s on "
355
-                    "network %(network)s"),
356
-                  {'port': context.current['id'],
357
-                   'network': context.network.current['id']})
358
-
359 347
     def check_segment(self, segment):
360 348
         """Verify a segment is valid for the OpenDaylight MechanismDriver.
361 349
 

+ 0
- 44
neutron/plugins/ml2/managers.py View File

@@ -471,47 +471,3 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
471 471
         LOG.warning(_("Failed to bind port %(port)s on host %(host)s"),
472 472
                     {'port': context._port['id'],
473 473
                      'host': binding.host})
474
-
475
-    def validate_port_binding(self, context):
476
-        """Check whether existing port binding is still valid.
477
-
478
-        :param context: PortContext instance describing the port
479
-        :returns: True if binding is valid, otherwise False
480
-
481
-        Called inside transaction context on session to validate that
482
-        the bound MechanismDriver's existing binding for the port is
483
-        still valid.
484
-        """
485
-        binding = context._binding
486
-        driver = self.mech_drivers.get(binding.driver, None)
487
-        if driver:
488
-            try:
489
-                return driver.obj.validate_port_binding(context)
490
-            except Exception:
491
-                LOG.exception(_("Mechanism driver %s failed in "
492
-                                "validate_port_binding"),
493
-                              driver.name)
494
-        return False
495
-
496
-    def unbind_port(self, context):
497
-        """Undo existing port binding.
498
-
499
-        :param context: PortContext instance describing the port
500
-
501
-        Called inside transaction context on session to notify the
502
-        bound MechanismDriver that its existing binding for the port
503
-        is no longer valid.
504
-        """
505
-        binding = context._binding
506
-        driver = self.mech_drivers.get(binding.driver, None)
507
-        if driver:
508
-            try:
509
-                driver.obj.unbind_port(context)
510
-            except Exception:
511
-                LOG.exception(_("Mechanism driver %s failed in "
512
-                                "unbind_port"),
513
-                              driver.name)
514
-        binding.vif_type = portbindings.VIF_TYPE_UNBOUND
515
-        binding.vif_details = ''
516
-        binding.driver = None
517
-        binding.segment = None

+ 6
- 7
neutron/plugins/ml2/plugin.py View File

@@ -226,11 +226,9 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
226 226
 
227 227
         if binding.vif_type != portbindings.VIF_TYPE_UNBOUND:
228 228
             if (not host_set and not vnic_type_set and not profile_set and
229
-                binding.segment and
230
-                self.mechanism_manager.validate_port_binding(mech_context)):
229
+                binding.segment):
231 230
                 return False
232
-            self.mechanism_manager.unbind_port(mech_context)
233
-            self._update_port_dict_binding(port, binding)
231
+            self._delete_port_binding(mech_context)
234 232
 
235 233
         # Return True only if an agent notification is needed.
236 234
         # This will happen if a new host, vnic_type, or profile was specified
@@ -294,10 +292,12 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
294 292
 
295 293
     def _delete_port_binding(self, mech_context):
296 294
         binding = mech_context._binding
295
+        binding.vif_type = portbindings.VIF_TYPE_UNBOUND
296
+        binding.vif_details = ''
297
+        binding.driver = None
298
+        binding.segment = None
297 299
         port = mech_context.current
298 300
         self._update_port_dict_binding(port, binding)
299
-        self.mechanism_manager.unbind_port(mech_context)
300
-        self._update_port_dict_binding(port, binding)
301 301
 
302 302
     def _ml2_extend_port_dict_binding(self, port_res, port_db):
303 303
         # None when called during unit tests for other plugins.
@@ -720,7 +720,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
720 720
             mech_context = driver_context.PortContext(self, context, port,
721 721
                                                       network)
722 722
             self.mechanism_manager.delete_port_precommit(mech_context)
723
-            self._delete_port_binding(mech_context)
724 723
             self._delete_port_security_group_bindings(context, id)
725 724
             LOG.debug(_("Calling base delete_port"))
726 725
             if l3plugin:

+ 0
- 8
neutron/tests/unit/ml2/_test_mech_agent.py View File

@@ -142,8 +142,6 @@ class AgentMechanismLocalTestCase(AgentMechanismBaseTestCase):
142 142
                                   self.LOCAL_SEGMENTS)
143 143
         self.driver.bind_port(context)
144 144
         self._check_bound(context, self.LOCAL_SEGMENTS[1])
145
-        self.assertTrue(self.driver.validate_port_binding(context))
146
-        self.driver.unbind_port(context)
147 145
 
148 146
     def test_type_local_dead(self):
149 147
         context = FakePortContext(self.AGENT_TYPE,
@@ -166,8 +164,6 @@ class AgentMechanismFlatTestCase(AgentMechanismBaseTestCase):
166 164
                                   self.FLAT_SEGMENTS)
167 165
         self.driver.bind_port(context)
168 166
         self._check_bound(context, self.FLAT_SEGMENTS[1])
169
-        self.assertTrue(self.driver.validate_port_binding(context))
170
-        self.driver.unbind_port(context)
171 167
 
172 168
     def test_type_flat_bad(self):
173 169
         context = FakePortContext(self.AGENT_TYPE,
@@ -191,8 +187,6 @@ class AgentMechanismVlanTestCase(AgentMechanismBaseTestCase):
191 187
                                   self.VLAN_SEGMENTS)
192 188
         self.driver.bind_port(context)
193 189
         self._check_bound(context, self.VLAN_SEGMENTS[1])
194
-        self.assertTrue(self.driver.validate_port_binding(context))
195
-        self.driver.unbind_port(context)
196 190
 
197 191
     def test_type_vlan_bad(self):
198 192
         context = FakePortContext(self.AGENT_TYPE,
@@ -215,8 +209,6 @@ class AgentMechanismGreTestCase(AgentMechanismBaseTestCase):
215 209
                                   self.GRE_SEGMENTS)
216 210
         self.driver.bind_port(context)
217 211
         self._check_bound(context, self.GRE_SEGMENTS[1])
218
-        self.assertTrue(self.driver.validate_port_binding(context))
219
-        self.driver.unbind_port(context)
220 212
 
221 213
     def test_type_gre_bad(self):
222 214
         context = FakePortContext(self.AGENT_TYPE,

+ 0
- 6
neutron/tests/unit/ml2/drivers/mechanism_logger.py View File

@@ -118,9 +118,3 @@ class LoggerMechanismDriver(api.MechanismDriver):
118 118
 
119 119
     def bind_port(self, context):
120 120
         self._log_port_call("bind_port", context)
121
-
122
-    def validate_port_binding(self, context):
123
-        self._log_port_call("validate_port_binding", context)
124
-
125
-    def unbind_port(self, context):
126
-        self._log_port_call("unbind_port", context)

+ 15
- 18
neutron/tests/unit/ml2/drivers/mechanism_test.py View File

@@ -21,7 +21,7 @@ class TestMechanismDriver(api.MechanismDriver):
21 21
     """Test mechanism driver for testing mechanism driver api."""
22 22
 
23 23
     def initialize(self):
24
-        pass
24
+        self.bound_ports = set()
25 25
 
26 26
     def _check_network_context(self, context, original_expected):
27 27
         assert(isinstance(context, api.NetworkContext))
@@ -87,13 +87,16 @@ class TestMechanismDriver(api.MechanismDriver):
87 87
 
88 88
         vif_type = context.current.get(portbindings.VIF_TYPE)
89 89
         assert(vif_type is not None)
90
+
90 91
         if vif_type in (portbindings.VIF_TYPE_UNBOUND,
91 92
                         portbindings.VIF_TYPE_BINDING_FAILED):
92 93
             assert(context.bound_segment is None)
93 94
             assert(context.bound_driver is None)
95
+            assert(context.current['id'] not in self.bound_ports)
94 96
         else:
95 97
             assert(isinstance(context.bound_segment, dict))
96 98
             assert(context.bound_driver == 'test')
99
+            assert(context.current['id'] in self.bound_ports)
97 100
 
98 101
         if original_expected:
99 102
             assert(isinstance(context.original, dict))
@@ -123,6 +126,9 @@ class TestMechanismDriver(api.MechanismDriver):
123 126
         self._check_port_context(context, False)
124 127
 
125 128
     def update_port_precommit(self, context):
129
+        if (context.original_bound_driver == 'test' and
130
+            context.bound_driver != 'test'):
131
+            self.bound_ports.remove(context.original['id'])
126 132
         self._check_port_context(context, True)
127 133
 
128 134
     def update_port_postcommit(self, context):
@@ -135,6 +141,12 @@ class TestMechanismDriver(api.MechanismDriver):
135 141
         self._check_port_context(context, False)
136 142
 
137 143
     def bind_port(self, context):
144
+        # REVISIT(rkukura): The upcoming fix for bug 1276391 will
145
+        # ensure the MDs see the unbinding of the port as a port
146
+        # update prior to re-binding, at which point this should be
147
+        # removed.
148
+        self.bound_ports.discard(context.current['id'])
149
+
138 150
         # REVISIT(rkukura): Currently, bind_port() is called as part
139 151
         # of either a create or update transaction. The fix for bug
140 152
         # 1276391 will change it to be called outside any transaction,
@@ -146,23 +158,8 @@ class TestMechanismDriver(api.MechanismDriver):
146 158
         if host == "host-ovs-no_filter":
147 159
             context.set_binding(segment, portbindings.VIF_TYPE_OVS,
148 160
                                 {portbindings.CAP_PORT_FILTER: False})
161
+            self.bound_ports.add(context.current['id'])
149 162
         elif host == "host-bridge-filter":
150 163
             context.set_binding(segment, portbindings.VIF_TYPE_BRIDGE,
151 164
                                 {portbindings.CAP_PORT_FILTER: True})
152
-
153
-    def validate_port_binding(self, context):
154
-        # REVISIT(rkukura): Currently, validate_port_binding() is
155
-        # called as part of either a create or update transaction. The
156
-        # fix for bug 1276391 will change it to be called outside any
157
-        # transaction (or eliminate it altogether), so the
158
-        # context.original* will no longer be available.
159
-        self._check_port_context(context, context.original is not None)
160
-        return True
161
-
162
-    def unbind_port(self, context):
163
-        # REVISIT(rkukura): Currently, unbind_port() is called as part
164
-        # of either an update or delete transaction. The fix for bug
165
-        # 1276391 will change it to be called outside any transaction
166
-        # (or eliminate it altogether), so the context.original* will
167
-        # no longer be available.
168
-        self._check_port_context(context, context.original is not None)
165
+            self.bound_ports.add(context.current['id'])

Loading…
Cancel
Save