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

[sonic-cfggen]: Update UT to add port lanes #10362

Merged
merged 14 commits into from
Apr 4, 2022

Conversation

ganglyu
Copy link
Contributor

@ganglyu ganglyu commented Mar 28, 2022

Signed-off-by: Gang Lv [email protected]

Why I did it

Need to run yang validation for sonic-cfggen unit test, and many unit test does not provide lanes for port table.

How I did it

Update port config file.

How to verify it

Run sonic-cfggen unit test,
Use below PR to verify
#10228

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

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

Description for the changelog

Fix #9553

Link to config_db schema for YANG module changes

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

@ganglyu ganglyu added the sonic-cfggen SONiC Configuration Generator Tool label Mar 28, 2022
@ganglyu ganglyu requested review from wen587 and qiluo-msft March 28, 2022 04:38
@ganglyu ganglyu requested a review from lguohan as a code owner March 28, 2022 04:38
ganglyu added 2 commits March 28, 2022 15:22
Signed-off-by: Gang Lv [email protected]
Signed-off-by: Gang Lv [email protected]
@ganglyu ganglyu marked this pull request as draft March 28, 2022 10:02
@ganglyu ganglyu marked this pull request as ready for review March 28, 2022 10:33
ganglyu added 2 commits March 28, 2022 23:01
Signed-off-by: Gang Lv [email protected]
Signed-off-by: Gang Lv [email protected]
@ganglyu
Copy link
Contributor Author

ganglyu commented Mar 28, 2022

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ganglyu ganglyu marked this pull request as draft March 29, 2022 02:43
@ganglyu ganglyu marked this pull request as ready for review March 29, 2022 06:57
@ganglyu
Copy link
Contributor Author

ganglyu commented Mar 29, 2022

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

<AlternateSpeeds i:nil="true"/>
<EnableFlowControl>true</EnableFlowControl>
<Index>1</Index>
<InterfaceName>Ethernet4</InterfaceName>
Copy link
Collaborator

@qiluo-msft qiluo-msft Mar 31, 2022

Choose a reason for hiding this comment

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

Ethernet4

Why Ethernet1/../4 are needed? I checked device/arista/x86_64-arista_7050_qx32s/Arista-7050QX-32S/port_config.ini. And seems they are not used. And alias Ethernet4 will conflict with SONiC port name. I am not sure if it is problem. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no speed for these ports.
We shouldn't update device/arista/x86_64-arista_7050_qx32s/Arista-7050QX-32S/port_config.ini, so I update this xml.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If port speed is not mandatory, no need to fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed this change

@qiluo-msft
Copy link
Collaborator

qiluo-msft commented Mar 31, 2022

    <EndPort>Ethernet1</EndPort>

Do you need to change this line at the same time?


In reply to: 1084154358


In reply to: 1084154358


In reply to: 1084154358


Refers to: src/sonic-config-engine/tests/sample_graph.xml:115 in f313124. [](commit_id = f313124, deletion_comment = False)

@ganglyu
Copy link
Contributor Author

ganglyu commented Mar 31, 2022

    <EndPort>Ethernet1</EndPort>

Do you need to change this line at the same time?

Refers to: src/sonic-config-engine/tests/sample_graph.xml:115 in f313124. [](commit_id = f313124, deletion_comment = False)

There's no Ethernet1 in port_config.ini, so I replace it with Ethernet4

@ganglyu ganglyu changed the title [sonic-cfggen]: Update UT to add port lanes and speed [sonic-cfggen]: Update UT to add port lanes Mar 31, 2022
@ganglyu ganglyu requested a review from qiluo-msft March 31, 2022 22:53
@ganglyu ganglyu requested a review from qiluo-msft April 1, 2022 12:18
@ganglyu ganglyu marked this pull request as draft April 1, 2022 23:31
output = self.run_script(argument)

#self.maxDiff = None
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 2, 2022

Choose a reason for hiding this comment

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

#self.maxDiff = None

Remove dead code. #Closed

@ganglyu ganglyu marked this pull request as ready for review April 2, 2022 01:17
Signed-off-by: Gang Lv [email protected]
@ganglyu ganglyu requested a review from qiluo-msft April 2, 2022 01:25
@@ -877,6 +879,7 @@ def test_minigraph_voq_inband_port(self):
self.assertDictEqual(
output_dict['Ethernet-IB0'],
{'lanes': '133',

'alias': 'Recycle0',
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 3, 2022

Choose a reason for hiding this comment

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

Could you remove the unnecessary new line? #Closed

@ganglyu ganglyu requested a review from qiluo-msft April 3, 2022 05:09
output = json.loads(self.run_script(argument))
self.assertDictEqual(output,
{"Ethernet0": { "admin_status": "up", "alias": "Ethernet1/1", "asic_port_name": "Eth0-ASIC0", "description": "01T2:Ethernet1:config_db", "index": "0", "lanes": "33,34,35,36", "mtu": "9100", "tpid": "0x8100", "pfc_asym": "off", "role": "Ext", "speed": "40000", "autoneg": "on" },
"Ethernet4": { "admin_status": "up", "alias": "Ethernet1/2", "asic_port_name": "Eth1-ASIC0", "description": "01T2:Ethernet2:config_db", "index": "1", "lanes": "29,30,31,32", "mtu": "9100", "tpid": "0x8100", "pfc_asym": "off", "role": "Ext", "speed": "40000", "autoneg": "on" },
"Ethernet8": { "admin_status": "up", "alias": "Ethernet1/3", "asic_port_name": "Eth2-ASIC0", "description": "Ethernet1/3:config_db", "index": "2", "lanes": "41,42,43,44", "mtu": "9100", "tpid": "0x8100", "pfc_asym": "off", "role": "Ext", "speed": "40000" },
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 3, 2022

Choose a reason for hiding this comment

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

Why admin_status disappear? Is it expected? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The actual output does not have admin_status

@ganglyu ganglyu merged commit 13aa233 into sonic-net:master Apr 4, 2022
@ganglyu ganglyu added the Request for 202111 Branch For PRs being requested for 202111 branch label Apr 14, 2022
SuvarnaMeenakshi added a commit to SuvarnaMeenakshi/sonic-buildimage that referenced this pull request Apr 14, 2022
as it was before PR sonic-net#10362.

Signed-off-by: Suvarna Meenakshi <[email protected]>
judyjoseph pushed a commit that referenced this pull request Apr 18, 2022
Why I did it
Need to run yang validation for sonic-cfggen unit test, and many unit test does not provide lanes for port table.

How I did it
Update port config file.

How to verify it
Run sonic-cfggen unit test,
Use below PR to verify
#10228

Signed-off-by: Gang Lv [email protected]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Included in 202111 Branch Request for 202111 Branch For PRs being requested for 202111 branch sonic-cfggen SONiC Configuration Generator Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[yang-models] port validation issue, lanes is mandatory
4 participants