Skip to content

[testbed-cli] Code change on add-topo and deploy-minigraph for deploying testbed with peers on multiple servers #15643

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

Merged
merged 10 commits into from
Jan 20, 2025

Conversation

w1nda
Copy link
Member

@w1nda w1nda commented Nov 20, 2024

Description of PR

Summary:
Fixes # (issue)
In PR #15547, we define server index in topology file which make topology file and testbed yaml file is coupled.
To decouple them, in new design #15395, we remove server index in topology file and add dut_interfaces in testbed yaml, so there is no more change on topology file schema.

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?

Decouple topology file and testbed file in design.

How did you do it?

Record all information in testbed schema

How did you verify/test it?

Deploy testbed

Any platform specific information?

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

Documentation

@w1nda w1nda requested review from r12f, Blueve and sdszhang November 20, 2024 16:27
@w1nda w1nda changed the title [testbed-cli-add-topo] Code change for deploying testbed with peers on multiple servers [testbed-cli] Code change on add-topo and deploy-minigraph for deploying testbed with peers on multiple servers Nov 20, 2024
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Pull request contains merge conflicts.

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@r12f r12f left a comment

Choose a reason for hiding this comment

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

< sign off removed here >

@r12f r12f self-requested a review January 18, 2025 01:14
Copy link
Contributor

@r12f r12f left a comment

Choose a reason for hiding this comment

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

discussed the naming of the 2 MultiServersUtils, seems to be very preferred to be the same. signed off, since other comments are already resolved.

@bingwang-ms
Copy link
Collaborator

@wangxin Could you please help review?

wangxin
wangxin previously approved these changes Jan 20, 2025
@wangxin
Copy link
Collaborator

wangxin commented Jan 20, 2025

