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

[yang] interface: add missing mac_addr leaf #20968

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[yang] interface: add missing mac_addr leaf #20968

wants to merge 1 commit into from

Conversation

bradh352
Copy link
Contributor

@bradh352 bradh352 commented Nov 29, 2024 •
edited
Loading

Why I did it

YANG configuration verification fails during config replace.

How I did it

mac_addr support was added in the INTERFACE, PORTCHANNEL_INTERFACE, and VLAN_INTERFACE tables by sonic-net/sonic-swss#814 in 2020 but the YANG models were never updated to reflect that change. Updated yang model to reflect change.

How to verify it

Add mac_addr to the various INTERFACE tables then run config replace and it succeeds.

Work item tracking

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305
  • 202411

Marked all releases that contained the change.

Tested branch (Please provide the tested image version)

master as of 20241109

Description for the changelog

YANG INTERFACE: add missing mac_addr leaf

Link to config_db schema for YANG module changes

There is no actual INTERFACE section in the docs (there probably should be).

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

Fixes #14253
Signed-off-by: Brad House (@bradh352)

@bradh352 bradh352 requested a review from qiluo-msft as a code owner November 29, 2024 13:34
@bradh352 bradh352 changed the title YANG sonic-interface: add missing mac_addr leaf [yang] interface: add missing mac_addr leaf Nov 29, 2024
@bradh352 bradh352 force-pushed the yang-macaddr branch 5 times, most recently from fc17991 to 546e620 Compare November 29, 2024 16:03
@bradh352 bradh352 mentioned this pull request Nov 29, 2024
Open
@bradh352
Copy link
Contributor Author

test failures are unrelated to this PR

@bradh352
Copy link
Contributor Author

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines Azure Pipelines
Copy link

Commenter does not have sufficient privileges for PR 20968 in repo sonic-net/sonic-buildimage

dgsudharsan
dgsudharsan suggested changes Dec 6, 2024
View reviewed changes
Copy link
Collaborator

@dgsudharsan dgsudharsan left a comment

Choose a reason for hiding this comment

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

Can you please add this config to sample_config_db.json as well us update the schema in Configuration.md?
In addition can you please add a test case covering MAC address with capital hex letters to see if it pass?

@dgsudharsan dgsudharsan added the YANG YANG model related changes label Dec 6, 2024
@bradh352
Copy link
Contributor Author

bradh352 commented Dec 6, 2024

@dgsudharsan I guess I'll need to break out an INTERFACE section since such a thing doesn't exist in the current docs and try to migrate the other info into there.

@bradh352
Copy link
Contributor Author

bradh352 commented Dec 6, 2024

@dgsudharsan ready for review

@dgsudharsan
Copy link
Collaborator

@bradh352 I think there is some problem with the diff in configuration.md. Due to formatting, it is showing a huge diff which will be very hard to manage especially when we plan to cherry-pick changes to different branches. Can you please ensure the diff shows only the changed content?

@bradh352
Copy link
Contributor Author

bradh352 commented Dec 6, 2024

@dgsudharsan I thought you might say that. I'm really frustrated with the markdown format used (inconsistently and incorrectly) in that doc, so I fixed a lot of it. It was worth a shot to clean it up and actually make, say, the Table of Contents links actually work. Oh well. I'll revert that stuff and just keep the changes relevant to this PR.

@bradh352
Copy link
Contributor Author

bradh352 commented Dec 6, 2024

@dgsudharsan hopefully that's acceptable now, please review

@bradh352
Copy link
Contributor Author

bradh352 commented Dec 6, 2024

found a couple minor bugs, one unexpected as I didn't realize the double-conversion flow for the sample db. those should be corrected now.

@bradh352
Copy link
Contributor Author

bradh352 commented Dec 6, 2024

@dgsudharsan I moved the most important parts of my markdown cleanup into PR #21072 it would be great if that could be merged. I provided the links to render mine vs original, you'll see the table of contents works on mine.

@bradh352
Copy link
Contributor Author

rebased to force rebuild to see if build general sonic CI build/test issues have been resolved

@bradh352
Copy link
Contributor Author

@dgsudharsan @qiluo-msft please merge

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bradh352
Copy link
Contributor Author

rebased for merge conflict after PR #21072

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bradh352
Copy link
Contributor Author

