From e7b84daa8774434f3b1382ce40741bebe248de12 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Sun, 25 Jun 2017 09:43:12 -0400 Subject: [PATCH] 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 --- .../functional/v2/legacy/test_os_services.py | 16 ++++++++++++++++ .../tests/functional/v2/test_os_services.py | 8 ++++++++ novaclient/tests/unit/v2/test_shell.py | 12 ++++++++++++ novaclient/v2/shell.py | 15 ++++++++++++--- ...cate-service-binary-arg-2d5c446f5a2409a7.yaml | 10 ++++++++++ 5 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/deprecate-service-binary-arg-2d5c446f5a2409a7.yaml diff --git a/novaclient/tests/functional/v2/legacy/test_os_services.py b/novaclient/tests/functional/v2/legacy/test_os_services.py index 3ffc2d81e..8af71611d 100644 --- a/novaclient/tests/functional/v2/legacy/test_os_services.py +++ b/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' diff --git a/novaclient/tests/functional/v2/test_os_services.py b/novaclient/tests/functional/v2/test_os_services.py index 9fef43397..3da8449c0 100644 --- a/novaclient/tests/functional/v2/test_os_services.py +++ b/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' diff --git a/novaclient/tests/unit/v2/test_shell.py b/novaclient/tests/unit/v2/test_shell.py index 15a267eaa..812efcfc5 100644 --- a/novaclient/tests/unit/v2/test_shell.py +++ b/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', diff --git a/novaclient/v2/shell.py b/novaclient/v2/shell.py index 951f6e9ed..d095baa77 100644 --- a/novaclient/v2/shell.py +++ b/novaclient/v2/shell.py @@ -3420,7 +3420,10 @@ def do_service_list(cs, args): @utils.arg('host', metavar='', help=_('Name of host.')) -@utils.arg('binary', metavar='', help=_('Service binary.')) +# TODO(mriedem): Eventually just hard-code the binary to "nova-compute". +@utils.arg('binary', metavar='', 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='', help=_('Name of host.')) -@utils.arg('binary', metavar='', help=_('Service binary.')) +# TODO(mriedem): Eventually just hard-code the binary to "nova-compute". +@utils.arg('binary', metavar='', help=_('Service binary. The only ' + 'meaningful binary is "nova-compute". (Deprecated)'), + default='nova-compute', nargs='?') @utils.arg( '--reason', metavar='', @@ -3447,7 +3453,10 @@ def do_service_disable(cs, args): @api_versions.wraps("2.11") @utils.arg('host', metavar='', help=_('Name of host.')) -@utils.arg('binary', metavar='', help=_('Service binary.')) +# TODO(mriedem): Eventually just hard-code the binary to "nova-compute". +@utils.arg('binary', metavar='', help=_('Service binary. The only ' + 'meaningful binary is "nova-compute". (Deprecated)'), + default='nova-compute', nargs='?') @utils.arg( '--unset', dest='force_down', diff --git a/releasenotes/notes/deprecate-service-binary-arg-2d5c446f5a2409a7.yaml b/releasenotes/notes/deprecate-service-binary-arg-2d5c446f5a2409a7.yaml new file mode 100644 index 000000000..254b42d68 --- /dev/null +++ b/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.