Skip to content

Commit ac3728e

Browse files
Zuulopenstack-gerrit
Zuul
authored and
openstack-gerrit
committed
Merge "Gracefully ERROR in _init_instance if vnic_type changed" into stable/xena
2 parents b27902b + 1a98a1a commit ac3728e

File tree

6 files changed

+252
-7
lines changed

6 files changed

+252
-7
lines changed

‎nova/compute/manager.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,6 +1151,20 @@ def _init_instance(self, context, instance):
11511151
'updated.', instance=instance)
11521152
self._set_instance_obj_error_state(instance)
11531153
return
1154+
except exception.PciDeviceNotFoundById:
1155+
# This is bug 1981813 where the bound port vnic_type has changed
1156+
# from direct to macvtap. Nova does not support that and it
1157+
# already printed an ERROR when the change is detected during
1158+
# _heal_instance_info_cache. Now we print an ERROR again and skip
1159+
# plugging the vifs but let the service startup continue to init
1160+
# the other instances
1161+
LOG.exception(
1162+
'Virtual interface plugging failed for instance. Probably the '
1163+
'vnic_type of the bound port has been changed. Nova does not '
1164+
'support such change.',
1165+
instance=instance
1166+
)
1167+
return
11541168

11551169
if instance.task_state == task_states.RESIZE_MIGRATING:
11561170
# We crashed during resize/migration, so roll back for safety

‎nova/network/neutron.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3236,6 +3236,25 @@ def _build_vif_model(self, context, client, current_neutron_port,
32363236
delegate_create=True,
32373237
)
32383238

3239+
def _log_error_if_vnic_type_changed(
3240+
self, port_id, old_vnic_type, new_vnic_type, instance
3241+
):
3242+
if old_vnic_type and old_vnic_type != new_vnic_type:
3243+
LOG.error(
3244+
'The vnic_type of the bound port %s has '
3245+
'been changed in neutron from "%s" to '
3246+
'"%s". Changing vnic_type of a bound port '
3247+
'is not supported by Nova. To avoid '
3248+
'breaking the connectivity of the instance '
3249+
'please change the port vnic_type back to '
3250+
'"%s".',
3251+
port_id,
3252+
old_vnic_type,
3253+
new_vnic_type,
3254+
old_vnic_type,
3255+
instance=instance
3256+
)
3257+
32393258
def _build_network_info_model(self, context, instance, networks=None,
32403259
port_ids=None, admin_client=None,
32413260
preexisting_port_ids=None,
@@ -3309,6 +3328,12 @@ def _build_network_info_model(self, context, instance, networks=None,
33093328
preexisting_port_ids)
33103329
for index, vif in enumerate(nw_info):
33113330
if vif['id'] == refresh_vif_id:
3331+
self._log_error_if_vnic_type_changed(
3332+
vif['id'],
3333+
vif['vnic_type'],
3334+
refreshed_vif['vnic_type'],
3335+
instance,
3336+
)
33123337
# Update the existing entry.
33133338
nw_info[index] = refreshed_vif
33143339
LOG.debug('Updated VIF entry in instance network '
@@ -3358,13 +3383,22 @@ def _build_network_info_model(self, context, instance, networks=None,
33583383
networks, port_ids = self._gather_port_ids_and_networks(
33593384
context, instance, networks, port_ids, client)
33603385

3386+
old_nw_info = instance.get_network_info()
33613387
nw_info = network_model.NetworkInfo()
33623388
for port_id in port_ids:
33633389
current_neutron_port = current_neutron_port_map.get(port_id)
33643390
if current_neutron_port:
33653391
vif = self._build_vif_model(
33663392
context, client, current_neutron_port, networks,
33673393
preexisting_port_ids)
3394+
for old_vif in old_nw_info:
3395+
if old_vif['id'] == port_id:
3396+
self._log_error_if_vnic_type_changed(
3397+
port_id,
3398+
old_vif['vnic_type'],
3399+
vif['vnic_type'],
3400+
instance,
3401+
)
33683402
nw_info.append(vif)
33693403
elif nw_info_refresh:
33703404
LOG.info('Port %s from network info_cache is no '

‎nova/tests/functional/libvirt/test_pci_sriov_servers.py‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -979,6 +979,14 @@ def test_change_bound_port_vnic_type_kills_compute_at_restart(self):
979979
self.host_mappings['compute1'].cell_mapping
980980
) as cctxt:
981981
compute.manager._heal_instance_info_cache(cctxt)
982+
self.assertIn(
983+
'The vnic_type of the bound port %s has been changed in '
984+
'neutron from "direct" to "macvtap". Changing vnic_type of a '
985+
'bound port is not supported by Nova. To avoid breaking the '
986+
'connectivity of the instance please change the port '
987+
'vnic_type back to "direct".' % port['id'],
988+
self.stdlog.logger.output,
989+
)
982990

983991
def fake_get_ifname_by_pci_address(pci_addr: str, pf_interface=False):
984992
# we want to fail the netdev lookup only if the pci_address is
@@ -1006,17 +1014,18 @@ def fake_get_ifname_by_pci_address(pci_addr: str, pf_interface=False):
10061014
'nova.pci.utils.get_ifname_by_pci_address',
10071015
side_effect=fake_get_ifname_by_pci_address,
10081016
):
1009-
# This is bug 1981813 as the compute service fails to start with an
1010-
# exception.
10111017
# Nova cannot prevent the vnic_type change on a bound port. Neutron
10121018
# should prevent that instead. But the nova-compute should still
10131019
# be able to start up and only log an ERROR for this instance in
10141020
# inconsistent state.
1015-
self.assertRaises(
1016-
exception.PciDeviceNotFoundById,
1017-
self.restart_compute_service,
1018-
'compute1',
1019-
)
1021+
self.restart_compute_service('compute1')
1022+
1023+
self.assertIn(
1024+
'Virtual interface plugging failed for instance. Probably the '
1025+
'vnic_type of the bound port has been changed. Nova does not '
1026+
'support such change.',
1027+
self.stdlog.logger.output,
1028+
)
10201029

10211030

10221031
class SRIOVAttachDetachTest(_PCIServersTestBase):

‎nova/tests/unit/compute/test_compute_mgr.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,6 +1306,36 @@ def test_init_instance_with_binding_failed_vif_type(self):
13061306
self.compute._init_instance(self.context, instance)
13071307
set_error_state.assert_called_once_with(instance)
13081308

1309+
def test_init_instance_vif_plug_fails_missing_pci(self):
1310+
instance = fake_instance.fake_instance_obj(
1311+
self.context,
1312+
uuid=uuids.instance,
1313+
info_cache=None,
1314+
power_state=power_state.RUNNING,
1315+
vm_state=vm_states.ACTIVE,
1316+
task_state=None,
1317+
host=self.compute.host,
1318+
expected_attrs=['info_cache'])
1319+
1320+
with test.nested(
1321+
mock.patch.object(context, 'get_admin_context',
1322+
return_value=self.context),
1323+
mock.patch.object(objects.Instance, 'get_network_info',
1324+
return_value=network_model.NetworkInfo()),
1325+
mock.patch.object(self.compute.driver, 'plug_vifs',
1326+
side_effect=exception.PciDeviceNotFoundById("pci-addr")),
1327+
mock.patch("nova.compute.manager.LOG.exception"),
1328+
) as (get_admin_context, get_nw_info, plug_vifs, log_exception):
1329+
# as this does not raise, we are sure that the compute service
1330+
# continues initializing the rest of the instances
1331+
self.compute._init_instance(self.context, instance)
1332+
log_exception.assert_called_once_with(
1333+
"Virtual interface plugging failed for instance. Probably the "
1334+
"vnic_type of the bound port has been changed. Nova does not "
1335+
"support such change.",
1336+
instance=instance
1337+
)
1338+
13091339
def _test__validate_pinning_configuration(self, supports_pcpus=True):
13101340
instance_1 = fake_instance.fake_instance_obj(
13111341
self.context, uuid=uuids.instance_1)

‎nova/tests/unit/network/test_neutron.py‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3392,6 +3392,155 @@ def test_build_network_info_model_empty(
33923392
mocked_client.list_ports.assert_called_once_with(
33933393
tenant_id=uuids.fake, device_id=uuids.instance)
33943394

3395+
@mock.patch.object(
3396+
neutronapi.API,
3397+
'_get_physnet_tunneled_info',
3398+
new=mock.Mock(return_value=(None, False)))
3399+
@mock.patch.object(
3400+
neutronapi.API,
3401+
'_get_preexisting_port_ids',
3402+
new=mock.Mock(return_value=[]))
3403+
@mock.patch.object(
3404+
neutronapi.API,
3405+
'_get_subnets_from_port',
3406+
new=mock.Mock(return_value=[model.Subnet(cidr='1.0.0.0/8')]))
3407+
@mock.patch.object(
3408+
neutronapi.API,
3409+
'_get_floating_ips_by_fixed_and_port',
3410+
new=mock.Mock(return_value=[{'floating_ip_address': '10.0.0.1'}]))
3411+
@mock.patch.object(neutronapi, 'get_client')
3412+
def test_build_network_info_model_full_vnic_type_change(
3413+
self, mock_get_client
3414+
):
3415+
mocked_client = mock.create_autospec(client.Client)
3416+
mock_get_client.return_value = mocked_client
3417+
fake_inst = objects.Instance()
3418+
fake_inst.project_id = uuids.fake
3419+
fake_inst.uuid = uuids.instance
3420+
fake_ports = [
3421+
{
3422+
"id": "port1",
3423+
"network_id": "net-id",
3424+
"tenant_id": uuids.fake,
3425+
"admin_state_up": True,
3426+
"status": "ACTIVE",
3427+
"fixed_ips": [{"ip_address": "1.1.1.1"}],
3428+
"mac_address": "de:ad:be:ef:00:01",
3429+
"binding:vif_type": model.VIF_TYPE_BRIDGE,
3430+
"binding:vnic_type": model.VNIC_TYPE_DIRECT,
3431+
"binding:vif_details": {},
3432+
},
3433+
]
3434+
mocked_client.list_ports.return_value = {'ports': fake_ports}
3435+
fake_inst.info_cache = objects.InstanceInfoCache.new(
3436+
self.context, uuids.instance)
3437+
fake_inst.info_cache.network_info = model.NetworkInfo.hydrate([])
3438+
3439+
# build the network info first
3440+
nw_infos = self.api._build_network_info_model(
3441+
self.context,
3442+
fake_inst,
3443+
force_refresh=True,
3444+
)
3445+
3446+
self.assertEqual(1, len(nw_infos))
3447+
fake_inst.info_cache.network_info = nw_infos
3448+
3449+
# change the vnic_type of the port and rebuild the network info
3450+
fake_ports[0]["binding:vnic_type"] = model.VNIC_TYPE_MACVTAP
3451+
with mock.patch(
3452+
"nova.network.neutron.API._log_error_if_vnic_type_changed"
3453+
) as mock_log:
3454+
nw_infos = self.api._build_network_info_model(
3455+
self.context,
3456+
fake_inst,
3457+
force_refresh=True,
3458+
)
3459+
3460+
mock_log.assert_called_once_with(
3461+
fake_ports[0]["id"], "direct", "macvtap", fake_inst)
3462+
self.assertEqual(1, len(nw_infos))
3463+
3464+
@mock.patch.object(
3465+
neutronapi.API,
3466+
'_get_physnet_tunneled_info',
3467+
new=mock.Mock(return_value=(None, False)))
3468+
@mock.patch.object(
3469+
neutronapi.API,
3470+
'_get_preexisting_port_ids',
3471+
new=mock.Mock(return_value=[]))
3472+
@mock.patch.object(
3473+
neutronapi.API,
3474+
'_get_subnets_from_port',
3475+
new=mock.Mock(return_value=[model.Subnet(cidr='1.0.0.0/8')]))
3476+
@mock.patch.object(
3477+
neutronapi.API,
3478+
'_get_floating_ips_by_fixed_and_port',
3479+
new=mock.Mock(return_value=[{'floating_ip_address': '10.0.0.1'}]))
3480+
@mock.patch.object(neutronapi, 'get_client')
3481+
def test_build_network_info_model_single_vnic_type_change(
3482+
self, mock_get_client
3483+
):
3484+
mocked_client = mock.create_autospec(client.Client)
3485+
mock_get_client.return_value = mocked_client
3486+
fake_inst = objects.Instance()
3487+
fake_inst.project_id = uuids.fake
3488+
fake_inst.uuid = uuids.instance
3489+
fake_ports = [
3490+
{
3491+
"id": "port1",
3492+
"network_id": "net-id",
3493+
"tenant_id": uuids.fake,
3494+
"admin_state_up": True,
3495+
"status": "ACTIVE",
3496+
"fixed_ips": [{"ip_address": "1.1.1.1"}],
3497+
"mac_address": "de:ad:be:ef:00:01",
3498+
"binding:vif_type": model.VIF_TYPE_BRIDGE,
3499+
"binding:vnic_type": model.VNIC_TYPE_DIRECT,
3500+
"binding:vif_details": {},
3501+
},
3502+
]
3503+
fake_nets = [
3504+
{
3505+
"id": "net-id",
3506+
"name": "foo",
3507+
"tenant_id": uuids.fake,
3508+
}
3509+
]
3510+
mocked_client.list_ports.return_value = {'ports': fake_ports}
3511+
fake_inst.info_cache = objects.InstanceInfoCache.new(
3512+
self.context, uuids.instance)
3513+
fake_inst.info_cache.network_info = model.NetworkInfo.hydrate([])
3514+
3515+
# build the network info first
3516+
nw_infos = self.api._build_network_info_model(
3517+
self.context,
3518+
fake_inst,
3519+
fake_nets,
3520+
[fake_ports[0]["id"]],
3521+
refresh_vif_id=fake_ports[0]["id"],
3522+
)
3523+
3524+
self.assertEqual(1, len(nw_infos))
3525+
fake_inst.info_cache.network_info = nw_infos
3526+
3527+
# change the vnic_type of the port and rebuild the network info
3528+
fake_ports[0]["binding:vnic_type"] = model.VNIC_TYPE_MACVTAP
3529+
with mock.patch(
3530+
"nova.network.neutron.API._log_error_if_vnic_type_changed"
3531+
) as mock_log:
3532+
nw_infos = self.api._build_network_info_model(
3533+
self.context,
3534+
fake_inst,
3535+
fake_nets,
3536+
[fake_ports[0]["id"]],
3537+
refresh_vif_id=fake_ports[0]["id"],
3538+
)
3539+
3540+
mock_log.assert_called_once_with(
3541+
fake_ports[0]["id"], "direct", "macvtap", fake_inst)
3542+
self.assertEqual(1, len(nw_infos))
3543+
33953544
@mock.patch.object(neutronapi, 'get_client')
33963545
def test_get_subnets_from_port(self, mock_get_client):
33973546
mocked_client = mock.create_autospec(client.Client)

‎releasenotes/notes/bug-1981813-vnic-type-change-9f3e16fae885b57f.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
fixes:
3+
- |
4+
`Bug #1981813 <https://bugs.launchpad.net/nova/+bug/1981813>`_: Now nova
5+
detects if the ``vnic_type`` of a bound port has been changed in neutron
6+
and leaves an ERROR message in the compute service log as such change on a
7+
bound port is not supported. Also the restart of the nova-compute service
8+
will not crash any more after such port change. Nova will log an ERROR and
9+
skip the initialization of the instance with such port during the startup.

0 commit comments

Comments
 (0)