From 0683fcd3f551dc55438d9f8415b8931f22635bd5 Mon Sep 17 00:00:00 2001 From: Giridhar Jayavelu Date: Wed, 9 Mar 2016 14:10:49 -0800 Subject: [PATCH] Handle TypeError when disabling host service When _set_host_enabled() in virt/libvirt/driver.py is called to change service status of a host from enabled to disabled without providing disable_reason, then a TypeError occurs while concatenating disabled_reason with DISABLE_PREFIX. This prevents the service status of the host from being updated. This patch handles the case when disable_reason is None. Also, unit tests have been fixed to validate the code path for changing the disabled state of a host. Closes-Bug: #1623738 Change-Id: Ib131f73444234723ef8bbb15bb4bd4a62a83320d --- nova/tests/unit/virt/libvirt/test_driver.py | 20 ++++++++++++++++---- nova/virt/libvirt/driver.py | 3 ++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 23f8b49a3787..0fcb56d7ab2a 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -1488,40 +1488,52 @@ class LibvirtConnTestCase(test.NoDBTestCase): self.assertRaises(exception.NovaException, drvr.set_admin_password, instance, "123") + @mock.patch.object(objects.Service, 'save') @mock.patch.object(objects.Service, 'get_by_compute_host') - def test_set_host_enabled_with_disable(self, mock_svc): + def test_set_host_enabled_with_disable(self, mock_svc, mock_save): # Tests disabling an enabled host. drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) svc = self._create_service(host='fake-mini') mock_svc.return_value = svc drvr._set_host_enabled(False) self.assertTrue(svc.disabled) + mock_save.assert_called_once_with() + @mock.patch.object(objects.Service, 'save') @mock.patch.object(objects.Service, 'get_by_compute_host') - def test_set_host_enabled_with_enable(self, mock_svc): + def test_set_host_enabled_with_enable(self, mock_svc, mock_save): # Tests enabling a disabled host. drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) svc = self._create_service(disabled=True, host='fake-mini') mock_svc.return_value = svc drvr._set_host_enabled(True) + # since disabled_reason is not set and not prefixed with "AUTO:", + # service should not be enabled. + mock_save.assert_not_called() self.assertTrue(svc.disabled) + @mock.patch.object(objects.Service, 'save') @mock.patch.object(objects.Service, 'get_by_compute_host') - def test_set_host_enabled_with_enable_state_enabled(self, mock_svc): + def test_set_host_enabled_with_enable_state_enabled(self, mock_svc, + mock_save): # Tests enabling an enabled host. drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) svc = self._create_service(disabled=False, host='fake-mini') mock_svc.return_value = svc drvr._set_host_enabled(True) self.assertFalse(svc.disabled) + mock_save.assert_not_called() + @mock.patch.object(objects.Service, 'save') @mock.patch.object(objects.Service, 'get_by_compute_host') - def test_set_host_enabled_with_disable_state_disabled(self, mock_svc): + def test_set_host_enabled_with_disable_state_disabled(self, mock_svc, + mock_save): # Tests disabling a disabled host. drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) svc = self._create_service(disabled=True, host='fake-mini') mock_svc.return_value = svc drvr._set_host_enabled(False) + mock_save.assert_not_called() self.assertTrue(svc.disabled) def test_set_host_enabled_swallows_exceptions(self): diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 89088027936e..894dde2480fd 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -3335,7 +3335,8 @@ class LibvirtDriver(driver.ComputeDriver): service.disabled = disable_service service.disabled_reason = ( DISABLE_PREFIX + disable_reason - if disable_service else DISABLE_REASON_UNDEFINED) + if disable_service and disable_reason else + DISABLE_REASON_UNDEFINED) service.save() LOG.debug('Updating compute service status to %s', status_name[disable_service])