diff --git a/ansible/roles/test/files/ptftests/advanced-reboot.py b/ansible/roles/test/files/ptftests/advanced-reboot.py index 2c6c2e1af9..c63a117d35 100644 --- a/ansible/roles/test/files/ptftests/advanced-reboot.py +++ b/ansible/roles/test/files/ptftests/advanced-reboot.py @@ -57,12 +57,12 @@ import re from collections import defaultdict import json -import paramiko import Queue import pickle from operator import itemgetter import scapy.all as scapyall import itertools +from device_connection import DeviceConnection from arista import Arista import sad_path as sp @@ -125,6 +125,7 @@ def __init__(self): self.test_params = testutils.test_params_get() self.check_param('verbose', False, required=False) self.check_param('dut_username', '', required=True) + self.check_param('dut_password', '', required=True) self.check_param('dut_hostname', '', required=True) self.check_param('reboot_limit_in_seconds', 30, required=False) self.check_param('reboot_type', 'fast-reboot', required=False) @@ -217,6 +218,12 @@ def __init__(self): self.allow_vlan_flooding = bool(self.test_params['allow_vlan_flooding']) + self.dut_connection = DeviceConnection( + self.test_params['dut_hostname'], + self.test_params['dut_username'], + password=self.test_params['dut_password'] + ) + return def read_json(self, name): @@ -411,7 +418,7 @@ def get_sad_info(self): def init_sad_oper(self): if self.sad_oper: self.log("Preboot/Inboot Operations:") - self.sad_handle = sp.SadTest(self.sad_oper, self.ssh_targets, self.portchannel_ports, self.vm_dut_map, self.test_params, self.dut_ssh, self.vlan_ports) + self.sad_handle = sp.SadTest(self.sad_oper, self.ssh_targets, self.portchannel_ports, self.vm_dut_map, self.test_params, self.vlan_ports) (self.ssh_targets, self.portchannel_ports, self.neigh_vm, self.vlan_ports), (log_info, fails) = self.sad_handle.setup() self.populate_fail_info(fails) for log in log_info: @@ -480,7 +487,6 @@ def setUp(self): self.reboot_type = self.test_params['reboot_type'] if self.reboot_type not in ['fast-reboot', 'warm-reboot']: raise ValueError('Not supported reboot_type %s' % self.reboot_type) - self.dut_ssh = self.test_params['dut_username'] + '@' + self.test_params['dut_hostname'] self.dut_mac = self.test_params['dut_mac'] # get VM info @@ -509,7 +515,7 @@ def setUp(self): self.from_server_dst_ports = self.portchannel_ports self.log("Test params:") - self.log("DUT ssh: %s" % self.dut_ssh) + self.log("DUT ssh: %s@%s" % (self.test_params['dut_username'], self.test_params['dut_hostname'])) self.log("DUT reboot limit in seconds: %s" % self.limit) self.log("DUT mac address: %s" % self.dut_mac) @@ -1004,7 +1010,7 @@ def reboot_dut(self): time.sleep(self.reboot_delay) self.log("Rebooting remote side") - stdout, stderr, return_code = self.cmd(["ssh", "-oStrictHostKeyChecking=no", self.dut_ssh, "sudo " + self.reboot_type]) + stdout, stderr, return_code = self.dut_connection.execCommand("sudo " + self.reboot_type) if stdout != []: self.log("stdout from %s: %s" % (self.reboot_type, str(stdout))) if stderr != []: diff --git a/ansible/roles/test/files/ptftests/device_connection.py b/ansible/roles/test/files/ptftests/device_connection.py new file mode 100644 index 0000000000..a29ea493b0 --- /dev/null +++ b/ansible/roles/test/files/ptftests/device_connection.py @@ -0,0 +1,63 @@ +import paramiko +import logging +from paramiko.ssh_exception import BadHostKeyException, AuthenticationException, SSHException + +logger = logging.getLogger(__name__) + +DEFAULT_CMD_EXECUTION_TIMEOUT_SEC = 10 + +class DeviceConnection: + ''' + DeviceConnection uses Paramiko module to connect to devices + + Paramiko module uses fallback mechanism where it would first try to use + ssh key and that fails, it will attempt username/password combination + ''' + def __init__(self, hostname, username, password=None): + ''' + Class constructor + + @param hostname: hostname of device to connect to + @param username: username for device connection + @param password: password for device connection + ''' + self.hostname = hostname + self.username = username + self.password = password + + def execCommand(self, cmd, timeout=DEFAULT_CMD_EXECUTION_TIMEOUT_SEC): + ''' + Executes command on remote device + + @param cmd: command to be run on remote device + @param timeout: timeout for command run session + @return: stdout, stderr, value + stdout is a list of lines of the remote stdout gathered during command execution + stderr is a list of lines of the remote stderr gathered during command execution + value: 0 if command execution raised no exception + nonzero if exception is raised + ''' + client = paramiko.SSHClient() + client.set_missing_host_key_policy(paramiko.AutoAddPolicy()) + + if isinstance(cmd, list): + cmd = ' '.join(cmd) + + stdOut = stdErr = [] + retValue = 1 + try: + client.connect(self.hostname, username=self.username, password=self.password, allow_agent=False) + si, so, se = client.exec_command(cmd, timeout=timeout) + stdOut = so.readlines() + stdErr = se.readlines() + retValue = 0 + except SSHException as sshException: + logger.error('SSH Command failed with message: %s' % sshException) + except AuthenticationException as authenticationException: + logger.error('SSH Authentiaction failure with message: %s' % authenticationException) + except BadHostKeyException as badHostKeyException: + logger.error('SSH Authentiaction failure with message: %s' % badHostKeyException) + finally: + client.close() + + return stdOut, stdErr, retValue diff --git a/ansible/roles/test/files/ptftests/sad_path.py b/ansible/roles/test/files/ptftests/sad_path.py index 8fcb5b7db5..85e61d20e5 100644 --- a/ansible/roles/test/files/ptftests/sad_path.py +++ b/ansible/roles/test/files/ptftests/sad_path.py @@ -1,25 +1,24 @@ import datetime import ipaddress import re -import subprocess import time from arista import Arista +from device_connection import DeviceConnection class SadTest(object): - def __init__(self, oper_type, vm_list, portchannel_ports, vm_dut_map, test_args, dut_ssh, vlan_ports): + def __init__(self, oper_type, vm_list, portchannel_ports, vm_dut_map, test_args, vlan_ports): self.oper_type = oper_type self.vm_list = vm_list self.portchannel_ports = portchannel_ports self.vm_dut_map = vm_dut_map self.test_args = test_args - self.dut_ssh = dut_ssh self.vlan_ports = vlan_ports self.fails_vm = set() self.fails_dut = set() self.log = [] - self.shandle = SadOper(self.oper_type, self.vm_list, self.portchannel_ports, self.vm_dut_map, self.test_args, self.dut_ssh, self.vlan_ports) + self.shandle = SadOper(self.oper_type, self.vm_list, self.portchannel_ports, self.vm_dut_map, self.test_args, self.vlan_ports) def setup(self): self.shandle.sad_setup(is_up=False) @@ -55,6 +54,7 @@ def __init__(self, oper_type, vm_list, portchannel_ports, vm_dut_map, test_args, self.portchannel_ports = portchannel_ports self.vm_dut_map = vm_dut_map self.test_args = test_args + self.dut_connection = DeviceConnection(test_args['dut_hostname'], test_args['dut_username'], password=test_args['dut_password']) self.vlan_ports = vlan_ports self.vlan_if_port = self.test_args['vlan_if_port'] self.neigh_vms = [] @@ -97,16 +97,6 @@ def extract_oper_info(self, oper_type): else: self.oper_type = oper_type - def cmd(self, cmds): - process = subprocess.Popen(cmds, - shell=False, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - stdout, stderr = process.communicate() - return_code = process.returncode - - return stdout, stderr, return_code - def select_vm(self): self.vm_list.sort() vm_len = len(self.vm_list) @@ -203,9 +193,8 @@ def retreive_logs(self): class SadOper(SadPath): - def __init__(self, oper_type, vm_list, portchannel_ports, vm_dut_map, test_args, dut_ssh, vlan_ports): + def __init__(self, oper_type, vm_list, portchannel_ports, vm_dut_map, test_args, vlan_ports): super(SadOper, self).__init__(oper_type, vm_list, portchannel_ports, vm_dut_map, test_args, vlan_ports) - self.dut_ssh = dut_ssh self.dut_needed = dict() self.lag_members_down = dict() self.neigh_lag_members_down = dict() @@ -335,7 +324,7 @@ def get_bgp_route_cnt(self, is_up=True, v4=True): else: cmd = 'show ipv6 bgp summary | sed \'1,/Neighbor/d;/^$/,$d\' | sed \'s/\s\s*/ /g\' | cut -d\' \' -f 1,10' - stdout, stderr, return_code = self.cmd(['ssh', '-oStrictHostKeyChecking=no', self.dut_ssh, cmd]) + stdout, stderr, return_code = self.dut_connection.execCommand(cmd) if return_code != 0: self.fails['dut'].add('%s: Failed to retreive BGP route info from DUT' % self.msg_prefix[1 - is_up]) self.fails['dut'].add('%s: Return code: %d' % (self.msg_prefix[1 - is_up], return_code)) @@ -345,15 +334,15 @@ def get_bgp_route_cnt(self, is_up=True, v4=True): def build_neigh_rt_map(self, neigh_rt_info): # construct neigh to route cnt map self.neigh_rt_map = dict() - for line in neigh_rt_info.strip().split('\n'): - key, value = line.split(' ') + for line in neigh_rt_info: + key, value = line.strip().split(' ') self.neigh_rt_map.update({key:value}) def verify_route_cnt(self, rt_incr, is_up=True, v4=True): neigh_rt_info, ret = self.get_bgp_route_cnt(is_up=is_up, v4=v4) if not ret: - for line in neigh_rt_info.strip().split('\n'): - neigh_ip, rt_cnt = line.split(' ') + for line in neigh_rt_info: + neigh_ip, rt_cnt = line.strip().split(' ') exp_cnt = int(self.neigh_rt_map[neigh_ip]) + rt_incr if int(rt_cnt) != exp_cnt: self.fails['dut'].add('%s: Route cnt incorrect for neighbor %s Expected: %d Obtained: %d' % (self.msg_prefix[is_up], neigh_ip, exp_cnt, int(rt_cnt))) @@ -386,7 +375,7 @@ def change_vlan_port_state(self, is_up=True): for intf, port in self.down_vlan_info: if not re.match('Ethernet\d+', intf): continue self.log.append('Changing state of %s from DUT side to %s' % (intf, state[is_up])) - stdout, stderr, return_code = self.cmd(['ssh', '-oStrictHostKeyChecking=no', self.dut_ssh, 'sudo config interface %s %s' % (state[is_up], intf)]) + stdout, stderr, return_code = self.dut_connection.execCommand('sudo config interface %s %s' % (state[is_up], intf)) if return_code != 0: self.fails['dut'].add('%s: State change not successful from DUT side for %s' % (self.msg_prefix[1 - is_up], intf)) self.fails['dut'].add('%s: Return code: %d' % (self.msg_prefix[1 - is_up], return_code)) @@ -400,9 +389,9 @@ def verify_vlan_port_state(self, state='down', pre_check=True): # extract the admin status pat = re.compile('(\S+\s+){7}%s' % state) for intf, port in self.down_vlan_info: - stdout, stderr, return_code = self.cmd(['ssh', '-oStrictHostKeyChecking=no', self.dut_ssh, 'show interfaces status %s' % intf]) + stdout, stderr, return_code = self.dut_connection.execCommand('show interfaces status %s' % intf) if return_code == 0: - for line in stdout.split('\n'): + for line in stdout: if intf in line: is_match = pat.match(line.strip()) if is_match: @@ -426,7 +415,7 @@ def change_bgp_dut_state(self, is_up=True): continue self.log.append('Changing state of BGP peer %s from DUT side to %s' % (self.neigh_bgps[vm][key], state[is_up])) - stdout, stderr, return_code = self.cmd(['ssh', '-oStrictHostKeyChecking=no', self.dut_ssh, 'sudo config bgp %s neighbor %s' % (state[is_up], self.neigh_bgps[vm][key])]) + stdout, stderr, return_code = self.dut_connection.execCommand('sudo config bgp %s neighbor %s' % (state[is_up], self.neigh_bgps[vm][key])) if return_code != 0: self.fails['dut'].add('State change not successful from DUT side for peer %s' % self.neigh_bgps[vm][key]) self.fails['dut'].add('Return code: %d' % return_code) @@ -442,9 +431,9 @@ def verify_bgp_dut_state(self, state='Idle'): if key not in ['v4', 'v6']: continue self.log.append('Verifying if the DUT side BGP peer %s is %s' % (self.neigh_bgps[vm][key], states)) - stdout, stderr, return_code = self.cmd(['ssh', '-oStrictHostKeyChecking=no', self.dut_ssh, 'show ip bgp neighbor %s' % self.neigh_bgps[vm][key]]) + stdout, stderr, return_code = self.dut_connection.execCommand('show ip bgp neighbor %s' % self.neigh_bgps[vm][key]) if return_code == 0: - for line in stdout.split('\n'): + for line in stdout: if 'BGP state' in line: curr_state = re.findall('BGP state = (\w+)', line)[0] bgp_state[vm][key] = (curr_state in states) @@ -507,7 +496,7 @@ def change_dut_lag_state(self, is_up=True): for intf in down_intfs: if not re.match('(PortChannel|Ethernet)\d+', intf): continue self.log.append('Changing state of %s from DUT side to %s' % (intf, state[is_up])) - stdout, stderr, return_code = self.cmd(['ssh', '-oStrictHostKeyChecking=no', self.dut_ssh, 'sudo config interface %s %s' % (state[is_up], intf)]) + stdout, stderr, return_code = self.dut_connection.execCommand('sudo config interface %s %s' % (state[is_up], intf)) if return_code != 0: self.fails['dut'].add('%s: State change not successful from DUT side for %s' % (self.msg_prefix[1 - is_up], intf)) self.fails['dut'].add('%s: Return code: %d' % (self.msg_prefix[1 - is_up], return_code)) @@ -549,9 +538,9 @@ def verify_dut_lag_state(self, pre_check=True): po_list.append(po_name) self.po_neigh_map[po_name] = self.neigh_names[vm] - stdout, stderr, return_code = self.cmd(['ssh', '-oStrictHostKeyChecking=no', self.dut_ssh, 'show interfaces portchannel']) + stdout, stderr, return_code = self.dut_connection.execCommand('show interfaces portchannel') if return_code == 0: - for line in stdout.split('\n'): + for line in stdout: for po_name in po_list: if po_name in line: is_match = pat.match(line) diff --git a/ansible/roles/test/tasks/ptf_runner_reboot.yml b/ansible/roles/test/tasks/ptf_runner_reboot.yml index 3111e8cbed..ba809ad1ee 100644 --- a/ansible/roles/test/tasks/ptf_runner_reboot.yml +++ b/ansible/roles/test/tasks/ptf_runner_reboot.yml @@ -51,7 +51,8 @@ ptf_qlen: 1000 ptf_test_params: - verbose=False - - dut_username=\"{{ ansible_ssh_user }}\" + - dut_username=\"{{ sonicadmin_user }}\" + - dut_password=\"{{ sonicadmin_password }}\" - dut_hostname=\"{{ ansible_host }}\" - reboot_limit_in_seconds={{ reboot_limit }} - reboot_type=\"{{ reboot_type }}\" diff --git a/tests/common/fixtures/advanced_reboot.py b/tests/common/fixtures/advanced_reboot.py index 0ad9b8646b..b108e5255c 100644 --- a/tests/common/fixtures/advanced_reboot.py +++ b/tests/common/fixtures/advanced_reboot.py @@ -115,9 +115,14 @@ def __buildTestbedData(self): self.rebootData['dut_hostname'] = self.mgFacts['minigraph_mgmt_interface']['addr'] self.rebootData['dut_mac'] = hostFacts['ansible_Ethernet0']['macaddress'] - self.rebootData['dut_username'] = hostFacts['ansible_env']['SUDO_USER'] self.rebootData['vlan_ip_range'] = self.mgFacts['minigraph_vlan_interfaces'][0]['subnet'] self.rebootData['dut_vlan_ip'] = self.mgFacts['minigraph_vlan_interfaces'][0]['addr'] + + invetory = self.duthost.host.options['inventory'].split('/')[-1] + secrets = self.duthost.host.options['variable_manager']._hostvars[self.duthost.hostname]['secret_group_vars'] + self.rebootData['dut_username'] = secrets[invetory]['sonicadmin_user'] + self.rebootData['dut_password'] = secrets[invetory]['sonicadmin_password'] + self.rebootData['default_ip_range'] = str( ipaddress.ip_interface(self.mgFacts['minigraph_vlan_interfaces'][0]['addr'] + '/16').network ) @@ -223,13 +228,24 @@ def __prepareTestbedSshKeys(self, dutUsername, dutIp): self.ptfhost.shell('ssh-keygen -f /root/.ssh/known_hosts -R ' + dutIp) logger.info('Generate public key for ptf host') - self.ptfhost.shell('ssh-keygen -b 2048 -t rsa -f /root/.ssh/id_rsa -q -N ""') - result = self.ptfhost.shell('cat /root/.ssh/id_rsa.pub') + self.ptfhost.file(path='/root/.ssh/', mode='u+rwx,g-rwx,o-rwx', state='directory') + result = self.ptfhost.openssh_keypair( + path='/root/.ssh/id_rsa', + size=2048, + force=True, + type='rsa', + mode='u=rw,g=,o=' + ) + # There is an error with id_rsa.pub access permissions documented in: + # https://github.com/ansible/ansible/issues/61411 + # @TODO: remove the following line when upgrading to Ansible 2.9x + self.ptfhost.file(path='/root/.ssh/id_rsa.pub', mode='u=rw,g=,o=') + cmd = ''' mkdir -p /home/{0}/.ssh && echo "{1}" >> /home/{0}/.ssh/authorized_keys && chown -R {0}:{0} /home/{0}/.ssh/ - '''.format(dutUsername, result['stdout']) + '''.format(dutUsername, result['public_key']) self.duthost.shell(cmd) def __handleMellanoxDut(self): @@ -423,6 +439,7 @@ def __runPtfRunner(self, rebootOper=None): platform="remote", params={ "dut_username" : self.rebootData['dut_username'], + "dut_password" : self.rebootData['dut_password'], "dut_hostname" : self.rebootData['dut_hostname'], "reboot_limit_in_seconds" : self.rebootLimit, "reboot_type" :self.rebootType, @@ -452,15 +469,17 @@ def __restorePrevImage(self): ''' Resotre previous image and reboot DUT ''' - logger.info('Restore current image') - self.duthost.shell('sonic_installer set_default {0}'.format(self.currentImage)) - - rebootDut( - self.duthost, - self.localhost, - reboot_type=self.rebootType.replace('-reboot', ''), - wait = 180 + self.readyTimeout - ) + currentImage = self.duthost.shell('sonic_installer list | grep Current | cut -f2 -d " "')['stdout'] + if currentImage != self.currentImage: + logger.info('Restore current image') + self.duthost.shell('sonic_installer set_default {0}'.format(self.currentImage)) + + rebootDut( + self.duthost, + self.localhost, + reboot_type=self.rebootType.replace('-reboot', ''), + wait = self.readyTimeout + ) def tearDown(self): '''