Skip to content

[upgrade_path]Bug fix in test_upgrade_path#2165

Closed
bingwang-ms wants to merge 3 commits intosonic-net:masterfrom
bingwang-ms:upgrade_path_test
Closed

[upgrade_path]Bug fix in test_upgrade_path#2165
bingwang-ms wants to merge 3 commits intosonic-net:masterfrom
bingwang-ms:upgrade_path_test

Conversation

@bingwang-ms
Copy link
Collaborator

Description of PR

Summary:
Fixes # (issue)
This PR is to fix several bug in test_upgrade_path.

  1. Fix bug in check_sonic_version_after_reboot;
  2. Add filter for ping_dataplane to cut down CPU usage (99.9% -> 40%);
  3. Refactor code in test_upgrade_path;
  4. Use creds to retrive ssh_user and password.

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

What is the motivation for this PR?

This PR is to fix several bug in test_upgrade_path.

How did you do it?

Fix several bugs.

  1. Fix bug in check_sonic_version_after_reboot
    The original position of check_sonic_version_after_reboot is too closer to reboot, as a result, it might get a incorrect version. This commit moved it to the end of test.
  2. Add filter for ping_dataplane to cut down CPU usage (99.9% -> 40%)
    A filter is applied before ping_dataplane to cut down CPU usage. At the very beginning of test, flooding ARP packets are captured and analyzed by ptf, which cousume lots of CPU. The filter only allow packets on port 5000 in, and the CPU usage is cut down from 99.9% to up to 40%. At the same time, the packet queue is also increase from 1000 to 10000 because the queue is full according debug log.
  3. Refactor code in test_upgrade_path
    Use fixture to handle ptf scripts.
  4. Use creds to retrive ssh_user and password

How did you verify/test it?

Verified on Arista-7260cx3

py.test upgrade_path/test_upgrade_path.py --inventory ../ansible/str,../ansible/veos --host-pattern str-7260cx3-acs-1 --module-path ../ansible/library/ --testbed vms7-t0-7260-1 --testbed_file ../ansible/testbed.csv --junit-xml=tr.xml --log-cli-level warn --base_image_list=http://100.127.20.23/installer/sonic/broadcom/internal-201811/sonic-aboot-broadcom-20181130.51.swi --target_image_list=http://100.127.20.23/installer/sonic/broadcom/internal-201811/sonic-aboot-broadcom.swi --restore_to_image=http://100.127.20.23/installer/sonic/broadcom/internal-201911/sonic-aboot-broadcom.swi
========================================================================================= test session starts =========================================================================================
platform linux2 -- Python 2.7.12, pytest-4.6.5, py-1.8.1, pluggy-0.13.1 -- /usr/bin/python
cachedir: .pytest_cache
ansible: 2.8.7
rootdir: /data/Networking-acs-sonic-mgmt/tests, inifile: pytest.ini
plugins: forked-1.1.3, xdist-1.28.0, ansible-2.2.2, repeat-0.8.0
collected 1 item                                                                                                                                                                                      

upgrade_path/test_upgrade_path.py::test_upgrade_path ^@^@^H^@^@PASSED                                                                                                                                     [100%]

------------------------------------------------------------------ generated xml file: /data/Networking-acs-sonic-mgmt/tests/tr.xml -------------------------------------------------------------------
===================================================================================== 1 passed in 1507.58 seconds =====================================================================================

Any platform specific information?

No.

Supported testbed topology if it's a new test case?

N/A

Documentation

No.

1. Fix bug in check_sonic_version_after_reboot
2. Add filter for ping_dataplane to cut down CPU usage (99.9% -> 40%)
3. Refactor code in test_upgrade_path
@lgtm-com
Copy link

lgtm-com bot commented Sep 1, 2020

This pull request introduces 1 alert when merging 1bcb7a3 into 528fb67 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@bingwang-ms
Copy link
Collaborator Author

bingwang-ms commented Sep 2, 2020

retest vsimage please

Sanitycheck and LogAnalyzer are disabled for upgrade_path because
1. The cold reboot at the beginning of test probably recover DUT;
2. Cold-reboot and warm-reboot during test may produce unexpected warning log.
from datetime import datetime

from tests.common.fixtures.ptfhost_utils import copy_ptftests_directory # lgtm[py/unused-import]
from tests.common.fixtures.ptfhost_utils import change_mac_addresses # lgtm[py/unused-import]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This required as all interface on new ptfdocker would have the same mac address.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'll recover it back.

@bingwang-ms
Copy link
Collaborator Author

Add and remove filter from fd frequently introduces a noticable delay. So I close this PR until fix it.

ZhaohuiS pushed a commit that referenced this pull request Jul 7, 2022
What is the motivation for this PR?
Update sub_port_interfaces TC according to changes from #2165 PR of sonic-swss

How did you do it?
Added function for removing ACL table when ingress/egress ACL binding is configured on LAG members.

Signed-off-by: Oleksandr Kozodoi <oleksandrx.kozodoi@intel.com>
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
Including change:

* 7ff8f75 2022-06-03 | Revert "[portsorch]: Prevent LAG member configuration when port has active ACL binding (sonic-net#2165)" (sonic-net#2306) (HEAD -> 202205, github/202205) [bingwang-ms]

Signed-off-by: Ying Xie <ying.xie@microsoft.com>
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
13ec600 [generic-config-updater] Add NTP validator (sonic-net#2212)
4fc09b1 [GCU] Handling non-compliant leaf-list with string values (sonic-net#2174)
ac89489 Modify override testcase to cover PORT admin_status (sonic-net#2165)
d7953d2 [GCU] Validate peer_group_range ip_range are correct (sonic-net#2145)
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
5bb99c7 Validate LAG has members before mirror session create (sonic-net#2130)
ec6c8af [vxlan] Remove tunnel map objects on VNET tunnel removal (sonic-net#2150)
7e7db19 [BFD]Registering BFD state change callback during session creation (sonic-net#2202)
618fe07 [VNET]Fixing nexthop group delete during route change (sonic-net#2198)
91b66df [portsorch]: Prevent LAG member configuration when port has active ACL binding (sonic-net#2165)
29de9d0 Remove redundant and problematic code to skip "pool" field in buffer profile handling (sonic-net#2197)
ded0b45 [PBH] Implement Edit Flows (sonic-net#2169)
2ee0f49 [neighsyncd] increase neighsyncd timeout (sonic-net#2209)
a0160c0 [QosOrch] The notifications cannot be drained in QosOrch in case the first one needs to retry (sonic-net#2206)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants