SAI validation #15324

opcoder0 · 2024-11-01T06:53:54Z

Description of PR

Introduces a new method to access SONiC Redis database instance using redis-py native Python library instead of relying on sonic-db-cli.
It also uses the Redis' pub-sub infrastructure to listen to key-space notifications to monitor DB changes instead of using the traditional wait_until which performs sleep->check cycle.
Adds changes to perform SAI validation for ACL tests.
Uses this library to monitor STATE_DB changes using the new notification library in test_bfd.py::test_bfd_basic.

Summary:
Fixes # Not applicable

Type of change

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

Back port request

Approach

What is the motivation for this PR?

Motivation is to add a simple and easy to use library that allows tests (where applicable) to perform SAI validation and allow tests to use event notification-based method to monitor DB changes.

How did you do it?

Refer to description above.

How did you verify/test it?

Verified ACL tests (on t0) and BFD basic tests (on t1) testbeds.

Any platform specific information?

No

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

NA

Documentation

TBD.

- Key changes detected

of N unique keys while waiting

wangxin · 2024-11-27T01:52:13Z

Is there a way to avoid committing a deb package into this repo? I have 2 concerns:

The repo should be used for source code only. Try not to commit any package that is publicly available to the repo unless there is a very very good reason.
The package version is kind of fixed. Extra effort would be required to upgrade version of the packages.

opcoder0 · 2024-11-27T14:47:28Z

Is there a way to avoid committing a deb package into this repo? I have 2 concerns:

The repo should be used for source code only. Try not to commit any package that is publicly available to the repo unless there is a very very good reason.

The package version is kind of fixed. Extra effort would be required to upgrade version of the packages.

Regarding (1.) As the package will need to be installed on the DUT which most likely won't have Internet access. And I am not sure if it is okay to assume the server (in MS / community test environments) on which the tests run have Internet access either. Hence, I checked them into the repository. I see similar practice followed for some files specific to Mellanox -

./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/sx-complib_1.mlnx.4.5.4414_amd64.deb
./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/sx-gen-utils_1.mlnx.4.5.4414_amd64.deb
./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/libnl-3-200_3.5.0-1_amd64.deb
./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/python-sdk-api_1.mlnx.4.5.4414_amd64.deb
./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/sxd-libs_1.mlnx.4.5.4414_amd64.deb
./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/applibs_1.mlnx.4.5.4414_amd64.deb

Regarding (2.) The two versions of socat correspond to the SONiC base OS versions (bullseye and bookworm).

wangxin · 2024-12-06T02:28:17Z

Is there a way to avoid committing a deb package into this repo? I have 2 concerns:

The repo should be used for source code only. Try not to commit any package that is publicly available to the repo unless there is a very very good reason.

The package version is kind of fixed. Extra effort would be required to upgrade version of the packages.

Regarding (1.) As the package will need to be installed on the DUT which most likely won't have Internet access. And I am not sure if it is okay to assume the server (in MS / community test environments) on which the tests run have Internet access either. Hence, I checked them into the repository. I see similar practice followed for some files specific to Mellanox -
./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/sx-complib_1.mlnx.4.5.4414_amd64.deb
./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/sx-gen-utils_1.mlnx.4.5.4414_amd64.deb
./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/libnl-3-200_3.5.0-1_amd64.deb
./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/python-sdk-api_1.mlnx.4.5.4414_amd64.deb
./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/sxd-libs_1.mlnx.4.5.4414_amd64.deb
./ansible/roles/test/files/mlnx/docker-tests-pfcgen-asic/pkgs/applibs_1.mlnx.4.5.4414_amd64.deb
Regarding (2.) The two versions of socat correspond to the SONiC base OS versions (bullseye and bookworm).

Most likely these packages are no longer being used. And this is not a good practice anyway. I don't feel it is a good idea to go further on this path.
In the code base, we can assume that either DUTs have direct internet access or via http proxy.
Maybe we can also consider building this package into SONiC image. Then in tests, the code can firstly check if there is such tool. If not, then try to install it.

