sonic-mgmt icon indicating copy to clipboard operation
sonic-mgmt copied to clipboard

Enhancing core_dump_and_config_check to be multi-asic aware

Open sanmalho-git opened this issue 2 years ago • 7 comments

Description of PR

Summary: Fixes # (issue)

Type of change

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

Back port request

  • [ ] 201911
  • [ ] 202012
  • [x] 202205

Approach

What is the motivation for this PR?

In our pipeline runs, autorestart tests against a multi-asic DUT were failing with error '

Feature 'x' auto-restart is not consistent across namespaces

The reason was that pretest goes and disables autorestart on all the containers. Then ACL tests run with change the config_db's and to retore in it's cleanup does a config load_minigraph. The above results in setting the autorestart state of the containers back to default of 'enabled'

Now, when check_dut_health_status kicks in, it takes a snapshot of the config_db.json before the ACL suite which has the autorestart state as 'disabled'. But, after the ACL suite, it detects that autorestart state has changed to 'enabled'. Thus, it tries to restore it.

However, when it restores, it only restores config_db.json, and not the other asics config_db's.

This results in the state of autorestart to be not consistent across namespaces - global has autorestart 'disabled', while namespace has autorestart 'enabled'.

How did you do it?

Fix for the above it to have enhance check_dut_health_status to be multi-asic aware.

  • It compares not just config_db.json, but also the config_db's of all the asics.
  • If it finds that config has changed, restore not just config_db.json, but also the config_db's of all the asics before rebooting the DUT.

How did you verify/test it?

Ran check_dut_health_status with changes to config_db's and validated that it is restored to what it is before the suite is run

Any platform specific information?

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

Documentation

sanmalho-git avatar Oct 12 '22 19:10 sanmalho-git

The pre-commit check detected issues in the files touched by this pull request. The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results: $(results)

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

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

azure-pipelines[bot] avatar Oct 13 '22 17:10 azure-pipelines[bot]

@SuvarnaMeenakshi - i will fix the merge conflict - can you please review

sanmalho-git avatar Oct 27 '22 17:10 sanmalho-git

The pre-commit check detected issues in the files touched by this pull request. The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

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:16:1: F401 'tests.common.fixtures.conn_graph_facts.conn_graph_facts' imported but unused
tests/conftest.py:28:1: F401 'tests.common.fixtures.duthost_utils.backup_and_restore_config_db_session' imported but unused
tests/conftest.py:29:1: F401 'tests.common.fixtures.ptfhost_utils.ptf_portmap_file' imported but unused
tests/conftest.py:30:1: F401 'tests.common.fixtures.ptfhost_utils.run_icmp_responder_session' imported but unused
tests/conftest.py:58:26: E261 at least two spaces before inline comment
tests/conftest.py:86:121: E501 line too long (123 > 120 characters)
tests/conftest.py:89:121: E501 line too long (122 > 120 characters)
tests/conftest.py:90:121: E501 line too long (139 > 120 characters)
tests/conftest.py:93:121: E501 line too long (176 > 120 characters)
tests/conftest.py:97:21: E128 continuation line under-indented for visual indent
tests/conftest.py:112:21: E128 continuation line under-indented for visual indent
...
[truncated extra lines, please run pre-commit locally to view full check results]

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

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

azure-pipelines[bot] avatar Oct 27 '22 21:10 azure-pipelines[bot]

@SuvarnaMeenakshi @judyjoseph are you expecting the PR owners to fix the pre-commit errors - even though they are in lines of code that the PR owner didn't touch.

sanmalho-git avatar Oct 28 '22 15:10 sanmalho-git

@SuvarnaMeenakshi @judyjoseph are you expecting the PR owners to fix the pre-commit errors - even though they are in lines of code that the PR owner didn't touch.

@sanmalho-git The log mentions that new issues must be fixed. Fixing old issue is not mandatory

SuvarnaMeenakshi avatar Oct 31 '22 18:10 SuvarnaMeenakshi

The pre-commit check detected issues in the files touched by this pull request. The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results: trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing ansible/config_sonic_basedon_testbed.yml

fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing tests/test_pretest.py

check yaml...............................................................Passed
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1
...
[truncated extra lines, please run pre-commit locally to view full check results]

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

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

azure-pipelines[bot] avatar Nov 08 '22 21:11 azure-pipelines[bot]

To ensure changes are uniform, changes should be done here as well, where golden config db is created and removed: https://github.com/sonic-net/sonic-mgmt/blob/master/ansible/config_sonic_basedon_testbed.yml#L584

https://github.com/sonic-net/sonic-mgmt/blob/master/tests/test_pretest.py#L291

Thanks for pointing this out. Have addressed this in the latest commit

sanmalho-git avatar Nov 08 '22 21:11 sanmalho-git

@anamehra fyi

SuvarnaMeenakshi avatar Nov 14 '22 22:11 SuvarnaMeenakshi

@sanmalho-git - can you resolve conflict and fix minor comments.

SuvarnaMeenakshi avatar Nov 16 '22 00:11 SuvarnaMeenakshi

The pre-commit check detected issues in the files touched by this pull request. The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results: trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing ansible/config_sonic_basedon_testbed.yml
Fixing tests/conftest.py

fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing tests/test_pretest.py

check yaml...............................................................Passed
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
...
[truncated extra lines, please run pre-commit locally to view full check results]

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

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

azure-pipelines[bot] avatar Nov 17 '22 21:11 azure-pipelines[bot]

@SuvarnaMeenakshi - have addressed the comments and rebased.

sanmalho-git avatar Nov 17 '22 21:11 sanmalho-git

@sanmalho-git Thank you for addressing the comments.

PR checks are failing. Checking one of the logs, I see this error: dhcp_relay/test_dhcp_relay.py::test_dhcp_relay_random_sport[dual] ERROR [100%]

==================================== ERRORS ==================================== ___________ ERROR at teardown of test_dhcp_relay_random_sport[dual] ____________ ... pre_running_config = duts_data[duthost.hostname]["pre_running_config"][cfg_context]

              cur_running_config = duts_data[duthost.hostname]["cur_running_config"][cfg_context]

E KeyError: None

SuvarnaMeenakshi avatar Nov 18 '22 20:11 SuvarnaMeenakshi

The pre-commit check detected issues in the files touched by this pull request. The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results: trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing ansible/config_sonic_basedon_testbed.yml
Fixing tests/conftest.py

fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing tests/test_pretest.py

check yaml...............................................................Passed
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
...
[truncated extra lines, please run pre-commit locally to view full check results]

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

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

azure-pipelines[bot] avatar Nov 18 '22 21:11 azure-pipelines[bot]

The pre-commit check detected issues in the files touched by this pull request. The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results: trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing ansible/config_sonic_basedon_testbed.yml
Fixing tests/conftest.py

fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing tests/test_pretest.py

check yaml...............................................................Passed
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
...
[truncated extra lines, please run pre-commit locally to view full check results]

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

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

azure-pipelines[bot] avatar Nov 21 '22 15:11 azure-pipelines[bot]