oVirt driver: Close connection after each API call
Currently a new connection is created for every API call which is
never closed. This results in open connections accumulating over time
due to periodic calls like power state polling, which is further
compounded by the number of ovirt nodes deployed.
This change converts _getvm into a generator based context manager so
that the connection can be closed after every call.
Change-Id: I30f9695c591f72dae21467b0b31f6f67fad7cc8a
(cherry picked from commit 343a57570e
)
This commit is contained in:
parent
ab5bb1d3c8
commit
67c24c47ca
|
@ -19,6 +19,9 @@ via oVirt sdk API.
|
||||||
|
|
||||||
For use in dev and test environments.
|
For use in dev and test environments.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import contextlib
|
||||||
|
|
||||||
from ironic.common import boot_devices
|
from ironic.common import boot_devices
|
||||||
from ironic.common import exception
|
from ironic.common import exception
|
||||||
from ironic.common.i18n import _
|
from ironic.common.i18n import _
|
||||||
|
@ -124,6 +127,7 @@ def _parse_driver_info(node):
|
||||||
return driver_info
|
return driver_info
|
||||||
|
|
||||||
|
|
||||||
|
@contextlib.contextmanager
|
||||||
def _getvm(driver_info):
|
def _getvm(driver_info):
|
||||||
address = driver_info['ovirt_address']
|
address = driver_info['ovirt_address']
|
||||||
username = driver_info['ovirt_username']
|
username = driver_info['ovirt_username']
|
||||||
|
@ -149,15 +153,20 @@ def _getvm(driver_info):
|
||||||
ca_file=ca_file)
|
ca_file=ca_file)
|
||||||
vms_service = connection.system_service().vms_service()
|
vms_service = connection.system_service().vms_service()
|
||||||
vmsearch = vms_service.list(search='name=%s' % name)
|
vmsearch = vms_service.list(search='name=%s' % name)
|
||||||
|
if vmsearch:
|
||||||
|
yield vms_service.vm_service(vmsearch[0].id)
|
||||||
|
else:
|
||||||
|
raise staging_exception.OVirtError(_("VM with name "
|
||||||
|
"%s was not found") % name)
|
||||||
except sdk.Error as e:
|
except sdk.Error as e:
|
||||||
LOG.error("Could not fetch information about VM vm %(name)s, "
|
LOG.error("Could not fetch information about VM vm %(name)s, "
|
||||||
"got error: %(error)s", {'name': name, 'error': e})
|
"got error: %(error)s", {'name': name, 'error': e})
|
||||||
raise staging_exception.OVirtError(err=e)
|
raise staging_exception.OVirtError(err=e)
|
||||||
if vmsearch:
|
finally:
|
||||||
return vms_service.vm_service(vmsearch[0].id)
|
try:
|
||||||
else:
|
connection.close()
|
||||||
raise staging_exception.OVirtError(_("VM with name "
|
except sdk.Error as e:
|
||||||
"%s was not found") % name)
|
LOG.warning("Error closing connection: %(error)s", {'error': e})
|
||||||
|
|
||||||
|
|
||||||
class OVirtPower(base.PowerInterface):
|
class OVirtPower(base.PowerInterface):
|
||||||
|
@ -188,7 +197,7 @@ class OVirtPower(base.PowerInterface):
|
||||||
"""
|
"""
|
||||||
driver_info = _parse_driver_info(task.node)
|
driver_info = _parse_driver_info(task.node)
|
||||||
vm_name = driver_info['ovirt_vm_name']
|
vm_name = driver_info['ovirt_vm_name']
|
||||||
vm = _getvm(driver_info)
|
with _getvm(driver_info) as vm:
|
||||||
status = vm.get().status.value
|
status = vm.get().status.value
|
||||||
if status not in OVIRT_TO_IRONIC_POWER_MAPPING:
|
if status not in OVIRT_TO_IRONIC_POWER_MAPPING:
|
||||||
msg = ("oVirt returned unknown state for node %(node)s "
|
msg = ("oVirt returned unknown state for node %(node)s "
|
||||||
|
@ -213,7 +222,7 @@ class OVirtPower(base.PowerInterface):
|
||||||
"""
|
"""
|
||||||
driver_info = _parse_driver_info(task.node)
|
driver_info = _parse_driver_info(task.node)
|
||||||
vm_name = driver_info['ovirt_vm_name']
|
vm_name = driver_info['ovirt_vm_name']
|
||||||
vm = _getvm(driver_info)
|
with _getvm(driver_info) as vm:
|
||||||
try:
|
try:
|
||||||
if target_state == states.POWER_OFF:
|
if target_state == states.POWER_OFF:
|
||||||
vm.stop()
|
vm.stop()
|
||||||
|
@ -231,7 +240,8 @@ class OVirtPower(base.PowerInterface):
|
||||||
raise exception.InvalidParameterValue(msg)
|
raise exception.InvalidParameterValue(msg)
|
||||||
except sdk.Error as e:
|
except sdk.Error as e:
|
||||||
LOG.error("Could not change status of VM vm %(name)s "
|
LOG.error("Could not change status of VM vm %(name)s "
|
||||||
"got error: %(error)s", {'name': vm_name, 'error': e})
|
"got error: %(error)s",
|
||||||
|
{'name': vm_name, 'error': e})
|
||||||
raise staging_exception.OVirtError(err=e)
|
raise staging_exception.OVirtError(err=e)
|
||||||
|
|
||||||
@task_manager.require_exclusive_lock
|
@task_manager.require_exclusive_lock
|
||||||
|
@ -291,7 +301,7 @@ class OVirtManagement(base.ManagementInterface):
|
||||||
oVirt operation.
|
oVirt operation.
|
||||||
"""
|
"""
|
||||||
driver_info = _parse_driver_info(task.node)
|
driver_info = _parse_driver_info(task.node)
|
||||||
vm = _getvm(driver_info)
|
with _getvm(driver_info) as vm:
|
||||||
boot_dev = vm.os.boot[0].get_dev()
|
boot_dev = vm.os.boot[0].get_dev()
|
||||||
persistent = True
|
persistent = True
|
||||||
ironic_boot_dev = OVIRT_TO_IRONIC_DEVICE_MAPPING.get(boot_dev)
|
ironic_boot_dev = OVIRT_TO_IRONIC_DEVICE_MAPPING.get(boot_dev)
|
||||||
|
@ -324,7 +334,7 @@ class OVirtManagement(base.ManagementInterface):
|
||||||
"Invalid boot device %s specified.") % device)
|
"Invalid boot device %s specified.") % device)
|
||||||
|
|
||||||
driver_info = _parse_driver_info(task.node)
|
driver_info = _parse_driver_info(task.node)
|
||||||
vm = _getvm(driver_info)
|
with _getvm(driver_info) as vm:
|
||||||
try:
|
try:
|
||||||
boot = otypes.Boot(devices=[otypes.BootDevice(boot_dev)])
|
boot = otypes.Boot(devices=[otypes.BootDevice(boot_dev)])
|
||||||
bootos = otypes.OperatingSystem(boot=boot)
|
bootos = otypes.OperatingSystem(boot=boot)
|
||||||
|
|
|
@ -62,7 +62,7 @@ class OVirtDriverTestCase(db_base.DbTestCase):
|
||||||
self.node['driver_info']['ovirt_address'] = u'127.0.0.1'
|
self.node['driver_info']['ovirt_address'] = u'127.0.0.1'
|
||||||
driver_info = ovirt_power._parse_driver_info(self.node)
|
driver_info = ovirt_power._parse_driver_info(self.node)
|
||||||
|
|
||||||
ovirt_power._getvm(driver_info)
|
with ovirt_power._getvm(driver_info):
|
||||||
ovirt_power.sdk.Connection.assert_called_with(
|
ovirt_power.sdk.Connection.assert_called_with(
|
||||||
ca_file=None, insecure='False', password='changeme',
|
ca_file=None, insecure='False', password='changeme',
|
||||||
url='https://127.0.0.1/ovirt-engine/api',
|
url='https://127.0.0.1/ovirt-engine/api',
|
||||||
|
@ -71,12 +71,14 @@ class OVirtDriverTestCase(db_base.DbTestCase):
|
||||||
url = ovirt_power.sdk.Connection.mock_calls[0][-1]['url']
|
url = ovirt_power.sdk.Connection.mock_calls[0][-1]['url']
|
||||||
self.assertIsInstance(url, six.string_types)
|
self.assertIsInstance(url, six.string_types)
|
||||||
|
|
||||||
|
ovirt_power.sdk.Connection.return_value.close.assert_called()
|
||||||
|
|
||||||
@mock.patch.object(ovirt_power, "sdk", create=True)
|
@mock.patch.object(ovirt_power, "sdk", create=True)
|
||||||
def test_getvm_unicode(self, sdk):
|
def test_getvm_unicode(self, sdk):
|
||||||
self.node['driver_info']['ovirt_address'] = u'host\u20141'
|
self.node['driver_info']['ovirt_address'] = u'host\u20141'
|
||||||
driver_info = ovirt_power._parse_driver_info(self.node)
|
driver_info = ovirt_power._parse_driver_info(self.node)
|
||||||
|
|
||||||
ovirt_power._getvm(driver_info)
|
with ovirt_power._getvm(driver_info):
|
||||||
ovirt_power.sdk.Connection.assert_called_with(
|
ovirt_power.sdk.Connection.assert_called_with(
|
||||||
ca_file=None, insecure='False', password='changeme',
|
ca_file=None, insecure='False', password='changeme',
|
||||||
url=u'https://host\u20141/ovirt-engine/api',
|
url=u'https://host\u20141/ovirt-engine/api',
|
||||||
|
@ -84,6 +86,7 @@ class OVirtDriverTestCase(db_base.DbTestCase):
|
||||||
)
|
)
|
||||||
url = ovirt_power.sdk.Connection.mock_calls[0][-1]['url']
|
url = ovirt_power.sdk.Connection.mock_calls[0][-1]['url']
|
||||||
self.assertIsInstance(url, six.text_type)
|
self.assertIsInstance(url, six.text_type)
|
||||||
|
ovirt_power.sdk.Connection.return_value.close.assert_called()
|
||||||
|
|
||||||
def test_get_properties(self):
|
def test_get_properties(self):
|
||||||
expected = list(ovirt_power.PROPERTIES.keys())
|
expected = list(ovirt_power.PROPERTIES.keys())
|
||||||
|
@ -138,18 +141,28 @@ class OVirtDriverTestCase(db_base.DbTestCase):
|
||||||
mock_power.assert_called_once_with(task.driver.management, task,
|
mock_power.assert_called_once_with(task.driver.management, task,
|
||||||
boot_devices.DISK)
|
boot_devices.DISK)
|
||||||
|
|
||||||
@mock.patch.object(ovirt_power, '_getvm')
|
@mock.patch.object(ovirt_power, "sdk", create=True)
|
||||||
def test_set_reboot_when_down(self, mock_vm):
|
def test_set_reboot_when_down(self, sdk):
|
||||||
mock_vm.return_value.get.return_value.status.value = 'down'
|
vm = mock.Mock()
|
||||||
|
vm.get.return_value.status.value = 'down'
|
||||||
|
conn = sdk.Connection.return_value
|
||||||
|
vms_service = conn.system_service.return_value.vms_service.return_value
|
||||||
|
vms_service.vm_service.return_value = vm
|
||||||
|
|
||||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||||
task.driver.power.reboot(task)
|
task.driver.power.reboot(task)
|
||||||
mock_vm.return_value.start.assert_called_once()
|
vm.start.assert_called_once()
|
||||||
|
conn.close.assert_called()
|
||||||
|
|
||||||
@mock.patch.object(ovirt_power, '_getvm')
|
@mock.patch.object(ovirt_power, "sdk", create=True)
|
||||||
def test_set_reboot_when_up(self, mock_vm):
|
def test_set_reboot_when_up(self, sdk):
|
||||||
mock_vm.return_value.get.return_value.status.value = 'up'
|
vm = mock.Mock()
|
||||||
|
vm.get.return_value.status.value = 'up'
|
||||||
|
conn = sdk.Connection.return_value
|
||||||
|
vms_service = conn.system_service.return_value.vms_service.return_value
|
||||||
|
vms_service.vm_service.return_value = vm
|
||||||
|
|
||||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||||
task.driver.power.reboot(task)
|
task.driver.power.reboot(task)
|
||||||
mock_vm.return_value.reboot.assert_called_once()
|
vm.reboot.assert_called_once()
|
||||||
|
conn.close.assert_called()
|
||||||
|
|
|
@ -0,0 +1,9 @@
|
||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
oVirt driver: Close connection after each API call
|
||||||
|
|
||||||
|
A new connection was being created for every API call which was
|
||||||
|
never closed. This resulted in open connections accumulating over time
|
||||||
|
due to periodic calls like power state polling, which was further
|
||||||
|
compounded by the number of ovirt nodes deployed.
|
Loading…
Reference in New Issue