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

[chassis]: support virtual-chassis setup in vs docker #4709

Merged
merged 1 commit into from
Jul 29, 2020

Conversation

BrynXu
Copy link
Contributor

@BrynXu BrynXu commented Jun 5, 2020

virtual-chassis test uses multiple vs instances to simulate a
modular switch and a redis-chassis service is required to run in
the vs instance that represents a supervisor card.
This change allows vs docker start redis-chassis service according
to external config file.

Signed-off-by: Honggang Xu hxu@arista.com

- Why I did it
To support virtual-chassis setup, so that we can test distributed forwarding feature in virtual sonic environment, see Distributed forwarding in a VOQ architecture HLD pull request at sonic-net/SONiC#622

- How I did it
The sonic-vs start.sh is enhanced to start new redis_chassis service if external chassis config file found. The config file doesn't exist in current vs environment, start.sh will behave like before.

- How to verify it
The swss/test still pass. The chassis_db service is verified in virtual-chassis topology and tests which are in following PRs.

- Description for the changelog
[vs] support chassis_db service in vs

- A picture of a cute animal (not mandatory but encouraged)

@ghost
Copy link

ghost commented Jun 5, 2020

CLA assistant check
All CLA requirements met.

@BrynXu BrynXu changed the title [vs]: support virtual-chassis setup in vs docker [chassis]: support virtual-chassis setup in vs docker Jun 6, 2020
@lgtm-com
Copy link

lgtm-com bot commented Jun 10, 2020

This pull request introduces 3 alerts when merging ca4b79aa358ecdd7e252bb870560413dd130c892 into f31eabb - view on LGTM.com

new alerts:

  • 3 for Unused import

@BrynXu BrynXu force-pushed the vschassisdb branch 2 times, most recently from 031a2d8 to 86072d9 Compare June 10, 2020 17:11
@lguohan
Copy link
Collaborator

lguohan commented Jun 10, 2020

retest baseimage please

@BrynXu
Copy link
Contributor Author

BrynXu commented Jun 10, 2020

retest vsimage please

1 similar comment
@BrynXu
Copy link
Contributor Author

BrynXu commented Jun 11, 2020

retest vsimage please

Copy link
Contributor

@daall daall left a comment

Choose a reason for hiding this comment

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

See comments/questions.

platform/vs/docker-sonic-vs/Dockerfile.j2 Outdated Show resolved Hide resolved
platform/vs/docker-sonic-vs.mk Outdated Show resolved Hide resolved
platform/vs/docker-sonic-vs/chassis_db.py Show resolved Hide resolved
platform/vs/docker-sonic-vs/chassis_db.py Show resolved Hide resolved
platform/vs/docker-sonic-vs/chassis_db.py Outdated Show resolved Hide resolved
platform/vs/docker-sonic-vs/start.sh Outdated Show resolved Hide resolved
@daall daall added the Chassis 🤖 Modular chassis support label Jun 26, 2020
@BrynXu BrynXu force-pushed the vschassisdb branch 5 times, most recently from 459c97c to 1004270 Compare July 7, 2020 00:39
@lgtm-com
Copy link

lgtm-com bot commented Jul 7, 2020

This pull request introduces 1 alert when merging 10042701d68e35f2cfb76bf208ab88f9908cff02 into 31baf38 - view on LGTM.com

new alerts:

  • 1 for Suspicious unused loop iteration variable

@lgtm-com
Copy link

lgtm-com bot commented Jul 7, 2020

This pull request introduces 1 alert when merging 8ed1c4bf2839e71a2b70446d6117cecfc527c07d into fa885c9 - view on LGTM.com

new alerts:

  • 1 for Suspicious unused loop iteration variable

@lgtm-com
Copy link

lgtm-com bot commented Jul 7, 2020

This pull request introduces 1 alert when merging aa7d4155aa1e73bfddd8e68eeaa7c5d0d8a3019b into fa885c9 - view on LGTM.com

new alerts:

  • 1 for Suspicious unused loop iteration variable

@BrynXu BrynXu force-pushed the vschassisdb branch 2 times, most recently from fc10ce4 to 8264c88 Compare July 8, 2020 01:43
saravanansv added a commit to saravanansv/sonic-swss that referenced this pull request Jul 8, 2020
sonic-net/sonic-buildimage#4709 is a PR for create/delete a virtual chassis.
Similar to create_vnet.sh, virtual_chassis.py is used to create the virtual chassis test
environment.
The changes here are to
1. Add a test_virtual_chassis.py to verify the virtual chassis
2. support virtual chassis topology in conftest.py to run the test_virtual_chassis.py.
   Notable changes in conftest.py are
   * Unlike create_vnet, virtual_chassis does not use a sw container.
   * New DockerVirtualChassisTopology class to setup the connections when the
   vitual chassis containers restart.
saravanansv added a commit to saravanansv/sonic-swss that referenced this pull request Jul 8, 2020
sonic-net/sonic-buildimage#4709 is a PR for create/delete a virtual chassis.
Similar to create_vnet.sh, virtual_chassis.py is used to create the virtual chassis test
environment.
The changes here are to
1. Add a test_virtual_chassis.py to verify the virtual chassis
2. support virtual chassis topology in conftest.py to run the test_virtual_chassis.py.
   Notable changes in conftest.py are
   * Unlike create_vnet, virtual_chassis does not use a sw container.
   * New DockerVirtualChassisTopology class to setup the connections when the
   vitual chassis containers restart.
Copy link
Contributor

@daall daall left a comment

Choose a reason for hiding this comment

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

See comments.

Also, I think for this PR it is OK but I think we should try to figure out how to use a template or something to generate the config files for the different SONiC instances. There's a lot of overlap and this might become tricky to maintain if we expand to more line cards, multiple supervisors, etc. in the future.

@daall daall requested a review from lguohan July 8, 2020 20:19
virtual-chassis test uses multiple vs instances to simulate a
modular switch and a redis-chassis service is required to run on
the vs instance that represents a supervisor card.
This change allows vs docker start redis-chassis service according
to external config file.

Signed-off-by: Honggang Xu <hxu@arista.com>
(cherry picked from commit c1d45cf81ce3238be2dcbccae98c0780944981ce)
Copy link
Contributor

@daall daall left a comment

Choose a reason for hiding this comment

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

@BrynXu @saravanansv @eswaranb LGTM, have you guys already run the pizza-box VS tests with this newly updated VS docker image?

@BrynXu
Copy link
Contributor Author

BrynXu commented Jul 21, 2020

retest vsimage please

@BrynXu @saravanansv @eswaranb LGTM, have you guys already run the pizza-box VS tests with this newly updated VS docker image?

yes, we run both pizza-box vs swss tests and virtual-chassis tests with docker vs image built with this PR.

@lguohan lguohan merged commit 311045f into sonic-net:master Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chassis 🤖 Modular chassis support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants