Skip to content

Changes to support other SpineRouter roles#22337

Merged
rlhui merged 9 commits intosonic-net:masterfrom
tjchadaga:spinerouter_checks
May 2, 2025
Merged

Changes to support other SpineRouter roles#22337
rlhui merged 9 commits intosonic-net:masterfrom
tjchadaga:spinerouter_checks

Conversation

@tjchadaga
Copy link
Contributor

@tjchadaga tjchadaga commented Apr 15, 2025

Why I did it

To support additional SpineRouter roles (UpperSpineRouter, FabricSpineRouter and LowerSpineRouter) for disaggregated chassis introduced in #22285

Work item tracking
  • Microsoft ADO (number only):

How I did it

Main changes include

  1. Wherever 'SpineRouter' check was used to run chassis-specific code, change the code to explicitly look for modular chassis linecard (upstreamLC/downstreamLC)
  2. Change BGPMON to use Loopback4096 on UpperSpineRouter as well
  3. Ensure anchor route configuration are processed on UpperSpineRouter and UpstreamLC of SpineRouter

How to verify it

Test with switch role set to one of the new SpineRouter roles

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tjchadaga tjchadaga requested a review from arlakshm April 15, 2025 21:47
@rlhui rlhui added the P0 Priority of the issue label Apr 16, 2025
Signed-off-by: Tejaswini Chadaga <tchadaga@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Comment on lines -60 to -66
if [[ "$(sonic-cfggen -d -v DEVICE_METADATA.localhost.type)" == *"SpineRouter"* ]] ; then
if [[ "$1" != "chassis" ]] ; then
echo "Please execute 'sudo config save' to preserve System mode in Normal state after reboot or config reload"
fi
else
echo "Please execute 'sudo config save' to preserve System mode in Normal state after reboot or config reload"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is some previous code cleanup issue. It is the same message being printed in all cases, so removed the conditional check

Comment on lines -61 to -70
if [[ "$(sonic-cfggen -d -v DEVICE_METADATA.localhost.type)" == *"SpineRouter"* ]] ; then
if [[ "$1" != "chassis" ]] ; then
echo "Please execute 'sudo config save' to preserve System mode in Maintenance after reboot or config reload"
if [[ $disaggregated_chassis -ne 1 ]]; then
echo -e "\nWARNING: Please execute 'TSA' on all other linecards of the chassis to fully isolate this device"
fi
fi
else
echo "Please execute 'sudo config save' to preserve System mode in Maintenance after reboot or config reload"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

line 62 was only applicable for rexec-based TSA where we were passing "TSA chassis" from sup. It is no longer needed.
Other changes are just making that warning message print on modular chassis alone by looking for upstream/downstreamLC

Copy link
Contributor

Choose a reason for hiding this comment

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

TSA from sup can still be done by DRI on sup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rlhui - no functional change to TSA from sup in this PR. This is just code cleanup while printing warning message on linecard. Instead of using Spinerouter to identify linecard, we have switched to using subtype UpsteamLC/DownstreamLC

Copy link
Contributor

@arlakshm arlakshm left a comment

Choose a reason for hiding this comment

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

as comments

@tjchadaga tjchadaga requested a review from arlakshm April 21, 2025 20:13
@arlakshm arlakshm requested a review from saksarav-nokia April 23, 2025 17:09
@saksarav-nokia
Copy link
Contributor

@tjchadaga , LGTM.
Does this cover advertising the routes to IBGP neighbors in disaggregated chassis or need to change the route_map in bgpd/templates/voq_chassis/policies.conf.j2?

@tjchadaga
Copy link
Contributor Author

@tjchadaga , LGTM. Does this cover advertising the routes to IBGP neighbors in disaggregated chassis or need to change the route_map in bgpd/templates/voq_chassis/policies.conf.j2?

Yes, it does. For UpperSpineRouter, the subtype 'UpstreamLC' will not be set in device metadata. So, the checks in the BGP templates to prevent advertising routes between asics should not apply.

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).


PLATFORM=${PLATFORM:-`sonic-cfggen -H -v DEVICE_METADATA.localhost.platform`}
type=`sonic-db-cli CONFIG_DB hget 'DEVICE_METADATA|localhost' 'type'`
subtype=`sonic-db-cli CONFIG_DB hget 'DEVICE_METADATA|localhost' 'subtype'`
Copy link
Contributor

Choose a reason for hiding this comment

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

