Browse Source

Allow to attach/detach VIF to portgroup

With this patch port groups are activated in Ironic.
When attaching a VIF to a node, it is attached to the first
free port group. If there are no free port groups, the first
available port (pxe_enabled has higher priority) is used
instead.

Related-Bug: #1618754
Related-Bug: #1582188
Co-Authored-By: Vladyslav Drok <vdrok@mirantis.com>
Change-Id: I0dca2c2d98184e370c08c3e05aa3edadead869af
tags/7.0.0
Vasyl Saienko 3 years ago
parent
commit
b83af0d65a
9 changed files with 531 additions and 115 deletions
  1. +7
    -1
      ironic/api/controllers/v1/portgroup.py
  2. +1
    -1
      ironic/common/exception.py
  3. +1
    -1
      ironic/common/utils.py
  4. +168
    -59
      ironic/drivers/modules/network/common.py
  5. +5
    -10
      ironic/drivers/modules/network/flat.py
  6. +25
    -1
      ironic/tests/unit/api/v1/test_portgroups.py
  7. +290
    -42
      ironic/tests/unit/drivers/modules/network/test_common.py
  8. +23
    -0
      ironic/tests/unit/drivers/modules/network/test_flat.py
  9. +11
    -0
      releasenotes/notes/allow-attach-vif-to-portgroup-ce24b72632ec5772.yaml

+ 7
- 1
ironic/api/controllers/v1/portgroup.py View File

@@ -28,6 +28,7 @@ from ironic.api import expose
from ironic.common import exception
from ironic.common.i18n import _
from ironic.common import policy
from ironic.common import utils as common_utils
from ironic import objects

METRICS = metrics_utils.get_metrics_logger(__name__)
@@ -446,8 +447,13 @@ class PortgroupsController(pecan.rest.RestController):
raise wsme.exc.ClientSideError(
error_msg, status_code=http_client.BAD_REQUEST)

pg_dict = portgroup.as_dict()
vif = pg_dict.get('extra', {}).get('vif_port_id')
if vif:
common_utils.warn_about_deprecated_extra_vif_port_id()

new_portgroup = objects.Portgroup(pecan.request.context,
**portgroup.as_dict())
**pg_dict)
new_portgroup.create()
# Set the HTTP Location Header
pecan.response.location = link.build_url('portgroups',


+ 1
- 1
ironic/common/exception.py View File

@@ -228,7 +228,7 @@ class VolumeTargetBootIndexAlreadyExists(Conflict):

class VifAlreadyAttached(Conflict):
_msg_fmt = _("Unable to attach VIF because VIF %(vif)s is already "
"attached to Ironic port %(port_uuid)s")
"attached to Ironic %(object_type)s %(object_uuid)s")


class NoFreePhysicalPorts(Invalid):


+ 1
- 1
ironic/common/utils.py View File