wangxin · 2024-12-09T02:08:42Z

tests/common/database is a new folder. Need to add a file __init__.py file under this folder because the parent folder is a python package.

wangxin · 2024-12-09T01:33:30Z

docs/sai_validation/sai_validation_design.md

+
+## Introduction
+
+SONiC management tests are Pytest modules running in the SONiC management container on the developer/CI/test environment and PTF tests running from the PTF container on the testbed server. As part of the setup and tear down activities the tests make configuration changes to SONiC, run the tests and verify if the tests ran successfully by making additional configuration checks and finally tear down the configuration changes. The tests use command line utilities on the DUT like `config`, `sonic-db-cli` or `redis-cli` to set and get configuration values. In some cases tests export / dump the contents of the database to examine its results and verify if tests ran successfully. These configuration changes are propogated to the ASIC through ASIC_DB. The aim of this design document is to identify a mechanism for tests to validate the configuration changes against ASIC_DB entries or SAI object types.


SONiC management tests -> SONiC tests in the sonic-mgmt repository?

wangxin · 2024-12-09T01:34:02Z

docs/sai_validation/sai_validation_design.md

+
+## Introduction
+
+SONiC management tests are Pytest modules running in the SONiC management container on the developer/CI/test environment and PTF tests running from the PTF container on the testbed server. As part of the setup and tear down activities the tests make configuration changes to SONiC, run the tests and verify if the tests ran successfully by making additional configuration checks and finally tear down the configuration changes. The tests use command line utilities on the DUT like `config`, `sonic-db-cli` or `redis-cli` to set and get configuration values. In some cases tests export / dump the contents of the database to examine its results and verify if tests ran successfully. These configuration changes are propogated to the ASIC through ASIC_DB. The aim of this design document is to identify a mechanism for tests to validate the configuration changes against ASIC_DB entries or SAI object types.


SONiC management container -> sonic-mgmt container.

wangxin · 2024-12-09T02:02:45Z

docs/sai_validation/sai_validation_design.md