subtype may not be defined for all sonic SpineRouter sku's?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is mainly used for reliable TSA from supervisor on modular chassis.
subtype is being used to check if the device is a linecard (UpstreamLC/DownstreamLC). If not set, assumption is that it is not a linecard.

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kenneth-arista
Copy link
Collaborator

lgtm

@rlhui
Copy link
Contributor

rlhui commented Apr 30, 2025

Can we be more descriptive on this PR?

@tjchadaga
Copy link
Contributor Author

Can we be more descriptive on this PR?

Sure. Added more details to the description

@rlhui rlhui merged commit 3ee2aae into sonic-net:master May 2, 2025
19 checks passed
arlakshm added a commit to Azure/sonic-buildimage-msft that referenced this pull request May 6, 2025
<!--
Please make sure you've read and understood our contributing guidelines:
     https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

** Make sure all your commits include a signature generated with `git
commit -s` **

If this is a bug fix, make sure your description includes "fixes #xxxx",
or
     "closes #xxxx" or "resolves #xxxx"

     Please provide the following information:
-->

Manual cherry-pick of
sonic-net/sonic-buildimage#22337

#### Why I did it
To support additional SpineRouter roles introduced in
sonic-net/sonic-buildimage#22285

##### Work item tracking
- Microsoft ADO **(number only)**:

#### How I did it
Added additional roles to checks of spinerouter, changed chassis
specific code to look for upstream/downstreamLC

#### How to verify it
Test with switch role set to one of the new SpineRouter roles

<!--
If PR needs to be backported, then the PR must be tested against the
base branch and the earliest backport release branch and provide tested
image version on these two branches. For example, if the PR is requested
for master, 202211 and 202012, then the requester needs to provide test
results on master and 202012.
-->

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205
- [ ] 202211

#### Tested branch (Please provide the tested image version)

<!--
- Please provide tested image version
- e.g.
- [x] 20201231.100
-->

- [ ] <!-- image version 1 -->
- [ ] <!-- image version 2 -->

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->

<!--
Ensure to add label/tag for the feature raised. example - PR#2174 under
sonic-utilities repo. where, Generic Config and Update feature has been
labelled as GCU.
-->

#### Link to config_db schema for YANG module changes
<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on
https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

#### A picture of a cute animal (not mandatory but encouraged)
yxieca pushed a commit that referenced this pull request Jul 29, 2025
Why I did it
This PR introduce a syntax error #22337, then part missed in prefix_list script, which would cause syntax error

admin@sonic:~$ prefix_list status
/usr/bin/prefix_list: line 43: syntax error near unexpected token `fi'
/usr/bin/prefix_list: line 43: `    fi'

How I did it
Fix syntax error

How to verify it
Run cli and no error

admin@sonic:~$ prefix_list status
Operation is only supported on Upstream SpineRouter.
mssonicbld added a commit to mssonicbld/sonic-buildimage that referenced this pull request Jul 30, 2025
<!--
     Please make sure you've read and understood our contributing guidelines:
     https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

     ** Make sure all your commits include a signature generated with `git commit -s` **

     If this is a bug fix, make sure your description includes "fixes #xxxx", or
     "closes #xxxx" or "resolves #xxxx"

     Please provide the following information:
-->

#### Why I did it
This PR introduce a syntax error sonic-net#22337, `then` part missed in prefix_list script, which would cause syntax error
```
admin@sonic:~$ prefix_list status
/usr/bin/prefix_list: line 43: syntax error near unexpected token `fi'
/usr/bin/prefix_list: line 43: `    fi'
```

##### Work item tracking
- Microsoft ADO **(number only)**:

#### How I did it
Fix syntax error

#### How to verify it
Run cli and no error
```
admin@sonic:~$ prefix_list status
Operation is only supported on Upstream SpineRouter.
```

<!--
If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012.
-->

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 202205
- [ ] 202211
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [x] 202505

#### Tested branch (Please provide the tested image version)

<!--
- Please provide tested image version
- e.g.
- [x] 20201231.100
-->

- [ ] <!-- image version 1 -->
- [ ] <!-- image version 2 -->

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->

<!--
 Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.
-->

#### Link to config_db schema for YANG module changes
<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

