Enforce placement minimum in nova.cmd.status

We keep forgetting to bump the minimum required placement version in
nova.cmd.status (and all the related bits and pieces) whenever we
change the report client to require a new version.

This patch interposes a check in the test_report_client functional suite
any time get/put/post/delete is called from the report client.  If we
see a microversion higher than the minumum specified in nova.cmd.status,
we raise an exception, which will blow up the test.  This should force
the author of a new patch on SchedulerReportClient to do the necessary
paperwork in that patch.

...assuming said author happens to write a test in test_report_client.
This pattern can and should be copied into other test suites where
report client tests are likely to be written, to broaden the scope of
this enforcement.

Change-Id: I5482b92f941261ab6ee6b7cd532ce268c31fe793

nova/cmd/status.py

@@ -51,6 +51,12 @@ CONF = nova.conf.CONF
 PLACEMENT_DOCS_LINK = 'https://docs.openstack.org/nova/latest' \
                       '/user/placement.html'
 
+# NOTE(efried): 1.25 is required by nova-scheduler to make granular
+# resource requests to GET /allocation_candidates.
+# NOTE: If you bump this version, remember to update the history
+# section in the nova-status man page (doc/source/cli/nova-status).
+MIN_PLACEMENT_MICROVERSION = "1.25"
+
 
 class UpgradeCheckCode(enum.IntEnum):
     """These are the status codes for the nova-status upgrade check command
@@ -199,11 +205,8 @@ class UpgradeCommands(object):
             versions = self._placement_get("/")
             max_version = pkg_resources.parse_version(
                 versions["versions"][0]["max_version"])
-            # NOTE(efried): 1.25 is required by nova-scheduler to make granular
-            # resource requests to GET /allocation_candidates.
-            # NOTE: If you bump this version, remember to update the history
-            # section in the nova-status man page (doc/source/cli/nova-status).
-            needs_version = pkg_resources.parse_version("1.25")
+            needs_version = pkg_resources.parse_version(
+                MIN_PLACEMENT_MICROVERSION)
             if max_version < needs_version:
                 msg = (_('Placement API version %(needed)s needed, '
                          'you have %(current)s.') %

nova/tests/functional/test_report_client.py

@@ -12,8 +12,10 @@
 #    under the License.
 
 import mock
+import pkg_resources
 
 from nova.api.openstack.placement import direct
+from nova.cmd import status
 from nova.compute import provider_tree
 from nova import conf
 from nova import context
@@ -25,11 +27,55 @@ from nova import exception
 from nova import objects
 from nova import rc_fields as fields
 from nova.scheduler.client import report
+from nova.scheduler import utils
 from nova import test
 from nova.tests import uuidsentinel as uuids
 
 CONF = conf.CONF
 
+CMD_STATUS_MIN_MICROVERSION = pkg_resources.parse_version(
+    status.MIN_PLACEMENT_MICROVERSION)
+
+
+class VersionCheckingReportClient(report.SchedulerReportClient):
+    """This wrapper around SchedulerReportClient checks microversions for
+    get/put/post/delete calls to validate that the minimum requirement enforced
+    in nova.cmd.status has been bumped appropriately when the report client
+    uses a new version. This of course relies on there being a test in this
+    module that hits the code path using that microversion. (This mechanism can
+    be copied into other func test suites where we hit the report client.)
+    """
+    @staticmethod
+    def _check_microversion(kwargs):
+        microversion = kwargs.get('version')
+        if not microversion:
+            return
+
+        seen_microversion = pkg_resources.parse_version(microversion)
+        if seen_microversion > CMD_STATUS_MIN_MICROVERSION:
+            raise ValueError(
+                "Report client is using microversion %s, but nova.cmd.status "
+                "is only requiring %s. See "
+                "I4369f7fb1453e896864222fa407437982be8f6b5 for an example of "
+                "how to bump the minimum requirement." %
+                (microversion, status.MIN_PLACEMENT_MICROVERSION))
+
+    def get(self, *args, **kwargs):
+        self._check_microversion(kwargs)
+        return super(VersionCheckingReportClient, self).get(*args, **kwargs)
+
+    def put(self, *args, **kwargs):
+        self._check_microversion(kwargs)
+        return super(VersionCheckingReportClient, self).put(*args, **kwargs)
+
+    def post(self, *args, **kwargs):
+        self._check_microversion(kwargs)
+        return super(VersionCheckingReportClient, self).post(*args, **kwargs)
+
+    def delete(self, *args, **kwargs):
+        self._check_microversion(kwargs)
+        return super(VersionCheckingReportClient, self).delete(*args, **kwargs)
+
 
 class SchedulerReportClientTestBase(test.TestCase):
 
@@ -51,7 +97,7 @@ class SchedulerReportClientTestBase(test.TestCase):
             """
             def __enter__(inner_self):
                 adap = super(ReportClientInterceptor, inner_self).__enter__()
-                client = report.SchedulerReportClient(adapter=adap)
+                client = VersionCheckingReportClient(adapter=adap)
                 # NOTE(efried): This `self` is the TestCase!
                 self._set_client(client)
                 return client
@@ -945,3 +991,9 @@ class SchedulerReportClientTests(SchedulerReportClientTestBase):
             self.client.aggregate_remove_host(
                 self.context, agg_uuid, self.compute_name)
         upd_aggs_mock.assert_not_called()
+
+    def test_alloc_cands_smoke(self):
+        """Simple call to get_allocation_candidates for version checking."""
+        with self._interceptor():
+            self.client.get_allocation_candidates(
+                self.context, utils.ResourceRequest())