Skip to content

[DT2] Support LowerSpineRouter as subtype#22465

Closed
bingwang-ms wants to merge 4 commits intosonic-net:masterfrom
bingwang-ms:support_lt2_in_minigraph_parser
Closed

[DT2] Support LowerSpineRouter as subtype#22465
bingwang-ms wants to merge 4 commits intosonic-net:masterfrom
bingwang-ms:support_lt2_in_minigraph_parser

Conversation

@bingwang-ms
Copy link
Contributor

@bingwang-ms bingwang-ms commented Apr 29, 2025

Why I did it

This PR is to support LowerSpineRouter as a subtype in minigraph parser.

Work item tracking
  • Microsoft ADO 32614515:

How I did it

If a device is SpineRouter, then try to parse SubType from minigraph.

How to verify it

The change is verified by UT.

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

This PR is to support LowerSpineRouter as a subtype in minigraph parser.

Link to config_db schema for YANG module changes

No YANG model change.

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

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@bingwang-ms bingwang-ms requested review from arlakshm and yxieca April 29, 2025 01:19
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bingwang-ms bingwang-ms marked this pull request as draft April 29, 2025 02:18
@bingwang-ms
Copy link
Contributor Author

minigraph.py is protected and can't be updated.

@bingwang-ms bingwang-ms marked this pull request as ready for review May 9, 2025 22:33
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

arlakshm
arlakshm previously approved these changes May 10, 2025
elif macsec_enabled == 'False':
results['DEVICE_METADATA']['localhost']['subtype'] = 'DownstreamLC'
# If a devive is SpineRouter but not a chassis, then it's a lower spine router
if not is_minigraph_for_chassis(chassis_type):
Copy link
Contributor

@arlakshm arlakshm May 10, 2025

Choose a reason for hiding this comment

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

this may not work. The minigraph of the upperT2 not is_minigraph_for_chassis(chassis_type) will also be True.

Copy link
Contributor

Choose a reason for hiding this comment

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

This may not break chassis, because in the minigraph generated in sonic-mgmt for T2 not is_minigraph_for_chassis(chassis_type) will be True

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 may not work. The minigraph of the upperT2 not is_minigraph_for_chassis(chassis_type) will also be True.

For UT2, results['DEVICE_METADATA']['localhost']['type'] will be UpperSpineRouter. So we should be good.

@bingwang-ms
Copy link
Contributor Author

@arlakshm Seems checking is_minigraph_for_chassis(chassis_type) is not always reliable. PR test failed because we don't have chassis_type in sample-chassis-packet-lc-graph.xml. Is it a miss or by design? Do we have any other reliable way to distinguish a chassis T2 and Non-chassis T2?

@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).

bingwang-ms added a commit to Azure/sonic-buildimage-msft that referenced this pull request May 16, 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
Cherry-pick from
sonic-net/sonic-buildimage#22465
This PR is to support `LowerSpineRouter` as a subtype in minigraph
parser.

##### Work item tracking
- Microsoft ADO **32614515**:

#### How I did it
If a device is `SpineRouter`, then try to parse `SubType` from
minigraph.

#### How to verify it
The change is verified by UT.

<!--
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
- [ ] 202305

#### 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.
-->
This PR is to support `LowerSpineRouter` as a subtype in minigraph
parser.

#### 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
-->
No YANG model change.

#### A picture of a cute animal (not mandatory but encouraged)
@bingwang-ms
Copy link
Contributor Author

Included in 202503 by PR Azure/sonic-buildimage-msft#1130

@bingwang-ms
Copy link
Contributor Author

Closing this PR as it's not needed any more

@bingwang-ms bingwang-ms closed this Dec 2, 2025
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.

3 participants