From 95cbc35b0bdae0de15569bc35fadbcf970da5b9e Mon Sep 17 00:00:00 2001
From: Matt Riedemann <mriedem.os@gmail.com>
Date: Wed, 28 Nov 2018 12:52:17 -0500
Subject: [PATCH] Provide a useful error message when trying to update
 non-compute services

Starting in Pike, we disallowed trying to update (enable/disable/force down)
non-nova-compute services because of multi-cell support using host mappings
to lookup service records, and simply because disabling non-compute services
doesn't do anything.

However, before microversion 2.53, the error the user gets back is confusing:

  HTTP exception thrown: Host 'p024.domain.com' is not mapped to any cell

This change provides a useful error message in this case and also changes
the 404 response to a 400 response to align with the type of error and the
behavior of the 2.53 microversion.

Change-Id: I44f09aec60b0b18c458f9ba6d8b725db962e9cc7
Closes-Bug: #1805164
---
 nova/api/openstack/compute/services.py             | 11 ++++++++++-
 .../unit/api/openstack/compute/test_services.py    | 14 ++++++++++++--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/nova/api/openstack/compute/services.py b/nova/api/openstack/compute/services.py
index 7a35c5665fc7..9a88cfcbd72b 100644
--- a/nova/api/openstack/compute/services.py
+++ b/nova/api/openstack/compute/services.py
@@ -172,6 +172,15 @@ class ServiceController(wsgi.Controller):
 
     def _update(self, context, host, binary, payload):
         """Do the actual PUT/update"""
+        # If the user tried to perform an action
+        # (disable/enable/force down) on a non-nova-compute
+        # service, provide a more useful error message.
+        if binary != 'nova-compute':
+            msg = (_(
+                'Updating a %(binary)s service is not supported. Only '
+                'nova-compute services can be updated.') % {'binary': binary})
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
         try:
             self.host_api.service_update(context, host, binary, payload)
         except (exception.HostBinaryNotFound,
@@ -324,7 +333,7 @@ class ServiceController(wsgi.Controller):
         # technically disable a nova-scheduler service, although that doesn't
         # really do anything within Nova and is just confusing. Now trying to
         # do that will fail as a nova-scheduler service won't have a host
-        # mapping so you'll get a 404. In this new microversion, we close that
+        # mapping so you'll get a 400. In this new microversion, we close that
         # old gap and make sure you can only enable/disable and set forced_down
         # on nova-compute services since those are the only ones that make
         # sense to update for those operations.
diff --git a/nova/tests/unit/api/openstack/compute/test_services.py b/nova/tests/unit/api/openstack/compute/test_services.py
index 0ebe374f73b1..458e673956f5 100644
--- a/nova/tests/unit/api/openstack/compute/test_services.py
+++ b/nova/tests/unit/api/openstack/compute/test_services.py
@@ -614,7 +614,7 @@ class ServicesTestV21(test.TestCase):
 
     def test_services_enable_with_invalid_binary(self):
         body = {'host': 'host1', 'binary': 'invalid'}
-        self.assertRaises(webob.exc.HTTPNotFound,
+        self.assertRaises(webob.exc.HTTPBadRequest,
                           self.controller.update,
                           self.req,
                           "enable",
@@ -637,7 +637,7 @@ class ServicesTestV21(test.TestCase):
 
     def test_services_disable_with_invalid_binary(self):
         body = {'host': 'host1', 'binary': 'invalid'}
-        self.assertRaises(webob.exc.HTTPNotFound,
+        self.assertRaises(webob.exc.HTTPBadRequest,
                           self.controller.update,
                           self.req,
                           "disable",
@@ -1079,6 +1079,16 @@ class ServicesTestV211(ServicesTestV21):
         self.assertRaises(exception.ValidationError,
             self.controller.update, req, 'force-down', body=req_body)
 
+    def test_update_forced_down_invalid_service(self):
+        req = fakes.HTTPRequest.blank('/fake/services',
+                                      use_admin_context=True,
+                                      version=self.wsgi_api_version)
+        req_body = {"forced_down": True,
+                    "host": "host1", "binary": "nova-scheduler"}
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.update, req, 'force-down',
+                          body=req_body)
+
 
 class ServicesTestV252(ServicesTestV211):
     """This is a boundary test to ensure that 2.52 behaves the same as 2.11."""