@@ -544,7 +544,7 @@ def warn_about_deprecated_extra_vif_port_id():
global warn_deprecated_extra_vif_port_id
if not warn_deprecated_extra_vif_port_id:
warn_deprecated_extra_vif_port_id = True
LOG.warning(_LW("Attaching VIF to a port via "
LOG.warning(_LW("Attaching VIF to a port/portgroup via "
"extra['vif_port_id'] is deprecated and will not "
"be supported in Pike release. API endpoint "
"v1/nodes/<node>/vifs should be used instead."))

+ 168
- 59
ironic/drivers/modules/network/common.py View File

@@ -13,6 +13,8 @@
# License for the specific language governing permissions and limitations
# under the License.

import collections

from neutronclient.common import exceptions as neutron_exceptions
from oslo_config import cfg
from oslo_log import log
@@ -30,6 +32,108 @@ LOG = log.getLogger(__name__)
TENANT_VIF_KEY = 'tenant_vif_port_id'


def _vif_attached(port_like_obj, vif_id):
"""Check if VIF is already attached to a port or portgroup.

Raises an exception if a VIF with id=vif_id is attached to the port-like
(Port or Portgroup) object. Otherwise, returns whether a VIF is attached.

:param port_like_obj: port-like object to check.
:param vif_id: identifier of the VIF to look for in port_like_obj.
:returns: True if a VIF (but not vif_id) is attached to port_like_obj,
False otherwise.
:raises: VifAlreadyAttached, if vif_id is attached to port_like_obj.
"""
attached_vif_id = port_like_obj.internal_info.get(
TENANT_VIF_KEY, port_like_obj.extra.get('vif_port_id'))
if attached_vif_id == vif_id:
raise exception.VifAlreadyAttached(
object_type=port_like_obj.__class__.__name__,
vif=vif_id, object_uuid=port_like_obj.uuid)
return attached_vif_id is not None


def _get_free_portgroups_and_ports(task, vif_id):
"""Get free portgroups and ports.

It only returns ports or portgroups that can be used for attachment of
vif_id.

:param task: a TaskManager instance.
:param vif_id: Name or UUID of a VIF.
:returns: tuple of: list of free portgroups, list of free ports.
:raises: VifAlreadyAttached, if vif_id is attached to any of the
node's ports or portgroups.
"""

# This list contains ports selected as candidates for attachment
free_ports = []
# This is a mapping of portgroup id to collection of its free ports
ports_by_portgroup = collections.defaultdict(list)
# This set contains IDs of portgroups that should be ignored, as they have
# at least one port with vif already attached to it
non_usable_portgroups = set()

for p in task.ports:
if _vif_attached(p, vif_id):
# Consider such portgroup unusable. The fact that we can have None
# added in this set is not a problem
non_usable_portgroups.add(p.portgroup_id)
continue
if p.portgroup_id is None:
# ports without portgroup_id are always considered candidates
free_ports.append(p)
else:
ports_by_portgroup[p.portgroup_id].append(p)

# This list contains portgroups selected as candidates for attachment
free_portgroups = []

for pg in task.portgroups:
if _vif_attached(pg, vif_id):
continue
if pg.id in non_usable_portgroups:
# This portgroup has vifs attached to its ports, consider its
# ports instead to avoid collisions
free_ports.extend(ports_by_portgroup[pg.id])
# Also ignore empty portgroups
elif ports_by_portgroup[pg.id]:
free_portgroups.append(pg)

return free_portgroups, free_ports


def get_free_port_like_object(task, vif_id):
"""Find free port-like object (portgroup or port) VIF will be attached to.

Ensures that VIF is not already attached to this node. It will return the
first free port group. If there are no free port groups, then the first
available port (pxe_enabled preferably) is used.

:param task: a TaskManager instance.
:param vif_id: Name or UUID of a VIF.
:raises: VifAlreadyAttached, if VIF is already attached to the node.
:raises: NoFreePhysicalPorts, if there is no port-like object VIF can be
attached to.
:returns: port-like object VIF will be attached to.
"""

free_portgroups, free_ports = _get_free_portgroups_and_ports(task, vif_id)

if free_portgroups:
# portgroups are higher priority
return free_portgroups[0]

if not free_ports:
raise exception.NoFreePhysicalPorts(vif=vif_id)

# Sort ports by pxe_enabled to ensure we always bind pxe_enabled ports
# first
sorted_free_ports = sorted(free_ports, key=lambda p: p.pxe_enabled,
reverse=True)
return sorted_free_ports[0]


class VIFPortIDMixin(object):

def port_changed(self, task, port_obj):
@@ -112,13 +216,25 @@ class VIFPortIDMixin(object):

context = task.context
portgroup_uuid = portgroup_obj.uuid
if 'address' in portgroup_obj.obj_what_changed():
pg_vif = portgroup_obj.extra.get('vif_port_id')
# NOTE(vsaienko) address is not mandatory field in portgroup.
# Do not touch neutron port if we removed address on portgroup.
if ('address' in portgroup_obj.obj_what_changed() and
portgroup_obj.address):
pg_vif = (portgroup_obj.internal_info.get(TENANT_VIF_KEY) or
portgroup_obj.extra.get('vif_port_id'))
if pg_vif:
neutron.update_port_address(pg_vif,
portgroup_obj.address,
token=context.auth_token)

if 'extra' in portgroup_obj.obj_what_changed():
original_portgroup = objects.Portgroup.get_by_id(context,
portgroup_obj.id)
if (portgroup_obj.extra.get('vif_port_id') and
portgroup_obj.extra['vif_port_id'] !=
original_portgroup.extra.get('vif_port_id')):
utils.warn_about_deprecated_extra_vif_port_id()

if ('standalone_ports_supported' in
portgroup_obj.obj_what_changed()):
if not portgroup_obj.standalone_ports_supported:
@@ -149,9 +265,9 @@ class VIFPortIDMixin(object):
entry with the ID of the VIF.
"""
vifs = []
for port in task.ports:
vif_id = port.internal_info.get(
TENANT_VIF_KEY, port.extra.get('vif_port_id'))
for port_like_obj in task.ports + task.portgroups:
vif_id = port_like_obj.internal_info.get(
TENANT_VIF_KEY, port_like_obj.extra.get('vif_port_id'))
if vif_id:
vifs.append({'id': vif_id})
return vifs
@@ -159,6 +275,10 @@ class VIFPortIDMixin(object):
def vif_attach(self, task, vif_info):
"""Attach a virtual network interface to a node

Attach a virtual interface to a node. It will use the first free port
group. If there are no free port groups, then the first available port
(pxe_enabled preferably) is used.

:param task: A TaskManager instance.
:param vif_info: a dictionary of information about a VIF.
It must have an 'id' key, whose value is a unique
@@ -166,53 +286,39 @@ class VIFPortIDMixin(object):
:raises: NetworkError, VifAlreadyAttached, NoFreePhysicalPorts
"""
vif_id = vif_info['id']
# Sort ports by pxe_enabled to ensure we always bind pxe_enabled ports
# first
sorted_ports = sorted(task.ports, key=lambda p: p.pxe_enabled,
reverse=True)
free_ports = []
# Check all ports to ensure this VIF isn't already attached
for port in sorted_ports:
port_id = port.internal_info.get(TENANT_VIF_KEY,
port.extra.get('vif_port_id'))
if port_id is None:
free_ports.append(port)
elif port_id == vif_id:
raise exception.VifAlreadyAttached(
vif=vif_id, port_uuid=port.uuid)

if not free_ports:
raise exception.NoFreePhysicalPorts(vif=vif_id)

# Get first free port
port = free_ports.pop(0)

# Check if the requested vif_id is a neutron port. If it is
# then attempt to update the port's MAC address.
try:
client = neutron.get_client(task.context.auth_token)
client.show_port(vif_id)
except neutron_exceptions.NeutronClientException:
# NOTE(sambetts): If a client error occurs this is because either
# neutron doesn't exist because we're running in standalone
# environment or we can't find a matching neutron port which means
# a user might be requesting a non-neutron port. So skip trying to
# update the neutron port MAC address in these cases.
pass
else:

port_like_obj = get_free_port_like_object(task, vif_id)

# Address is optional for portgroups
if port_like_obj.address:
# Check if the requested vif_id is a neutron port. If it is
# then attempt to update the port's MAC address.
try:
neutron.update_port_address(vif_id, port.address)
except exception.FailedToUpdateMacOnPort:
raise exception.NetworkError(_(
"Unable to attach VIF %(vif)s because Ironic can not "
"update Neutron port %(port)s MAC address to match "
"physical MAC address %(mac)s") % {
'vif': vif_id, 'port': vif_id, 'mac': port.address})

int_info = port.internal_info
client = neutron.get_client(task.context.auth_token)
client.show_port(vif_id)
except neutron_exceptions.NeutronClientException:
# NOTE(sambetts): If a client error occurs this is because
# either neutron doesn't exist because we're running in
# standalone environment or we can't find a matching neutron
# port which means a user might be requesting a non-neutron
# port. So skip trying to update the neutron port MAC address
# in these cases.
pass
else:
try:
neutron.update_port_address(vif_id, port_like_obj.address)
except exception.FailedToUpdateMacOnPort:
raise exception.NetworkError(_(
"Unable to attach VIF %(vif)s because Ironic can not "
"update Neutron port %(port)s MAC address to match "
"physical MAC address %(mac)s") % {
'vif': vif_id, 'port': vif_id,
'mac': port_like_obj.address})

int_info = port_like_obj.internal_info
int_info[TENANT_VIF_KEY] = vif_id
port.internal_info = int_info
port.save()
port_like_obj.internal_info = int_info
port_like_obj.save()

def vif_detach(self, task, vif_id):
"""Detach a virtual network interface from a node
@@ -221,18 +327,21 @@ class VIFPortIDMixin(object):
:param vif_id: A VIF ID to detach
:raises: VifNotAttached
"""
for port in task.ports:

ports = [p for p in task.ports if p.portgroup_id is None]
portgroups = task.portgroups
for port_like_obj in portgroups + ports:
# FIXME(sambetts) Remove this when we no longer support a nova
# driver that uses port.extra
if (port.extra.get("vif_port_id") == vif_id or
port.internal_info.get(TENANT_VIF_KEY) == vif_id):
int_info = port.internal_info
extra = port.extra
if (port_like_obj.extra.get("vif_port_id") == vif_id or
port_like_obj.internal_info.get(TENANT_VIF_KEY) == vif_id):
int_info = port_like_obj.internal_info
extra = port_like_obj.extra
int_info.pop(TENANT_VIF_KEY, None)
extra.pop('vif_port_id', None)
port.extra = extra
port.internal_info = int_info
port.save()
port_like_obj.extra = extra
port_like_obj.internal_info = int_info
port_like_obj.save()
break
else:
raise exception.VifNotAttached(vif=vif_id, node=task.node.uuid)
@@ -252,5 +361,5 @@ class VIFPortIDMixin(object):

return (p_obj.internal_info.get('cleaning_vif_port_id') or
p_obj.internal_info.get('provisioning_vif_port_id') or
p_obj.internal_info.get('tenant_vif_port_id') or
p_obj.internal_info.get(TENANT_VIF_KEY) or
p_obj.extra.get('vif_port_id') or None)

+ 5
- 10
ironic/drivers/modules/network/flat.py View File

@@ -69,17 +69,12 @@ class FlatNetwork(common.VIFPortIDMixin, neutron.NeutronNetworkInterfaceMixin,
if not host_id:
return

# FIXME(sambetts): Uncomment when we support vifs attached to
# portgroups
#
# ports = [p for p in task.ports if not p.portgroup_id]
# portgroups = task.portgroups

client = neutron.get_client(task.context.auth_token)
for port_like_obj in task.ports: # + portgroups:
vif_port_id = (port_like_obj.extra.get('vif_port_id') or
port_like_obj.internal_info.get(
'tenant_vif_port_id'))
for port_like_obj in task.ports + task.portgroups:
vif_port_id = (
port_like_obj.internal_info.get('tenant_vif_port_id') or
port_like_obj.extra.get('vif_port_id')
)
if not vif_port_id:
continue
body = {


+ 25
- 1
ironic/tests/unit/api/v1/test_portgroups.py View File

@@ -30,6 +30,7 @@ from ironic.api.controllers import v1 as api_v1
from ironic.api.controllers.v1 import portgroup as api_portgroup
from ironic.api.controllers.v1 import utils as api_utils
from ironic.common import exception
from ironic.common import utils as common_utils
from ironic.conductor import rpcapi
from ironic.tests import base
from ironic.tests.unit.api import base as test_api_base
@@ -875,8 +876,10 @@ class TestPost(test_api_base.BaseApiTest):
super(TestPost, self).setUp()
self.node = obj_utils.create_test_node(self.context)

@mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id',
autospec=True)
@mock.patch.object(timeutils, 'utcnow', autospec=True)
def test_create_portgroup(self, mock_utcnow):
def test_create_portgroup(self, mock_utcnow, mock_warn):
pdict = apiutils.post_get_test_portgroup()
test_time = datetime.datetime(2000, 1, 1, 0, 0)
mock_utcnow.return_value = test_time
@@ -895,6 +898,7 @@ class TestPost(test_api_base.BaseApiTest):
expected_location = '/v1/portgroups/%s' % pdict['uuid']
self.assertEqual(urlparse.urlparse(response.location).path,
expected_location)
self.assertEqual(0, mock_warn.call_count)

@mock.patch.object(timeutils, 'utcnow', autospec=True)
def test_create_portgroup_v123(self, mock_utcnow):
@@ -956,6 +960,26 @@ class TestPost(test_api_base.BaseApiTest):
headers=self.headers)
self.assertEqual(pdict['extra'], result['extra'])

@mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id',
autospec=True)
def test_create_portgroup_with_extra_vif_port_id_deprecated(
self, mock_warn):
pgdict = apiutils.post_get_test_portgroup(extra={'vif_port_id': 'foo'})
response = self.post_json('/portgroups', pgdict, headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.CREATED, response.status_int)
self.assertEqual(1, mock_warn.call_count)

@mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id',
autospec=True)
def test_create_portgroup_with_no_extra(self, mock_warn):
pgdict = apiutils.post_get_test_portgroup()
del pgdict['extra']
response = self.post_json('/portgroups', pgdict, headers=self.headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.CREATED, response.status_int)
self.assertEqual(0, mock_warn.call_count)

def test_create_portgroup_no_address(self):
pdict = apiutils.post_get_test_portgroup()
del pdict['address']


+ 290
- 42
ironic/tests/unit/drivers/modules/network/test_common.py View File

@@ -26,6 +26,165 @@ from ironic.tests.unit.objects import utils as obj_utils
CONF = cfg.CONF


class TestCommonFunctions(db_base.DbTestCase):

def setUp(self):
super(TestCommonFunctions, self).setUp()
self.config(enabled_drivers=['fake'])
mgr_utils.mock_the_extension_manager()
self.node = obj_utils.create_test_node(self.context,
network_interface='neutron')
self.port = obj_utils.create_test_port(
self.context, node_id=self.node.id, address='52:54:00:cf:2d:32')
self.vif_id = "fake_vif_id"

def _objects_setup(self):
pg1 = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id)
pg1_ports = []
# This portgroup contains 2 ports, both of them without VIF
for i in range(2):
pg1_ports.append(obj_utils.create_test_port(
self.context, node_id=self.node.id,
address='52:54:00:cf:2d:0%d' % i,
uuid=uuidutils.generate_uuid(), portgroup_id=pg1.id))
pg2 = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id, address='00:54:00:cf:2d:04',
name='foo2', uuid=uuidutils.generate_uuid())
pg2_ports = []
# This portgroup contains 3 ports, one of them with 'some-vif'
# attached, so the two free ones should be considered standalone
for i in range(2, 4):
pg2_ports.append(obj_utils.create_test_port(
self.context, node_id=self.node.id,
address='52:54:00:cf:2d:0%d' % i,
uuid=uuidutils.generate_uuid(), portgroup_id=pg2.id))
pg2_ports.append(obj_utils.create_test_port(
self.context, node_id=self.node.id,
address='52:54:00:cf:2d:04',
extra={'vif_port_id': 'some-vif'},
uuid=uuidutils.generate_uuid(), portgroup_id=pg2.id))
# This portgroup has 'some-vif-2' attached to it and contains one port,
# so neither portgroup nor port can be considered free
pg3 = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id, address='00:54:00:cf:2d:05',
name='foo3', uuid=uuidutils.generate_uuid(),
internal_info={common.TENANT_VIF_KEY: 'some-vif-2'})
pg3_ports = [obj_utils.create_test_port(
self.context, node_id=self.node.id,
address='52:54:00:cf:2d:05', uuid=uuidutils.generate_uuid(),
portgroup_id=pg3.id)]
return pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports

def test__get_free_portgroups_and_ports(self):
pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup()
with task_manager.acquire(self.context, self.node.id) as task:
free_portgroups, free_ports = (
common._get_free_portgroups_and_ports(task, self.vif_id))
self.assertItemsEqual(
[self.port.uuid] + [p.uuid for p in pg2_ports[:2]],
[p.uuid for p in free_ports])
self.assertItemsEqual([pg1.uuid], [p.uuid for p in free_portgroups])

def test_get_free_port_like_object_ports(self):
with task_manager.acquire(self.context, self.node.id) as task:
res = common.get_free_port_like_object(task, self.vif_id)
self.assertEqual(self.port.uuid, res.uuid)

def test_get_free_port_like_object_ports_pxe_enabled_first(self):
self.port.pxe_enabled = False
self.port.save()
other_port = obj_utils.create_test_port(
self.context, node_id=self.node.id, address='52:54:00:cf:2d:33',
uuid=uuidutils.generate_uuid())
with task_manager.acquire(self.context, self.node.id) as task:
res = common.get_free_port_like_object(task, self.vif_id)
self.assertEqual(other_port.uuid, res.uuid)

def test_get_free_port_like_object_portgroup_first(self):
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id)
obj_utils.create_test_port(
self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
uuid=uuidutils.generate_uuid(), portgroup_id=pg.id)
with task_manager.acquire(self.context, self.node.id) as task:
res = common.get_free_port_like_object(task, self.vif_id)
self.assertEqual(pg.uuid, res.uuid)

def test_get_free_port_like_object_ignores_empty_portgroup(self):
obj_utils.create_test_portgroup(self.context, node_id=self.node.id)
with task_manager.acquire(self.context, self.node.id) as task:
res = common.get_free_port_like_object(task, self.vif_id)
self.assertEqual(self.port.uuid, res.uuid)

def test_get_free_port_like_object_ignores_standalone_portgroup(self):
self.port.destroy()
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id)
obj_utils.create_test_port(
self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
uuid=uuidutils.generate_uuid(), portgroup_id=pg.id,
extra={'vif_port_id': 'some-vif'})
free_port = obj_utils.create_test_port(
self.context, node_id=self.node.id, address='52:54:00:cf:2d:02',
uuid=uuidutils.generate_uuid(), portgroup_id=pg.id)
with task_manager.acquire(self.context, self.node.id) as task:
res = common.get_free_port_like_object(task, self.vif_id)
self.assertEqual(free_port.uuid, res.uuid)

def test_get_free_port_like_object_vif_attached_to_portgroup(self):
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id,
internal_info={common.TENANT_VIF_KEY: self.vif_id})
obj_utils.create_test_port(
self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
uuid=uuidutils.generate_uuid(), portgroup_id=pg.id)
with task_manager.acquire(self.context, self.node.id) as task:
self.assertRaisesRegex(
exception.VifAlreadyAttached,
r"already attached to Ironic Portgroup",
common.get_free_port_like_object, task, self.vif_id)

def test_get_free_port_like_object_vif_attached_to_portgroup_extra(self):
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id,
extra={'vif_port_id': self.vif_id})
obj_utils.create_test_port(
self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
uuid=uuidutils.generate_uuid(), portgroup_id=pg.id)
with task_manager.acquire(self.context, self.node.id) as task:
self.assertRaisesRegex(
exception.VifAlreadyAttached,
r"already attached to Ironic Portgroup",
common.get_free_port_like_object, task, self.vif_id)

def test_get_free_port_like_object_vif_attached_to_port(self):
self.port.internal_info = {common.TENANT_VIF_KEY: self.vif_id}
self.port.save()
with task_manager.acquire(self.context, self.node.id) as task:
self.assertRaisesRegex(
exception.VifAlreadyAttached,
r"already attached to Ironic Port\b",
common.get_free_port_like_object, task, self.vif_id)

def test_get_free_port_like_object_vif_attached_to_port_extra(self):
self.port.extra = {'vif_port_id': self.vif_id}
self.port.save()
with task_manager.acquire(self.context, self.node.id) as task:
self.assertRaisesRegex(
exception.VifAlreadyAttached,
r"already attached to Ironic Port\b",
common.get_free_port_like_object, task, self.vif_id)

def test_get_free_port_like_object_nothing_free(self):
self.port.extra = {'vif_port_id': 'another-vif'}
self.port.save()
with task_manager.acquire(self.context, self.node.id) as task:
self.assertRaises(exception.NoFreePhysicalPorts,
common.get_free_port_like_object,
task, self.vif_id)


class TestVifPortIDMixin(db_base.DbTestCase):

def setUp(self):
@@ -47,17 +206,33 @@ class TestVifPortIDMixin(db_base.DbTestCase):
vif_id = uuidutils.generate_uuid()
self.port.extra = {'vif_port_id': vif_id}
self.port.save()
pg_vif_id = uuidutils.generate_uuid()
portgroup = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id,
address='52:54:00:00:00:00',
internal_info={common.TENANT_VIF_KEY: pg_vif_id})
obj_utils.create_test_port(
self.context, node_id=self.node.id, portgroup_id=portgroup.id,
address='52:54:00:cf:2d:01', uuid=uuidutils.generate_uuid())
with task_manager.acquire(self.context, self.node.id) as task:
vifs = self.interface.vif_list(task)
self.assertEqual([{'id': vif_id}], vifs)
self.assertItemsEqual([{'id': pg_vif_id}, {'id': vif_id}], vifs)

def test_vif_list_internal(self):
vif_id = uuidutils.generate_uuid()
self.port.internal_info = {common.TENANT_VIF_KEY: vif_id}
self.port.save()
pg_vif_id = uuidutils.generate_uuid()
portgroup = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id,
address='52:54:00:00:00:00',
internal_info={common.TENANT_VIF_KEY: pg_vif_id})
obj_utils.create_test_port(
self.context, node_id=self.node.id, portgroup_id=portgroup.id,
address='52:54:00:cf:2d:01', uuid=uuidutils.generate_uuid())
with task_manager.acquire(self.context, self.node.id) as task:
vifs = self.interface.vif_list(task)
self.assertEqual([{'id': vif_id}], vifs)
self.assertItemsEqual([{'id': pg_vif_id}, {'id': vif_id}], vifs)

def test_vif_list_extra_and_internal_priority(self):
vif_id = uuidutils.generate_uuid()
@@ -69,19 +244,42 @@ class TestVifPortIDMixin(db_base.DbTestCase):
vifs = self.interface.vif_list(task)
self.assertEqual([{'id': vif_id}], vifs)

@mock.patch.object(neutron_common, 'get_client')
@mock.patch.object(neutron_common, 'update_port_address')
def test_vif_attach(self, mock_upa, mock_client):
@mock.patch.object(common, 'get_free_port_like_object', autospec=True)
@mock.patch.object(neutron_common, 'get_client', autospec=True)
@mock.patch.object(neutron_common, 'update_port_address', autospec=True)
def test_vif_attach(self, mock_upa, mock_client, moc_gfp):
self.port.extra = {}
self.port.save()
vif = {'id': "fake_vif_id"}
moc_gfp.return_value = self.port
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.vif_attach(task, vif)
self.port.refresh()
self.assertEqual("fake_vif_id", self.port.internal_info.get(
common.TENANT_VIF_KEY))
mock_client.assert_called_once_with(None)
mock_upa.assert_called_once_with("fake_vif_id", self.port.address)

@mock.patch.object(common, 'get_free_port_like_object', autospec=True)
@mock.patch.object(neutron_common, 'get_client')
@mock.patch.object(neutron_common, 'update_port_address')
def test_vif_attach_portgroup_no_address(self, mock_upa, mock_client,
mock_gfp):
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id, address=None)
mock_gfp.return_value = pg
obj_utils.create_test_port(
self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
uuid=uuidutils.generate_uuid(), portgroup_id=pg.id)
vif = {'id': "fake_vif_id"}
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.vif_attach(task, vif)
pg.refresh()
self.assertEqual(vif['id'],
pg.internal_info[common.TENANT_VIF_KEY])
self.assertFalse(mock_upa.called)
self.assertFalse(mock_client.called)

@mock.patch.object(neutron_common, 'get_client')
@mock.patch.object(neutron_common, 'update_port_address')
def test_vif_attach_update_port_exception(self, mock_upa, mock_client):
@@ -94,42 +292,7 @@ class TestVifPortIDMixin(db_base.DbTestCase):
self.assertRaisesRegexp(
exception.NetworkError, "can not update Neutron port",
self.interface.vif_attach, task, vif)

def test_attach_port_no_ports_left_extra(self):
vif = {'id': "fake_vif_id"}
with task_manager.acquire(self.context, self.node.id) as task:
self.assertRaisesRegexp(
exception.NoFreePhysicalPorts, "not enough free physical",
self.interface.vif_attach, task, vif)

def test_attach_port_no_ports_left_internal_info(self):
self.port.internal_info = {
common.TENANT_VIF_KEY: self.port.extra['vif_port_id']}
self.port.extra = {}
self.port.save()
vif = {'id': "fake_vif_id"}
with task_manager.acquire(self.context, self.node.id) as task:
self.assertRaisesRegexp(
exception.NoFreePhysicalPorts, "not enough free physical",
self.interface.vif_attach, task, vif)

def test_attach_port_vif_already_attached_extra(self):
vif = {'id': self.port.extra['vif_port_id']}
with task_manager.acquire(self.context, self.node.id) as task:
self.assertRaisesRegexp(
exception.VifAlreadyAttached, "already attached",
self.interface.vif_attach, task, vif)

def test_attach_port_vif_already_attach_internal_info(self):
vif = {'id': self.port.extra['vif_port_id']}
self.port.internal_info = {
common.TENANT_VIF_KEY: self.port.extra['vif_port_id']}
self.port.extra = {}
self.port.save()
with task_manager.acquire(self.context, self.node.id) as task:
self.assertRaisesRegexp(
exception.VifAlreadyAttached, "already attached",
self.interface.vif_attach, task, vif)
mock_client.assert_called_once_with(None)

def test_vif_detach_in_extra(self):
with task_manager.acquire(self.context, self.node.id) as task:
@@ -151,6 +314,36 @@ class TestVifPortIDMixin(db_base.DbTestCase):
self.assertFalse('vif_port_id' in self.port.extra)
self.assertFalse(common.TENANT_VIF_KEY in self.port.internal_info)

def test_vif_detach_in_extra_portgroup(self):
vif_id = uuidutils.generate_uuid()
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id,
extra={'vif_port_id': vif_id})
obj_utils.create_test_port(
self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
portgroup_id=pg.id, uuid=uuidutils.generate_uuid()
)
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.vif_detach(task, vif_id)
pg.refresh()
self.assertFalse('vif_port_id' in pg.extra)
self.assertFalse(common.TENANT_VIF_KEY in pg.internal_info)

def test_vif_detach_in_internal_info_portgroup(self):
vif_id = uuidutils.generate_uuid()
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id,
internal_info={common.TENANT_VIF_KEY: vif_id})
obj_utils.create_test_port(
self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
portgroup_id=pg.id, uuid=uuidutils.generate_uuid()
)
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.vif_detach(task, vif_id)
pg.refresh()
self.assertFalse('vif_port_id' in pg.extra)
self.assertFalse(common.TENANT_VIF_KEY in pg.internal_info)

def test_vif_detach_not_attached(self):
with task_manager.acquire(self.context, self.node.id) as task:
self.assertRaisesRegexp(
@@ -466,6 +659,58 @@ class TestVifPortIDMixin(db_base.DbTestCase):
mac_update_mock.assert_called_once_with('fake-id', new_address,
token=self.context.auth_token)

@mock.patch.object(neutron_common, 'update_port_address', autospec=True)
def test_update_portgroup_remove_address(self, mac_update_mock):
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id,
extra={'vif_port_id': 'fake-id'})
pg.address = None
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.portgroup_changed(task, pg)
self.assertFalse(mac_update_mock.called)

@mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id',
autospec=True)
@mock.patch.object(neutron_common, 'update_port_address', autospec=True)
def test_update_portgroup_vif(self, mac_update_mock, mock_warn):
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id)
extra = {'vif_port_id': 'foo'}
pg.extra = extra
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.portgroup_changed(task, pg)
self.assertFalse(mac_update_mock.called)
self.assertEqual(1, mock_warn.call_count)

@mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id',
autospec=True)
@mock.patch.object(neutron_common, 'update_port_address', autospec=True)
def test_update_portgroup_vif_removal_no_deprecation(self, mac_update_mock,
mock_warn):
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id, extra={'vif_port_id': 'foo'})
pg.extra = {}
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.portgroup_changed(task, pg)
self.assertFalse(mac_update_mock.called)
self.assertFalse(mock_warn.called)

@mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id',
autospec=True)
@mock.patch.object(neutron_common, 'update_port_address', autospec=True)
def test_update_portgroup_extra_new_key(self, mac_update_mock, mock_warn):
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id,
extra={'vif_port_id': 'vif-id'})
expected_extra = pg.extra
expected_extra['foo'] = 'bar'
pg.extra = expected_extra
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.portgroup_changed(task, pg)
self.assertFalse(mac_update_mock.called)
self.assertFalse(mock_warn.called)
self.assertEqual(expected_extra, pg.extra)

@mock.patch.object(neutron_common, 'update_port_address', autospec=True)
def test_update_portgroup_address_fail(self, mac_update_mock):
pg = obj_utils.create_test_portgroup(
@@ -482,8 +727,10 @@ class TestVifPortIDMixin(db_base.DbTestCase):
mac_update_mock.assert_called_once_with('fake-id', new_address,
token=self.context.auth_token)

@mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id',
autospec=True)
@mock.patch.object(neutron_common, 'update_port_address', autospec=True)
def test_update_portgroup_address_no_vif(self, mac_update_mock):
def test_update_portgroup_address_no_vif(self, mac_update_mock, mock_warn):
pg = obj_utils.create_test_portgroup(
self.context, node_id=self.node.id)
new_address = '11:22:33:44:55:bb'
@@ -492,6 +739,7 @@ class TestVifPortIDMixin(db_base.DbTestCase):
self.interface.portgroup_changed(task, pg)
self.assertEqual(new_address, pg.address)
self.assertFalse(mac_update_mock.called)
self.assertFalse(mock_warn.called)

@mock.patch.object(neutron_common, 'update_port_address', autospec=True)
def test_update_portgroup_nostandalone_ports_pxe_ports_exc(


+ 23
- 0
ironic/tests/unit/drivers/modules/network/test_flat.py View File

@@ -132,6 +132,29 @@ class TestFlatInterface(db_base.DbTestCase):
self.interface.add_provisioning_network(task)
upd_mock.assert_called_once_with('foo', exp_body)

@mock.patch.object(neutron, 'get_client')
def test_add_provisioning_network_set_binding_host_id_portgroup(
self, client_mock):
upd_mock = mock.Mock()
client_mock.return_value.update_port = upd_mock
instance_info = self.node.instance_info
instance_info['nova_host_id'] = 'nova_host_id'
self.node.instance_info = instance_info
self.node.save()
internal_info = {'tenant_vif_port_id': 'foo'}
utils.create_test_portgroup(
self.context, node_id=self.node.id, internal_info=internal_info,
uuid=uuidutils.generate_uuid())
utils.create_test_port(
self.context, node_id=self.node.id, address='52:54:00:cf:2d:33',
extra={'vif_port_id': 'bar'}, uuid=uuidutils.generate_uuid())
exp_body = {'port': {'binding:host_id': 'nova_host_id'}}
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.add_provisioning_network(task)
upd_mock.assert_has_calls([
mock.call('bar', exp_body), mock.call('foo', exp_body)
])

@mock.patch.object(neutron, 'get_client')
def test_add_provisioning_network_no_binding_host_id(
self, client_mock):


+ 11
- 0
releasenotes/notes/allow-attach-vif-to-portgroup-ce24b72632ec5772.yaml View File

@@ -0,0 +1,11 @@
---
features:
- Enables port group usage when attaching/detaching VIFs.
When attaching a VIF to a node, it is attached to the first free port
group. Port group is considered free if it has no VIFs attached to any of
its ports. Otherwise, only the unattached ports of this portgroup are
available for attachment. If there are no free port groups, the first
available port (pxe_enabled has higher priority) is used instead.
deprecations:
- Using portgroup.extra['vif_port_id'] for attaching/detaching
VIFs to port groups is deprecated and will be removed in Pike release.

Loading…
Cancel
Save