From d78179cb57dcc2d169353711cb4d412bc8626f71 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Thu, 10 Mar 2016 11:08:08 +0000 Subject: [PATCH] SSH driver: Remove pipes from virsh's list_{all, running} This patch is removing the use of pipes from the virsh's list_all and list_running commands. The pipes do hide real errors making Ironic believe that the domain is shutdown while in reality it's activated. Nonetheless, it's better to have simpler commands for sanity's sake. Closes-Bug: #1555565 Change-Id: I62a5b292891a44e3f4b82edd90b6e4402c50fabf --- ironic/drivers/modules/ssh.py | 6 ++---- ironic/tests/unit/db/utils.py | 4 ++-- ironic/tests/unit/drivers/modules/test_ssh.py | 5 +++++ 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/ironic/drivers/modules/ssh.py b/ironic/drivers/modules/ssh.py index ec85cb165d..fb005aeb9f 100644 --- a/ironic/drivers/modules/ssh.py +++ b/ironic/drivers/modules/ssh.py @@ -196,10 +196,8 @@ def _get_command_sets(virt_type): 'start_cmd': 'start {_NodeName_}', 'stop_cmd': 'destroy {_NodeName_}', 'reboot_cmd': 'reset {_NodeName_}', - 'list_all': "list --all | tail -n +2 | awk -F\" \" '{print $2}'", - 'list_running': ( - "list --all|grep running | " - "awk -v qc='\"' -F\" \" '{print qc$2qc}'"), + 'list_all': 'list --all --name', + 'list_running': 'list --name', 'get_node_macs': ( "dumpxml {_NodeName_} | " "awk -F \"'\" '/mac address/{print $2}'| tr -d ':'"), diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index 2b1ab636e6..e336967034 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -40,12 +40,12 @@ def get_test_ipmi_bridging_parameters(): } -def get_test_ssh_info(auth_type='password'): +def get_test_ssh_info(auth_type='password', virt_type='virsh'): result = { "ssh_address": "1.2.3.4", "ssh_username": "admin", "ssh_port": 22, - "ssh_virt_type": "vbox", + "ssh_virt_type": virt_type, } if 'password' == auth_type: result['ssh_password'] = 'fake' diff --git a/ironic/tests/unit/drivers/modules/test_ssh.py b/ironic/tests/unit/drivers/modules/test_ssh.py index 20c7c36e7c..76a1e0387d 100644 --- a/ironic/tests/unit/drivers/modules/test_ssh.py +++ b/ironic/tests/unit/drivers/modules/test_ssh.py @@ -318,6 +318,9 @@ class SSHPrivateMethodsTestCase(db_base.DbTestCase): pstate = ssh._get_power_status(self.sshclient, info) self.assertEqual(states.POWER_OFF, pstate) + ssh_cmd = "%s %s" % (info['cmd_set']['base_cmd'], + info['cmd_set']['list_running']) + exec_ssh_mock.assert_called_once_with(self.sshclient, ssh_cmd) @mock.patch.object(processutils, 'ssh_execute', autospec=True) def test__get_hosts_name_for_node_match(self, exec_ssh_mock): @@ -1099,6 +1102,8 @@ class SSHDriverTestCase(db_base.DbTestCase): def test_console_validate_not_virsh(self): with task_manager.acquire( self.context, self.node.uuid, shared=True) as task: + task.node.driver_info = db_utils.get_test_ssh_info( + virt_type='vbox') self.assertRaisesRegex(exception.InvalidParameterValue, 'not supported for non-virsh types', task.driver.console.validate, task)