@w1nda Can you resolve merge conflicts?

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wangxin wangxin merged commit 48ac199 into sonic-net:master Jan 20, 2025
24 checks passed
Blueve pushed a commit that referenced this pull request Jan 23, 2025
…t.yml (#16634)

Summary:
Fixes # (issue)
In PR: #15643, it add a new optional variable clean_before_add for template arista_7260_connect.j2 to indicate whether clean vlan range before add vlan range, however, this variable are defined in fanout_connect.yml which will call rootfanout_connect.yml, this will be a bug, because when rootfanout_connect.yml is called by other playbook, it will raise error: clean_before_add undefined, so, to fix this bug, we move the clean_before_add definition from fanout_connect.yml to
rootfanout_connect.yml.

What is the motivation for this PR?
this will be a bug, because when rootfanout_connect.yml is called by other playbook, it will raise error: clean_before_add undefined

How did you do it?
move the clean_before_add definition from fanout_connect.yml to
rootfanout_connect.yml.

How did you verify/test it?
local run playbook
mssonicbld added a commit to mssonicbld/sonic-mgmt.msft that referenced this pull request Feb 28, 2025
… servers

<!--
Please make sure you've read and understood our contributing guidelines;
https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md

Please provide following information to help code review process a bit easier:
-->
### Description of PR
<!--
- Please include a summary of the change and which issue is fixed.
- Please also include relevant motivation and context. Where should reviewer start? background context?
- List any dependencies that are required for this change.
-->

Summary:
Fixes # (issue)
To leverage the servers instead of a single server for deploying a single testbed, we proposal this design for deploying testbeds with multiple servers.

Related PRs:
| PR title | State | Context |
| ------ | ------ | -------|
| [[testbed-cli] Code change on add-topo and deploy-minigraph for deploying testbed with peers on multiple servers](sonic-net/sonic-mgmt#15643) | ![state](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-mgmt/15643) | ![context](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-mgmt/15643) |
| [[Pending #15643][testbed] ptf data plane connection for multi-servers testbed](sonic-net/sonic-mgmt#15881) | ![state](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-mgmt/15881) | ![context](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-mgmt/15881) |
| [[sonic-mgmt-docker-image] Support ptf dataplane packet poll with multiple ptf nn agents connection](sonic-net/sonic-buildimage#21070)  | ![state](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-buildimage/21070) | ![context](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-buildimage/21070) |
### Type of change

<!--
- Fill x for your type of change.
- e.g.
- [x] Bug fix
-->

- [ ] 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?
When deploying a testbed with a vast number of virtual ceos neighbors, we will create ceos containers on same server, however, the server doesn't have infinite resources such as memory to deploy that.
To leverage the servers instead of a single server, we proposal this design for deploying testbeds with multiple servers.
#### How did you do it?
Design for deploying testbed with multiple servers.
#### How did you verify/test it?

#### Any platform specific information?

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

### Documentation
<!--
(If it's a new feature, new test case)
Did you update documentation/Wiki relevant to your implementation?
Link to the wiki page?
-->
mssonicbld added a commit to Azure/sonic-mgmt.msft that referenced this pull request Feb 28, 2025
… servers (#125)

<!--
Please make sure you've read and understood our contributing guidelines;
https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md

Please provide following information to help code review process a bit easier:
-->
### Description of PR
<!--
- Please include a summary of the change and which issue is fixed.
- Please also include relevant motivation and context. Where should reviewer start? background context?
- List any dependencies that are required for this change.
-->

Summary:
Fixes # (issue)
To leverage the servers instead of a single server for deploying a single testbed, we proposal this design for deploying testbeds with multiple servers.

Related PRs:
| PR title | State | Context |
| ------ | ------ | -------|
| [[testbed-cli] Code change on add-topo and deploy-minigraph for deploying testbed with peers on multiple servers](sonic-net/sonic-mgmt#15643) | ![state](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-mgmt/15643) | ![context](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-mgmt/15643) |
| [[Pending #15643][testbed] ptf data plane connection for multi-servers testbed](sonic-net/sonic-mgmt#15881) | ![state](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-mgmt/15881) | ![context](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-mgmt/15881) |
| [[sonic-mgmt-docker-image] Support ptf dataplane packet poll with multiple ptf nn agents connection](sonic-net/sonic-buildimage#21070) | ![state](https://img.shields.io/github/pulls/detail/state/sonic-net/sonic-buildimage/21070) | ![context](https://img.shields.io/github/status/contexts/pulls/sonic-net/sonic-buildimage/21070) |
### Type of change

<!--
- Fill x for your type of change.
- e.g.
- [x] Bug fix
-->

- [ ] 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?
When deploying a testbed with a vast number of virtual ceos neighbors, we will create ceos containers on same server, however, the server doesn't have infinite resources such as memory to deploy that.
To leverage the servers instead of a single server, we proposal this design for deploying testbeds with multiple servers.
#### How did you do it?
Design for deploying testbed with multiple servers.
#### How did you verify/test it?

#### Any platform specific information?

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

### Documentation
<!--
(If it's a new feature, new test case)
Did you update documentation/Wiki relevant to your implementation?
Link to the wiki page?
-->
nnelluri-cisco pushed a commit to nnelluri-cisco/sonic-mgmt that referenced this pull request Mar 15, 2025
…ing testbed with peers on multiple servers (sonic-net#15643)

In PR sonic-net#15547, we define server index in topology file which make topology file and testbed yaml file is coupled.
To decouple them, in new design sonic-net#15395, we remove server index in topology file and add dut_interfaces in testbed yaml, so there is no more change on topology file schema.

What is the motivation for this PR?
Decouple topology file and testbed file in design.

How did you do it?
Record all information in testbed schema

How did you verify/test it?
Deploy testbed
nnelluri-cisco pushed a commit to nnelluri-cisco/sonic-mgmt that referenced this pull request Mar 15, 2025
…t.yml (sonic-net#16634)

Summary:
Fixes # (issue)
In PR: sonic-net#15643, it add a new optional variable clean_before_add for template arista_7260_connect.j2 to indicate whether clean vlan range before add vlan range, however, this variable are defined in fanout_connect.yml which will call rootfanout_connect.yml, this will be a bug, because when rootfanout_connect.yml is called by other playbook, it will raise error: clean_before_add undefined, so, to fix this bug, we move the clean_before_add definition from fanout_connect.yml to
rootfanout_connect.yml.

What is the motivation for this PR?
this will be a bug, because when rootfanout_connect.yml is called by other playbook, it will raise error: clean_before_add undefined

How did you do it?
move the clean_before_add definition from fanout_connect.yml to
rootfanout_connect.yml.

How did you verify/test it?
local run playbook
nhe-NV pushed a commit to nhe-NV/sonic-mgmt that referenced this pull request May 12, 2025
…t.yml (sonic-net#25)

<!--
Please make sure you've read and understood our contributing guidelines;
https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md

Please provide following information to help code review process a bit easier:
-->
### Description of PR
<!--
- Please include a summary of the change and which issue is fixed.
- Please also include relevant motivation and context. Where should reviewer start? background context?
- List any dependencies that are required for this change.
-->

Summary:
Fixes # (issue)
In PR: sonic-net#15643, it add a new optional variable `clean_before_add` for template `arista_7260_connect.j2` to indicate whether clean vlan range before add vlan range, however, this variable are defined in `fanout_connect.yml` which will call `rootfanout_connect.yml`, this will be a bug, because when `rootfanout_connect.yml` is called by other playbook, it will raise error: clean_before_add undefined, so, to fix this bug, we move the `clean_before_add` definition from `fanout_connect.yml` to
 `rootfanout_connect.yml`.
### Type of change

<!--
- Fill x for your type of change.
- e.g.
- [x] Bug fix
-->

- [x] Bug fix
- [ ] Testbed and Framework(new/improvement)
- [ ] New Test case
 - [ ] Skipped for non-supported platforms
- [ ] Test case improvement

### Back port request
- [ ] 202012
- [ ] 202205
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411

### Approach
#### What is the motivation for this PR?
this will be a bug, because when `rootfanout_connect.yml` is called by other playbook, it will raise error: clean_before_add undefined
#### How did you do it?
 move the `clean_before_add` definition from `fanout_connect.yml` to
 `rootfanout_connect.yml`.
#### How did you verify/test it?
local run playbook
#### Any platform specific information?

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

### Documentation
<!--
(If it's a new feature, new test case)
Did you update documentation/Wiki relevant to your implementation?
Link to the wiki page?
-->
nhe-NV pushed a commit to nhe-NV/sonic-mgmt that referenced this pull request May 12, 2025
…ing testbed with peers on multiple servers (sonic-net#15643)

In PR sonic-net#15547, we define server index in topology file which make topology file and testbed yaml file is coupled.
To decouple them, in new design sonic-net#15395, we remove server index in topology file and add dut_interfaces in testbed yaml, so there is no more change on topology file schema.

What is the motivation for this PR?
Decouple topology file and testbed file in design.

How did you do it?
Record all information in testbed schema

How did you verify/test it?
Deploy testbed
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.

5 participants