openstack
/
neutron
Code Issues Proposed changes

macvtap: Mech driver detects invalid migration 

As of today, Macvtap agent allows invalid live migrations
from a host with interface mapping 'physnet1:eth1' to a host
with mapping 'physnet1:eth2'. Migration places the macvtap on
the target on 'eth2' instead of 'eth1'. This fix detects
such a migration in the mech driver and lets the portbinding fail.

Nova will put the instance in error state. The instance might
still be able to access the wrong network it migrated to, as
today there is no communication to the agent after a failed binding
after a migration. But at least the user is now aware that
something went wrong.

Change-Id: I92cd6411fe88483b921c92f219afa8e846cc0137
Depends-On: Ib1cc44bf9d01baf4d1f1d26c2a368a5ca7c6ab68
Partial-Bug: #1550400
This commit is contained in:
Andreas Scheuring 2016-08-29 17:08:34 +02:00
parent ab324cf161
commit 6865f4d9f2
3 changed files with 120 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
neutron/plugins/ml2/drivers/macvtap/mech_driver/mech_macvtap.py
View File

@ -14,6 +14,7 @@
# License for the specific language governing permissions and limitations
# under the License.
from neutron._i18n import _LE
from neutron_lib import constants
from oslo_log import log
@ -54,6 +55,22 @@ class MacvtapMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase):
"""Macvtap driver vlan transparency support."""
return False
def _is_live_migration(self, context):
# We cannot just check if
# context.original['host_id'] != context.current['host_id']
# This condition is also true, if nova does a reschedule of a
# instance when something went wrong during spawn. In this case,
# context.original['host_id'] is set to the failed host.
# The only safe way to detect a migration is to look into the binding
# profiles 'migrating_to' attribute, which is set by Nova since patch
# https://review.openstack.org/#/c/275073/.
port_profile = context.original.get(portbindings.PROFILE)
if port_profile and port_profile.get('migrating_to', None):
LOG.debug("Live migration with profile %s detected.", port_profile)
return True
else:
return False
def try_to_bind_segment_for_agent(self, context, segment, agent):
if self.check_segment_for_agent(segment, agent):
vif_details_segment = self.vif_details
@ -69,6 +86,35 @@ class MacvtapMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase):
else:
macvtap_src = interface
if self._is_live_migration(context):
# We can use the original port here, as during live migration
# portbinding is done after the migration happened. Nova will
# not do a reschedule of the instance migration if binding
# fails, but just set the instance into error state.
# Due to that we can be sure that the original port is the
# migration source port.
orig_vif_details = context.original[portbindings.VIF_DETAILS]
orig_source = orig_vif_details[
portbindings.VIF_DETAILS_MACVTAP_SOURCE]
if orig_source != macvtap_src:
source_host = context.original[portbindings.HOST_ID]
target_host = agent['host']
LOG.error(_LE("Vif binding denied by mechanism driver. "
"MacVTap source device '%(target_dev)s' on "
"the migration target '%(target_host)s'is "
"not equal to device '%(source_dev)s' on "
"the migration source '%(source_host)s. "
"Make sure that the "
"interface mapping of macvtap "
"agent on both hosts is equal "
"for the physical network '%(physnet)s'!"),
{'source_dev': orig_source,
'target_dev': macvtap_src,
'target_host': target_host,
'source_host': source_host,
'physnet': segment['physical_network']})
return False
vif_details_segment['physical_interface'] = interface
vif_details_segment['macvtap_source'] = macvtap_src
vif_details_segment['macvtap_mode'] = MACVTAP_MODE_BRIDGE

6
neutron/tests/unit/plugins/ml2/_test_mech_agent.py
View File

@ -41,7 +41,8 @@ class FakeNetworkContext(api.NetworkContext):
class FakePortContext(api.PortContext):
def __init__(self, agent_type, agents, segments,
vnic_type=portbindings.VNIC_NORMAL):
vnic_type=portbindings.VNIC_NORMAL,
original=None):
self._agent_type = agent_type
self._agents = agents
self._network_context = FakeNetworkContext(segments)
@ -49,6 +50,7 @@ class FakePortContext(api.PortContext):
self._bound_segment_id = None
self._bound_vif_type = None
self._bound_vif_details = None
self._original = original
@property
def current(self):
@ -57,7 +59,7 @@ class FakePortContext(api.PortContext):
@property
def original(self):
return None
return self._original or {}
@property
def status(self):

75
neutron/tests/unit/plugins/ml2/drivers/macvtap/mech_driver/test_mech_macvtap.py
View File

