openstack/ironic
Code Issues Proposed changes

Rely on portgroup standalone_ports_supported

Browse Source 
Some hardware doesn't support portgroup fallback to single interface
fashion. This imposes additional restrictions on ports, for example
such ports will not support booting by PXE.
This patch adds restrictions setting pxe_enabled=True on ports that
are members of portgroups with standalone_ports_supported=False. And
vice versa portgroup.standalone_ports_supported can't be set to False
until portgroup contains ports with pxe_enabled=True.

Setting 'vif_port_id' means that we are using this port in standalone
mode. This patch also ensures that 'vif_port_id' can't be updated on
ports which are members of portgroups with
standalone_ports_supported=False. And vice versa updating portgroup
standalone_ports_supported=False is not allowed if portgroup contains
ports with extra/vif_port_id set.

Partial-bug: #1526403

Change-Id: I9b4682918725bed2da0b7c89666e2c37d8826290
This commit is contained in:
Vasyl Saienko 2016-08-25 11:25:32 -04:00
parent e1f4cbdab3
commit c8d518898e
5 changed files with 407 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ironic/api/controllers/v1/port.py
View File

@ -492,7 +492,7 @@ class PortsController(rest.RestController):
"""Create a new port.
:param port: a port within the request body.
:raises: NotAcceptable, HTTPNotFound
:raises: NotAcceptable, HTTPNotFound, Conflict
"""
cdict = pecan.request.context.to_dict()
policy.authorize('baremetal:port:create', cdict, cdict)
@ -508,6 +508,20 @@ class PortsController(rest.RestController):
'portgroup_uuid' in pdict):
raise exception.NotAcceptable()
extra = pdict.get('extra')
vif = extra.get('vif_port_id') if extra else None
if (pdict.get('portgroup_uuid') and
(pdict.get('pxe_enabled') or vif)):
rpc_pg = objects.Portgroup.get_by_uuid(pecan.request.context,
pdict['portgroup_uuid'])
if not rpc_pg.standalone_ports_supported:
msg = _("Port group %s doesn't support standalone ports. "
"This port cannot be created as a member of that "
"port group because either 'extra/vif_port_id' "
"was specified or 'pxe_enabled' was set to True.")
raise exception.Conflict(
msg % pdict['portgroup_uuid'])
new_port = objects.Port(pecan.request.context,
**pdict)

46
ironic/conductor/manager.py
View File

