openstack
/
ironic
Code Issues Proposed changes

Refactor get_physnets_by_portgroup_id 

There is a lot of common code between the functions
ironic.conductor.utils.validate_port_physnets() and
ironic.drivers.modules.network.common._get_physnets_by_portgroup().

This change refactors the code, adding a new utility method,
get_physnets_by_portgroup_id, which returns the physical networks
associated with a portgroup. There should be at most one such physical
network, and the presence of multiple will cause
PortgroupPhysnetInconsistent to be raised.

Change-Id: I8f01dcc5eaa0c8511ce77622e41db88e27791327
Related-Bug: #1666009
This commit is contained in:
Mark Goddard 2017-07-11 13:34:33 +01:00
parent 5ea8d9f354
commit 43fa6fc294
4 changed files with 112 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
ironic/common/network.py
View File

@ -12,6 +12,8 @@
# License for the specific language governing permissions and limitations
# under the License.
from ironic.common import exception
def get_node_vif_ids(task):
"""Get all VIF ids for a node.
@ -78,3 +80,31 @@ def get_physnets_for_node(task):
return set(port.physical_network
for port in task.ports
if port.physical_network is not None)
def get_physnets_by_portgroup_id(task, portgroup_id, exclude_port=None):
"""Return the set of physical networks associated with a portgroup.
:param task: a TaskManager instance.
:param portgroup_id: ID of the portgroup.
:param exclude_port: A Port object to exclude from the determination of the
portgroup's physical network, or None.
:returns: The set of physical networks associated with the portgroup. The
set will contain zero or one physical networks.
:raises: PortgroupPhysnetInconsistent if the portgroup's ports are not
assigned the same physical network.
"""
pg_ports = get_ports_by_portgroup_id(task, portgroup_id)
if exclude_port is not None and 'id' in exclude_port:
exclude_port_id = exclude_port.id
else:
exclude_port_id = None
pg_physnets = set(port.physical_network
for port in pg_ports
if port.id != exclude_port_id)
# Sanity check: all ports should have the same physical network.
if len(pg_physnets) > 1:
portgroup = get_portgroup_by_id(task, portgroup_id)
raise exception.PortgroupPhysnetInconsistent(
portgroup=portgroup.uuid, physical_networks=", ".join(pg_physnets))
return pg_physnets

14
ironic/conductor/utils.py
View File

@ -599,21 +599,13 @@ def validate_port_physnet(task, port_obj):
return
# Determine the current physical network of the portgroup.
pg_ports = network.get_ports_by_portgroup_id(task, port_obj.portgroup_id)
port_obj_id = port_obj.id if 'id' in port_obj else None
pg_physnets = {port.physical_network
for port in pg_ports if port.id != port_obj_id}
pg_physnets = network.get_physnets_by_portgroup_id(task,
port_obj.portgroup_id,
exclude_port=port_obj)
if not pg_physnets:
return
# Sanity check that all existing ports in the group have the same
# physical network (should never happen).
if len(pg_physnets) > 1:
portgroup = network.get_portgroup_by_id(task, port_obj.portgroup_id)
raise exception.PortgroupPhysnetInconsistent(
portgroup=portgroup.uuid, physical_networks=", ".join(pg_physnets))
# Check that the port has the same physical network as any existing
# member ports.
pg_physnet = pg_physnets.pop()

23
ironic/drivers/modules/network/common.py
View File

@ -140,25 +140,6 @@ def _get_free_portgroups_and_ports(task, vif_id, physnets):
return free_port_like_objs
def _get_physnet_for_portgroup(task, portgroup):
"""Return the physical network associated with a portgroup.
:param task: a TaskManager instance.
:param portgroup: a Portgroup object.
:returns: The physical network associated with the portgroup.
:raises: PortgroupPhysnetInconsistent if the portgroup's ports are not
assigned the same physical network.
"""
pg_ports = network.get_ports_by_portgroup_id(task, portgroup.id)
pg_physnets = {port.physical_network for port in pg_ports}
# Sanity check: there should be at least one port in the portgroup and
# all ports should have the same physical network.
if len(pg_physnets) != 1:
raise exception.PortgroupPhysnetInconsistent(
portgroup=portgroup.uuid, physical_networks=", ".join(pg_physnets))
return pg_physnets.pop()
def get_free_port_like_object(task, vif_id, physnets):
"""Find free port-like object (portgroup or port) VIF will be attached to.
@ -207,7 +188,9 @@ def get_free_port_like_object(task, vif_id, physnets):
"""
is_pg = isinstance(port_like_obj, objects.Portgroup)
if is_pg:
pg_physnet = _get_physnet_for_portgroup(task, port_like_obj)
pg_physnets = network.get_physnets_by_portgroup_id(
task, port_like_obj.id)
pg_physnet = pg_physnets.pop()
physnet_matches = pg_physnet in physnets
pxe_enabled = True
else:

76
ironic/tests/unit/common/test_network.py
View File

@ -15,6 +15,7 @@
from oslo_utils import uuidutils
from ironic.common import exception
from ironic.common import network
from ironic.conductor import task_manager
from ironic.tests.unit.conductor import mgr_utils
@ -227,3 +228,78 @@ class GetPhysnetsForNodeTestCase(db_base.DbTestCase):
with task_manager.acquire(self.context, node.uuid) as task:
res = network.get_physnets_for_node(task)
self.assertEqual({'physnet1', 'physnet2'}, res)
class GetPhysnetsByPortgroupID(db_base.DbTestCase):
def setUp(self):
super(GetPhysnetsByPortgroupID, self).setUp()
self.node = object_utils.create_test_node(self.context, driver='fake')
self.portgroup = object_utils.create_test_portgroup(
self.context, node_id=self.node.id)
def _test(self, expected_result, exclude_port=None):
with task_manager.acquire(self.context, self.node.uuid) as task:
result = network.get_physnets_by_portgroup_id(task,
self.portgroup.id,
exclude_port)
self.assertEqual(expected_result, result)
def test_empty(self):
self._test(set())
def test_one_port(self):
object_utils.create_test_port(self.context, node_id=self.node.id,
portgroup_id=self.portgroup.id,
physical_network='physnet1')
self._test({'physnet1'})
def test_two_ports(self):
object_utils.create_test_port(self.context, node_id=self.node.id,
portgroup_id=self.portgroup.id,
physical_network='physnet1')
object_utils.create_test_port(self.context, node_id=self.node.id,
uuid=uuidutils.generate_uuid(),
address='00:11:22:33:44:55',
portgroup_id=self.portgroup.id,
physical_network='physnet1')
self._test({'physnet1'})
def test_exclude_port(self):
object_utils.create_test_port(self.context, node_id=self.node.id,
portgroup_id=self.portgroup.id,
physical_network='physnet1')
port2 = object_utils.create_test_port(self.context,
node_id=self.node.id,
uuid=uuidutils.generate_uuid(),
address='00:11:22:33:44:55',
portgroup_id=self.portgroup.id,
physical_network='physnet2')
self._test({'physnet1'}, port2)
def test_exclude_port_no_id(self):
# During port creation there may be no 'id' field.
object_utils.create_test_port(self.context, node_id=self.node.id,
portgroup_id=self.portgroup.id,
physical_network='physnet1')
port2 = object_utils.get_test_port(self.context,
node_id=self.node.id,
uuid=uuidutils.generate_uuid(),
address='00:11:22:33:44:55',
portgroup_id=self.portgroup.id,
physical_network='physnet2')
self._test({'physnet1'}, port2)
def test_two_ports_inconsistent(self):
object_utils.create_test_port(self.context, node_id=self.node.id,
portgroup_id=self.portgroup.id,
physical_network='physnet1')
object_utils.create_test_port(self.context, node_id=self.node.id,
uuid=uuidutils.generate_uuid(),
address='00:11:22:33:44:55',
portgroup_id=self.portgroup.id,
physical_network='physnet2')
with task_manager.acquire(self.context, self.node.uuid) as task:
self.assertRaises(exception.PortgroupPhysnetInconsistent,
network.get_physnets_by_portgroup_id,
task, self.portgroup.id)