#### A picture of a cute animal (not mandatory but encouraged)
mssonicbld added a commit that referenced this pull request Jul 30, 2025
<!--
 Please make sure you've read and understood our contributing guidelines:
 https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

 failure_prs.log Make sure all your commits include a signature generated with `git commit -s` **

 If this is a bug fix, make sure your description includes "fixes #xxxx", or
 "closes #xxxx" or "resolves #xxxx"

 Please provide the following information:
-->

#### Why I did it
This PR introduce a syntax error #22337, `then` part missed in prefix_list script, which would cause syntax error
```
admin@sonic:~$ prefix_list status
/usr/bin/prefix_list: line 43: syntax error near unexpected token `fi'
/usr/bin/prefix_list: line 43: ` fi'
```

##### Work item tracking
- Microsoft ADO **(number only)**:

#### How I did it
Fix syntax error

#### How to verify it
Run cli and no error
```
admin@sonic:~$ prefix_list status
Operation is only supported on Upstream SpineRouter.
```

<!--
If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012.
-->

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 202205
- [ ] 202211
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [x] 202505

#### Tested branch (Please provide the tested image version)

<!--
- Please provide tested image version
- e.g.
- [x] 20201231.100
-->

- [ ] <!-- image version 1 -->
- [ ] <!-- image version 2 -->

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->

<!--
 Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.
-->

#### Link to config_db schema for YANG module changes
<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

#### A picture of a cute animal (not mandatory but encouraged)
rlhui pushed a commit that referenced this pull request Aug 27, 2025
Fix incorrect checking introduced by #22337
When invoking prefix_list script in supported roles, the checking function should return rather than exit 0. Exiting 0 would directly terminate the script
mssonicbld added a commit to mssonicbld/sonic-buildimage that referenced this pull request Aug 28, 2025
<!--
     Please make sure you've read and understood our contributing guidelines:
     https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

     ** Make sure all your commits include a signature generated with `git commit -s` **

     If this is a bug fix, make sure your description includes "fixes #xxxx", or
     "closes #xxxx" or "resolves #xxxx"

     Please provide the following information:
-->

#### Why I did it
Fix incorrect checking introduced by sonic-net#22337
When invoking prefix_list script in supported roles, the checking function should return rather than exit 0. Exiting 0 would directly terminate the script

##### Work item tracking
- Microsoft ADO **(number only)**:

#### How I did it
When invoking prefix_list script in supported roles, the checking function should return rather than exit 0

#### How to verify it
Run the script

<!--
If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012.
-->

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 202205
- [ ] 202211
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [ ] 202505

#### Tested branch (Please provide the tested image version)

<!--
- Please provide tested image version
- e.g.
- [x] 20201231.100
-->

- [ ] <!-- image version 1 -->
- [ ] <!-- image version 2 -->

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->

<!--
 Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.
-->

#### Link to config_db schema for YANG module changes
<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

#### A picture of a cute animal (not mandatory but encouraged)
mssonicbld added a commit that referenced this pull request Sep 1, 2025
<!--
 Please make sure you've read and understood our contributing guidelines:
 https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

 failure_prs.log skip_prs.log Make sure all your commits include a signature generated with `git commit -s` **

 If this is a bug fix, make sure your description includes "fixes #xxxx", or
 "closes #xxxx" or "resolves #xxxx"

 Please provide the following information:
-->

#### Why I did it
Fix incorrect checking introduced by #22337
When invoking prefix_list script in supported roles, the checking function should return rather than exit 0. Exiting 0 would directly terminate the script

##### Work item tracking
- Microsoft ADO **(number only)**:

#### How I did it
When invoking prefix_list script in supported roles, the checking function should return rather than exit 0

#### How to verify it
Run the script

<!--
If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012.
-->

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 202205
- [ ] 202211
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [ ] 202505

#### Tested branch (Please provide the tested image version)

<!--
- Please provide tested image version
- e.g.
- [x] 20201231.100
-->

- [ ] <!-- image version 1 -->
- [ ] <!-- image version 2 -->

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->

<!--
 Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.
-->

#### Link to config_db schema for YANG module changes
<!--
Provide a link to config_db schema for the table for which YANG model
is defined
Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md
-->

#### A picture of a cute animal (not mandatory but encouraged)
FengPan-Frank pushed a commit to FengPan-Frank/sonic-buildimage that referenced this pull request Dec 4, 2025
Fix incorrect checking introduced by sonic-net#22337
When invoking prefix_list script in supported roles, the checking function should return rather than exit 0. Exiting 0 would directly terminate the script

Signed-off-by: Feng Pan <fenpan@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants