Add a function to compare running config. by yutongzhang-microsoft · Pull Request #8744 · sonic-net/sonic-mgmt

yutongzhang-microsoft · 2023-06-28T06:20:56Z

Description of PR

We only use == to compare running config before, but our running config is a nested dict, and if the value is disordered, it will wrongly return false. Just in module acl/test_acl_outer_vlan.py, it somehow change the order of part of the running dict from {"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel104", "PortChannel103"]}} to {"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel103", "PortChannel104"]}}. And this disorder will cause config reload after running all cases in this module. In this PR, I add a function to compare running config, which will avoid such wrong judgement caused by order.

Summary:
Fixes # (issue)

Type of change

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

Back port request

201911
202012
202205

Approach

What is the motivation for this PR?

We only use == to compare running config before, but our running config is a nested dict, and if the value is disordered, it will wrongly return false. Just in module acl/test_acl_outer_vlan.py, it somehow change the order of part of the running dict from {"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel104", "PortChannel103"]}} to {"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel103", "PortChannel104"]}}. And this disorder will cause config reload after running all cases in this module. In this PR, I add a function to compare running config, which will avoid such wrong judgement caused by order.

How did you do it?

Add a function to compare two running config dict.

How did you verify/test it?

06:35:15 conftest.core_dump_and_config_check      L2115 INFO   | Core dump and config check passed for acl/test_acl_outer_vlan.py

Any platform specific information?

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

Documentation

mssonicbld · 2023-06-28T06:21:57Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/conftest.py:1879:1: E302 expected 2 blank lines, found 1 tests/conftest.py:1891:1: E302 expected 2 blank lines, found 1 check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-06-28T06:24:53Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/conftest.py:1879:1: E302 expected 2 blank lines, found 1 tests/conftest.py:1891:1: E302 expected 2 blank lines, found 1 check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-06-28T06:26:28Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/conftest.py:1879:1: E302 expected 2 blank lines, found 1 tests/conftest.py:1891:1: E302 expected 2 blank lines, found 1 check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

yutongzhang-microsoft · 2023-06-28T09:10:50Z

/azp run

azure-pipelines · 2023-06-28T09:11:00Z

Azure Pipelines successfully started running 1 pipeline(s).

wangxin · 2023-06-28T10:12:41Z

tests/conftest.py

+        return True
+    if type(pre_running_config) != type(cur_running_config):
+        return False
+    if sorted(pre_running_config.keys()) != sorted(cur_running_config.keys()):


How about using set to do the key comparison? Then no need to sort the list.

mssonicbld · 2023-06-29T02:23:35Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/conftest.py:1886:8: F632 use ==/!= to compare constant literals (str, bytes, int, float, tuple) check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

Description of PR We only use `==` to compare running config before, but our running config is a nested dict, and if the value is disordered, it will wrongly return false. Just in module `acl/test_acl_outer_vlan.py`, it somehow change the order of part of the running dict from `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel104", "PortChannel103"]}}` to `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel103", "PortChannel104"]}}`. And this disorder will cause config reload after running all cases in this module. In this PR, I add a function to compare running config, which will avoid such wrong judgement caused by order. What is the motivation for this PR? We only use `==` to compare running config before, but our running config is a nested dict, and if the value is disordered, it will wrongly return false. Just in module `acl/test_acl_outer_vlan.py`, it somehow change the order of part of the running dict from `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel104", "PortChannel103"]}}` to `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel103", "PortChannel104"]}}`. And this disorder will cause config reload after running all cases in this module. In this PR, I add a function to compare running config, which will avoid such wrong judgement caused by order. How did you do it? Add a function to compare two running config dict. How did you verify/test it? ``` 06:35:15 conftest.core_dump_and_config_check L2115 INFO | Core dump and config check passed for acl/test_acl_outer_vlan.py ``` Signed-off-by: Yutong Zhang <yutongzhang@microsoft.com>

mssonicbld · 2023-07-07T08:35:58Z

Cherry-pick PR to 202205: #8846

mssonicbld · 2023-07-07T08:35:58Z

@yutongzhang-microsoft PR conflicts with 202012 branch

Description of PR We only use `==` to compare running config before, but our running config is a nested dict, and if the value is disordered, it will wrongly return false. Just in module `acl/test_acl_outer_vlan.py`, it somehow change the order of part of the running dict from `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel104", "PortChannel103"]}}` to `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel103", "PortChannel104"]}}`. And this disorder will cause config reload after running all cases in this module. In this PR, I add a function to compare running config, which will avoid such wrong judgement caused by order. What is the motivation for this PR? We only use `==` to compare running config before, but our running config is a nested dict, and if the value is disordered, it will wrongly return false. Just in module `acl/test_acl_outer_vlan.py`, it somehow change the order of part of the running dict from `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel104", "PortChannel103"]}}` to `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel103", "PortChannel104"]}}`. And this disorder will cause config reload after running all cases in this module. In this PR, I add a function to compare running config, which will avoid such wrong judgement caused by order. How did you do it? Add a function to compare two running config dict. How did you verify/test it? ``` 06:35:15 conftest.core_dump_and_config_check L2115 INFO | Core dump and config check passed for acl/test_acl_outer_vlan.py ``` Signed-off-by: Yutong Zhang <yutongzhang@microsoft.com> Co-authored-by: jianquanye@microsoft.com

