SONiC icon indicating copy to clipboard operation
SONiC copied to clipboard

SAG HLD

Open superchild opened this issue 3 years ago • 13 comments

Static Anycast Gateway (SAG) HLD

Repo PR title State Context
sonic-swss-common [schema] Add SAG table for static anycast gateway
sonic-swss [SAG]: Add SAG implementation
sonic-utilities Add SAG implementation
sonic-buildimage Add SAG SONiC Yang
sonic-mgmt Add static-anycast-gateway(SAG) test plan
sonic-mgmt Add static-anycast-gateway(SAG) test cases

superchild avatar Aug 13 '21 03:08 superchild

CLA assistant check
All CLA requirements met.

ghost avatar Aug 13 '21 03:08 ghost

@kperumalbfn @venkatmahalingam @preetham-singh @hasan-brcm @dgsudharsan @liat-grozovik
All comments are replied and the HLD is updated. Please help to check again.

superchild avatar Sep 07 '21 05:09 superchild

All related PR's are sent and updated tracking table in this PR. Please help to review it.

superchild avatar Oct 21 '21 02:10 superchild

@superchild would you please add the test PR into the code PR list? Thanks.

zhangyanzhao avatar Oct 26 '21 15:10 zhangyanzhao

@superchild would you please add the test PR into the code PR list? Thanks.

@zhangyanzhao What is the test PR? The unit test code is included in the following 3 PRs

  • sonic-swss: https://github.com/Azure/sonic-swss/pull/1974
  • sonic-utilities: https://github.com/Azure/sonic-utilities/pull/1887
  • sonic-buildimage(yang model): https://github.com/Azure/sonic-buildimage/pull/9018

sonic-swss-common#540 and sonic-utilities#1887 don't have reviewer yet, could you please assign someone help to review?

Thanks.

superchild avatar Oct 27 '21 00:10 superchild

Update the sonic-mgmt related test plan /code PR for this feature

superchild avatar Apr 19 '22 12:04 superchild

@superchild looks like all code PRs are still open, need defer this feature to next release (202211). Thanks.

zhangyanzhao avatar Jun 02 '22 00:06 zhangyanzhao

@zhangyanzhao These PRs are already ready but always doesn't get feedback of the reviewer, is that possible to push reviewers and get this feature to include in this release? (some PR still lack of authorized reviewer to merge)

superchild avatar Jun 02 '22 01:06 superchild

@superchild looks like all code PRs are still open, need defer this feature to next release (202211). Thanks.

Is there a reason SAG is not listed under the 202211 section on the roadmap? https://github.com/sonic-net/SONiC/wiki/Sonic-Roadmap-Planning

ITJamie avatar Aug 22 '22 08:08 ITJamie

@superchild looks like all code PRs are still open, need defer this feature to next release (202211). Thanks.

Is there a reason SAG is not listed under the 202211 section on the roadmap? https://github.com/sonic-net/SONiC/wiki/Sonic-Roadmap-Planning

@superchild - Also interested to know the answer to the question above. When will this feature and all PR's be merged ?

eddyk-nvidia avatar Oct 16 '22 08:10 eddyk-nvidia

It's not merged due to code PR isn't merged by the reviewer.
The related code PR had some discussion.
@zhangyanzhao is there any other way to speed up the review process and get merged by the reviewer?
In previous release, some code PR is approved by the reviewer but can't be merged due to their permission.
When time goes by, the code is out-of-date with the base branch, there's much effort to merge/resolve the conflict again.
Please help if there's any other suggestion, then I'll follow and sync the PR again. Thanks.

superchild avatar Oct 17 '22 09:10 superchild

This is not in 202211 release plan.

@superchild looks like all code PRs are still open, need defer this feature to next release (202211). Thanks.

Is there a reason SAG is not listed under the 202211 section on the roadmap? https://github.com/sonic-net/SONiC/wiki/Sonic-Roadmap-Planning

This is not in 202211 release plan.

zhangyanzhao avatar Nov 03 '22 20:11 zhangyanzhao

@superchild would you please add the test PR into the code PR list? Thanks.

