From fc1e18e3e4a28b1163e7a597ee8b07d465d7a174 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Thu, 20 Jul 2017 10:41:54 +0100 Subject: [PATCH] Rolling upgrades support for create_port RPCAPI The new field 'physical_network' was added to Port in version 1.7, and port creation was moved from the API to the conductor service in commit 9e3f4121865807294dd3768b095b67eb01ef539e. This change adds a method can_send_create_port() to the conductor RPCAPI that allows the caller to determine whether the conductor is able to create ports. During a rolling upgrade this may return False, and the API will need to determine whether it is able to create the port locally as was done previously. The create_port RPC method was added to support validation of the physical_network field of ports in portgroups. A port may therefore be safely created in the API service if it is not a member of a portgroup. If the port being created is a member of a portgroup, then it cannot be safely validated by the API service and the request must be rejected. Change-Id: I8c417cba085f61c3d2ffe1f7e97c64fa85a014cb Related-Bug: #1666009 Related-Bug: #1526283 --- ironic/api/controllers/v1/port.py | 31 +++++++++++++------ ironic/conductor/rpcapi.py | 4 +++ ironic/tests/unit/api/v1/test_ports.py | 29 +++++++++++++++++ ironic/tests/unit/conductor/test_rpcapi.py | 15 +++++++++ ...ort-physical-network-a7009dc514353796.yaml | 5 +++ 5 files changed, 75 insertions(+), 9 deletions(-) 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.