[202305] Fix LAG downtime calculation with SONiC neighbors (sonic-net#11243)

* 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 <sarcot@microsoft.com>

Author: saiarcot895

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

@@ -1371,14 +1371,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:
@@ -1579,7 +1583,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 = stdout.read()
+                stderrOutput = stderr.read()
+                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))
@@ -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()))