From 9003906bdc47169572ec34a142e5abfbadceb934 Mon Sep 17 00:00:00 2001 From: Douglas Viroel Date: Tue, 19 Aug 2025 17:22:52 -0300 Subject: [PATCH] Fix NovaHelper microversion comparison Fixes the microversion comparison in both enable and disable nova-compute service methods in NovaHelper. The previous implementation was incorrect and started to fail for microversion greather than 2.99. Closes-Bug: #2120586 Assisted-By: Cursor (claude-4-sonnet) Change-Id: I69da7f10cd5b42f7d4613d8947bca3e382815c3f Signed-off-by: Douglas Viroel --- ...a-microversion-check-9022a378b75d046f.yaml | 7 +++++ watcher/common/nova_helper.py | 6 ++-- watcher/tests/common/test_nova_helper.py | 31 +++++++++++++++++-- 3 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/bug-2120586-fix-nova-microversion-check-9022a378b75d046f.yaml diff --git a/releasenotes/notes/bug-2120586-fix-nova-microversion-check-9022a378b75d046f.yaml b/releasenotes/notes/bug-2120586-fix-nova-microversion-check-9022a378b75d046f.yaml new file mode 100644 index 000000000..b4c750391 --- /dev/null +++ b/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 diff --git a/watcher/common/nova_helper.py b/watcher/common/nova_helper.py index e0635dcb8..fb31e54fe 100644 --- a/watcher/common/nova_helper.py +++ b/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', diff --git a/watcher/tests/common/test_nova_helper.py b/watcher/tests/common/test_nova_helper.py index 5e6401516..70d55a217 100644 --- a/watcher/tests/common/test_nova_helper.py +++ b/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') @@ -458,15 +461,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() @@ -474,15 +487,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): @@ -612,13 +637,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)