Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sonic-mgmt: fix t0-isolated-d128u128s2 topo #15542

Merged

Conversation

ccroy-arista
Copy link
Contributor

@ccroy-arista ccroy-arista commented Nov 13, 2024

Description of PR

This PR contains the following changes, in order to achieve a functional t0-isolated-d128u128s2 topo:

  • The field 'bp_interfaces' in the t0-isolated-d128u128s2 yml file is corrected to 'bp_interface' i.e. w/o the 's'.
  • Added more VMs, to support the larger number of VMs needed for the topo.
  • Added the missing leaf template file for the topo.
  • Fixed the synthesis of the MACs used in ansible roles so that it does not error out after 256 interfaces.

Additionally, this PR fixed the 'bp_interfaces' for t0-isolated-d128u128s1 yml file.

Summary:
Fixes # (issue)

Type of change

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

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

Approach

What is the motivation for this PR?

To achieve a functional t0-isolated-d128u128s2 for use in sonic-mgmt tests.

How did you do it?

Made a series of fixes/updates as detailed in the summary above.

How did you verify/test it?

Confirmed that the testbed server can now be successfully configured as a result of these changes, and subsequently ran the sonic-mgmt test suite.

In order to have a funtional
t0-isolated-d128u128s2 topo, the following
changes have been made:

- The field 'bp_interfaces' in the
t0-isolated-d128u128s2 yml file is corrected to
'bp_interface' i.e. w/o the 's'.

- Added more VMs, to support the larger number
of VMs needed for the topo.

- Added the missing leaf template file for the
topo.

- Fixed the synthesis of the MACs used in
ansible roles so that it does not error out
after 256 interfaces.
@mssonicbld
Copy link
Collaborator

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.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing ansible/roles/eos/templates/t0-isolated-d128u128s2-leaf.j2

check yaml...............................................................Passed
check for added large files..............................................Passed
check python ast.....................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
check conditional mark sort..............................................Passed

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>

@sdszhang sdszhang requested a review from r12f November 14, 2024 05:08
@sdszhang
Copy link
Contributor

@r12f for viz.

@r12f
Copy link
Contributor

r12f commented Nov 14, 2024

a few issues is already fixed by this PR: https://github.com/sonic-net/sonic-mgmt/pull/15454/files.

this PR is missing the template, because the topo is generated. it will be better to get the generator fixed too.

@r12f
Copy link
Contributor

r12f commented Nov 14, 2024

@ccroy-arista , this PR is merged now: #15454.

you might need to merge with the master branch, and I believe certain issues will be gone.

This change reduces risk of mac duplication
from commit 29b01dd.
The field 'bp_interfaces' in the
t0-isolated-d128u128s1 yml file should be
'bp_interface' i.e. w/o the 's'.
@Janetxxx Janetxxx mentioned this pull request Nov 18, 2024
8 tasks
@StormLiangMS StormLiangMS merged commit f992360 into sonic-net:master Nov 18, 2024
16 checks passed
@ccroy-arista ccroy-arista deleted the fix-t0-isolated-d128u128s2-topo branch November 18, 2024 18:07
sreejithsreekumaran pushed a commit to sreejithsreekumaran/sonic-mgmt that referenced this pull request Nov 20, 2024
Description of PR
This PR contains the following changes, in order to achieve a functional t0-isolated-d128u128s2 topo:

The field 'bp_interfaces' in the t0-isolated-d128u128s2 yml file is corrected to 'bp_interface' i.e. w/o the 's'.
Added more VMs, to support the larger number of VMs needed for the topo.
Added the missing leaf template file for the topo.
Fixed the synthesis of the MACs used in ansible roles so that it does not error out after 256 interfaces.
Additionally, this PR fixed the 'bp_interfaces' for t0-isolated-d128u128s1 yml file.
yutongzhang-microsoft pushed a commit to yutongzhang-microsoft/sonic-mgmt that referenced this pull request Nov 21, 2024
Description of PR
This PR contains the following changes, in order to achieve a functional t0-isolated-d128u128s2 topo:

The field 'bp_interfaces' in the t0-isolated-d128u128s2 yml file is corrected to 'bp_interface' i.e. w/o the 's'.
Added more VMs, to support the larger number of VMs needed for the topo.
Added the missing leaf template file for the topo.
Fixed the synthesis of the MACs used in ansible roles so that it does not error out after 256 interfaces.
Additionally, this PR fixed the 'bp_interfaces' for t0-isolated-d128u128s1 yml file.
@mssonicbld
Copy link
Collaborator

@ccroy-arista PR conflicts with 202405 branch

@ccroy-arista
Copy link
Contributor Author

ccroy-arista commented Nov 28, 2024

A recent merge from a different PR contained a fix that is also present in this PR, hence the conflict with 202405.

I will open up a separate PR for 202405 branch.

@ccroy-arista ccroy-arista restored the fix-t0-isolated-d128u128s2-topo branch November 28, 2024 01:44
ccroy-arista added a commit to ccroy-arista/sonic-mgmt that referenced this pull request Nov 28, 2024
The merged cherry-pick to 202405 for sonic-net#15744 mirrors a change in PR sonic-net#15542, which is being blocked from backporting to 202405 as a result.

Remove this ansible/roles/eos/templates/t0-isolated-d128u128s2-leaf.j2 so that a 202405 cherry-pick PR can be opened for sonic-net#15542.
@ccroy-arista ccroy-arista deleted the fix-t0-isolated-d128u128s2-topo branch November 28, 2024 02:09
@ccroy-arista
Copy link
Contributor Author

PR for manual cherry-pick to 202405 posted: #15786

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants