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

Parse platform.json, hwsku.json for generate minigraph #2887

Merged
merged 2 commits into from
Feb 17, 2021

Conversation

dmytroxshevchuk
Copy link
Contributor

@dmytroxshevchuk dmytroxshevchuk commented Jan 29, 2021

Description of PR

After transition from port_config.ini to platform.json, hwsku.json, we can`t fully deprecate and remove port_config.ini cause of port_mgmt/ansible use port_config.ini for generate minigraph. So currently we have configuration(platform.json, hwsku.json) using by sonic for configure ports and configuration(port_config.ini) using by sonic_mgmt for generate minigraph. So changes in platform.json and hwsku.json will not affect minigraph because of port_config.ini so we have configuration mismatch.
PR add functionality to sonic_mgmt for parse platform.json and hwsku.json and use this configuration for generation minigraph.

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

What is the motivation for this PR?

  • fully deprecate and remove port_config.ini
  • avoid mismatch configurations between port_config.ini and platform.json, hwsku.json

How did you do it?

Add functionality for parsing platform.json, hwsku.json and use this configuration for generate minigraph.

How did you verify/test it?

Compare generated minigraph/config_db configurations with platform.json, hwsku.json.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

@dmytroxshevchuk dmytroxshevchuk changed the title add platform.json, hwsku.json support WIP: add platform.json, hwsku.json support Jan 29, 2021
@dmytroxshevchuk dmytroxshevchuk changed the title WIP: add platform.json, hwsku.json support WIP: parse platform.json, hwsku.json in case generate minigraph Jan 29, 2021
@dmytroxshevchuk dmytroxshevchuk changed the title WIP: parse platform.json, hwsku.json in case generate minigraph WIP: parse platform.json, hwsku.json for generate minigraph Jan 29, 2021
@dmytroxshevchuk dmytroxshevchuk changed the title WIP: parse platform.json, hwsku.json for generate minigraph Parse platform.json, hwsku.json for generate minigraph Jan 29, 2021
@SavchukRomanLv
Copy link
Contributor

@wangxin @yxieca could you please take a look? Thank you in advance!

@yxieca yxieca requested review from jleveque, theasianpianist and a team February 2, 2021 16:59
@yxieca
Copy link
Collaborator

yxieca commented Feb 2, 2021

@jleveque @theasianpianist , the change looks good to me. Can you guys also take a look?

ansible/library/port_alias.py Outdated Show resolved Hide resolved
ansible/library/port_alias.py Outdated Show resolved Hide resolved
@theasianpianist
Copy link
Contributor

Please use the testmg.sh script to verify your changes.

@yxieca yxieca merged commit ec8185a into sonic-net:master Feb 17, 2021
yxieca added a commit that referenced this pull request Feb 19, 2021
@yxieca
Copy link
Collaborator

yxieca commented Feb 19, 2021

@dmytroxshevchuk I need to revert this PR. We found following issue with platform.json as grant truth:

For example, on a Mellanox 2700 device, if we want to deploy the 100G topology:

The platform.json's alias table:
"alias_at_lanes": "etp1a, etp1b, etp1c, etp1d",

ept1a would be used for Ethernet0's alias and that causes minigraph generation error.

With non-breakout topologies, the alias should be etp1.

yxieca added a commit that referenced this pull request Feb 20, 2021
@dmytroxshevchuk
Copy link
Contributor Author

@yxieca issue with platform.json was fixed in sonic-buildimage
issue for that: sonic-net/sonic-buildimage#6024
PR for that: sonic-net/sonic-buildimage#6831
So I created another PR with the same changes considering on Mellanox platform.json issue.
Please take a look:
#3469

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.

5 participants