From c0c409084d711c270112d884e565a20fedacd806 Mon Sep 17 00:00:00 2001 From: "Vincent S. Cojot" Date: Fri, 29 Apr 2016 17:27:35 -0400 Subject: [PATCH] Make the ssh driver work on headless VirtualBox machines On a workstation where an X display might not currently be running or a workstation where someone else is logged on the graphical desktop this small change helps get things working for using VirtualBox as the hypervisor. Use VirtualBox 3.2 or above for headless mode support. This patch also ensures that set_boot_device() always works by powering off the VM before setting the device. VirtualBox does not allow boot device to be set while the machine is on. Closes-Bug: 1561624 Change-Id: If8d989cc3964e849544b139959b329a3fd05d451 --- ironic/drivers/modules/ssh.py | 46 ++++++++++++-- ironic/tests/unit/conductor/test_manager.py | 6 +- ironic/tests/unit/drivers/modules/test_ssh.py | 62 +++++++++++++++++++ 3 files changed, 108 insertions(+), 6 deletions(-) diff --git a/ironic/drivers/modules/ssh.py b/ironic/drivers/modules/ssh.py index 13c46b5dc3..38da701ed8 100644 --- a/ironic/drivers/modules/ssh.py +++ b/ironic/drivers/modules/ssh.py @@ -34,6 +34,8 @@ from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log as logging from oslo_utils import excutils +from oslo_utils import strutils + import retrying from ironic.common import boot_devices @@ -84,7 +86,11 @@ OTHER_PROPERTIES = { 'ssh_password': _("password to use for authentication or for unlocking a " "private key. One of this, ssh_key_contents, or " "ssh_key_filename must be specified."), - 'ssh_port': _("port on the node to connect to; default is 22. Optional.") + 'ssh_port': _("port on the node to connect to; default is 22. Optional."), + 'vbox_use_headless': _("True or False (Default). Optional. " + "In the case of VirtualBox 3.2 and above, allows " + "the user to use a headless remote VirtualBox " + "machine.") } COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy() COMMON_PROPERTIES.update(OTHER_PROPERTIES) @@ -131,9 +137,22 @@ def _get_boot_device_map(virt_type): {'virt_type': virt_type}) -def _get_command_sets(virt_type): +def _get_command_sets(virt_type, use_headless=False): """Retrieves the virt_type-specific commands to control power + :param virt_type: Hypervisor type (virsh, vmware, vbox, parallels, + xenserver) + :param use_headless: boolean value, defaults to False. + use_headless is used by some Hypervisors (only VBox v3.2 and above) + to determine if the hypervisor is being used on a headless box. + This is only relevant to Desktop Hypervisors that have different + CLI settings depending upon the availability of a graphical + environment working on the hypervisor itself. Again, only VBox + makes this distinction and allows "--type headless" to some of + its sub-commands. This is needed for support of tripleo with + VBox as the Hypervisor but some other Hypervisors could make + use of it in the future (Parallels, VMWare Workstation, etc...) + Required commands are as follows: base_cmd: Used by most sub-commands as the primary executable @@ -147,9 +166,12 @@ def _get_command_sets(virt_type): get_boot_device / set_boot_device: Gets or sets the primary boot device """ if virt_type == 'vbox': + vbox_headless_str = '' + if use_headless: + vbox_headless_str = ' --type headless' return { 'base_cmd': 'LC_ALL=C /usr/bin/VBoxManage', - 'start_cmd': 'startvm {_NodeName_}', + 'start_cmd': 'startvm {_NodeName_}%s' % vbox_headless_str, 'stop_cmd': 'controlvm {_NodeName_} poweroff', 'reboot_cmd': 'controlvm {_NodeName_} reset', 'list_all': "list vms|awk -F'\"' '{print $2}'", @@ -367,6 +389,8 @@ def _parse_driver_info(node): port = utils.validate_network_port(port, 'ssh_port') key_contents = info.get('ssh_key_contents') key_filename = info.get('ssh_key_filename') + use_headless = strutils.bool_from_string(info.get('vbox_use_headless', + False)) virt_type = info.get('ssh_virt_type') terminal_port = info.get('ssh_terminal_port') @@ -379,12 +403,13 @@ def _parse_driver_info(node): 'host': address, 'username': username, 'port': port, + 'use_headless': use_headless, 'virt_type': virt_type, 'uuid': node.uuid, 'terminal_port': terminal_port } - cmd_set = _get_command_sets(virt_type) + cmd_set = _get_command_sets(virt_type, use_headless) res['cmd_set'] = cmd_set # Only one credential may be set (avoids complexity around having @@ -738,6 +763,19 @@ class SSHManagement(base.ManagementInterface): driver_info['macs'] = driver_utils.get_node_mac_addresses(task) ssh_obj = _get_connection(node) + node_name = _get_hosts_name_for_node(ssh_obj, driver_info) + virt_type = driver_info['virt_type'] + use_headless = driver_info['use_headless'] + + if virt_type == 'vbox': + if use_headless: + current_pstate = _get_power_status(ssh_obj, driver_info) + if current_pstate == states.POWER_ON: + LOG.debug("Forcing VBox VM %s to power off " + "in order to set the boot device.", + node_name) + _power_off(ssh_obj, driver_info) + boot_device_map = _get_boot_device_map(driver_info['virt_type']) try: _set_boot_device(ssh_obj, driver_info, boot_device_map[device]) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 48fbb2c1b3..9f327fa8be 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -3803,7 +3803,8 @@ class ManagerTestProperties(tests_db_base.DbTestCase): self._check_driver_properties("fake_ipminative", expected) def test_driver_properties_fake_ssh(self): - expected = ['ssh_address', 'ssh_username', 'ssh_virt_type', + expected = ['ssh_address', 'ssh_username', + 'vbox_use_headless', 'ssh_virt_type', 'ssh_key_contents', 'ssh_key_filename', 'ssh_password', 'ssh_port', 'ssh_terminal_port'] self._check_driver_properties("fake_ssh", expected) @@ -3843,7 +3844,8 @@ class ManagerTestProperties(tests_db_base.DbTestCase): def test_driver_properties_pxe_ssh(self): expected = ['deploy_kernel', 'deploy_ramdisk', - 'ssh_address', 'ssh_username', 'ssh_virt_type', + 'ssh_address', 'ssh_username', + 'vbox_use_headless', 'ssh_virt_type', 'ssh_key_contents', 'ssh_key_filename', 'ssh_password', 'ssh_port', 'ssh_terminal_port', 'deploy_forces_oob_reboot'] diff --git a/ironic/tests/unit/drivers/modules/test_ssh.py b/ironic/tests/unit/drivers/modules/test_ssh.py index d4ac3e66fb..6b088182b7 100644 --- a/ironic/tests/unit/drivers/modules/test_ssh.py +++ b/ironic/tests/unit/drivers/modules/test_ssh.py @@ -773,6 +773,34 @@ class SSHDriverTestCase(db_base.DbTestCase): '--boot1 net') % fake_name mock_exc.assert_called_once_with(mock.ANY, expected_cmd) + @mock.patch.object(ssh, '_get_power_status', autospec=True) + @mock.patch.object(ssh, '_get_connection', autospec=True) + @mock.patch.object(ssh, '_get_hosts_name_for_node', autospec=True) + @mock.patch.object(ssh, '_ssh_execute', autospec=True) + def test_management_interface_set_boot_device_vbox_with_power_on( + self, mock_exc, mock_h, mock_get_conn, mock_get_power): + fake_name = 'fake-name' + mock_h.return_value = fake_name + mock_get_conn.return_value = self.sshclient + # NOTE(jroll) _power_off calls _get_power_state twice + mock_get_power.side_effect = [ + states.POWER_ON, states.POWER_ON, states.POWER_OFF + ] + with task_manager.acquire(self.context, self.node.uuid) as task: + task.node['driver_info']['ssh_virt_type'] = 'vbox' + task.node['driver_info']['vbox_use_headless'] = True + self.driver.management.set_boot_device(task, boot_devices.PXE) + + expected_cmds = [ + mock.call(mock.ANY, + 'LC_ALL=C /usr/bin/VBoxManage ' + 'controlvm %s poweroff' % fake_name), + mock.call(mock.ANY, + 'LC_ALL=C /usr/bin/VBoxManage ' + 'modifyvm %s --boot1 net' % fake_name) + ] + self.assertEqual(expected_cmds, mock_exc.call_args_list) + @mock.patch.object(ssh, '_get_connection', autospec=True) @mock.patch.object(ssh, '_get_hosts_name_for_node', autospec=True) @mock.patch.object(ssh, '_ssh_execute', autospec=True) @@ -1007,6 +1035,40 @@ class SSHDriverTestCase(db_base.DbTestCase): "vm-shutdown uuid=fakevm force=true") mock_exc.assert_called_once_with(mock.ANY, expected_cmd) + @mock.patch.object(ssh, '_get_connection', autospec=True) + @mock.patch.object(ssh, '_get_hosts_name_for_node', autospec=True) + @mock.patch.object(ssh, '_ssh_execute', autospec=True) + @mock.patch.object(ssh, '_get_power_status', autospec=True) + def test_start_command_vbox(self, mock_power, mock_exc, mock_h, + mock_get_conn): + mock_power.side_effect = [states.POWER_OFF, states.POWER_ON] + nodename = 'fakevm' + mock_h.return_value = nodename + mock_get_conn.return_value = self.sshclient + with task_manager.acquire(self.context, self.node.uuid) as task: + task.node['driver_info']['ssh_virt_type'] = 'vbox' + self.driver.power.set_power_state(task, states.POWER_ON) + expected_cmd = 'LC_ALL=C /usr/bin/VBoxManage startvm fakevm' + mock_exc.assert_called_once_with(mock.ANY, expected_cmd) + + @mock.patch.object(ssh, '_get_connection', autospec=True) + @mock.patch.object(ssh, '_get_hosts_name_for_node', autospec=True) + @mock.patch.object(ssh, '_ssh_execute', autospec=True) + @mock.patch.object(ssh, '_get_power_status', autospec=True) + def test_start_command_vbox_headless(self, mock_power, mock_exc, mock_h, + mock_get_conn): + mock_power.side_effect = [states.POWER_OFF, states.POWER_ON] + nodename = 'fakevm' + mock_h.return_value = nodename + mock_get_conn.return_value = self.sshclient + with task_manager.acquire(self.context, self.node.uuid) as task: + task.node['driver_info']['ssh_virt_type'] = 'vbox' + task.node['driver_info']['vbox_use_headless'] = True + self.driver.power.set_power_state(task, states.POWER_ON) + expected_cmd = ('LC_ALL=C /usr/bin/VBoxManage ' + 'startvm fakevm --type headless') + mock_exc.assert_called_once_with(mock.ANY, expected_cmd) + def test_management_interface_validate_good(self): with task_manager.acquire(self.context, self.node.uuid) as task: task.driver.management.validate(task)