@ -1644,7 +1644,8 @@ class ConductorManager(base_manager.BaseConductorManager):
exception.FailedToUpdateMacOnPort,
exception.MACAlreadyExists,
exception.InvalidState,
exception.FailedToUpdateDHCPOptOnPort)
exception.FailedToUpdateDHCPOptOnPort,
exception.Conflict)
def update_port(self, context, port_obj):
"""Update a port.
@ -1658,6 +1659,9 @@ class ConductorManager(base_manager.BaseConductorManager):
:raises: InvalidState if port connectivity attributes
are updated while node not in a MANAGEABLE or ENROLL or
INSPECTING state or not in MAINTENANCE mode.
:raises: Conflict if trying to set extra/vif_port_id or
pxe_enabled=True on port which is a member of portgroup with
standalone_ports_supported=False.
"""
port_uuid = port_obj.uuid
LOG.debug("RPC update_port called for port %s.", port_uuid)
@ -1665,6 +1669,10 @@ class ConductorManager(base_manager.BaseConductorManager):
with task_manager.acquire(context, port_obj.node_id,
purpose='port update') as task:
node = task.node
portgroup_obj = None
if port_obj.portgroup_id:
portgroup_obj = [pg for pg in task.portgroups if
pg.id == port_obj.portgroup_id][0]
# Only allow updating MAC addresses for active nodes if maintenance
# mode is on.
@ -1717,12 +1725,12 @@ class ConductorManager(base_manager.BaseConductorManager):
"address."),
{'port': port_uuid, 'instance': node.instance_uuid})
vif = port_obj.extra.get('vif_port_id')
if 'extra' in port_obj.obj_what_changed():
orignal_port = objects.Port.get_by_id(context, port_obj.id)
updated_client_id = port_obj.extra.get('client-id')
if (orignal_port.extra.get('client-id') !=
updated_client_id):
vif = port_obj.extra.get('vif_port_id')
# DHCP Option with opt_value=None will remove it
# from the neutron port
if vif:
@ -1742,6 +1750,19 @@ class ConductorManager(base_manager.BaseConductorManager):
{'port': port_uuid,
'instance': node.instance_uuid})
if portgroup_obj and ((set(port_obj.obj_what_changed()) &
{'pxe_enabled', 'portgroup_id'}) or vif):
if ((port_obj.pxe_enabled or vif) and not
portgroup_obj.standalone_ports_supported):
msg = (_("Port group %(portgroup)s doesn't support "
"standalone ports. This port %(port)s cannot be "
" a member of that port group because either "
"'extra/vif_port_id' was specified or "
"'pxe_enabled' was set to True.") %
{"portgroup": portgroup_obj.uuid,
"port": port_uuid})
raise exception.Conflict(msg)
port_obj.save()
return port_obj
@ -1751,7 +1772,8 @@ class ConductorManager(base_manager.BaseConductorManager):
exception.FailedToUpdateMacOnPort,
exception.PortgroupMACAlreadyExists,
exception.PortgroupNotEmpty,
exception.InvalidState)
exception.InvalidState,
exception.Conflict)
def update_portgroup(self, context, portgroup_obj):
"""Update a portgroup.
@ -1767,6 +1789,9 @@ class ConductorManager(base_manager.BaseConductorManager):
in MAINTENANCE mode.
:raises: PortgroupNotEmpty if there are ports associated with this
portgroup.
:raises: Conflict when trying to set standalone_ports_supported=False
on portgroup with ports that has pxe_enabled=True and vice
versa.
"""
portgroup_uuid = portgroup_obj.uuid
LOG.debug("RPC update_portgroup called for portgroup %s.",
@ -1829,6 +1854,21 @@ class ConductorManager(base_manager.BaseConductorManager):
'instance': node.instance_uuid,
'node': node.uuid})
if ('standalone_ports_supported' in
portgroup_obj.obj_what_changed()):
if not portgroup_obj.standalone_ports_supported:
ports = [p for p in task.ports if
p.portgroup_id == portgroup_obj.id]
for p in ports:
extra = p.extra
vif = extra.get('vif_port_id')
if vif or p.pxe_enabled:
msg = _("standalone_ports_supported can not be "
"set to False, because the port group %s "
"contains ports with 'extra/vif_port_id' "
"or pxe_enabled=True") % portgroup_uuid
raise exception.Conflict(msg)
portgroup_obj.save()
return portgroup_obj

109
ironic/tests/unit/api/v1/test_ports.py
View File

