diff --git a/os_xenapi/tests/utils/test_iptable.py b/os_xenapi/tests/utils/test_iptable.py index 60ddddf..5592ea1 100644 --- a/os_xenapi/tests/utils/test_iptable.py +++ b/os_xenapi/tests/utils/test_iptable.py @@ -48,36 +48,33 @@ class XenapiIptableTestCase(base.TestCase): def test_configure_dom0_iptables(self): client = mock.Mock() - client.ssh.side_effect = [sshclient.SshExecCmdFailure( - command="fake_cmd", - stdout="fake_out", - stderr="fake_err"), - None, - None, - sshclient.SshExecCmdFailure( - command="fake_cmd", - stdout="fake_out", - stderr="fake_err"), - None, - sshclient.SshExecCmdFailure( - command="fake_cmd", - stdout="fake_out", - stderr="fake_err"), - None, - None] + client.ssh.side_effect = [(1, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (1, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (1, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err')] xs_chain = 'XenServer-Neutron-INPUT' - expect_call1 = mock.call('iptables -t filter -L %s' % xs_chain) - expect_call2 = mock.call('iptables -t filter --new %s' % xs_chain) + expect_call1 = mock.call('iptables -t filter -L %s' % xs_chain, + allowed_return_codes=[0, 1]) + expect_call2 = mock.call('iptables -t filter --new %s' % xs_chain, + allowed_return_codes=[0]) expect_call3 = mock.call('iptables -t filter -I INPUT -j %s' - % xs_chain) + % xs_chain, allowed_return_codes=[0]) expect_call4 = mock.call('iptables -t filter -C %s -p tcp -m ' - 'tcp --dport 6640 -j ACCEPT' % xs_chain) + 'tcp --dport 6640 -j ACCEPT' % xs_chain, + allowed_return_codes=[0, 1]) expect_call5 = mock.call('iptables -t filter -I %s -p tcp -m ' - 'tcp --dport 6640 -j ACCEPT' % xs_chain) + 'tcp --dport 6640 -j ACCEPT' % xs_chain, + allowed_return_codes=[0]) expect_call6 = mock.call('iptables -t filter -C %s -p udp -m ' - 'multiport --dport 4789 -j ACCEPT' % xs_chain) + 'multiport --dport 4789 -j ACCEPT' % xs_chain, + allowed_return_codes=[0, 1]) expect_call7 = mock.call('iptables -t filter -I %s -p udp -m ' - 'multiport --dport 4789 -j ACCEPT' % xs_chain) + 'multiport --dport 4789 -j ACCEPT' % xs_chain, + allowed_return_codes=[0]) expect_call8 = mock.call("service iptables save") expect_calls = [expect_call1, expect_call2, expect_call3, expect_call4, expect_call5, expect_call6, expect_call7, expect_call8] @@ -86,28 +83,30 @@ class XenapiIptableTestCase(base.TestCase): @mock.patch.object(himn, 'get_local_himn_eth') @mock.patch.object(common_function, 'execute') - def test_configure_himn_forwards(self, mock_execute, mock_get_eth): + @mock.patch.object(common_function, 'detailed_execute') + def test_configure_himn_forwards(self, mock_detail_execute, + mock_execute, mock_get_eth): mock_get_eth.return_value = 'fake_eth' fack_end_point = ['br-storage', 'br-mgmt'] - mock_execute.side_effect = [None, - None, - exception.ExecuteCommandFailed('fake_cmd'), - None, - exception.ExecuteCommandFailed('fake_cmd'), - None, - exception.ExecuteCommandFailed('fake_cmd'), - None, - exception.ExecuteCommandFailed('fake_cmd'), - None, - exception.ExecuteCommandFailed('fake_cmd'), - None, - exception.ExecuteCommandFailed('fake_cmd'), - None, - exception.ExecuteCommandFailed('fake_cmd'), - None, - None, - None, - None] + mock_execute.side_effect = [None, None] + mock_detail_execute.side_effect = [ + (1, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (1, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (1, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (1, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (1, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (1, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (1, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err'), + (0, 'fake_out', 'fake_err')] expect_call1 = mock.call( 'sed', @@ -116,67 +115,80 @@ class XenapiIptableTestCase(base.TestCase): expect_call2 = mock.call('sysctl', 'net.ipv4.ip_forward=1') expect_call3 = mock.call('iptables', '-t', 'nat', '-C', 'POSTROUTING', - '-o', fack_end_point[0], '-j', 'MASQUERADE') + '-o', fack_end_point[0], '-j', 'MASQUERADE', + allowed_return_codes=[0, 1]) expect_call4 = mock.call('iptables', '-t', 'nat', '-I', 'POSTROUTING', - '-o', fack_end_point[0], '-j', 'MASQUERADE') + '-o', fack_end_point[0], '-j', 'MASQUERADE', + allowed_return_codes=[0]) expect_call5 = mock.call('iptables', '-t', 'nat', '-C', 'POSTROUTING', - '-o', fack_end_point[1], '-j', 'MASQUERADE') + '-o', fack_end_point[1], '-j', 'MASQUERADE', + allowed_return_codes=[0, 1]) expect_call6 = mock.call('iptables', '-t', 'nat', '-I', 'POSTROUTING', - '-o', fack_end_point[1], '-j', 'MASQUERADE') + '-o', fack_end_point[1], '-j', 'MASQUERADE', + allowed_return_codes=[0]) expect_call7 = mock.call('iptables', '-t', 'filter', '-C', 'FORWARD', '-i', fack_end_point[0], '-o', 'fake_eth', '-m', 'state', '--state', - 'RELATED,ESTABLISHED', '-j', 'ACCEPT') + 'RELATED,ESTABLISHED', '-j', 'ACCEPT', + allowed_return_codes=[0, 1]) expect_call8 = mock.call('iptables', '-t', 'filter', '-I', 'FORWARD', '-i', fack_end_point[0], '-o', 'fake_eth', '-m', 'state', '--state', - 'RELATED,ESTABLISHED', '-j', 'ACCEPT') + 'RELATED,ESTABLISHED', '-j', 'ACCEPT', + allowed_return_codes=[0]) expect_call9 = mock.call('iptables', '-t', 'filter', '-C', 'FORWARD', '-i', fack_end_point[1], '-o', 'fake_eth', '-m', 'state', '--state', - 'RELATED,ESTABLISHED', '-j', 'ACCEPT') + 'RELATED,ESTABLISHED', '-j', 'ACCEPT', + allowed_return_codes=[0, 1]) expect_call10 = mock.call('iptables', '-t', 'filter', '-I', 'FORWARD', '-i', fack_end_point[1], '-o', 'fake_eth', '-m', 'state', '--state', - 'RELATED,ESTABLISHED', '-j', 'ACCEPT') + 'RELATED,ESTABLISHED', '-j', 'ACCEPT', + allowed_return_codes=[0]) expect_call11 = mock.call('iptables', '-t', 'filter', '-C', 'FORWARD', '-i', 'fake_eth', '-o', fack_end_point[0], - '-j', 'ACCEPT') + '-j', 'ACCEPT', allowed_return_codes=[0, 1]) expect_call12 = mock.call('iptables', '-t', 'filter', '-I', 'FORWARD', '-i', 'fake_eth', '-o', fack_end_point[0], - '-j', 'ACCEPT') + '-j', 'ACCEPT', allowed_return_codes=[0]) expect_call13 = mock.call('iptables', '-t', 'filter', '-C', 'FORWARD', '-i', 'fake_eth', '-o', fack_end_point[1], - '-j', 'ACCEPT') + '-j', 'ACCEPT', allowed_return_codes=[0, 1]) expect_call14 = mock.call('iptables', '-t', 'filter', '-I', 'FORWARD', '-i', 'fake_eth', '-o', fack_end_point[1], - '-j', 'ACCEPT') + '-j', 'ACCEPT', allowed_return_codes=[0]) expect_call15 = mock.call('iptables', '-t', 'filter', '-C', 'INPUT', - '-i', 'fake_eth', '-j', 'ACCEPT') + '-i', 'fake_eth', '-j', 'ACCEPT', + allowed_return_codes=[0, 1]) expect_call16 = mock.call('iptables', '-t', 'filter', '-I', 'INPUT', - '-i', 'fake_eth', '-j', 'ACCEPT') + '-i', 'fake_eth', '-j', 'ACCEPT', + allowed_return_codes=[0]) - expect_call17 = mock.call('iptables', '-t', 'filter', '-S', 'FORWARD') - expect_call18 = mock.call('iptables', '-t', 'nat', '-S', 'POSTROUTING') + expect_call17 = mock.call('iptables', '-t', 'filter', '-S', 'FORWARD', + allowed_return_codes=[0]) + expect_call18 = mock.call('iptables', '-t', 'nat', '-S', 'POSTROUTING', + allowed_return_codes=[0]) + + detail_execute_expect_calls = [expect_call3, expect_call4, + expect_call7, expect_call8, + expect_call11, expect_call12, + expect_call5, expect_call6, + expect_call9, expect_call10, + expect_call13, expect_call14, + expect_call15, expect_call16, + expect_call17, expect_call18] - expect_calls = [expect_call1, expect_call2, - expect_call3, expect_call4, - expect_call7, expect_call8, - expect_call11, expect_call12, - expect_call5, expect_call6, - expect_call9, expect_call10, - expect_call13, expect_call14, - expect_call15, expect_call16, - expect_call17, expect_call18] iptables.configure_himn_forwards(fack_end_point, 'fake_dom0_himn_ip') mock_get_eth.assert_called_once_with('fake_dom0_himn_ip') - mock_execute.assert_has_calls(expect_calls) + mock_execute.assert_has_calls([expect_call1, expect_call2]) + mock_detail_execute.assert_has_calls(detail_execute_expect_calls) @mock.patch.object(himn, 'get_local_himn_eth') @mock.patch.object(common_function, 'execute') @@ -187,21 +199,22 @@ class XenapiIptableTestCase(base.TestCase): iptables.configure_himn_forwards, 'fake_end_point', 'fake_dom0_himn_ip') - @mock.patch.object(common_function, 'execute') + @mock.patch.object(common_function, 'detailed_execute') def test_execute_local_iptables_cmd(self, mock_execute): fake_rule_spec = 'fake_rule' - mock_execute.return_value = 'success' + mock_execute.return_value = (0, 'fake_out', 'fake_err') - execute_result = iptables.execute_iptables_cmd('fake_table', - 'fake_action', - 'fake_chain', - fake_rule_spec) - self.assertTrue(execute_result) + ret, out, err = iptables.execute_iptables_cmd('fake_table', + 'fake_action', + 'fake_chain', + fake_rule_spec) + self.assertEqual(ret, 0) mock_execute.assert_called_once_with('iptables', '-t', 'fake_table', 'fake_action', 'fake_chain', - fake_rule_spec) + fake_rule_spec, + allowed_return_codes=[0]) - @mock.patch.object(common_function, 'execute') + @mock.patch.object(common_function, 'detailed_execute') def test_execute_local_iptables_cmd_failed(self, mock_execute): fake_rule_spec = 'fake_rule' mock_execute.side_effect = [exception.ExecuteCommandFailed('fake_cmd')] @@ -213,50 +226,48 @@ class XenapiIptableTestCase(base.TestCase): mock_execute.assert_called_once_with('iptables', '-t', 'fake_table', 'fake_action', 'fake_chain', - fake_rule_spec) + fake_rule_spec, + allowed_return_codes=[0]) - @mock.patch.object(common_function, 'execute') + @mock.patch.object(common_function, 'detailed_execute') def test_execute_local_iptables_cmd_expect_failed(self, mock_execute): fake_rule_spec = 'fake_rule' - mock_execute.side_effect = [exception.ExecuteCommandFailed('fake_cmd')] + mock_execute.return_value = (1, 'fake_out', 'fake_err') - execute_result = iptables.execute_iptables_cmd('fake_table', - 'fake_action', - 'fake_chain', - fake_rule_spec, - None, - True) - self.assertFalse(execute_result) + ret, out, err = iptables.execute_iptables_cmd( + 'fake_table', 'fake_action', 'fake_chain', + rule_spec=fake_rule_spec, client=None, allowed_return_codes=[0, 1]) + self.assertEqual(ret, 1) mock_execute.assert_called_once_with('iptables', '-t', 'fake_table', 'fake_action', 'fake_chain', - fake_rule_spec) + fake_rule_spec, + allowed_return_codes=[0, 1]) - @mock.patch.object(common_function, 'execute') + @mock.patch.object(common_function, 'detailed_execute') def test_execute_local_iptables_cmd_no_rule_spec(self, mock_execute): - mock_execute.return_value = 'success' + mock_execute.return_value = (0, 'fake_out', 'fake_err') - execute_result = iptables.execute_iptables_cmd('fake_table', - 'fake_action', - 'fake_chain', - None) - self.assertTrue(execute_result) + ret, out, err = iptables.execute_iptables_cmd('fake_table', + 'fake_action', + 'fake_chain') + self.assertEqual(ret, 0) mock_execute.assert_called_once_with('iptables', '-t', 'fake_table', - 'fake_action', 'fake_chain') + 'fake_action', 'fake_chain', + allowed_return_codes=[0]) def test_execute_remote_iptables_cmd(self): fake_client = mock.Mock() fake_rule_spec = 'fake_rule' - fake_client.ssh.return_value = 'success' + fake_client.ssh.return_value = (0, 'fake_out', 'fake_err') - execute_result = iptables.execute_iptables_cmd('fake_table', - 'fake_action', - 'fake_chain', - fake_rule_spec, - fake_client) - self.assertTrue(execute_result) + ret, out, err = iptables.execute_iptables_cmd( + 'fake_table', 'fake_action', 'fake_chain', + rule_spec=fake_rule_spec, client=fake_client) + self.assertEqual(ret, 0) fake_client.ssh.assert_called_once_with('iptables -t fake_table ' + 'fake_action fake_chain ' + - fake_rule_spec) + fake_rule_spec, + allowed_return_codes=[0]) def test_execute_remote_iptables_cmd_failed(self): fake_client = mock.Mock() @@ -274,36 +285,35 @@ class XenapiIptableTestCase(base.TestCase): fake_client.ssh.assert_called_once_with('iptables -t fake_table ' + 'fake_action fake_chain ' + - fake_rule_spec) + fake_rule_spec, + allowed_return_codes=[0]) def test_execute_remote_iptables_cmd_expect_failed(self): fake_client = mock.Mock() fake_rule_spec = 'fake_rule' - fake_client.ssh.side_effect = [sshclient.SshExecCmdFailure( - command="fake_cmd", - stdout="fake_out", - stderr="fake_err")] + fake_client.ssh.return_value = (1, 'fake_out', 'fake_err') - execute_result = iptables.execute_iptables_cmd('fake_table', - 'fake_action', - 'fake_chain', - fake_rule_spec, - fake_client, - True) - self.assertFalse(execute_result) + ret, out, err = iptables.execute_iptables_cmd('fake_table', + 'fake_action', + 'fake_chain', + fake_rule_spec, + fake_client, + [0, 1]) + self.assertEqual(ret, 1) fake_client.ssh.assert_called_once_with('iptables -t fake_table ' + 'fake_action fake_chain ' + - fake_rule_spec) + fake_rule_spec, + allowed_return_codes=[0, 1]) def test_execute_remote_iptables_cmd_no_rule_spec(self): fake_client = mock.Mock() - fake_client.ssh.return_value = 'success' + fake_client.ssh.return_value = (0, 'fake_out', 'fake_err') - execute_result = iptables.execute_iptables_cmd('fake_table', - 'fake_action', - 'fake_chain', - None, - fake_client) - self.assertTrue(execute_result) + ret, out, err = iptables.execute_iptables_cmd('fake_table', + 'fake_action', + 'fake_chain', + client=fake_client) + self.assertEqual(ret, 0) fake_client.ssh.assert_called_once_with("iptables -t fake_table " - "fake_action fake_chain") + "fake_action fake_chain", + allowed_return_codes=[0]) diff --git a/os_xenapi/tests/utils/test_sshclient.py b/os_xenapi/tests/utils/test_sshclient.py index a7b6374..522a4a4 100644 --- a/os_xenapi/tests/utils/test_sshclient.py +++ b/os_xenapi/tests/utils/test_sshclient.py @@ -52,7 +52,7 @@ class SshClientTestCase(base.TestCase): client = sshclient.SSHClient('ip', 'username', password='password', log=mock_log) - out, err = client.ssh('fake_command', output=True) + return_code, out, err = client.ssh('fake_command', output=True) mock_log.debug.assert_called() mock_exec.assert_called() @@ -78,6 +78,25 @@ class SshClientTestCase(base.TestCase): self.assertRaises(sshclient.SshExecCmdFailure, client.ssh, 'fake_command', output=True) + @mock.patch.object(paramiko.SSHClient, 'set_missing_host_key_policy') + @mock.patch.object(paramiko.SSHClient, 'connect') + @mock.patch.object(paramiko.SSHClient, 'exec_command') + def test_ssh_allow_error_return(self, mock_exec, mock_conn, mock_set): + mock_log = mock.Mock() + mock_channel = mock.Mock() + mock_exec.return_value = (fake_channel_file(['input']), + fake_channel_file(['info'], mock_channel), + fake_channel_file(['err'])) + mock_channel.recv_exit_status.return_value = 1 + + client = sshclient.SSHClient('ip', 'username', password='password', + log=mock_log) + return_code, out, err = client.ssh('fake_command', output=True, + allowed_return_codes=[0, 1]) + mock_exec.assert_called_once_with('fake_command', get_pty=True) + mock_channel.recv_exit_status.assert_called_once() + self.assertEqual(return_code, 1) + @mock.patch.object(paramiko.SSHClient, 'set_missing_host_key_policy') @mock.patch.object(paramiko.SSHClient, 'connect') @mock.patch.object(paramiko.SSHClient, 'open_sftp') @@ -87,6 +106,7 @@ class SshClientTestCase(base.TestCase): mock_open.return_value = mock_sftp client = sshclient.SSHClient('ip', 'username', password='password', + log=mock_log) client.scp('source_file', 'dest_file') diff --git a/os_xenapi/utils/iptables.py b/os_xenapi/utils/iptables.py index 16c0825..a76f056 100644 --- a/os_xenapi/utils/iptables.py +++ b/os_xenapi/utils/iptables.py @@ -36,21 +36,24 @@ def exit_with_error(err_msg): def configure_dom0_iptables(client): xs_chain = 'XenServer-Neutron-INPUT' # Check XenServer specific chain, create if not exist - if not execute_iptables_cmd('filter', '-L', xs_chain, client=client, - expect_exception=True): + ret, out, err = execute_iptables_cmd('filter', '-L', xs_chain, + client=client, + allowed_return_codes=[0, 1]) + if ret == 1: execute_iptables_cmd('filter', '--new', xs_chain, client=client) rule_spec = ('-j %s' % xs_chain) - execute_iptables_cmd('filter', '-I', 'INPUT', rule_spec, client) + execute_iptables_cmd('filter', '-I', 'INPUT', rule_spec=rule_spec, + client=client) # Check XenServer rule for ovs native mode, create if not exist rule_spec = ('-p tcp -m tcp --dport %s -j ACCEPT' % OVS_NATIVE_TCP_PORT) - ensure_iptables('filter', xs_chain, rule_spec, client) + ensure_iptables('filter', xs_chain, rule_spec, client=client) # Check XenServer rule for vxlan, create if not exist rule_spec = ('-p udp -m multiport --dport %s -j ACCEPT' % VXLAN_UDP_PORT) - ensure_iptables('filter', xs_chain, rule_spec, client) + ensure_iptables('filter', xs_chain, rule_spec, client=client) # Persist iptables rules client.ssh('service iptables save') @@ -86,12 +89,17 @@ def configure_himn_forwards(forwarding_interfaces, dom0_himn_ip): def ensure_iptables(table, chain, rule_spec, client=None): - if not execute_iptables_cmd(table, '-C', chain, rule_spec, client, True): - execute_iptables_cmd(table, '-I', chain, rule_spec, client) + ret, _, _ = execute_iptables_cmd( + table, '-C', chain, rule_spec=rule_spec, client=client, + allowed_return_codes=[0, 1]) + # if the return value is 1, the rule is not exists + if ret == 1: + execute_iptables_cmd(table, '-I', chain, rule_spec=rule_spec, + client=client) def execute_iptables_cmd(table, action, chain, rule_spec=None, client=None, - expect_exception=False): + allowed_return_codes=[0]): """This function is used to run iptables command. Users could run command to configure iptables for remote and local hosts. @@ -113,27 +121,15 @@ def execute_iptables_cmd(table, action, chain, rule_spec=None, client=None, % {'table': table, 'action': action, 'chain': chain, 'rule_spec': rule_spec}) command = command.strip() - try: - client.ssh(command) - except sshclient.SshExecCmdFailure: - if expect_exception: - return False - else: - raise + return client.ssh(command, allowed_return_codes=allowed_return_codes) else: if rule_spec: rule_spec = rule_spec.split() else: rule_spec = [] command = ['iptables', '-t', table, action, chain] + rule_spec - try: - common_function.execute(*command) - except exception.ExecuteCommandFailed: - if expect_exception: - return False - else: - raise - return True + return common_function.detailed_execute( + *command, allowed_return_codes=allowed_return_codes) def config_iptables(client, forwarding_interfaces=None): diff --git a/os_xenapi/utils/sshclient.py b/os_xenapi/utils/sshclient.py index 67cd598..41ea83c 100644 --- a/os_xenapi/utils/sshclient.py +++ b/os_xenapi/utils/sshclient.py @@ -14,12 +14,14 @@ This defines a class for SSH client which can be used to scp files to remote hosts or execute commands in remote hosts. """ - +import logging import paramiko from os_xenapi.client.exception import OsXenApiException from os_xenapi.client.i18n import _ +LOG = logging.getLogger('SSHClient') + class SshExecCmdFailure(OsXenApiException): msg_fmt = _("Failed to execute: %(command)s\n" @@ -43,7 +45,8 @@ class SSHClient(object): def __del__(self): self.client.close() - def ssh(self, command, get_pty=True, output=False): + def ssh(self, command, get_pty=True, output=False, + allowed_return_codes=[0]): if self.log: self.log.debug("Executing command: [%s]" % command) stdin, stdout, stderr = self.client.exec_command( @@ -56,13 +59,13 @@ class SSHClient(object): if err: self.log.error(err) ret = stdout.channel.recv_exit_status() - if ret: - if self.log: - self.log.debug("FAILED executing command: [%s]" - "-(ret=%s)" % (command, ret)) + if ret in allowed_return_codes: + LOG.info('Swallowed acceptable return code of %d', ret) + else: + LOG.warn('unacceptable return code: %d', ret) raise SshExecCmdFailure(command=command, stdout=out, stderr=err) - return out, err + return ret, out, err def scp(self, source, dest): if self.log: