Deprecate binary argument in nova service enable/disable/force-down CLIs

Change If1e03c9343b8cc9c34bd51c2b4d25acdb21131ff in the API in Pike
makes the os-services API look up services records via the host mapping
in the API datadabase to determine which cell the service lives in.
The host mappings only exist for nova-compute services, which means
you can only enable/disable/force-down nova-compute services now, which
is realistically the only service that ever made sense for those actions.

That change broke some functional tests in novaclient though which loop
through all services and disables them or forces them down. This change
fixes those tests by only attempting the action on nova-compute services.

This change also deprecates the binary argument to the service
enable/disable/force-down commands since the only value that ever really
worked or does anything is nova-compute, so a future version of the CLI
should just hard-code that value.

Change-Id: Idd0d2be960ca0ed59097c10c931da47a1a3e66fb
Closes-Bug: #1700359

novaclient/tests/functional/v2/legacy/test_os_services.py

@@ -31,6 +31,14 @@ class TestOsServicesNovaClient(base.ClientTestBase):
         # in serial way (https://review.openstack.org/#/c/217768/), but
         # it's a potential issue for making these tests parallel in the future
         for serv in self.client.services.list():
+            # In Pike the os-services API was made multi-cell aware and it
+            # looks up services by host, which uses the host mapping record
+            # in the API DB which is only populated for nova-compute services,
+            # effectively making it impossible to perform actions like enable
+            # or disable non-nova-compute services since the API won't be able
+            # to find them. So filter out anything that's not nova-compute.
+            if serv.binary != 'nova-compute':
+                continue
             host = self._get_column_value_from_single_row_table(
                 self.nova('service-list --binary %s' % serv.binary), 'Host')
             service = self.nova('service-disable %s %s' % (host, serv.binary))
@@ -46,6 +54,14 @@ class TestOsServicesNovaClient(base.ClientTestBase):
 
     def test_os_service_disable_log_reason(self):
         for serv in self.client.services.list():
+            # In Pike the os-services API was made multi-cell aware and it
+            # looks up services by host, which uses the host mapping record
+            # in the API DB which is only populated for nova-compute services,
+            # effectively making it impossible to perform actions like enable
+            # or disable non-nova-compute services since the API won't be able
+            # to find them. So filter out anything that's not nova-compute.
+            if serv.binary != 'nova-compute':
+                continue
             host = self._get_column_value_from_single_row_table(
                 self.nova('service-list --binary %s' % serv.binary), 'Host')
             service = self.nova('service-disable --reason test_disable %s %s'

novaclient/tests/functional/v2/test_os_services.py

@@ -20,6 +20,14 @@ class TestOsServicesNovaClientV211(test_os_services.TestOsServicesNovaClient):
 
     def test_os_services_force_down_force_up(self):
         for serv in self.client.services.list():
+            # In Pike the os-services API was made multi-cell aware and it
+            # looks up services by host, which uses the host mapping record
+            # in the API DB which is only populated for nova-compute services,
+            # effectively making it impossible to perform actions like enable
+            # or disable non-nova-compute services since the API won't be able
+            # to find them. So filter out anything that's not nova-compute.
+            if serv.binary != 'nova-compute':
+                continue
             host = self._get_column_value_from_single_row_table(
                 self.nova('service-list --binary %s' % serv.binary), 'Host')
             service = self.nova('service-force-down %s %s'

novaclient/tests/unit/v2/test_shell.py

@@ -2201,11 +2201,23 @@ class ShellTest(utils.TestCase):
         body = {'host': 'host1', 'binary': 'nova-cert'}
         self.assert_called('PUT', '/os-services/enable', body)
 
+    def test_services_enable_default_binary(self):
+        """Tests that the default binary is nova-compute if not specified."""
+        self.run_command('service-enable host1')
+        body = {'host': 'host1', 'binary': 'nova-compute'}
+        self.assert_called('PUT', '/os-services/enable', body)
+
     def test_services_disable(self):
         self.run_command('service-disable host1 nova-cert')
         body = {'host': 'host1', 'binary': 'nova-cert'}
         self.assert_called('PUT', '/os-services/disable', body)
 
+    def test_services_disable_default_binary(self):
+        """Tests that the default binary is nova-compute if not specified."""
+        self.run_command('service-disable host1')
+        body = {'host': 'host1', 'binary': 'nova-compute'}
+        self.assert_called('PUT', '/os-services/disable', body)
+
     def test_services_disable_with_reason(self):
         self.run_command('service-disable host1 nova-cert --reason no_reason')
         body = {'host': 'host1', 'binary': 'nova-cert',

novaclient/v2/shell.py

@@ -3420,7 +3420,10 @@ def do_service_list(cs, args):
 
 
 @utils.arg('host', metavar='<hostname>', help=_('Name of host.'))
-@utils.arg('binary', metavar='<binary>', help=_('Service binary.'))
+# TODO(mriedem): Eventually just hard-code the binary to "nova-compute".
+@utils.arg('binary', metavar='<binary>', help=_('Service binary. The only '
+           'meaningful binary is "nova-compute". (Deprecated)'),
+           default='nova-compute', nargs='?')
 def do_service_enable(cs, args):
     """Enable the service."""
     result = cs.services.enable(args.host, args.binary)
@@ -3428,7 +3431,10 @@ def do_service_enable(cs, args):
 
 
 @utils.arg('host', metavar='<hostname>', help=_('Name of host.'))
-@utils.arg('binary', metavar='<binary>', help=_('Service binary.'))
+# TODO(mriedem): Eventually just hard-code the binary to "nova-compute".
+@utils.arg('binary', metavar='<binary>', help=_('Service binary. The only '
+           'meaningful binary is "nova-compute". (Deprecated)'),
+           default='nova-compute', nargs='?')
 @utils.arg(
     '--reason',
     metavar='<reason>',
@@ -3447,7 +3453,10 @@ def do_service_disable(cs, args):
 
 @api_versions.wraps("2.11")
 @utils.arg('host', metavar='<hostname>', help=_('Name of host.'))
-@utils.arg('binary', metavar='<binary>', help=_('Service binary.'))
+# TODO(mriedem): Eventually just hard-code the binary to "nova-compute".
+@utils.arg('binary', metavar='<binary>', help=_('Service binary. The only '
+           'meaningful binary is "nova-compute". (Deprecated)'),
+           default='nova-compute', nargs='?')
 @utils.arg(
     '--unset',
     dest='force_down',

releasenotes/notes/deprecate-service-binary-arg-2d5c446f5a2409a7.yaml

@@ -0,0 +1,10 @@
+---
+deprecations:
+  - |
+    The ``binary`` argument to the ``nova service-enable``,
+    ``nova service-disable``, and ``nova service-force-down`` commands has been
+    deprecated. The only binary that it makes sense to use is ``nova-compute``
+    since disabling a service like ``nova-scheduler`` or ``nova-conductor``
+    does not actually do anything, and starting in the 16.0.0 Pike release the
+    compute API will not be able to look up services other than
+    ``nova-compute`` for these operations.