diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index a0a8cc9d21..a44279f286 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -530,6 +530,17 @@ class PortsController(rest.RestController): pdict = port.as_dict() self._check_allowed_port_fields(pdict) + create_remotely = pecan.request.rpcapi.can_send_create_port() + if (not create_remotely and pdict.get('portgroup_uuid')): + # NOTE(mgoddard): In RPC API v1.41, port creation was moved to the + # conductor service to facilitate validation of the physical + # network field of ports in portgroups. During a rolling upgrade, + # the RPCAPI will reject the create_port method, so we need to + # create the port locally. If the port is a member of a portgroup, + # we are unable to perform the validation and must reject the + # request. + raise exception.NotAcceptable() + extra = pdict.get('extra') vif = extra.get('vif_port_id') if extra else None if vif: @@ -560,16 +571,18 @@ class PortsController(rest.RestController): **notify_extra) with notify.handle_error_notification(context, rpc_port, 'create', **notify_extra): - # TODO(mgoddard): In RPC API v1.41, port creation was moved to the + # NOTE(mgoddard): In RPC API v1.41, port creation was moved to the # conductor service to facilitate validation of the physical - # network field of ports in portgroups. Further consideration is - # required determine how best to support rolling upgrades from a - # release in which ports are created by the API service to one in - # which they are created by the conductor service, while ensuring - # that all required validation is performed. - topic = pecan.request.rpcapi.get_topic_for(rpc_node) - new_port = pecan.request.rpcapi.create_port(context, rpc_port, - topic) + # network field of ports in portgroups. During a rolling upgrade, + # the RPCAPI will reject the create_port method, so we need to + # create the port locally. + if create_remotely: + topic = pecan.request.rpcapi.get_topic_for(rpc_node) + new_port = pecan.request.rpcapi.create_port(context, rpc_port, + topic) + else: + rpc_port.create() + new_port = rpc_port notify.emit_end_notification(context, new_port, 'create', **notify_extra) # Set the HTTP Location Header diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index c3984f6a67..4108641252 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -153,6 +153,10 @@ class ConductorAPI(object): host = random.choice(list(ring.nodes)) return self.topic + "." + host + def can_send_create_port(self): + """Return whether the RPCAPI supports the create_port method.""" + return self.client.can_send_version("1.41") + def create_node(self, context, node_obj, topic=None): """Synchronously, have a conductor validate and create a node. diff --git a/ironic/tests/unit/api/v1/test_ports.py b/ironic/tests/unit/api/v1/test_ports.py index d07b133d81..09a01a89b9 100644 --- a/ironic/tests/unit/api/v1/test_ports.py +++ b/ironic/tests/unit/api/v1/test_ports.py @@ -1533,6 +1533,20 @@ class TestPost(test_api_base.BaseApiTest): self.assertTrue(response.json['error_message']) self.assertFalse(mock_create.called) + @mock.patch.object(rpcapi.ConductorAPI, 'can_send_create_port', + autospec=True) + def test_create_port_cannot_send_create_port(self, mock_cscp, mock_create): + mock_cscp.return_value = False + pdict = post_get_test_port( + node_uuid=self.node.uuid, + portgroup_uuid=None) + response = self.post_json('/ports', pdict, headers=self.headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.CREATED, response.status_int) + self.assertFalse(mock_create.called) + mock_cscp.assert_called_once_with(mock.ANY) + objects.Port.get(self.context, pdict['uuid']) + def test_create_port_portgroup(self, mock_create): pdict = post_get_test_port( portgroup_uuid=self.portgroup.uuid, @@ -1567,6 +1581,21 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) self.assertFalse(mock_create.called) + @mock.patch.object(rpcapi.ConductorAPI, 'can_send_create_port', + autospec=True) + def test_create_port_portgroup_cannot_send_create_port(self, mock_cscp, + mock_create): + mock_cscp.return_value = False + pdict = post_get_test_port( + node_uuid=self.node.uuid, + portgroup_uuid=self.portgroup.uuid) + response = self.post_json('/ports', pdict, expect_errors=True, + headers=self.headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + self.assertFalse(mock_create.called) + mock_cscp.assert_called_once_with(mock.ANY) + @mock.patch.object(notification_utils, '_emit_api_notification') def test_create_port_address_already_exist(self, mock_notify, mock_create): address = 'AA:AA:AA:11:22:33' diff --git a/ironic/tests/unit/conductor/test_rpcapi.py b/ironic/tests/unit/conductor/test_rpcapi.py index 04212a0abd..4e2b353dba 100644 --- a/ironic/tests/unit/conductor/test_rpcapi.py +++ b/ironic/tests/unit/conductor/test_rpcapi.py @@ -146,6 +146,21 @@ class RPCAPITestCase(db_base.DbTestCase): self.assertEqual('fake-topic.fake-host', rpcapi.get_topic_for_driver('fake-driver')) + def _test_can_send_create_port(self, can_send): + rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic') + with mock.patch.object(rpcapi.client, + "can_send_version") as mock_can_send_version: + mock_can_send_version.return_value = can_send + result = rpcapi.can_send_create_port() + self.assertEqual(can_send, result) + mock_can_send_version.assert_called_once_with("1.41") + + def test_can_send_create_port_True(self): + self._test_can_send_create_port(True) + + def test_can_send_create_port_False(self): + self._test_can_send_create_port(False) + def _test_rpcapi(self, method, rpc_method, **kwargs): rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic') diff --git a/releasenotes/notes/port-physical-network-a7009dc514353796.yaml b/releasenotes/notes/port-physical-network-a7009dc514353796.yaml index 97c36cac0d..6fe1c8c452 100644 --- a/releasenotes/notes/port-physical-network-a7009dc514353796.yaml +++ b/releasenotes/notes/port-physical-network-a7009dc514353796.yaml @@ -32,3 +32,8 @@ upgrade: ``physical_network`` field. Attachment of Virtual Interfaces (VIFs) will continue to function as in the previous release until any ports have their physical network field set. + + During a live upgrade to this release, the ``physical_network`` field will + not be available. It will also not be possible to create ports which are + members of a port group during a live upgrade, as the API service will be + unable to validate the consistency of the request.