@ -16,6 +16,7 @@
from neutron_lib import constants
from neutron.extensions import portbindings
from neutron.plugins.ml2 import driver_api as api
from neutron.plugins.ml2.drivers.macvtap.mech_driver import mech_macvtap
from neutron.tests.unit.plugins.ml2 import _test_mech_agent as base
@ -31,9 +32,11 @@ class MacvtapMechanismBaseTestCase(base.AgentMechanismBaseTestCase):
BAD_MAPPINGS = {'wrong_physical_network': 'wrong_if'}
BAD_CONFIGS = {'interface_mappings': BAD_MAPPINGS}
AGENTS = [{'alive': True,
'configurations': GOOD_CONFIGS,
'host': 'host'}]
AGENT = {'alive': True,
'configurations': GOOD_CONFIGS,
'host': 'host'}
AGENTS = [AGENT]
AGENTS_DEAD = [{'alive': False,
'configurations': GOOD_CONFIGS,
'host': 'dead_host'}]
@ -55,8 +58,64 @@ class MacvtapMechanismGenericTestCase(MacvtapMechanismBaseTestCase,
pass
class MacvtapMechanismMigrationTestCase(object):
# MIGRATION_SEGMENT must be overridden for the specific type being tested
MIGRATION_SEGMENT = None
MIGRATION_SEGMENTS = [MIGRATION_SEGMENT]
def test__is_live_migration_true(self):
original = {"binding:profile": {"migrating_to": "host"}}
self._test__is_live_migration(True, original)
def test__is_live_migration_false(self):
self._test__is_live_migration(False, {})
def _test__is_live_migration(self, expected, original):
context = base.FakePortContext(self.AGENT_TYPE,
self.AGENTS,
self.MIGRATION_SEGMENTS,
vnic_type=self.VNIC_TYPE,
original=original)
self.assertEqual(expected, self.driver._is_live_migration(context))
def _test_try_to_bind_segment_for_agent_migration(self, expected,
original):
context = base.FakePortContext(self.AGENT_TYPE,
self.AGENTS,
self.MIGRATION_SEGMENTS,
vnic_type=self.VNIC_TYPE,
original=original)
result = self.driver.try_to_bind_segment_for_agent(
context, self.MIGRATION_SEGMENT, self.AGENT)
self.assertEqual(expected, result)
def test_try_to_bind_segment_for_agent_migration_abort(self):
original = {"binding:profile": {"migrating_to": "host"},
"binding:vif_details": {"macvtap_source": "bad_source"},
"binding:host_id": "source_host"}
self._test_try_to_bind_segment_for_agent_migration(False, original)
def test_try_to_bind_segment_for_agent_migration_ok(self):
macvtap_src = "fake_if"
seg_id = self.MIGRATION_SEGMENT.get(api.SEGMENTATION_ID)
if seg_id:
# In the vlan case, macvtap source name ends with .vlan_id
macvtap_src += "." + str(seg_id)
original = {"binding:profile": {"migrating_to": "host"},
"binding:vif_details": {"macvtap_source": macvtap_src},
"binding:host_id": "source_host"}
self._test_try_to_bind_segment_for_agent_migration(True, original)
class MacvtapMechanismFlatTestCase(MacvtapMechanismBaseTestCase,
base.AgentMechanismFlatTestCase):
base.AgentMechanismFlatTestCase,
MacvtapMechanismMigrationTestCase):
MIGRATION_SEGMENT = {api.ID: 'flat_segment_id',
api.NETWORK_TYPE: 'flat',
api.PHYSICAL_NETWORK: 'fake_physical_network'}
def test_type_flat_vif_details(self):
context = base.FakePortContext(self.AGENT_TYPE,
self.AGENTS,
@ -75,7 +134,13 @@ class MacvtapMechanismFlatTestCase(MacvtapMechanismBaseTestCase,
class MacvtapMechanismVlanTestCase(MacvtapMechanismBaseTestCase,
base.AgentMechanismVlanTestCase):
base.AgentMechanismVlanTestCase,
MacvtapMechanismMigrationTestCase):
MIGRATION_SEGMENT = {api.ID: 'vlan_segment_id',
api.NETWORK_TYPE: 'vlan',
api.PHYSICAL_NETWORK: 'fake_physical_network',
api.SEGMENTATION_ID: 1234}
def test_type_vlan_vif_details(self):
context = base.FakePortContext(self.AGENT_TYPE,
self.AGENTS,