Description of PR There is a conflict when cherry pick PR (#8744) into 202012 branch, so raise this PR to cherry pick. co-authorized by: jianquanye@microsoft.com

Description of PR We only use `==` to compare running config before, but our running config is a nested dict, and if the value is disordered, it will wrongly return false. Just in module `acl/test_acl_outer_vlan.py`, it somehow change the order of part of the running dict from `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel104", "PortChannel103"]}}` to `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel103", "PortChannel104"]}}`. And this disorder will cause config reload after running all cases in this module. In this PR, I add a function to compare running config, which will avoid such wrong judgement caused by order. What is the motivation for this PR? We only use `==` to compare running config before, but our running config is a nested dict, and if the value is disordered, it will wrongly return false. Just in module `acl/test_acl_outer_vlan.py`, it somehow change the order of part of the running dict from `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel104", "PortChannel103"]}}` to `{"DATAACL": {"ports": ["PortChannel101", "PortChannel102", "PortChannel103", "PortChannel104"]}}`. And this disorder will cause config reload after running all cases in this module. In this PR, I add a function to compare running config, which will avoid such wrong judgement caused by order. How did you do it? Add a function to compare two running config dict. How did you verify/test it? ``` 06:35:15 conftest.core_dump_and_config_check L2115 INFO | Core dump and config check passed for acl/test_acl_outer_vlan.py ``` Signed-off-by: Yutong Zhang <yutongzhang@microsoft.com>

yutongzhang-microsoft requested a review from wangxin June 28, 2023 06:20

Add a function to compare running comfig.

93114a7

yutongzhang-microsoft force-pushed the yutongzhang/compare_running_config branch from ead8671 to 93114a7 Compare June 28, 2023 06:25

Modify code format

e65ba81

wangxin reviewed Jun 28, 2023

View reviewed changes

Fix error

4c15cc6

yutongzhang-microsoft added 2 commits June 29, 2023 10:24

Fix error

7e5b200

Modify the structure of function compare_running_config

853cf37

yutongzhang-microsoft added Request for 202012 branch Request for 202205 branch labels Jun 30, 2023

wangxin approved these changes Jun 30, 2023

View reviewed changes

yutongzhang-microsoft merged commit ad4700f into sonic-net:master Jun 30, 2023

yutongzhang-microsoft deleted the yutongzhang/compare_running_config branch June 30, 2023 02:44

yutongzhang-microsoft added Approved for 202205 branch Approved for 202012 Branch labels Jul 7, 2023

mssonicbld added Created PR to 202205 branch Cherry Pick Conflict_202012 labels Jul 7, 2023

mssonicbld mentioned this pull request Jul 7, 2023

[action] [PR:8744] Add a function to compare running config. #8846

Merged

6 tasks

mssonicbld added Included in 202205 branch and removed Created PR to 202205 branch labels Jul 7, 2023

yutongzhang-microsoft mentioned this pull request Jul 10, 2023

[202012]Add a function to compare running config. #8880

Merged

6 tasks

yutongzhang-microsoft added the Included in 202012 branch label Jul 10, 2023

yejianquan pushed a commit that referenced this pull request Jul 10, 2023

Add a function to compare running config. (#8880)

1bb0112

Description of PR There is a conflict when cherry pick PR (#8744) into 202012 branch, so raise this PR to cherry pick. co-authorized by: jianquanye@microsoft.com

yutongzhang-microsoft removed the Cherry Pick Conflict_202012 label Aug 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a function to compare running config. #8744

Add a function to compare running config. #8744
yutongzhang-microsoft merged 5 commits intosonic-net:masterfrom
yutongzhang-microsoft:yutongzhang/compare_running_config

yutongzhang-microsoft commented Jun 28, 2023 •

edited

Loading

Uh oh!

mssonicbld commented Jun 28, 2023

Uh oh!

mssonicbld commented Jun 28, 2023

Uh oh!

mssonicbld commented Jun 28, 2023

Uh oh!

yutongzhang-microsoft commented Jun 28, 2023

Uh oh!

azure-pipelines bot commented Jun 28, 2023

Uh oh!

wangxin Jun 28, 2023

Uh oh!

yutongzhang-microsoft Jun 29, 2023

Uh oh!

mssonicbld commented Jun 29, 2023

Uh oh!

mssonicbld commented Jul 7, 2023

Uh oh!

mssonicbld commented Jul 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

yutongzhang-microsoft commented Jun 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

mssonicbld commented Jun 28, 2023

Uh oh!

mssonicbld commented Jun 28, 2023

Uh oh!

mssonicbld commented Jun 28, 2023

Uh oh!

yutongzhang-microsoft commented Jun 28, 2023

Uh oh!

azure-pipelines bot commented Jun 28, 2023

Uh oh!

wangxin Jun 28, 2023

Choose a reason for hiding this comment

Uh oh!

yutongzhang-microsoft Jun 29, 2023

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Jun 29, 2023

Uh oh!

mssonicbld commented Jul 7, 2023

Uh oh!

mssonicbld commented Jul 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yutongzhang-microsoft commented Jun 28, 2023 •

edited

Loading