@qiluo-msft please approve and merge

@bradh352
Copy link
Contributor Author

@qiluo-msft @dgsudharsan what's the status of merging this? seems pretty trivial at this point. All tests requested were added, everything is passing.

@dgsudharsan
Copy link
Collaborator

@bradh352 I don't have merge permissions. It should be @qiluo-msft or @lguohan

bluecmd pushed a commit to kamelnetworks/sonic-buildimage that referenced this pull request Dec 24, 2024
@bradh352 @bluecmd
kamel: YANG sonic-interface: add missing mac_addr leaf (PR sonic-net#…
3730972 
…20968)

mac_addr support was added in the INTERFACES table by
sonic-net/sonic-swss#814 in 2020
but the YANG models were never updated to reflect that change.

Signed-off-by: Brad House (@bradh352)
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bradh352
YANG sonic-interface: add missing mac_addr leaf
ffbbb37 
mac_addr support was added in the INTERFACES table by
sonic-net/sonic-swss#814 in 2020
but the YANG models were never updated to reflect that change.

Signed-off-by: Brad House (@bradh352)
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Dec 24, 2024
@bradh352
YANG sonic-interface: add missing mac_addr leaf (PR sonic-net#20968)
71f0d62 
mac_addr support was added in the INTERFACES table by
sonic-net/sonic-swss#814 in 2020
but the YANG models were never updated to reflect that change.

Signed-off-by: Brad House (@bradh352)
bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Dec 24, 2024
@bradh352
YANG sonic-interface: add missing mac_addr leaf (PR sonic-net#20968)
a8ce506 
mac_addr support was added in the INTERFACES table by
sonic-net/sonic-swss#814 in 2020
but the YANG models were never updated to reflect that change.

Signed-off-by: Brad House (@bradh352)
bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Dec 24, 2024
@bradh352
YANG sonic-interface: add missing mac_addr leaf (PR sonic-net#20968)
7ba2d3b 
mac_addr support was added in the INTERFACES table by
sonic-net/sonic-swss#814 in 2020
but the YANG models were never updated to reflect that change.

Signed-off-by: Brad House (@bradh352)
github-actions bot pushed a commit to bradh352/sonic-buildimage that referenced this pull request Dec 24, 2024
@bradh352
YANG sonic-interface: add missing mac_addr leaf (PR sonic-net#20968)
5afc7bf 
mac_addr support was added in the INTERFACES table by
sonic-net/sonic-swss#814 in 2020
but the YANG models were never updated to reflect that change.

Signed-off-by: Brad House (@bradh352)
bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Dec 24, 2024
@bradh352
YANG sonic-interface: add missing mac_addr leaf (PR sonic-net#20968)
8eb3f5f 
mac_addr support was added in the INTERFACES table by
sonic-net/sonic-swss#814 in 2020
but the YANG models were never updated to reflect that change.

Signed-off-by: Brad House (@bradh352)
bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Dec 24, 2024
@bradh352
YANG sonic-interface: add missing mac_addr leaf (PR sonic-net#20968)
e6fa442 
mac_addr support was added in the INTERFACES table by
sonic-net/sonic-swss#814 in 2020
but the YANG models were never updated to reflect that change.

Signed-off-by: Brad House (@bradh352)
bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Dec 24, 2024
@bradh352
YANG sonic-interface: add missing mac_addr leaf (PR sonic-net#20968)
191f9a5 
mac_addr support was added in the INTERFACES table by
sonic-net/sonic-swss#814 in 2020
but the YANG models were never updated to reflect that change.

Signed-off-by: Brad House (@bradh352)
bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Dec 24, 2024
@bradh352
YANG sonic-interface: add missing mac_addr leaf (PR sonic-net#20968)
50af577 
mac_addr support was added in the INTERFACES table by
sonic-net/sonic-swss#814 in 2020
but the YANG models were never updated to reflect that change.

Signed-off-by: Brad House (@bradh352)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgsudharsan dgsudharsan dgsudharsan approved these changes

@qiluo-msft qiluo-msft Awaiting requested review from qiluo-msft qiluo-msft is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
YANG YANG model related changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[yang] interface yang model is missing mac_addr key
3 participants
@bradh352 @dgsudharsan @mssonicbld