Fix compute service set handling for 2.53+

With compute API microversion 2.53 there is a single
PUT /os-services/{service_id} API which takes the service
id as a UUID. Since the openstack compute service set
command only takes --host and --service (binary) to identify
the service, this change checks if 2.53 or greater is being
used and if so, looks up the service by host and binary and
calls the appropriate methods in novaclient.

If the command cannot uniquely identify a compute service
with the given host and binary, an error is raised. A future
change could add an --id option to be used with 2.53+ to
pass the service id (as UUID) directly to avoid the host/binary
filtering.

Conflicts:
    openstackclient/compute/v2/service.py
Note(elod.illes): conflict is due to missing patch on stable/queens:
I0a87e02e71ff025d30181fc17ebcd003a590f110

Change-Id: I868e0868e8eb17e7e34eef3d2d58dceedd29c2b0
Story: 2005349
Task: 30302
(cherry picked from commit 4bd53dc1090fda86f6ce25b76a079e250c9206d8)
(cherry picked from commit 100d34c54ecdfedf6fb40a2686e1aae1f54df97e)
(cherry picked from commit fc5f2978c0f4f1e3360325b00c1f1b99f1e6318f)

1 files changed, 101 insertions, 4 deletions

diff --git a/openstackclient/tests/unit/compute/v2/test_service.py b/openstackclient/tests/unit/compute/v2/test_service.py
index 8403efc9..f2a91393 100644
--- a/openstackclient/tests/unit/compute/v2/test_service.py
+++ b/openstackclient/tests/unit/compute/v2/test_service.py
@@ -16,7 +16,9 @@
 import mock
 from mock import call
 
+from novaclient import api_versions
 from osc_lib import exceptions
+import six
 
 from openstackclient.compute.v2 import service
 from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes
@@ -342,7 +344,7 @@ class TestServiceSet(TestService):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
         self.service_mock.force_down.assert_called_once_with(
-            self.service.host, self.service.binary, force_down=False)
+            self.service.host, self.service.binary, False)
         self.assertNotCalled(self.service_mock.enable)
         self.assertNotCalled(self.service_mock.disable)
         self.assertIsNone(result)
@@ -361,7 +363,7 @@ class TestServiceSet(TestService):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
         self.service_mock.force_down.assert_called_once_with(
-            self.service.host, self.service.binary, force_down=True)
+            self.service.host, self.service.binary, True)
         self.assertNotCalled(self.service_mock.enable)
         self.assertNotCalled(self.service_mock.disable)
         self.assertIsNone(result)
@@ -384,7 +386,7 @@ class TestServiceSet(TestService):
         self.service_mock.enable.assert_called_once_with(
             self.service.host, self.service.binary)
         self.service_mock.force_down.assert_called_once_with(
-            self.service.host, self.service.binary, force_down=True)
+            self.service.host, self.service.binary, True)
         self.assertIsNone(result)
 
     def test_service_set_enable_and_state_down_with_exception(self):
@@ -407,4 +409,99 @@ class TestServiceSet(TestService):
             self.assertRaises(exceptions.CommandError,
                               self.cmd.take_action, parsed_args)
             self.service_mock.force_down.assert_called_once_with(
-                self.service.host, self.service.binary, force_down=True)
+                self.service.host, self.service.binary, True)
+
+    def test_service_set_2_53_disable_down(self):
+        # Tests disabling and forcing down a compute service with microversion
+        # 2.53 which requires looking up the service by host and binary.
+        arglist = [
+            '--disable',
+            '--down',
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('disable', True),
+            ('down', True),
+            ('host', self.service.host),
+            ('service', self.service.binary),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.53')
+        service_id = '339478d0-0b95-4a94-be63-d5be05dfeb1c'
+        self.service_mock.list.return_value = [mock.Mock(id=service_id)]
+        result = self.cmd.take_action(parsed_args)
+        self.service_mock.disable.assert_called_once_with(service_id)
+        self.service_mock.force_down.assert_called_once_with(service_id, True)
+        self.assertIsNone(result)
+
+    def test_service_set_2_53_disable_reason(self):
+        # Tests disabling with reason a compute service with microversion
+        # 2.53 which requires looking up the service by host and binary.
+        reason = 'earthquake'
+        arglist = [
+            '--disable',
+            '--disable-reason', reason,
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('disable', True),
+            ('disable_reason', reason),
+            ('host', self.service.host),
+            ('service', self.service.binary),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.53')
+        service_id = '339478d0-0b95-4a94-be63-d5be05dfeb1c'
+        self.service_mock.list.return_value = [mock.Mock(id=service_id)]
+        result = self.cmd.take_action(parsed_args)
+        self.service_mock.disable_log_reason.assert_called_once_with(
+            service_id, reason)
+        self.assertIsNone(result)
+
+    def test_service_set_2_53_enable_up(self):
+        # Tests enabling and bringing up a compute service with microversion
+        # 2.53 which requires looking up the service by host and binary.
+        arglist = [
+            '--enable',
+            '--up',
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('enable', True),
+            ('up', True),
+            ('host', self.service.host),
+            ('service', self.service.binary),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.53')
+        service_id = '339478d0-0b95-4a94-be63-d5be05dfeb1c'
+        self.service_mock.list.return_value = [mock.Mock(id=service_id)]
+        result = self.cmd.take_action(parsed_args)
+        self.service_mock.enable.assert_called_once_with(service_id)
+        self.service_mock.force_down.assert_called_once_with(service_id, False)
+        self.assertIsNone(result)
+
+    def test_service_set_find_service_by_host_and_binary_no_results(self):
+        # Tests that no compute services are found by host and binary.
+        self.service_mock.list.return_value = []
+        ex = self.assertRaises(exceptions.CommandError,
+                               self.cmd._find_service_by_host_and_binary,
+                               self.service_mock, 'fake-host', 'nova-compute')
+        self.assertIn('Compute service for host "fake-host" and binary '
+                      '"nova-compute" not found.', six.text_type(ex))
+
+    def test_service_set_find_service_by_host_and_binary_many_results(self):
+        # Tests that more than one compute service is found by host and binary.
+        self.service_mock.list.return_value = [mock.Mock(), mock.Mock()]
+        ex = self.assertRaises(exceptions.CommandError,
+                               self.cmd._find_service_by_host_and_binary,
+                               self.service_mock, 'fake-host', 'nova-compute')
+        self.assertIn('Multiple compute services found for host "fake-host" '
+                      'and binary "nova-compute". Unable to proceed.',
+                      six.text_type(ex))