@zhangyanzhao What is the test PR? The unit test code is included in the following 3 PRs

sonic-swss-common#540 and sonic-utilities#1887 don't have reviewer yet, could you please assign someone help to review?

Thanks.

@superchild this is not in 202211 release plan. We need the reviewer to finish the review, approve the PRs to merge.

zhangyanzhao avatar Nov 03 '22 20:11 zhangyanzhao

@superchild what are your plans? do you plan to drive this PR?

Yuval-Mellanox avatar Nov 22 '22 09:11 Yuval-Mellanox

This feature is added into 202305 release plan. Let's finish the code review and merge the code @venkatmahalingam @hasan-brcm

zhangyanzhao avatar Jan 03 '23 21:01 zhangyanzhao

@zhangyanzhao Thanks for your help, I'll merge the master branch's change for 202305 release.

superchild avatar Jan 04 '23 01:01 superchild

Reviewers, can you please help to finish your review on this feature? Thanks. @gord1306 @kperumalbfn @venkatmahalingam @preetham-singh @hasan-brcm

zhangyanzhao avatar Feb 01 '23 07:02 zhangyanzhao

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: superchild / name: Jimi Chen (cd6941fcd13c926e7e29a4c65a0d35a9f07cac7b, b0b5c10fa4416e36b60fd11b8dacb12714e98213, ed64df7a08fb937cf322677ff6aff0eccc50016d, 950267e2217e43aefcc53f7a5d4bfa0ef60b9217, 63c7a3bc2c1d9945639aa19cead72c8b3d8eba87, 3237e5032108204ed2c8f817b12d3bf53dc1f4fa, 5dbc369e93247665e5cfb12504ba4460497b7da1)

@superchild could you please sign the easyCLA?

liat-grozovik avatar Feb 01 '23 08:02 liat-grozovik

@liat-grozovik thanks for your reminder, already signed.

superchild avatar Feb 01 '23 12:02 superchild

@liat-grozovik can you please let me know if we can merge this PR? Nvidia is the registered reviewer. Thanks.

zhangyanzhao avatar May 08 '23 05:05 zhangyanzhao

It was reviewed by Nvidia and approved (@dgsudharsan and @Junchao-Mellanox ). please go a head and merge the HLD. as for code PRs, need to see the review and feedback

liat-grozovik avatar May 08 '23 06:05 liat-grozovik

General comments:

  • The HLD is missing a high level description before even discussing the config db which should be followed by yang model. Please check the HLD Template guidelines (https://github.com/Azure/SONiC/blob/master/doc/hld_template.md) it is very helpful to understand the feature and be able to review the code later on.
  • The CLI as described is very high level and the part of the interface does not define and of the checkers expected on the CLI level. The CLI itself IMO should be described in the same way we have in the command reference md file.
  • The flow is very high level and does to explain clearly (although there is a flow image). i don't see any reference to the SAI it self and the parameters used. Missing flow of vlan interface delete, not the sag part.
  • The default behaviour should be clearly mentioned. it is disabled by default.
  • It was mentioned it must go with VXLAN/EVPN. Is there any validation? what will happen if not? what tis the expected behaviour? ignored? error?

@superchild can you please address this comment? Thanks.

zhangyanzhao avatar Jun 14 '23 04:06 zhangyanzhao

HLD has been reviewed in SONiC community on 8/24/2021

zhangyanzhao avatar Jan 24 '24 21:01 zhangyanzhao

HLD was reviewed and approved by reviewers, merge the HLD.

zhangyanzhao avatar Jan 24 '24 21:01 zhangyanzhao

@liat-grozovik NVIDIA is the reviewing company, can you please merge this PR? Thanks.

zhangyanzhao avatar Jan 24 '24 21:01 zhangyanzhao

New Updated PRs: sonic-swss : https://github.com/sonic-net/sonic-swss/pull/3167 sonic-utility: https://github.com/sonic-net/sonic-utilities/pull/3339 sonic-buildimage: https://github.com/sonic-net/sonic-buildimage/pull/19069

yanjundeng avatar Jun 01 '24 00:06 yanjundeng