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

[multi-asic]: Skip must field check in config_db for default namespace in multi-asic platform #10617

Closed

Conversation

SuvarnaMeenakshi
Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi commented Apr 20, 2022

Signed-off-by: Suvarna Meenakshi sumeenak@microsoft.com

Why I did it

For multi-asic VS testbed bring up, minigraph is copied to DUT and topology service is started.
topology service uses hwsku from minigraph to run the topology script for the right hwsku, using the command below:

  HWSKU=`sonic-cfggen -m /etc/sonic/minigraph.xml -v "DEVICE_METADATA['localhost']['hwsku']" 2>&1`

The above command fails after PR 10228.
After #10228; https://github.com/Azure/sonic-buildimage/blob/master/files/image_config/topology/topology.sh#L14 fails to get hwsku as the must-have fields are not present in default namespace for multi-asic platform.

How I did it

Check must-have fields only for single asic or only on namespaces in multi-asic platform.

How to verify it

Able to generate config_db in multi-asic platform without warning message.
Able to start topology service in multi-asic vs platform.

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Link to config_db schema for YANG module changes

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

default namepsace in multi-asic platform.

Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
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.

lgtm

@qiluo-msft
Copy link
Collaborator

The _must_field_by_yang is reverted, so this PR is not needed any more. You may check conflicting for details.

@ganglyu
Copy link
Contributor

ganglyu commented Apr 27, 2022

If multi-asic platform has a port without lanes and speed, maybe we should modify sonic-port.yang?
@SuvarnaMeenakshi Can you explain the scenario?

@SuvarnaMeenakshi
Copy link
Contributor Author

SuvarnaMeenakshi commented Apr 28, 2022

platform has a port without lanes and speed, maybe we should modify sonic-port.yang

In multi-asic, currently the PORT table is generated for default namespace, though the default namespace does not have any asic or PORT associated. PORT table is generated because minigraph.xml contains PORTS for the device. But as there is no port_config.ini for default namespace, there is no lanes generated.
@arlakshm Currently lanes and speed are mandatory fields in sonic-port.yang, can this be made non-mandatory fields?

@SuvarnaMeenakshi
Copy link
Contributor Author

closing this PR as #10683 reverts the PR #10228.

@ganglyu
Copy link
Contributor

ganglyu commented Apr 28, 2022

platform has a port without lanes and speed, maybe we should modify sonic-port.yang

In multi-asic, currently the PORT table is generated for default namespace, though the default namespace does not have any asic or PORT associated. PORT table is generated because minigraph.xml contains PORTS for the device. But as there is no port_config.ini for default namespace, there is no lanes generated. @arlakshm Currently lanes and speed are mandatory fields in sonic-port.yang, can this be made non-mandatory fields?

Maybe we can delete PORT table in this case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants