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.

Sign up for GitHub

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

Jump to bottom

Switchport Modes Port & Port Channel Yang Model Configurations #13580

Merged
merged 250 commits into from
Jan 26, 2024
Merged

Switchport Modes Port & Port Channel Yang Model Configurations #13580

merged 250 commits into from
Jan 26, 2024
+276 −106

Conversation

ridahanif96
Copy link
Contributor

@ridahanif96 ridahanif96 commented Feb 1, 2023 •
edited
Loading

Why I did it

  • Modified "sonic-port.yang" for adding support in Port Yang model for the "mode" attribute for adding port modes

  • Modified "sonic-portchannel.yang" for adding support in Port Channel Yang model for the "mode" attribute for adding port modes

  • Updated tests for these modifications

How to verify it

  • Added support to align SONiC yang with Config_db

@ham-xa ham-xa mentioned this pull request Feb 1, 2023
Merged
@ridahanif96 ridahanif96 marked this pull request as ready for review February 2, 2023 18:28
@zhangyanzhao zhangyanzhao added the YANG YANG model related changes label Feb 2, 2023
@zhangyanzhao
Copy link
Collaborator

Please help on the review. Thanks.

@ganglyu
Copy link
Contributor

ganglyu commented Feb 4, 2023

Please fix unit test:

libyang[0]: Invalid keyword "routed|access|trunk.". (path: /sonic-port:sonic-port/PORT/PORT_LIST/mode)
libyang[0]: Module "sonic-port" parsing failed.
libyang[0]: Importing "sonic-port" module into "sonic-buffer-port-egress-profile-list" failed.
libyang[0]: Module "sonic-buffer-port-egress-profile-list" parsing failed.

ganglyu
ganglyu previously approved these changes Feb 6, 2023
View reviewed changes
@zhangyanzhao
Copy link
Collaborator

This will target 202305 release.

@zhangyanzhao
Copy link
Collaborator

@ridahanif96 will fix the build failure.

ganglyu
ganglyu reviewed Jan 3, 2024
View reviewed changes
src/sonic-yang-models/yang-models/sonic-vlan.yang Outdated
@@ -208,6 +210,7 @@ module sonic-vlan {
type inet:ipv6-address;
}


Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary empty line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

ganglyu
ganglyu reviewed Jan 3, 2024
View reviewed changes
src/sonic-yang-models/yang-models/sonic-vlan.yang Outdated
@@ -221,6 +224,8 @@ module sonic-vlan {
leaf mac {
type yang:mac-address;
}


Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary empty line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

ganglyu
ganglyu reviewed Jan 3, 2024
View reviewed changes
src/sonic-yang-models/yang-models/sonic-vlan.yang Outdated
@@ -240,22 +245,29 @@ module sonic-vlan {
}
}

leaf port {
leaf port
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't change format here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

ganglyu
ganglyu reviewed Jan 3, 2024
View reviewed changes
src/sonic-yang-models/yang-models/sonic-vlan.yang Outdated
/* key elements are mandatory by default */
type union {
type leafref {
type leafref
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't change format here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

ganglyu
ganglyu reviewed Jan 7, 2024
View reviewed changes
src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json Outdated
@@ -89,7 +89,7 @@
"description": "Ethernet0",
"mtu": 9000,
"name": "Ethernet0",
"speed": 25000
"speed": 25000
Copy link
Contributor

Choose a reason for hiding this comment

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

space at end of line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed!

ganglyu
ganglyu reviewed Jan 7, 2024
View reviewed changes
src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json Outdated
@@ -193,7 +193,7 @@
"description": "Ethernet1",
"mtu": 9000,
"name": "Ethernet1",
"speed": 25000
"speed": 25000
Copy link
Contributor

Choose a reason for hiding this comment

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

space at end of line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed!

ganglyu
ganglyu approved these changes Jan 10, 2024
View reviewed changes
Copy link
Contributor

@ganglyu ganglyu left a comment

Choose a reason for hiding this comment

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

lgtm

@ridahanif96 ridahanif96 mentioned this pull request Jan 11, 2024
Merged
@ridahanif96
Copy link
Contributor Author

@qiluo-msft please help merge this PR! Thanks!

@ridahanif96
Copy link
Contributor Author

@qiluo-msft can you pls help merge this PR! Thanks

@zongbintu
Copy link

@qiluo-msft can you pls help merge this PR! Thanks

We need the port mode change.
Thanks for the PR.
May I ask when this PR is planned to be merge.

@ridahanif96
Copy link
Contributor Author

@qiluo-msft pls merge this PR

@qiluo-msft qiluo-msft merged commit 88f80fb into sonic-net:master Jan 26, 2024
18 checks passed
@zhangyanzhao
Copy link
Collaborator

This is disruptive change per @qiluo-msft

qiluo-msft
qiluo-msft reviewed Mar 21, 2024
View reviewed changes
src/sonic-yang-models/yang-models/sonic-port.yang
leaf mode {
description "SwitchPort Modes possible values are routed|access|trunk. Default value for mode is routed";
type stypes:switchport_mode;
default "routed";
Copy link
Collaborator

Choose a reason for hiding this comment

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

The default value will introduce disruptive change. The current behavior is conditional "default value".

@ridahanif96 ridahanif96 mentioned this pull request Mar 21, 2024
Merged
qiluo-msft pushed a commit that referenced this pull request Mar 27, 2024
@ridahanif96
fix for yang disreptive change (#18428)
17d728e 
#### Why I did it
Removed Default Mode value from sonic-port.yang & sonic-portchannel.yang to avoid disruptive change

#### How I did it
This PR is created in reference with [PR](#13580)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@venkatmahalingam venkatmahalingam venkatmahalingam approved these changes

@ganglyu ganglyu ganglyu approved these changes

@isabelmsft isabelmsft Awaiting requested review from isabelmsft

@yxieca yxieca Awaiting requested review from yxieca

Assignees
No one assigned
Labels
YANG YANG model related changes
Projects
yang
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@ridahanif96 @zhangyanzhao @ganglyu @mssonicbld @qiluo-msft @anonymity-web @zongbintu @venkatmahalingam