tests requiring interfaces to be operationally fail after running test_po_cleanup in regression runs

[sanmalho-git]

Description

PR #4888 introduced the call to 'duthost.critical_services_fully_started' which checks if all the critical services dockers are in running state.

Since, test_po_cleanup restarts the swss docker (which restarts syncd docker as well), it does not wait long enough for syncd to fully initialize (all ports are operationally up).

Thus, in regression runs, if tests are executing that require all ports to be operationally up (like test_interfaces sanity check) fail.

Steps to reproduce the issue:

1. Run test_po_cleanup in test_po_cleanup.py.
2. As soon as the test is done, monitor 'show interface status' - and ports won't show up.
3. After a while (2 mins or so for Nokia's linecards), 'show interface status' would have the ports.

Describe the results you received:

Describe the results you expected:
The test should check if syncd is fully initialized at the end.

Additional information you deem important:

**Output of `show version`:**

```
(paste your output here)
```

**Attach debug file `sudo generate_dump`:**

```
(paste your output here)
```

[sanmalho-git]

@AntonHryshchuk @judyjoseph - could you please check into this issue.

[wenyiz2021]

Updated PR: #5453 to address this issue.
I changed timeout to 800sec for my dut to pass.

[sanmalho-git]

@wenyiz2021 - changing the time will not fix the issue #5453. We are using wait_until (which will poll) until all critical services are up (critical processes are in running state). At this point the wait_until will return. However, after the processes like syncd are started, on some DUTs it takes couple of minutes h/w to be initialized, after which poirts become operationally up - that is the check that is missing.

[wenyiz2021]

@sanmalho-git I don't understand what check is missing?
original issue was from #4887 that "The existed test did the restart of swss docker and in the end, did cleanup by config reload, where the was a delay for bringup all critical processes." Thus a wait time for 300 sec was added before exiting test_po_cleanup.
However, in my case it needs more time for swss/teamd to come back on some asics for MASIC devices, thus changing timeout to 800 helped.
Normally 800 sec, which are 10+min/asic are long enough for containers to establish as I think.

[sanmalho-git]

The issue I observed is when running test_po_cleanup.py on a multi-asic DUT. Lets assume that we have 2 asics, then the order sequence of test execution is:

pc/test_po_cleanup.py::test_po_cleanup[ixre-egl-board1-0]
pc/test_po_cleanup.py::test_po_cleanup_after_reload[ixre-egl-board1]
pc/test_po_cleanup.py::test_po_cleanup[ixre-egl-board1-1]

The test_po_cleanup_after_reload is not run with asic1 as the test is not parameterized with 'enum_asic_index', similar to test_po_cleanup. When the test_po_cleanup.py is done, that last test run is test_po_cleanup on asic1 - which does swss1 restart, and waits only for critical processes in swss1 to start. However, finishing the initialization of the h/w after the swss1 critical processes have started takes time (in our case we are dealing with a J2C+ chip, which takes a couple of minutes to initialize - for 'show interface status' to even show the ports) - which is why the next test in the regression run fails.

The check required could be checking all interfaces that are defined in the conn_graph are operationally up, or checking bgp peers are established.

One other option would be to add 'enum_asic_index' as a parameter to test_po_cleanup_after_reload as well, which would resolve the issue that I was seeing - as config reload would take care of it.

[wenyiz2021]

The check required could be checking all interfaces that are defined in the conn_graph are operationally up, or checking bgp peers are established.
-- Think the current wait_until for critical services is doing that, it makes sure all docker processes are running, if any failed, either bgp neighbors, or interfaces will be failing too. Changing 300->800 also worked because it is long enough to "finish the initialization of the h/w after the swss1 critical processes have started"

One other option would be to add 'enum_asic_index' as a parameter to test_po_cleanup_after_reload as well, which would resolve the issue that I was seeing - as config reload would take care of it.

-- Think you are right. I observed the same for a 6 asic device: test_po_cleanup_after_reload was done during test_po_cleanup per asic test -- after test_po_cleanup of asic0 but before asic1.
Maybe better to do:
test_po_cleanup for asic0 -> test_po_cleanup_after_reload for asic0 -> test_po_cleanup for asic1 -> test_po_cleanup_after_reload for asic1 etc..

[sanmalho-git]

SonicHost.critical_services_fully_started calls critical_services_status, which does a
docker ps --filter status=running --format {{.Names}}
That docker status will be 'running' as soon that it is up. As an example, some of syslog messages on our DUT:

Apr  1 13:28:35.940361 ixre-egl-board27 NOTICE root: Started swss0 service...  
Apr  1 13:28:38.034832 ixre-egl-board27 NOTICE root: Started syncd0 service...
Apr  1 13:29:39.310345 ixre-egl-board27 INFO syncd0#syncd: [06:00.0] SAI_API_SWITCH:brcm_sai_dnx_create_switch:6787 SAI Timing: SDK init time 56 seconds

In the above example, at '13:28:35.940361', docker status for swss0 would be 'running'. But SDK init took 56 seconds, which is when the ports would show up in 'show interface status'

[wenyiz2021]

I have merged: #5453
Please check if that helps

[wenyiz2021]

Added config_reload at end of test_po_cleanup.py, also let this test base on duthost, not on asics.
with the config_reload, all interfaces and ports should be good. Please double check. thanks!