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 9e3f412186.

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

@@ -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

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.
 

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'

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')
 

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.