Merge "Fix NovaHelper microversion comparison"

releasenotes/notes/bug-2120586-fix-nova-microversion-check-9022a378b75d046f.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixed nova client microversion comparison in enable and disable compute
+    service methods. The code was incorrectly comparing API versions, which
+    caused failures for microversions greater than 2.99. For more details,
+    see the bug report: https://bugs.launchpad.net/watcher/+bug/2120586

watcher/common/nova_helper.py

@@ -423,7 +423,8 @@ class NovaHelper(object):
                             "for the instance %s" % instance_id)
 
     def enable_service_nova_compute(self, hostname):
-        if float(CONF.nova_client.api_version) < 2.53:
+        if (api_versions.APIVersion(version_str=CONF.nova_client.api_version) <
+                api_versions.APIVersion(version_str='2.53')):
             status = self.nova.services.enable(
                 host=hostname, binary='nova-compute').status == 'enabled'
         else:
@@ -435,7 +436,8 @@ class NovaHelper(object):
         return status
 
     def disable_service_nova_compute(self, hostname, reason=None):
-        if float(CONF.nova_client.api_version) < 2.53:
+        if (api_versions.APIVersion(version_str=CONF.nova_client.api_version) <
+                api_versions.APIVersion(version_str='2.53')):
             status = self.nova.services.disable_log_reason(
                 host=hostname,
                 binary='nova-compute',

watcher/tests/common/test_nova_helper.py

@@ -29,8 +29,11 @@ from watcher.common import clients
 from watcher.common import exception
 from watcher.common import nova_helper
 from watcher.common import utils
+from watcher import conf
 from watcher.tests import base
 
+CONF = conf.CONF
+
 
 @mock.patch.object(clients.OpenStackClients, 'nova')
 @mock.patch.object(clients.OpenStackClients, 'neutron')
@@ -504,15 +507,25 @@ class TestNovaHelper(base.TestCase):
         nova_services.enable.return_value = mock.MagicMock(
             status='enabled')
 
+        CONF.set_override('api_version', '2.52', group='nova_client')
+
         result = nova_util.enable_service_nova_compute('nanjing')
         self.assertTrue(result)
 
+        nova_util.nova.services.enable.assert_called_with(
+            host='nanjing', binary='nova-compute')
+
         nova_services.enable.return_value = mock.MagicMock(
             status='disabled')
 
+        CONF.set_override('api_version', '2.56', group='nova_client')
+
         result = nova_util.enable_service_nova_compute('nanjing')
         self.assertFalse(result)
 
+        nova_util.nova.services.enable.assert_called_with(
+            service_uuid=mock.ANY)
+
     def test_disable_service_nova_compute(self, mock_glance, mock_cinder,
                                           mock_neutron, mock_nova):
         nova_util = nova_helper.NovaHelper()
@@ -520,15 +533,27 @@ class TestNovaHelper(base.TestCase):
         nova_services.disable_log_reason.return_value = mock.MagicMock(
             status='enabled')
 
-        result = nova_util.disable_service_nova_compute('nanjing')
+        CONF.set_override('api_version', '2.52', group='nova_client')
+
+        result = nova_util.disable_service_nova_compute(
+            'nanjing', reason='test')
         self.assertFalse(result)
 
+        nova_services.disable_log_reason.assert_called_with(
+            host='nanjing', binary='nova-compute', reason='test')
+
         nova_services.disable_log_reason.return_value = mock.MagicMock(
             status='disabled')
 
-        result = nova_util.disable_service_nova_compute('nanjing')
+        CONF.set_override('api_version', '2.56', group='nova_client')
+
+        result = nova_util.disable_service_nova_compute(
+            'nanjing', reason='test2')
         self.assertTrue(result)
 
+        nova_util.nova.services.disable_log_reason.assert_called_with(
+            service_uuid=mock.ANY, reason='test2')
+
     @mock.patch.object(time, 'sleep', mock.Mock())
     def test_create_instance(self, mock_glance, mock_cinder,
                              mock_neutron, mock_nova):
@@ -658,13 +683,13 @@ class TestNovaHelper(base.TestCase):
             "in-use",
             timeout=2)
 
+    @mock.patch.object(api_versions, 'APIVersion', mock.MagicMock())
     def test_check_nova_api_version(self, mock_glance, mock_cinder,
                                     mock_neutron, mock_nova):
         nova_util = nova_helper.NovaHelper()
 
         # verify that the method will return True when the version of nova_api
         # is supported.
-        api_versions.APIVersion = mock.MagicMock()
         result = nova_util._check_nova_api_version(nova_util.nova, "2.56")
         self.assertTrue(result)