@ -955,6 +955,18 @@ class TestPost(test_api_base.BaseApiTest):
self.assertEqual(urlparse.urlparse(response.location).path,
expected_location)
def test_create_port_min_api_version(self):
pdict = post_get_test_port(
node_uuid=self.node.uuid)
pdict.pop('local_link_connection')
pdict.pop('pxe_enabled')
pdict.pop('extra')
headers = {api_base.Version.string: str(api_v1.MIN_VER)}
response = self.post_json('/ports', pdict, headers=headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.CREATED, response.status_int)
self.assertEqual(self.node.uuid, response.json['node_uuid'])
def test_create_port_doesnt_contain_id(self):
with mock.patch.object(self.dbapi, 'create_port',
wraps=self.dbapi.create_port) as cp_mock:
@ -1194,6 +1206,103 @@ class TestPost(test_api_base.BaseApiTest):
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.FORBIDDEN, response.status_int)
def _test_create_port(self, has_vif=False, in_portgroup=False,
pxe_enabled=True, standalone_ports=True,
http_status=http_client.CREATED):
extra = {}
if has_vif:
extra = {'vif_port_id': uuidutils.generate_uuid()}
pdict = post_get_test_port(
node_uuid=self.node.uuid,
pxe_enabled=pxe_enabled,
extra=extra)
if not in_portgroup:
pdict.pop('portgroup_uuid')
else:
self.portgroup.standalone_ports_supported = standalone_ports
self.portgroup.save()
expect_errors = http_status != http_client.CREATED
response = self.post_json('/ports', pdict, headers=self.headers,
expect_errors=expect_errors)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_status, response.status_int)
if not expect_errors:
expected_portgroup_uuid = pdict.get('portgroup_uuid', None)
self.assertEqual(expected_portgroup_uuid,
response.json['portgroup_uuid'])
self.assertEqual(extra, response.json['extra'])
def test_create_port_novif_pxe_noportgroup(self):
self._test_create_port(has_vif=False, in_portgroup=False,
pxe_enabled=True,
http_status=http_client.CREATED)
def test_create_port_novif_nopxe_noportgroup(self):
self._test_create_port(has_vif=False, in_portgroup=False,
pxe_enabled=False,
http_status=http_client.CREATED)
def test_create_port_vif_pxe_noportgroup(self):
self._test_create_port(has_vif=True, in_portgroup=False,
pxe_enabled=True,
http_status=http_client.CREATED)
def test_create_port_vif_nopxe_noportgroup(self):
self._test_create_port(has_vif=True, in_portgroup=False,
pxe_enabled=False,
http_status=http_client.CREATED)
def test_create_port_novif_pxe_portgroup_standalone_ports(self):
self._test_create_port(has_vif=False, in_portgroup=True,
pxe_enabled=True,
standalone_ports=True,
http_status=http_client.CREATED)
def test_create_port_novif_pxe_portgroup_nostandalone_ports(self):
self._test_create_port(has_vif=False, in_portgroup=True,
pxe_enabled=True,
standalone_ports=False,
http_status=http_client.CONFLICT)
def test_create_port_novif_nopxe_portgroup_standalone_ports(self):
self._test_create_port(has_vif=False, in_portgroup=True,
pxe_enabled=False,
standalone_ports=True,
http_status=http_client.CREATED)
def test_create_port_novif_nopxe_portgroup_nostandalone_ports(self):
self._test_create_port(has_vif=False, in_portgroup=True,
pxe_enabled=False,
standalone_ports=False,
http_status=http_client.CREATED)
def test_create_port_vif_pxe_portgroup_standalone_ports(self):
self._test_create_port(has_vif=True, in_portgroup=True,
pxe_enabled=True,
standalone_ports=True,
http_status=http_client.CREATED)
def test_create_port_vif_pxe_portgroup_nostandalone_ports(self):
self._test_create_port(has_vif=True, in_portgroup=True,
pxe_enabled=True,
standalone_ports=False,
http_status=http_client.CONFLICT)
def test_create_port_vif_nopxe_portgroup_standalone_ports(self):
self._test_create_port(has_vif=True, in_portgroup=True,
pxe_enabled=False,
standalone_ports=True,
http_status=http_client.CREATED)
def test_create_port_vif_nopxe_portgroup_nostandalone_ports(self):
self._test_create_port(has_vif=True, in_portgroup=True,
pxe_enabled=False,
standalone_ports=False,
http_status=http_client.CONFLICT)
@mock.patch.object(rpcapi.ConductorAPI, 'destroy_port')
class TestDelete(test_api_base.BaseApiTest):

215
ironic/tests/unit/conductor/test_manager.py
View File

