[advanced reboot] Add Paramiko module for device connection

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 != []:

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

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)

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 }}\"

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):
         '''