diff options
| -rw-r--r-- | ironic/common/neutron.py | 6 | ||||
| -rw-r--r-- | ironic/drivers/modules/network/common.py | 6 | ||||
| -rw-r--r-- | ironic/drivers/modules/network/neutron.py | 6 | ||||
| -rw-r--r-- | ironic/tests/unit/common/test_neutron.py | 8 | ||||
| -rw-r--r-- | ironic/tests/unit/drivers/modules/network/test_neutron.py | 90 | ||||
| -rw-r--r-- | releasenotes/notes/portgroup-mac-649ed31c3525e4f0.yaml | 7 |
6 files changed, 116 insertions, 7 deletions
diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index c6c84f3df..e81f360aa 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -104,7 +104,7 @@ def update_neutron_port(context, port_id, attrs, client=None): return client.update_port(port_id, **attrs) -def unbind_neutron_port(port_id, client=None, context=None): +def unbind_neutron_port(port_id, client=None, context=None, reset_mac=True): """Unbind a neutron port Remove a neutron port's binding profile and host ID so that it returns to @@ -113,6 +113,7 @@ def unbind_neutron_port(port_id, client=None, context=None): :param port_id: Neutron port ID. :param client: Optional a Neutron client object. :param context: request context + :param reset_mac: reset mac address :type context: ironic.common.context.RequestContext :raises: NetworkError """ @@ -126,7 +127,8 @@ def unbind_neutron_port(port_id, client=None, context=None): # Exception PortBound will be raised by neutron as it refuses to # update the mac address of a bound port if we attempt to unbind and # reset the mac in the same call. - update_neutron_port(context, port_id, attrs_reset_mac, client) + if reset_mac: + update_neutron_port(context, port_id, attrs_reset_mac, client) # NOTE(vsaienko): Ignore if port was deleted before calling vif detach. except openstack_exc.ResourceNotFound: LOG.info('Port %s was not found while unbinding.', port_id) diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py index 3798c2bd4..f10089d26 100644 --- a/ironic/drivers/modules/network/common.py +++ b/ironic/drivers/modules/network/common.py @@ -268,8 +268,10 @@ def plug_port_to_tenant_network(task, port_like_obj, client=None): # because other port attributes may have been set by the user or # nova. port_attrs = {'binding:vnic_type': neutron.VNIC_BAREMETAL, - 'binding:host_id': node.uuid, - 'mac_address': port_like_obj.address} + 'binding:host_id': node.uuid} + # NOTE(kaifeng) Only update mac address when it's available + if port_like_obj.address: + port_attrs['mac_address'] = port_like_obj.address binding_profile = {'local_link_information': local_link_info} if local_group_info: binding_profile['local_group_information'] = local_group_info diff --git a/ironic/drivers/modules/network/neutron.py b/ironic/drivers/modules/network/neutron.py index 379be9e5c..68520cbf1 100644 --- a/ironic/drivers/modules/network/neutron.py +++ b/ironic/drivers/modules/network/neutron.py @@ -235,7 +235,11 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, link_info = port_like_obj.local_link_connection neutron.wait_for_host_agent(client, link_info['hostname']) - neutron.unbind_neutron_port(vif_port_id, context=task.context) + # NOTE(kaifeng) address is optional for port group, avoid to + # regenerate mac when the address is absent. + reset_mac = bool(port_like_obj.address) + neutron.unbind_neutron_port(vif_port_id, context=task.context, + reset_mac=reset_mac) def need_power_on(self, task): """Check if the node has any Smart NIC ports diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index 07dcb290e..2263dd2c7 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/ironic/tests/unit/common/test_neutron.py @@ -1207,6 +1207,14 @@ class TestUnbindPort(base.TestCase): neutron.unbind_neutron_port(port_id, context=self.context) mock_unp.assert_has_calls(update_calls) + def test_unbind_neutron_port_not_reset_mac(self, mock_unp): + port_id = 'fake-port-id' + attr_unbind = {'binding:host_id': '', 'binding:profile': {}} + neutron.unbind_neutron_port(port_id, context=self.context, + reset_mac=False) + mock_unp.assert_called_once_with(self.context, port_id, attr_unbind, + None) + @mock.patch.object(neutron, 'LOG', autospec=True) def test_unbind_neutron_port_not_found(self, mock_log, mock_unp): port_id = 'fake-port-id' diff --git a/ironic/tests/unit/drivers/modules/network/test_neutron.py b/ironic/tests/unit/drivers/modules/network/test_neutron.py index bee21422c..895c8ec5a 100644 --- a/ironic/tests/unit/drivers/modules/network/test_neutron.py +++ b/ironic/tests/unit/drivers/modules/network/test_neutron.py @@ -533,7 +533,8 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.unconfigure_tenant_networks(task) mock_unbind_port.assert_called_once_with( - self.port.extra['vif_port_id'], context=task.context) + self.port.extra['vif_port_id'], context=task.context, + reset_mac=True) @mock.patch.object(neutron_common, 'get_client', autospec=True) @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True) @@ -550,9 +551,36 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.unconfigure_tenant_networks(task) mock_unbind_port.assert_called_once_with( - self.port.extra['vif_port_id'], context=task.context) + self.port.extra['vif_port_id'], context=task.context, + reset_mac=True) wait_agent_mock.assert_called_once_with(nclient, 'hostname') + @mock.patch.object(neutron_common, 'unbind_neutron_port', autospec=True) + def test_unconfigure_tenant_networks_portgroup_1(self, mock_unbind_port): + pg = utils.create_test_portgroup( + self.context, node_id=self.node.id, address='ff:54:00:cf:2d:32', + internal_info={'tenant_vif_port_id': uuidutils.generate_uuid()}) + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.unconfigure_tenant_networks(task) + mock_unbind_port.assert_has_calls([ + mock.call(self.port.extra['vif_port_id'], context=task.context, + reset_mac=True), + mock.call(pg.internal_info['tenant_vif_port_id'], + context=task.context, reset_mac=True)]) + + @mock.patch.object(neutron_common, 'unbind_neutron_port', autospec=True) + def test_unconfigure_tenant_networks_portgroup_2(self, mock_unbind_port): + pg = utils.create_test_portgroup( + self.context, node_id=self.node.id, address=None, + internal_info={'tenant_vif_port_id': uuidutils.generate_uuid()}) + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.unconfigure_tenant_networks(task) + mock_unbind_port.assert_has_calls([ + mock.call(self.port.extra['vif_port_id'], context=task.context, + reset_mac=True), + mock.call(pg.internal_info['tenant_vif_port_id'], + context=task.context, reset_mac=False)]) + def test_configure_tenant_networks_no_ports_for_node(self): n = utils.create_test_node(self.context, network_interface='neutron', uuid=uuidutils.generate_uuid()) @@ -755,6 +783,64 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): call2_attrs)] ) + @mock.patch.object(neutron_common, 'get_neutron_port_data', autospec=True) + @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True) + @mock.patch.object(neutron_common, 'update_neutron_port', autospec=True) + @mock.patch.object(neutron_common, 'get_client', autospec=True) + @mock.patch.object(neutron_common, 'get_local_group_information', + autospec=True) + def test_configure_tenant_networks_with_portgroups_no_address( + self, glgi_mock, client_mock, update_mock, wait_agent_mock, + port_data_mock): + pg = utils.create_test_portgroup( + self.context, node_id=self.node.id, address=None, + extra={'vif_port_id': uuidutils.generate_uuid()}) + port1 = utils.create_test_port( + self.context, node_id=self.node.id, address='ff:54:00:cf:2d:33', + uuid=uuidutils.generate_uuid(), + portgroup_id=pg.id, + local_link_connection={'switch_id': '0a:1b:2c:3d:4e:ff', + 'port_id': 'Ethernet1/1', + 'switch_info': 'switch2'} + ) + port2 = utils.create_test_port( + self.context, node_id=self.node.id, address='ff:54:00:cf:2d:34', + uuid=uuidutils.generate_uuid(), + portgroup_id=pg.id, + local_link_connection={'switch_id': '0a:1b:2c:3d:4e:ff', + 'port_id': 'Ethernet1/2', + 'switch_info': 'switch2'} + ) + local_group_info = {'a': 'b'} + glgi_mock.return_value = local_group_info + expected_attrs = {'binding:vnic_type': 'baremetal', + 'binding:host_id': self.node.uuid} + call1_attrs = copy.deepcopy(expected_attrs) + call1_attrs['binding:profile'] = { + 'local_link_information': [self.port.local_link_connection] + } + call1_attrs['mac_address'] = '52:54:00:cf:2d:32' + call2_attrs = copy.deepcopy(expected_attrs) + call2_attrs['binding:profile'] = { + 'local_link_information': [port1.local_link_connection, + port2.local_link_connection], + 'local_group_information': local_group_info + } + with task_manager.acquire(self.context, self.node.id) as task: + # Override task.portgroups here, to have ability to check + # that mocked get_local_group_information was called with + # this portgroup object. + task.portgroups = [pg] + self.interface.configure_tenant_networks(task) + client_mock.assert_called_once_with(context=task.context) + glgi_mock.assert_called_once_with(task, pg) + update_mock.assert_has_calls( + [mock.call(self.context, self.port.extra['vif_port_id'], + call1_attrs), + mock.call(self.context, pg.extra['vif_port_id'], + call2_attrs)] + ) + def test_need_power_on_true(self): self.port.is_smartnic = True self.port.save() diff --git a/releasenotes/notes/portgroup-mac-649ed31c3525e4f0.yaml b/releasenotes/notes/portgroup-mac-649ed31c3525e4f0.yaml new file mode 100644 index 000000000..2859fd695 --- /dev/null +++ b/releasenotes/notes/portgroup-mac-649ed31c3525e4f0.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes the issue that when the MAC address of a port group is not set and + been attached to instance, the landed bond port cannot get IP address + due to inconsistent MAC address between the tenant port and the initially + allocated one in the config drive. |