diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index a64dbd7be1..7fac0da03a 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -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) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 85924db553..6909245876 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -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 diff --git a/ironic/tests/unit/api/v1/test_ports.py b/ironic/tests/unit/api/v1/test_ports.py index 1b8b717c9e..43bcc34090 100644 --- a/ironic/tests/unit/api/v1/test_ports.py +++ b/ironic/tests/unit/api/v1/test_ports.py @@ -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): diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 2a966e14dc..5a9beb0128 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -3111,6 +3111,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') @@ -3470,6 +3607,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): diff --git a/releasenotes/notes/rely-on-standalone-ports-supported-8153e1135787828b.yaml b/releasenotes/notes/rely-on-standalone-ports-supported-8153e1135787828b.yaml new file mode 100644 index 0000000000..23fb687e5b --- /dev/null +++ b/releasenotes/notes/rely-on-standalone-ports-supported-8153e1135787828b.yaml @@ -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.