diff --git a/hpe3parclient/ssh.py b/hpe3parclient/ssh.py index 5ce91878..7408eab1 100644 --- a/hpe3parclient/ssh.py +++ b/hpe3parclient/ssh.py @@ -208,52 +208,54 @@ def strip_input_from_output(cmd, output): in the output so that it knows what it is stripping (or else it raises an exception). """ - - # Keep output lines after the 'exit'. - # 'exit' is the last of the stdin. + # Search for the prompt. It may or may not be the first line, because + # on fast connections we get a "dump" of stdin with all the commands + # we've sent, but we don't on slow connections. for i, line in enumerate(output): - if line == 'exit': + prompt_pct = line.find('% setclienv csvtable 1') + if prompt_pct >= 0: + prompt = line[:prompt_pct + 1] + ' ' output = output[i + 1:] break else: - reason = "Did not find 'exit' in output." - HPE3PARSSHClient.raise_stripper_error(reason, output) - - if not output: - reason = "Did not find any output after 'exit'." - HPE3PARSSHClient.raise_stripper_error(reason, output) - - # The next line is prompt plus setclienv command. - # Use this to get the prompt string. - prompt_pct = output[0].find('% setclienv csvtable 1') - if prompt_pct < 0: reason = "Did not find '% setclienv csvtable 1' in output." HPE3PARSSHClient.raise_stripper_error(reason, output) - prompt = output[0][0:prompt_pct + 1] - del output[0] - # Next find the prompt plus the command. + # Some systems return additional empty prompts, others return an + # extra \r after commands. In both cases this prevents us from finding + # the output delimiters, and makes us return useless data to caller, so + # fix them. + output = [line.rstrip('\r\n') for line in output if line != prompt] + + # Output starts right after the command we sent. # It might be broken into multiple lines, so loop and # append until we find the whole prompt plus command. command_string = ' '.join(cmd) if re.match('|'.join(tpd_commands), command_string): escp_command_string = command_string.replace('"', '\\"') command_string = "Tpd::rtpd " + '"' + escp_command_string + '"' - seek = ' '.join((prompt, command_string)) + seek = prompt + command_string found = '' for i, line in enumerate(output): - found = ''.join((found, line.rstrip('\r\n'))) + found = found + line if found == seek: - # Found the whole thing. Use the rest as output now. - output = output[i + 1:] + # Output is right after this line, drop everything before it + del output[:i + 1] break else: HPE3PARSSHClient._logger.debug("Command: %s" % command_string) reason = "Did not find match for command in output" HPE3PARSSHClient.raise_stripper_error(reason, output) - # Always strip the last 2 - return output[:len(output) - 2] + # Output stops right before exit command is executed + try: + exit_index = output.index(prompt + 'exit') + del output[exit_index:] + except ValueError: + reason = "Did not find 'exit' in output." + HPE3PARSSHClient.raise_stripper_error(reason, output) + + return output def run(self, cmd, multi_line_stripper=False): """Runs a CLI command over SSH, without doing any result parsing.""" diff --git a/test/test_HPE3ParClient_FilePersona.py b/test/test_HPE3ParClient_FilePersona.py index d2544db0..74787464 100644 --- a/test/test_HPE3ParClient_FilePersona.py +++ b/test/test_HPE3ParClient_FilePersona.py @@ -408,7 +408,7 @@ def validate_vfs(self, fpgname=None, vfsname=None, no_vfsname=None, # Validate contents if fpgname is not None: success_message = None - not_found_message = 'Invalid VFS %s\r' % vfsname + not_found_message = 'Invalid VFS %s' % vfsname self.assertIn(message, (success_message, not_found_message)) elif total == 0: self.assertEqual('', message) @@ -456,7 +456,7 @@ def test_getfpg_empty(self): @print_header_and_footer def test_getfpg_bogus(self): result = self.cl.getfpg('bogus1', 'bogus2', 'bogus3') - expected_message = 'File Provisioning Group: bogus1 not found\r' + expected_message = 'File Provisioning Group: bogus1 not found' self.assertEqual(expected_message, result['message']) self.assertEqual(0, result['total']) self.assertEqual([], result['members']) @@ -474,7 +474,7 @@ def test_createfpg_bogus_cpg(self): bogus_cpgname = 'thiscpgdoesnotexist' result = self.cl.createfpg(bogus_cpgname, fpgname, '1X') self.assertEqual( - 'Error: Invalid CPG name: %s\r' % bogus_cpgname, + 'Error: Invalid CPG name: %s' % bogus_cpgname, result[0]) self.validate_fpg(expected_count=fpg_count) @@ -498,7 +498,7 @@ def test_createfpg_bad_size(self): result = self.cl.createfpg(cpgname, fpgname, '1X') self.assertEqual( - 'The suffix, X, for size is invalid.\r', result[0]) + 'The suffix, X, for size is invalid.', result[0]) self.validate_fpg(expected_count=fpg_count) @@ -552,7 +552,7 @@ def test_createfpg_twice_and_remove(self): # Create same FPG again to test createfpg already exists error result = self.cl.createfpg(cpgname, fpgname, '1T', wait=True) - expected = ('Error: FPG %s already exists\r' % + expected = ('Error: FPG %s already exists' % fpgname) self.assertEqual(expected, result[0]) self.validate_fpg(fpgname=fpgname, expected_count=fpg_count + 1) @@ -614,7 +614,7 @@ def test_createvfs_bogus_bgrace(self): fpg=fpgname, bgrace='bogus', igrace='60', wait=True) - self.assertEqual('bgrace value should be between 1 and 2147483647\r', + self.assertEqual('bgrace value should be between 1 and 2147483647', result[0]) @unittest.skipIf(is_live_test() and skip_file_persona(), SKIP_MSG) @@ -627,7 +627,7 @@ def test_createvfs_bogus_igrace(self): fpg=fpgname, bgrace='60', igrace='bogus', wait=True) - self.assertEqual('igrace value should be between 1 and 2147483647\r', + self.assertEqual('igrace value should be between 1 and 2147483647', result[0]) def get_fsips(self, fpgname, vfsname): @@ -654,7 +654,7 @@ def get_fsips(self, fpgname, vfsname): result = self.cl.getfsip(vfsname, fpg='bogus') self.debug_print(result) expected = { - 'message': 'File Provisioning Group: bogus not found\r', + 'message': 'File Provisioning Group: bogus not found', 'total': 0, 'members': [] } @@ -662,7 +662,7 @@ def get_fsips(self, fpgname, vfsname): result = self.cl.getfsip('bogus', fpg=fpgname) self.debug_print(result) expected = { - 'message': 'Invalid VFS bogus\r', + 'message': 'Invalid VFS bogus', 'total': 0, 'members': [] } @@ -736,13 +736,13 @@ def create_fsnap(self, fpgname, vfsname, fstore, tag): # Test error messages with bogus names result = self.cl.createfsnap('bogus', fstore, tag, fpg=fpgname) - self.assertEqual(['Virtual Server bogus does not exist on FPG %s\r' % + self.assertEqual(['Virtual Server bogus does not exist on FPG %s' % fpgname], result) result = self.cl.createfsnap(vfsname, 'bogus', tag, fpg=fpgname) - self.assertEqual(['File Store bogus does not exist on FPG %s\r' % + self.assertEqual(['File Store bogus does not exist on FPG %s' % fpgname], result) result = self.cl.createfsnap(vfsname, fstore, tag, fpg='bogus') - self.assertEqual(['FPG bogus not found\r'], result) + self.assertEqual(['FPG bogus not found'], result) result = self.cl.getfsnap('bogus', fpg=fpgname, vfs=vfsname, fstore=fstore, @@ -754,7 +754,7 @@ def create_fsnap(self, fpgname, vfsname, fstore, tag): expected = { 'members': [], 'message': 'SnapShot bogus does not exist on FPG %s path ' - '%s/%s\r' % (fpgname, vfsname, fstore), + '%s/%s' % (fpgname, vfsname, fstore), 'total': 0} self.assertEqual(expected, result) @@ -797,7 +797,7 @@ def create_fsnap(self, fpgname, vfsname, fstore, tag): self.assertEqual([], result) success = [] - running = ['Reclamation already running on %s\r' % fpgname] + running = ['Reclamation already running on %s' % fpgname] expected_in = (success, running) # After first one expect 'running', but to avoid timing issues in # the test results accept either success or running. @@ -833,14 +833,14 @@ def create_fsnap(self, fpgname, vfsname, fstore, tag): self.assertEqual([], result) result = self.cl.startfsnapclean(fpgname, resume=True) - self.assertEqual(['No reclamation task running on FPG %s\r' % fpgname], + self.assertEqual(['No reclamation task running on FPG %s' % fpgname], result) def remove_fstore(self, fpgname, vfsname, fstore): self.cl.removefsnap(vfsname, fstore, fpg=fpgname) result = self.cl.startfsnapclean(fpgname, reclaimStrategy='maxspeed') success = [] - running = ['Reclamation already running on %s\r' % fpgname] + running = ['Reclamation already running on %s' % fpgname] expected_in = (success, running) self.assertIn(result, expected_in) @@ -855,7 +855,7 @@ def remove_share(self, protocol, fpgname, vfsname, share_name): fpg=fpgname, fstore=share_name) if protocol == 'nfs': expected = ['%s Delete Export failed with error: ' - 'share %s does not exist\r' % + 'share %s does not exist' % (protocol.upper(), share_name)] self.assertEqual(expected, result) else: @@ -869,9 +869,9 @@ def remove_share(self, protocol, fpgname, vfsname, share_name): if protocol == 'nfs': expected = [ '%s Delete Export failed with error: ' - 'File Store bogus was not found\r' % protocol.upper()] + 'File Store bogus was not found' % protocol.upper()] else: - expected = ['Could not find Store=bogus\r'] + expected = ['Could not find Store=bogus'] self.assertEqual(expected, result) @unittest.skipIf(skip_file_persona(), SKIP_MSG) @@ -890,7 +890,7 @@ def test_create_and_remove_shares(self): result = self.cl.createvfs('127.0.0.2', '255.255.255.0', vfsname, fpg=fpgname, wait=True) - expected = ('VFS "%s" already exists within FPG %s\r' % + expected = ('VFS "%s" already exists within FPG %s' % (vfsname, fpgname)) self.assertEqual(expected, result[0]) self.validate_vfs(vfsname=vfsname, fpgname=fpgname, @@ -985,12 +985,12 @@ def test_removevfs_bogus(self): self.assertRaises(AttributeError, self.cl.removevfs, None) result = self.cl.removevfs('bogus') vfs_not_found = ('Virtual file server bogus was not found in any ' - 'existing file provisioning group.\r') + 'existing file provisioning group.') self.assertEqual(vfs_not_found, result[0]) self.assertRaises(AttributeError, self.cl.removevfs, None, fpg='bogus') result = self.cl.removevfs('bogus', fpg='bogus') - fpg_not_found = 'File Provisioning Group: bogus not found\r' + fpg_not_found = 'File Provisioning Group: bogus not found' self.assertEqual(fpg_not_found, result[0]) # testing diff --git a/test/test_HPE3ParClient_FilePersona_Mock.py b/test/test_HPE3ParClient_FilePersona_Mock.py index e241a793..eec7f5e6 100644 --- a/test/test_HPE3ParClient_FilePersona_Mock.py +++ b/test/test_HPE3ParClient_FilePersona_Mock.py @@ -258,17 +258,17 @@ def test_strip_input_from_output(self): 'CSIM-EOS08_1611165 cli% createvfs -fpg marktestfpg -wait ' '127.0.0.2 255.255.255.\r', '0 UT5_VFS_150651\r', - 'VFS UT5_VFS_150651 already exists within FPG marktestfpg\r', + 'VFS UT5_VFS_150651 already exists within FPG marktestfpg', 'CSIM-EOS08_1611165 cli% exit\r', '' ] expected = [ - 'VFS UT5_VFS_150651 already exists within FPG marktestfpg\r'] + 'VFS UT5_VFS_150651 already exists within FPG marktestfpg'] actual = ssh.HPE3PARSSHClient.strip_input_from_output(cmd, out) self.assertEqual(expected, actual) - def test_strip_input_from_output_no_exit(self): + def test_strip_input_from_output_no_stdin(self): cmd = [ 'createvfs', '-fpg', @@ -279,10 +279,6 @@ def test_strip_input_from_output_no_exit(self): 'UT5_VFS_150651' ] out = [ - 'setclienv csvtable 1', - 'createvfs -fpg marktestfpg -wait 127.0.0.2 255.255.255.0 ' - 'UT5_VFS_150651', - 'XXXt', # Don't match 'CSIM-EOS08_1611165 cli% setclienv csvtable 1\r', 'CSIM-EOS08_1611165 cli% createvfs -fpg marktestfpg -wait ' '127.0.0.2 255.255.255.\r', @@ -291,9 +287,11 @@ def test_strip_input_from_output_no_exit(self): 'CSIM-EOS08_1611165 cli% exit\r', '' ] - self.assertRaises(exceptions.SSHException, - ssh.HPE3PARSSHClient.strip_input_from_output, - cmd, out) + expected = [ + 'VFS UT5_VFS_150651 already exists within FPG marktestfpg'] + + actual = ssh.HPE3PARSSHClient.strip_input_from_output(cmd, out) + self.assertEqual(expected, actual) def test_strip_input_from_output_no_setclienv(self): cmd = [ diff --git a/test/test_HPE3ParClient_MockSSH.py b/test/test_HPE3ParClient_MockSSH.py index 6f122a14..71a244d3 100644 --- a/test/test_HPE3ParClient_MockSSH.py +++ b/test/test_HPE3ParClient_MockSSH.py @@ -210,6 +210,14 @@ def test_sanitize_cert(self): self.assertEqual(expected, out) def test_strip_input_from_output(self): + def gen_output(*args): + result = [] + for arg in args: + if isinstance(arg, list): + result.extend(arg) + else: + result.append(arg) + return result cmd = ['foo', '-v'] # nothing after exit output = ['exit'] @@ -224,30 +232,68 @@ def test_strip_input_from_output(self): cmd, output) # no setclienv csv - output = [cmd, 'exit', 'out'] + output = cmd + ['exit', 'out'] self.assertRaises(exceptions.SSHException, ssh.HPE3PARSSHClient.strip_input_from_output, cmd, output) # command not in output after exit - output = [cmd, 'exit', 'PROMPT% setclienv csvtable 1'] + output = gen_output('setclienv csvtable 1', + cmd, + 'exit', + 'PROMPT% setclienv csvtable 1') + self.assertRaises(exceptions.SSHException, + ssh.HPE3PARSSHClient.strip_input_from_output, + cmd, + output) + # Missing exit prompt because output was cut + output = gen_output('setclienv csvtable 1', + cmd, + 'exit', + 'PROMPT% setclienv csvtable 1', + 'PROMPT% foo -v', + 'out1', + 'out2') self.assertRaises(exceptions.SSHException, ssh.HPE3PARSSHClient.strip_input_from_output, cmd, output) # success - output = [cmd, - 'setclienv csvtable 1', - 'exit', - 'PROMPT% setclienv csvtable 1', - 'PROMPT% foo -v', - 'out1', - 'out2', - 'out3', - '------', - 'totals'] + expected = ['out1', 'out2', 'out3', '------', 'totals'] + output = gen_output('setclienv csvtable 1', + cmd, + 'exit', + 'PROMPT% setclienv csvtable 1', + 'PROMPT% foo -v', + 'out1', + 'out2', + 'out3', + '------', + 'totals', + 'PROMPT% exit') + result = ssh.HPE3PARSSHClient.strip_input_from_output(cmd, output) + self.assertEqual(expected, result) + + # succeed even on slow connection that's missing the stdin dump + output = output[4:] + result = ssh.HPE3PARSSHClient.strip_input_from_output(cmd, output) + self.assertEqual(expected, result) + + # succeed with empty prompts and extra \r on commands + output = gen_output('setclienv csvtable 1', + cmd, + 'exit', + 'PROMPT% setclienv csvtable 1\r', + 'PROMPT% ', + 'PROMPT% foo -v\r', + 'out1', + 'out2', + 'out3', + '------', + 'totals', + 'PROMPT% exit\r') result = ssh.HPE3PARSSHClient.strip_input_from_output(cmd, output) - self.assertEqual(['out1', 'out2', 'out3'], result) + self.assertEqual(expected, result) @mock.patch('hpe3parclient.client.ssh.HPE3PARSSHClient', spec=True) def test_verify_get_port(self, mock_ssh_client):