Skip to content

Commit d2045c0

Browse files
Zuulopenstack-gerrit
Zuul
authored and
openstack-gerrit
committed
Merge "Do not make actual device changes in bind_port()"
2 parents d418157 + 0088fde commit d2045c0

File tree

2 files changed

+154
-51
lines changed

2 files changed

+154
-51
lines changed

‎networking_generic_switch/generic_switch_mech.py‎

Lines changed: 34 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from neutron.db import provisioning_blocks
1818
from neutron_lib.api.definitions import portbindings
1919
from neutron_lib.callbacks import resources
20+
from neutron_lib import constants as const
2021
from neutron_lib.plugins.ml2 import api
2122
from oslo_log import log as logging
2223

@@ -340,20 +341,50 @@ def update_port_postcommit(self, context):
340341
state changes that it does not know or care about.
341342
"""
342343
port = context.current
344+
network = context.network.current
343345
if self._is_port_bound(port):
344346
binding_profile = port['binding:profile']
345347
local_link_information = binding_profile.get(
346348
'local_link_information')
347349
if not local_link_information:
348350
return
351+
# Necessary because the "provisioning_complete" event triggers
352+
# an additional call to update_port_postcommit(). We don't
353+
# want to configure the port a second time.
354+
if port['status'] == const.PORT_STATUS_ACTIVE:
355+
LOG.debug("Port %(port_id)s is already active, "
356+
"not doing anything",
357+
{'port_id': port['id']})
358+
return
359+
# If binding has already succeeded, we should have valid links
360+
# at this point, but check just in case.
361+
if not self._is_link_valid(port, network):
362+
return
363+
is_802_3ad = self._is_802_3ad(port)
349364
for link in local_link_information:
365+
port_id = link.get('port_id')
350366
switch_info = link.get('switch_info')
351367
switch_id = link.get('switch_id')
352368
switch = device_utils.get_switch_device(
353369
self.switches, switch_info=switch_info,
354370
ngs_mac_address=switch_id)
355-
if not switch:
356-
return
371+
372+
# If segmentation ID is None, set vlan 1
373+
segmentation_id = network.get('provider:segmentation_id') or 1
374+
LOG.debug("Putting switch port %(switch_port)s on "
375+
"%(switch_info)s in vlan %(segmentation_id)s",
376+
{'switch_port': port_id, 'switch_info': switch_info,
377+
'segmentation_id': segmentation_id})
378+
# Move port to network
379+
if is_802_3ad and hasattr(switch, 'plug_bond_to_network'):
380+
switch.plug_bond_to_network(port_id, segmentation_id)
381+
else:
382+
switch.plug_port_to_network(port_id, segmentation_id)
383+
LOG.info("Successfully plugged port %(port_id)s in segment "
384+
"%(segment_id)s on device %(device)s",
385+
{'port_id': port['id'], 'device': switch_info,
386+
'segment_id': segmentation_id})
387+
357388
provisioning_blocks.provisioning_complete(
358389
context._plugin_context, port['id'], resources.PORT,
359390
GENERIC_SWITCH_ENTITY)
@@ -446,32 +477,7 @@ def bind_port(self, context):
446477
if not self._is_link_valid(port, network):
447478
return
448479

449-
is_802_3ad = self._is_802_3ad(port)
450-
for link in local_link_information:
451-
port_id = link.get('port_id')
452-
switch_info = link.get('switch_info')
453-
switch_id = link.get('switch_id')
454-
switch = device_utils.get_switch_device(
455-
self.switches, switch_info=switch_info,
456-
ngs_mac_address=switch_id)
457-
458-
segments = context.segments_to_bind
459-
# If segmentation ID is None, set vlan 1
460-
segmentation_id = segments[0].get('segmentation_id') or 1
461-
LOG.debug("Putting port %(port_id)s on %(switch_info)s "
462-
"to vlan: %(segmentation_id)s",
463-
{'port_id': port_id, 'switch_info': switch_info,
464-
'segmentation_id': segmentation_id})
465-
# Move port to network
466-
if is_802_3ad and hasattr(switch, 'plug_bond_to_network'):
467-
switch.plug_bond_to_network(port_id, segmentation_id)
468-
else:
469-
switch.plug_port_to_network(port_id, segmentation_id)
470-
LOG.info("Successfully bound port %(port_id)s in segment "
471-
"%(segment_id)s on device %(device)s",
472-
{'port_id': port['id'], 'device': switch_info,
473-
'segment_id': segmentation_id})
474-
480+
segments = context.segments_to_bind
475481
context.set_binding(segments[0][api.ID],
476482
portbindings.VIF_TYPE_OTHER, {})
477483
provisioning_blocks.add_provisioning_component(

‎networking_generic_switch/tests/unit/test_generic_switch_mech.py‎

Lines changed: 120 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ def test_delete_port_postcommit_failure(self, m_list):
342342
driver.delete_port_postcommit,
343343
mock_context)
344344

345-
def test_delete_port_potcommit_unknown_switch(self, m_list):
345+
def test_delete_port_postcommit_unknown_switch(self, m_list):
346346
driver = gsm.GenericSwitchDriver()
347347
driver.initialize()
348348
mock_context = mock.create_autospec(driver_context.PortContext)
@@ -408,12 +408,14 @@ def test_update_port_postcommit_not_bound(self, m_pc, m_list):
408408
},
409409
'binding:vnic_type': 'baremetal',
410410
'id': '123',
411-
'binding:vif_type': 'unbound'}
411+
'binding:vif_type': 'unbound',
412+
'status': 'DOWN'}
412413
mock_context.original = {'binding:profile': {},
413414
'binding:vnic_type': 'baremetal',
414415
'id': '123',
415416
'binding:vif_type': 'unbound'}
416417
driver.update_port_postcommit(mock_context)
418+
self.switch_mock.plug_port_to_network.assert_not_called()
417419
self.assertFalse(m_pc.called)
418420

419421
@mock.patch.object(provisioning_blocks, 'provisioning_complete')
@@ -433,12 +435,14 @@ def test_update_port_postcommit_not_baremetal(self, m_pc, m_list):
433435
},
434436
'binding:vnic_type': 'mcvtap',
435437
'id': '123',
436-
'binding:vif_type': 'other'}
438+
'binding:vif_type': 'other',
439+
'status': 'DOWN'}
437440
mock_context.original = {'binding:profile': {},
438441
'binding:vnic_type': 'mcvtap',
439442
'id': '123',
440443
'binding:vif_type': 'unbound'}
441444
driver.update_port_postcommit(mock_context)
445+
self.switch_mock.plug_port_to_network.assert_not_called()
442446
self.assertFalse(m_pc.called)
443447

444448
@mock.patch.object(provisioning_blocks, 'provisioning_complete')
@@ -450,12 +454,14 @@ def test_update_port_postcommit_no_llc(self, m_pc, m_list):
450454
mock_context.current = {'binding:profile': {},
451455
'binding:vnic_type': 'baremetal',
452456
'id': '123',
453-
'binding:vif_type': 'other'}
457+
'binding:vif_type': 'other',
458+
'status': 'DOWN'}
454459
mock_context.original = {'binding:profile': {},
455460
'binding:vnic_type': 'baremetal',
456461
'id': '123',
457462
'binding:vif_type': 'unbound'}
458463
driver.update_port_postcommit(mock_context)
464+
self.switch_mock.plug_port_to_network.assert_not_called()
459465
self.assertFalse(m_pc.called)
460466

461467
@mock.patch.object(provisioning_blocks, 'provisioning_complete')
@@ -475,12 +481,14 @@ def test_update_port_postcommit_not_managed_by_ngs(self, m_pc, m_list):
475481
},
476482
'binding:vnic_type': 'baremetal',
477483
'id': '123',
478-
'binding:vif_type': 'other'}
484+
'binding:vif_type': 'other',
485+
'status': 'DOWN'}
479486
mock_context.original = {'binding:profile': {},
480487
'binding:vnic_type': 'baremetal',
481488
'id': '123',
482489
'binding:vif_type': 'unbound'}
483490
driver.update_port_postcommit(mock_context)
491+
self.switch_mock.plug_port_to_network.assert_not_called()
484492
self.assertFalse(m_pc.called)
485493

486494
@mock.patch.object(provisioning_blocks, 'provisioning_complete')
@@ -500,13 +508,20 @@ def test_update_port_postcommit_complete_provisioning(self, m_pc, m_list):
500508
},
501509
'binding:vnic_type': 'baremetal',
502510
'id': '123',
503-
'binding:vif_type': 'other'}
511+
'binding:vif_type': 'other',
512+
'status': 'DOWN'}
504513
mock_context.original = {'binding:profile': {},
505514
'binding:vnic_type': 'baremetal',
506515
'id': '123',
507516
'binding:vif_type': 'unbound'}
517+
mock_context.network = mock.Mock()
518+
mock_context.network.current = {
519+
'provider:segmentation_id': 42,
520+
'provider:physical_network': 'physnet1'
521+
}
508522
driver.update_port_postcommit(mock_context)
509-
self.switch_mock.plug_port_to_network.assert_not_called()
523+
self.switch_mock.plug_port_to_network.assert_called_once_with(
524+
2222, 42)
510525
m_pc.assert_called_once_with(mock_context._plugin_context,
511526
mock_context.current['id'],
512527
resources.PORT,
@@ -535,13 +550,23 @@ def test_update_portgroup_postcommit_complete_provisioning(self,
535550
},
536551
'binding:vnic_type': 'baremetal',
537552
'id': '123',
538-
'binding:vif_type': 'other'}
553+
'binding:vif_type': 'other',
554+
'status': 'DOWN'}
539555
mock_context.original = {'binding:profile': {},
540556
'binding:vnic_type': 'baremetal',
541557
'id': '123',
542558
'binding:vif_type': 'unbound'}
559+
mock_context.network = mock.Mock()
560+
mock_context.network.current = {
561+
'provider:segmentation_id': 42,
562+
'provider:physical_network': 'physnet1'
563+
}
543564
driver.update_port_postcommit(mock_context)
544-
self.switch_mock.plug_port_to_network.assert_not_called()
565+
self.switch_mock.plug_port_to_network.assert_has_calls(
566+
[mock.call(2222, 42),
567+
mock.call(3333, 42)]
568+
)
569+
self.switch_mock.plug_bond_to_network.assert_not_called()
545570
m_pc.assert_has_calls([mock.call(mock_context._plugin_context,
546571
mock_context.current['id'],
547572
resources.PORT,
@@ -574,18 +599,97 @@ def test_update_portgroup_postcommit_complete_provisioning_802_3ad(self,
574599
},
575600
'binding:vnic_type': 'baremetal',
576601
'id': '123',
577-
'binding:vif_type': 'other'}
602+
'binding:vif_type': 'other',
603+
'status': 'DOWN'}
578604
mock_context.original = {'binding:profile': {},
579605
'binding:vnic_type': 'baremetal',
580606
'id': '123',
581607
'binding:vif_type': 'unbound'}
608+
mock_context.network = mock.Mock()
609+
mock_context.network.current = {
610+
'provider:segmentation_id': 42,
611+
'provider:physical_network': 'physnet1'
612+
}
582613
driver.update_port_postcommit(mock_context)
583-
self.switch_mock.plug_bond_to_network.assert_not_called()
614+
self.switch_mock.plug_bond_to_network.assert_has_calls(
615+
[mock.call(2222, 42),
616+
mock.call(3333, 42)]
617+
)
618+
self.switch_mock.plug_port_to_network.assert_not_called()
584619
m_pc.assert_has_calls([mock.call(mock_context._plugin_context,
585620
mock_context.current['id'],
586621
resources.PORT,
587622
'GENERICSWITCH')])
588623

624+
@mock.patch.object(provisioning_blocks, 'provisioning_complete')
625+
def test_update_port_postcommit_with_physnet(self, m_pc, m_list):
626+
driver = gsm.GenericSwitchDriver()
627+
driver.initialize()
628+
mock_context = mock.create_autospec(driver_context.PortContext)
629+
mock_context._plugin_context = mock.MagicMock()
630+
mock_context.current = {'binding:profile':
631+
{'local_link_information':
632+
[
633+
{
634+
'switch_info': 'foo',
635+
'port_id': 2222
636+
}
637+
]
638+
},
639+
'binding:vnic_type': 'baremetal',
640+
'id': '123',
641+
'binding:vif_type': 'other',
642+
'status': 'DOWN'}
643+
mock_context.original = {'binding:profile': {},
644+
'binding:vnic_type': 'baremetal',
645+
'id': '123',
646+
'binding:vif_type': 'unbound'}
647+
mock_context.network = mock.Mock()
648+
mock_context.network.current = {
649+
'provider:physical_network': 'physnet1'
650+
}
651+
self.switch_mock._get_physical_networks.return_value = ['physnet1']
652+
653+
driver.update_port_postcommit(mock_context)
654+
self.switch_mock.plug_port_to_network.assert_called_once_with(
655+
2222, 1)
656+
m_pc.assert_called_once_with(mock_context._plugin_context,
657+
mock_context.current['id'],
658+
resources.PORT,
659+
'GENERICSWITCH')
660+
661+
@mock.patch.object(provisioning_blocks, 'provisioning_complete')
662+
def test_update_port_postcommit_with_different_physnet(self, m_pc, m_list):
663+
driver = gsm.GenericSwitchDriver()
664+
driver.initialize()
665+
mock_context = mock.create_autospec(driver_context.PortContext)
666+
mock_context._plugin_context = mock.MagicMock()
667+
mock_context.current = {'binding:profile':
668+
{'local_link_information':
669+
[
670+
{
671+
'switch_info': 'foo',
672+
'port_id': 2222
673+
}
674+
]
675+
},
676+
'binding:vnic_type': 'baremetal',
677+
'id': '123',
678+
'binding:vif_type': 'other',
679+
'status': 'DOWN'}
680+
mock_context.original = {'binding:profile': {},
681+
'binding:vnic_type': 'baremetal',
682+
'id': '123',
683+
'binding:vif_type': 'unbound'}
684+
mock_context.network.current = {
685+
'provider:physical_network': 'physnet1'
686+
}
687+
self.switch_mock._get_physical_networks.return_value = ['physnet2']
688+
689+
driver.update_port_postcommit(mock_context)
690+
self.switch_mock.plug_port_to_network.assert_not_called()
691+
m_pc.assert_not_called()
692+
589693
@mock.patch.object(provisioning_blocks, 'provisioning_complete')
590694
def test_update_port_postcommit_unbind_not_bound(self, m_pc, m_list):
591695
driver = gsm.GenericSwitchDriver()
@@ -775,13 +879,12 @@ def test_bind_port(self, m_apc, m_list):
775879
]
776880

777881
driver.bind_port(mock_context)
778-
self.switch_mock.plug_port_to_network.assert_called_once_with(
779-
2222, 1)
780882
mock_context.set_binding.assert_called_with(123, 'other', {})
781883
m_apc.assert_called_once_with(mock_context._plugin_context,
782884
mock_context.current['id'],
783885
resources.PORT,
784886
'GENERICSWITCH')
887+
self.switch_mock.plug_port_to_network.assert_not_called()
785888

786889
@mock.patch.object(provisioning_blocks, 'add_provisioning_component')
787890
def test_bind_portgroup(self, m_apc, m_list):
@@ -815,17 +918,14 @@ def test_bind_portgroup(self, m_apc, m_list):
815918
]
816919

817920
driver.bind_port(mock_context)
818-
self.switch_mock.plug_port_to_network.assert_has_calls(
819-
[mock.call(2222, 1),
820-
mock.call(3333, 1)]
821-
)
822921
mock_context.set_binding.assert_has_calls(
823922
[mock.call(123, 'other', {})]
824923
)
825924
m_apc.assert_has_calls([mock.call(mock_context._plugin_context,
826925
mock_context.current['id'],
827926
resources.PORT,
828927
'GENERICSWITCH')])
928+
self.switch_mock.plug_port_to_network.assert_not_called()
829929

830930
@mock.patch.object(provisioning_blocks, 'add_provisioning_component')
831931
def test_bind_portgroup_802_3ad(self, m_apc, m_list):
@@ -863,17 +963,15 @@ def test_bind_portgroup_802_3ad(self, m_apc, m_list):
863963
]
864964

865965
driver.bind_port(mock_context)
866-
self.switch_mock.plug_bond_to_network.assert_has_calls(
867-
[mock.call(2222, 1),
868-
mock.call(3333, 1)]
869-
)
870966
mock_context.set_binding.assert_has_calls(
871967
[mock.call(123, 'other', {})]
872968
)
873969
m_apc.assert_has_calls([mock.call(mock_context._plugin_context,
874970
mock_context.current['id'],
875971
resources.PORT,
876972
'GENERICSWITCH')])
973+
self.switch_mock.plug_port_to_network.assert_not_called()
974+
self.switch_mock.plug_bond_to_network.assert_not_called()
877975

878976
@mock.patch.object(provisioning_blocks, 'add_provisioning_component')
879977
def test_bind_port_with_physnet(self, m_apc, m_list):
@@ -904,13 +1002,12 @@ def test_bind_port_with_physnet(self, m_apc, m_list):
9041002
self.switch_mock._get_physical_networks.return_value = ['physnet1']
9051003

9061004
driver.bind_port(mock_context)
907-
self.switch_mock.plug_port_to_network.assert_called_once_with(
908-
2222, 1)
9091005
mock_context.set_binding.assert_called_with(123, 'other', {})
9101006
m_apc.assert_called_once_with(mock_context._plugin_context,
9111007
mock_context.current['id'],
9121008
resources.PORT,
9131009
'GENERICSWITCH')
1010+
self.switch_mock.plug_port_to_network.assert_not_called()
9141011

9151012
@mock.patch.object(provisioning_blocks, 'add_provisioning_component')
9161013
def test_bind_port_unknown_switch(self, m_apc, m_list):

0 commit comments

Comments
 (0)