@ -3110,6 +3110,143 @@ class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin,
port.refresh()
self.assertEqual(True, port.pxe_enabled)
def _test_update_port(self, has_vif=False, in_portgroup=False,
pxe_enabled=True, standalone_ports=True,
expect_errors=False):
node = obj_utils.create_test_node(self.context, driver='fake',
provision_state=states.ENROLL)
pg = obj_utils.create_test_portgroup(
self.context, node_id=node.id,
standalone_ports_supported=standalone_ports)
extra_vif = {'vif_port_id': uuidutils.generate_uuid()}
if has_vif:
extra = extra_vif
opposite_extra = {}
else:
extra = {}
opposite_extra = extra_vif
opposite_pxe_enabled = not pxe_enabled
pg_id = None
if in_portgroup:
pg_id = pg.id
ports = []
# Update only portgroup id on existed port with different
# combinations of pxe_enabled/vif_port_id
p1 = obj_utils.create_test_port(self.context, node_id=node.id,
uuid=uuidutils.generate_uuid(),
address="aa:bb:cc:dd:ee:01",
extra=extra,
pxe_enabled=pxe_enabled)
p1.portgroup_id = pg_id
ports.append(p1)
# Update portgroup_id/pxe_enabled/vif_port_id in one request
p2 = obj_utils.create_test_port(self.context, node_id=node.id,
uuid=uuidutils.generate_uuid(),
address="aa:bb:cc:dd:ee:02",
extra=opposite_extra,
pxe_enabled=opposite_pxe_enabled)
p2.extra = extra
p2.pxe_enabled = pxe_enabled
p2.portgroup_id = pg_id
ports.append(p2)
# Update portgroup_id and pxe_enabled
p3 = obj_utils.create_test_port(self.context, node_id=node.id,
uuid=uuidutils.generate_uuid(),
address="aa:bb:cc:dd:ee:03",
extra=extra,
pxe_enabled=opposite_pxe_enabled)
p3.pxe_enabled = pxe_enabled
p3.portgroup_id = pg_id
ports.append(p3)
# Update portgroup_id and vif_port_id
p4 = obj_utils.create_test_port(self.context, node_id=node.id,
uuid=uuidutils.generate_uuid(),
address="aa:bb:cc:dd:ee:04",
pxe_enabled=pxe_enabled,
extra=opposite_extra)
p4.extra = extra
p4.portgroup_id = pg_id
ports.append(p4)
for port in ports:
if not expect_errors:
res = self.service.update_port(self.context, port)
self.assertEqual(port.pxe_enabled, res.pxe_enabled)
self.assertEqual(port.portgroup_id, res.portgroup_id)
self.assertEqual(port.extra, res.extra)
else:
self.assertRaises(messaging.rpc.ExpectedException,
self.service.update_port,
self.context, port)
def test_update_port_novif_pxe_noportgroup(self):
self._test_update_port(has_vif=False, in_portgroup=False,
pxe_enabled=True,
expect_errors=False)
def test_update_port_novif_nopxe_noportgroup(self):
self._test_update_port(has_vif=False, in_portgroup=False,
pxe_enabled=False,
expect_errors=False)
def test_update_port_vif_pxe_noportgroup(self):
self._test_update_port(has_vif=True, in_portgroup=False,
pxe_enabled=True,
expect_errors=False)
def test_update_port_vif_nopxe_noportgroup(self):
self._test_update_port(has_vif=True, in_portgroup=False,
pxe_enabled=False,
expect_errors=False)
def test_update_port_novif_pxe_portgroup_standalone_ports(self):
self._test_update_port(has_vif=False, in_portgroup=True,
pxe_enabled=True, standalone_ports=True,
expect_errors=False)
def test_update_port_novif_pxe_portgroup_nostandalone_ports(self):
self._test_update_port(has_vif=False, in_portgroup=True,
pxe_enabled=True, standalone_ports=False,
expect_errors=True)
def test_update_port_novif_nopxe_portgroup_standalone_ports(self):
self._test_update_port(has_vif=False, in_portgroup=True,
pxe_enabled=False, standalone_ports=True,
expect_errors=False)
def test_update_port_novif_nopxe_portgroup_nostandalone_ports(self):
self._test_update_port(has_vif=False, in_portgroup=True,
pxe_enabled=False, standalone_ports=False,
expect_errors=False)
def test_update_port_vif_pxe_portgroup_standalone_ports(self):
self._test_update_port(has_vif=True, in_portgroup=True,
pxe_enabled=True, standalone_ports=True,
expect_errors=False)
def test_update_port_vif_pxe_portgroup_nostandalone_ports(self):
self._test_update_port(has_vif=True, in_portgroup=True,
pxe_enabled=True, standalone_ports=False,
expect_errors=True)
def test_update_port_vif_nopxe_portgroup_standalone_ports(self):
self._test_update_port(has_vif=True, in_portgroup=True,
pxe_enabled=True, standalone_ports=True,
expect_errors=False)
def test_update_port_vif_nopxe_portgroup_nostandalone_ports(self):
self._test_update_port(has_vif=True, in_portgroup=True,
pxe_enabled=False, standalone_ports=False,
expect_errors=True)
def test__filter_out_unsupported_types_all(self):
self._start_service()
CONF.set_override('send_sensor_data_types', ['All'], group='conductor')
@ -3469,6 +3606,84 @@ class UpdatePortgroupTestCase(mgr_utils.ServiceSetUpMixin,
self.assertEqual(new_address, pg.address)
self.assertFalse(mac_update_mock.called)
def _test_update_portgroup(self, has_vif=False, with_ports=False,
pxe_enabled=True, standalone_ports=True,
expect_errors=False):
node = obj_utils.create_test_node(self.context, driver='fake',
provision_state=states.ENROLL)
# NOTE(vsaienko) make sure that old values are opposite to new,
# to guarantee that object.what_changes() returns true.
old_standalone_ports_supported = not standalone_ports
pg = obj_utils.create_test_portgroup(
self.context, node_id=node.id,
standalone_ports_supported=old_standalone_ports_supported)
if with_ports:
extra = {}
if has_vif:
extra = {'vif_port_id': uuidutils.generate_uuid()}
obj_utils.create_test_port(
self.context, node_id=node.id, extra=extra,
pxe_enabled=pxe_enabled, portgroup_id=pg.id)
pg.standalone_ports_supported = standalone_ports
if not expect_errors:
res = self.service.update_portgroup(self.context, pg)
self.assertEqual(pg.standalone_ports_supported,
res.standalone_ports_supported)
else:
self.assertRaises(messaging.rpc.ExpectedException,
self.service.update_portgroup,
self.context, pg)
def test_update_portgroup_standalone_ports_noports(self):
self._test_update_portgroup(with_ports=False, standalone_ports=True,
expect_errors=False)
def test_update_portgroup_standalone_ports_novif_pxe_ports(self):
self._test_update_portgroup(with_ports=True, standalone_ports=True,
has_vif=False, pxe_enabled=True,
expect_errors=False)
def test_update_portgroup_nostandalone_ports_novif_pxe_ports(self):
self._test_update_portgroup(with_ports=True, standalone_ports=False,
has_vif=False, pxe_enabled=True,
expect_errors=True)
def test_update_portgroup_nostandalone_ports_novif_nopxe_ports(self):
self._test_update_portgroup(with_ports=True, standalone_ports=False,
has_vif=False, pxe_enabled=False,
expect_errors=False)
def test_update_portgroup_standalone_ports_novif_nopxe_ports(self):
self._test_update_portgroup(with_ports=True, standalone_ports=True,
has_vif=False, pxe_enabled=False,
expect_errors=False)
def test_update_portgroup_standalone_ports_vif_pxe_ports(self):
self._test_update_portgroup(with_ports=True, standalone_ports=True,
has_vif=True, pxe_enabled=True,
expect_errors=False)
def test_update_portgroup_nostandalone_ports_vif_pxe_ports(self):
self._test_update_portgroup(with_ports=True, standalone_ports=False,
has_vif=True, pxe_enabled=True,
expect_errors=True)
def test_update_portgroup_standalone_ports_vif_nopxe_ports(self):
self._test_update_portgroup(with_ports=True, standalone_ports=True,
has_vif=True, pxe_enabled=False,
expect_errors=False)
def test_update_portgroup_nostandalone_ports_vif_nopxe_ports(self):
self._test_update_portgroup(with_ports=True, standalone_ports=False,
has_vif=True, pxe_enabled=False,
expect_errors=True)
@mgr_utils.mock_record_keepalive
class RaidTestCases(mgr_utils.ServiceSetUpMixin, tests_db_base.DbTestCase):

25
releasenotes/notes/rely-on-standalone-ports-supported-8153e1135787828b.yaml Normal file
View File

@ -0,0 +1,25 @@
---
other:
- |
Some combinations of port group protocols and hardware might not support
falling back to single interface mode. If a static port group was created
under such circumstances (where
``portgroup.standalone_ports_supported = False``), additional restrictions
apply to such ports and port groups, for example such ports will not
support booting over PXE.
Certain restrictions are imposed on values of port properties for ports
belonging to a port group:
* ``port.pxe_enabled`` cannot be set to True if the port is a member of
a port group with portgroup.standalone_ports_supported already
set to False.
* ``portgroup.standalone_ports_supported`` cannot be set to False on a
portgroup if at least one port in that port group has
``port.pxe_enabled=True``
* ``port.extra.vif_port_id`` cannot be set on a port that is a member of
a port group with ``portgroup.standalone_ports_supported=False`` as
setting it means that we using port in single interface mode.
* ``portgroup.standalone_ports_supported`` cannot be set to False on a
port group if it has at least one port with ``port.extra.vif_port_id``
set.