+
+- **Using threading and polling** as implemented in [SWSS VS Tests](https://github.com/sonic-net/sonic-swss/blob/master/tests/README.md), [conftest](https://github.com/sonic-net/sonic-swss/blob/master/tests/conftest.py), [dvslib/dvs_database](https://github.com/sonic-net/sonic-swss/blob/master/tests/dvslib/dvs_database.py)
+
+- **Using Redis pubsub** This approach takes advantage of Redis pubsub to get notification of keyspace changes instead of using threading or polling to watch for key value changes. This mechanism improves performance and predictability of watching for keyspace changes. The tests don't have to rely on threads or sleep timers to check for value changes. The keyspace notifications are enabled by default for `ASIC_DB`. Keyspace notifications are enabled by default (`AKE`) in SONiC Redis instance. There are two kinds of message types received in pubsub `psubscribe` and `pmessage`.


In "Keyspace notifications are enabled by default (AKE) in SONiC Redis instance."

What does AKE mean?

AKE are flags which translate to "all the events except for new key events (n) and missed events (m)". More details are available here https://redis.io/docs/latest/develop/use/keyspace-notifications/#configuration. I have updated the design doc with this link too.

wangxin · 2024-12-09T02:11:16Z

tests/templates/socat-sonicdb.service.j2

+After=database.service
+
+[Service]
+ExecStart=/usr/bin/socat TCP4-LISTEN:6381,fork TCP:localhost:6379


The template has port 6381 hard coded. The added pytest option --exposed-db-port is not actually being used.

wangxin · 2024-12-09T03:21:38Z

tests/bfd/test_bfd.py

-
-        time.sleep(1)
+        # check all STATE_DB BFD_SESSION_TABLE neighbors' state is Up
+        status, actual_wait = wait_until_condition(q, prefix, neighbor_addrs,


If exception is raised in wait_until_condition, it would return None. Can this tuple assignment pass if wait_until_condition returns None? If not, then there is a problem here. And other wait_until_* tools have the similar problem.

Fixed. ThreadPoolExecutor.submit raises RuntimeError, TypeError and BrokenThreadPool all of these shouldn't be handled. The concurrent.futures.Future.result submit() can raise TimeoutError, CancelledError and Exception which have been handled by logging and returning appropriate return values.

opcoder0 · 2024-12-11T04:48:18Z

Most likely these packages are no longer being used. And this is not a good practice anyway. I don't feel it is a good idea to go further on this path. In the code base, we can assume that either DUTs have direct internet access or via http proxy. Maybe we can also consider building this package into SONiC image. Then in tests, the code can firstly check if there is such tool. If not, then try to install it.

Thanks @wangxin. I agree that committing Debian packages to the repository is not the best way forward. I am checking with the SONiC team to see if socat can be / is part of the image. I observed a few builds 202311 and 202405 already had socat on them. However I am not sure if they are there across all the builds based on bullseye and bookworm. I observed a dirty build based on master didn't have it for some reason.

Modify template to use exposed db port; Update documentation; Add __init__.py to tests/common/database;

mssonicbld · 2024-12-16T05:49:03Z

/azp run

azure-pipelines · 2024-12-16T05:49:09Z

Pull request contains merge conflicts.

mssonicbld · 2024-12-17T01:24:11Z

/azp run

azure-pipelines · 2024-12-17T01:24:23Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2024-12-24T01:29:57Z

/azp run

azure-pipelines · 2024-12-24T01:30:09Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2024-12-24T04:59:56Z

/azp run

azure-pipelines · 2024-12-24T05:00:59Z

Azure Pipelines successfully started running 1 pipeline(s).

opcoder0 added 23 commits August 22, 2024 06:42

Initial commit with draft design - in progress

96094a0

Remove test category column and fix formatting

30727ce

Add everflow; fix description

abb1b88

Add tests/fdb, tests/fib

65d2e0e

Update design document; Write pubsub based sonic database access library

e0fbbbf

Merge branch 'master' into feat/sai-validation

f4bd2d7

Add logging to test bfd and sonic db library

9e510de

Merge branch 'master' into feat/sai-validation

7b41e62

Add wait_for_n_keys; update usage in bfd and acl

25ab88d

Basic working:

c3624dc

- Key changes detected

Add to design doc; Keep count

7fd5761

of N unique keys while waiting

refactor code to count rules

a3767a0

Reduce logging; Add SAI validation to TestIncrementalAcl

a1a3178

add log to test_bfd.py and make basic test_basic_new

da86d65

Add new subscription based API for watching Sonic DB keys

b2b410f

Merge branch 'master' into feat/sai-validation

c7e1adc

Fix failures; Move changes to test_bfd.py; remove test_bfd_new.py;

d7d0ea8

Add socat files

fc76650

remove test_bfd_new.py

778acb1

Expose redis via db fixtures; Assert ACL test;

e456688

Merge branch 'master' into feat/sai-validation

7a55ec3

Remove vs config files from commit

c51dc9f

Remove extra debug log entry

6cbabd6

opcoder0 requested review from yutongzhang-microsoft and xwjiang-ms November 1, 2024 07:01

opcoder0 added 2 commits November 1, 2024 07:08

Fix merge issues in conftest.py

b1fa4e9

Remove debug log entries from test_acl.py

6548a75

opcoder0 marked this pull request as ready for review November 4, 2024 05:44

opcoder0 requested review from wangxin and yxieca as code owners November 4, 2024 05:44

opcoder0 added 3 commits November 6, 2024 07:22

Get duthost mgmt IP from fixture, make expose port configurable

d6523c5

Merge branch 'master' into feat/sai-validation

33bf4b2

Merge branch 'master' into feat/sai-validation

ca07542

wangxin reviewed Dec 9, 2024

View reviewed changes

opcoder0 added 2 commits December 11, 2024 05:21

Handle exceptions in wait_until_* functions;

44e891b

Modify template to use exposed db port; Update documentation; Add __init__.py to tests/common/database;

Remove socat package and get it from SONiC swss container

31afed8

Merge branch 'master' into feat/sai-validation

4188ba2

opcoder0 added 2 commits December 24, 2024 01:29

Add ACL rule validation functions

1da54da

Merge branch 'master' into feat/sai-validation

32f6c41

Cancel future that timeout; Skip validation for egress stage

3e5ea5b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SAI validation #15324

SAI validation #15324

opcoder0 commented Nov 1, 2024

wangxin commented Nov 27, 2024

opcoder0 commented Nov 27, 2024

wangxin commented Dec 6, 2024

wangxin commented Dec 9, 2024

wangxin Dec 9, 2024

wangxin Dec 9, 2024

wangxin Dec 9, 2024

opcoder0 Dec 11, 2024

wangxin Dec 9, 2024

wangxin Dec 9, 2024

opcoder0 Dec 11, 2024

opcoder0 commented Dec 11, 2024

mssonicbld commented Dec 16, 2024

azure-pipelines bot commented Dec 16, 2024

mssonicbld commented Dec 17, 2024

azure-pipelines bot commented Dec 17, 2024

mssonicbld commented Dec 24, 2024

azure-pipelines bot commented Dec 24, 2024

mssonicbld commented Dec 24, 2024

azure-pipelines bot commented Dec 24, 2024


		## Introduction

		SONiC management tests are Pytest modules running in the SONiC management container on the developer/CI/test environment and PTF tests running from the PTF container on the testbed server. As part of the setup and tear down activities the tests make configuration changes to SONiC, run the tests and verify if the tests ran successfully by making additional configuration checks and finally tear down the configuration changes. The tests use command line utilities on the DUT like `config`, `sonic-db-cli` or `redis-cli` to set and get configuration values. In some cases tests export / dump the contents of the database to examine its results and verify if tests ran successfully. These configuration changes are propogated to the ASIC through ASIC_DB. The aim of this design document is to identify a mechanism for tests to validate the configuration changes against ASIC_DB entries or SAI object types.


		- Using threading and polling as implemented in [SWSS VS Tests](https://github.com/sonic-net/sonic-swss/blob/master/tests/README.md), [conftest](https://github.com/sonic-net/sonic-swss/blob/master/tests/conftest.py), [dvslib/dvs_database](https://github.com/sonic-net/sonic-swss/blob/master/tests/dvslib/dvs_database.py)

		- Using Redis pubsub This approach takes advantage of Redis pubsub to get notification of keyspace changes instead of using threading or polling to watch for key value changes. This mechanism improves performance and predictability of watching for keyspace changes. The tests don't have to rely on threads or sleep timers to check for value changes. The keyspace notifications are enabled by default for `ASIC_DB`. Keyspace notifications are enabled by default (`AKE`) in SONiC Redis instance. There are two kinds of message types received in pubsub `psubscribe` and `pmessage`.

SAI validation #15324

Are you sure you want to change the base?

SAI validation #15324

Conversation

opcoder0 commented Nov 1, 2024

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

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

Documentation

wangxin commented Nov 27, 2024

opcoder0 commented Nov 27, 2024

wangxin commented Dec 6, 2024

wangxin commented Dec 9, 2024

wangxin Dec 9, 2024

Choose a reason for hiding this comment

wangxin Dec 9, 2024

Choose a reason for hiding this comment

wangxin Dec 9, 2024

Choose a reason for hiding this comment

opcoder0 Dec 11, 2024

Choose a reason for hiding this comment

wangxin Dec 9, 2024

Choose a reason for hiding this comment

wangxin Dec 9, 2024

Choose a reason for hiding this comment

opcoder0 Dec 11, 2024

Choose a reason for hiding this comment

opcoder0 commented Dec 11, 2024

mssonicbld commented Dec 16, 2024

azure-pipelines bot commented Dec 16, 2024

mssonicbld commented Dec 17, 2024

azure-pipelines bot commented Dec 17, 2024

mssonicbld commented Dec 24, 2024

azure-pipelines bot commented Dec 24, 2024

mssonicbld commented Dec 24, 2024

azure-pipelines bot commented Dec 24, 2024