Fix Python 3 syntax errors in sonic.py (sonic-net#10107)

* Fix Python 3 syntax errors in sonic.py

This file is used with the advanced-reboot test case with SONiC
neighbors.

Signed-off-by: Saikrishna Arcot <[email protected]>

* Raise exceptions from the VM threads as failures in warm-reboot

If there's some exception that happens from the VM thread, raise this as
a failure in the overall warm reboot. This has two purposes: to make any
failures more clearer/easier to find when debugging, and to make sure
that the test fails if something goes wrong there, even if the warm
reboot itself succeeded.

Signed-off-by: Saikrishna Arcot <[email protected]>

* Remove whitespace

Signed-off-by: Saikrishna Arcot <[email protected]>

* Log output on stderr as well

Signed-off-by: Saikrishna Arcot <[email protected]>

* For SONiC neighbors, in neigh_lag_status_check, use existing info in cli_info

In the neigh_lag_status_check function, when the neighbor device is
checked to see if there were any LAG flaps, instead of trying to get
this information from the neighbor VM, just use the existing info we
have in self.cli_info already. Otherwise, LAG flaps that may have
happened prior to the test starting would show up here.

Signed-off-by: Saikrishna Arcot <[email protected]>

* Read stdout first, before reading stderr, since the buffer might be full

Signed-off-by: Saikrishna Arcot <[email protected]>

---------

Signed-off-by: Saikrishna Arcot <[email protected]>

ansible/roles/test/files/ptftests/py3/advanced-reboot.py

@@ -1385,14 +1385,18 @@ def neigh_lag_status_check(self):
         Ensure there are no interface flaps after warm-boot
         """
         for neigh in self.ssh_targets:
-            self.test_params['port_channel_intf_idx'] = [x['ptf_ports'][0] for x in self.vm_dut_map.values()
-                                                         if x['mgmt_addr'] == neigh]
-            self.neigh_handle = HostDevice.getHostDeviceInstance(self.test_params['neighbor_type'], neigh,
-                                                                 None, self.test_params)
-            self.neigh_handle.connect()
-            fails, flap_cnt = self.neigh_handle.verify_neigh_lag_no_flap()
-            self.neigh_handle.disconnect()
-            self.fails[neigh] |= fails
+            flap_cnt = None
+            if self.test_params['neighbor_type'] == "sonic":
+                flap_cnt = self.cli_info[neigh][1]
+            else:
+                self.test_params['port_channel_intf_idx'] = [x['ptf_ports'][0] for x in self.vm_dut_map.values()
+                                                             if x['mgmt_addr'] == neigh]
+                self.neigh_handle = HostDevice.getHostDeviceInstance(self.test_params['neighbor_type'], neigh,
+                                                                     None, self.test_params)
+                self.neigh_handle.connect()
+                fails, flap_cnt = self.neigh_handle.verify_neigh_lag_no_flap()
+                self.neigh_handle.disconnect()
+                self.fails[neigh] |= fails
             if not flap_cnt:
                 self.log("No LAG flaps seen on %s after warm boot" % neigh)
             else:
@@ -1593,7 +1597,24 @@ def peer_state_check(self, ip, queue):
                                                      if x['mgmt_addr'] == ip]
         ssh = HostDevice.getHostDeviceInstance(self.test_params['neighbor_type'], ip, queue,
                                                self.test_params, log_cb=self.log)
-        self.fails[ip], self.info[ip], self.cli_info[ip], self.logs_info[ip], self.lacp_pdu_times[ip] = ssh.run()
+        try:
+            self.fails[ip], self.info[ip], self.cli_info[ip], self.logs_info[ip], self.lacp_pdu_times[ip] = ssh.run()
+        except Exception:
+            traceback_msg = traceback.format_exc()
+            self.log("Error in HostDevice: {}".format(traceback_msg))
+            self.fails[ip] = set()
+            self.fails[ip].add("HostDevice hit an exception")
+            self.info[ip] = set()
+            self.cli_info[ip] = {
+                    "lacp": [0, 0],
+                    "po": [0, 0],
+                    "bgp_v4": [0, 0],
+                    "bgp_v6": [0, 0],
+                    }
+            self.logs_info[ip] = {}
+            self.lacp_pdu_times[ip] = {
+                    "lacp_all": []
+                    }
         self.log('SSH thread for VM {} finished'.format(ip))
 
         lacp_pdu_times = self.lacp_pdu_times[ip]

ansible/roles/test/files/ptftests/sonic.py

@@ -61,7 +61,19 @@ def do_cmd(self, cmd):
             attempts += 1
             try:
                 stdin, stdout, stderr = self.conn.exec_command(cmd, timeout=Sonic.SSH_CMD_TIMEOUT)
-                return stdout.read()
+                # Warning: This is a bit fragile. Both stdout and stderr use
+                # the same SSH channel, and if the buffer for that channel is
+                # full, then either stdout or stderr will block until there's
+                # space in the channel.
+                #
+                # Therefore, fully read stdout first before trying to read
+                # stderr. This assumes there's plenty of data on stdout to read,
+                # but not much on stderr.
+                stdoutOutput = six.ensure_str(stdout.read())
+                stderrOutput = six.ensure_str(stderr.read()).strip()
+                if len(stderrOutput) > 0:
+                    self.log("Output on stderr from command '{}': '{}'".format(cmd, stderrOutput))
+                return stdoutOutput
             except socket.timeout:
                 self.log("Timeout when running command: {}".format(cmd))
                 return ""
@@ -175,12 +187,12 @@ def run(self):
         self.disconnect()
 
         # save data for troubleshooting
-        with open("/tmp/%s.data.pickle" % self.ip, "w") as fp:
+        with open("/tmp/%s.data.pickle" % self.ip, "wb") as fp:
             pickle.dump(data, fp)
 
         # save debug data for troubleshooting
         if self.DEBUG:
-            with open("/tmp/%s.raw.pickle" % self.ip, "w") as fp:
+            with open("/tmp/%s.raw.pickle" % self.ip, "wb") as fp:
                 pickle.dump(debug_data, fp)
             with open("/tmp/%s.logging" % self.ip, "w") as fp:
                 fp.write("\n".join(log_lines))
@@ -309,7 +321,7 @@ def check_lag_flaps(self, interface, log_lines, start_time):
         return 0, num_lag_flaps
 
     def parse_lacp(self, output):
-        return six.ensure_str(output).find('Bundled') != -1
+        return output.find('Bundled') != -1
 
     def parse_bgp_neighbor_once(self, output):
         is_gr_ipv4_enabled = False
@@ -377,7 +389,7 @@ def parse_bgp_route(self, output, expects):
         return set(expects).issubset(prefixes)
 
     def parse_supported_show_lacp_command(self):
-        show_lacp_command = "show lacp neighbor"
+        show_lacp_command = "show interface portchannel"
         self.log("show lacp command is '{}'".format(show_lacp_command))
         return show_lacp_command
 
@@ -481,6 +493,8 @@ def verify_neigh_lag_no_flap(self):
         # Note: this function may have false-positives (with regards to link flaps). The start time used here is
         # the system's boot time, not the test start time, which means any LAG flaps before the start of the test
         # would get included here.
+        #
+        # You probably really don't want to call this function.
         log_lines = self.do_cmd("sudo cat /var/log/teamd.log{,.1}").split('\n')
         boot_time = datetime.datetime.strptime(self.do_cmd("uptime -s").strip(), "%Y-%m-%d %H:%M:%S")
         _, flap_cnt = self.check_lag_flaps("PortChannel1", log_lines, time.mktime(boot_time.timetuple()))