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

[yang] update crm yang model for dynamic port breakout #6395

Merged
merged 3 commits into from
Feb 24, 2021

Conversation

dmytroxshevchuk
Copy link
Contributor

@dmytroxshevchuk dmytroxshevchuk commented Jan 8, 2021

- Why I did it
Fix DPB crash caused of new entries in config db.
Details: #6331

- How I did it
Updated crm yang model

- How to verify it

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

  • 201811
  • 201911
  • 202006
  • 202012

- Description for the changelog

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

@dmytroxshevchuk dmytroxshevchuk changed the title DPB: update crm yang model for dynamic port breakout yang: update crm yang model for dynamic port breakout Jan 8, 2021
@dmytroxshevchuk dmytroxshevchuk changed the title yang: update crm yang model for dynamic port breakout [yang] update crm yang model for dynamic port breakout Jan 8, 2021
@dmytroxshevchuk
Copy link
Contributor Author

retest vsimage please

@@ -293,6 +293,66 @@ module sonic-crm {
type threshold;
}

leaf dnat_entry_threshold_type {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Changes looks good to me, but we need to add tests.


To add Test for sonic-yang-module:

1.) Add example Config at:
https://github.com/Azure/sonic-buildimage/blob/e52581e919c95e861cf630c9c7e1a840fc5e8ebd/src/sonic-yang-models/tests/yang_model_tests/yangTest.json#L2

Example config should be in sonic-yang format.

2.) Add matching detail and expected error string at:
https://github.com/Azure/sonic-buildimage/blob/e52581e919c95e861cf630c9c7e1a840fc5e8ebd/src/sonic-yang-models/tests/yang_model_tests/test_yang_model.py#L52

If You add new leaf in YANG Models:
Then add corresponding config in same format as in configDB.json at:
https://github.com/Azure/sonic-buildimage/blob/4cf9316ec349f80deceb0d7665b0aaf85bfe2599/src/sonic-yang-models/tests/yang_model_tests/yangTest.json#L1038

And build both:
sonic_yang_mgmt-1.0-py3-none-any.whl
sonic_yang_models-1.0-py3-none-any.whl

Build tests will make sure, Translation works for new leafs in YANG models.


@dmytroxshevchuk dmytroxshevchuk force-pushed the dpb_update_crm_yang branch 4 times, most recently from a7f850e to 27326cd Compare January 20, 2021 09:44
@dmytroxshevchuk
Copy link
Contributor Author

@praveen-li done, also fixed small issues/typos in CRM, please check

@praveen-li
Copy link
Collaborator

@praveen-li done, also fixed small issues/typos in CRM, please check

It will be great if we can avoid the force push until necessary, thanks.

@@ -186,6 +186,10 @@ def initTest(self):
about high threshold being lower than low threshold.',
'eStr': ['high_threshold should be more than low_threshold']
},
'CRM_WITH_WRONG_THRESHOLD_TYPE': {
'desc': 'CRM_WITH_WRONG_THRESHOLD_TYPE must condition failure.',
'eStr': self.defaultYANGFailure['None']
Copy link
Collaborator

Choose a reason for hiding this comment

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

If it is "must condition failure" then estr should be self.defaultYANGFailure['must'] + field name.

@dmytroxshevchuk
Copy link
Contributor Author

@praveen-li done, please check

@@ -1743,6 +1953,15 @@
"acl_counter_high_threshold": "85",
"acl_counter_low_threshold": "70",
"acl_counter_threshold_type": "percentage",
"snat_entry_threshold_type": "percentage",
Copy link
Collaborator

Choose a reason for hiding this comment

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

kindly remove it from here because this config is just to test an unknown table.

@@ -200,7 +200,7 @@ module sonic-crm {
leaf ipv6_neighbor_high_threshold {
must "(current() > ../ipv6_neighbor_low_threshold)"
{
error-message "high_threshold should be more that low_threshold";
error-message "high_threshold should be more than low_threshold";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool, Thanks for fixing this.

praveen-li
praveen-li previously approved these changes Jan 22, 2021
Copy link
Collaborator

@praveen-li praveen-li left a comment

Choose a reason for hiding this comment

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

LGTM, just remove extra config from CRM.

@dmytroxshevchuk
Copy link
Contributor Author

@praveen-li removed extra fields from CRM yang test model. Please quickly double check and I hope we can merge it.

@dmytroxshevchuk
Copy link
Contributor Author

retest vsimage please

praveen-li
praveen-li previously approved these changes Jan 27, 2021
Copy link
Collaborator

@praveen-li praveen-li left a comment

Choose a reason for hiding this comment

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

LGTM, please make sure there are no test failures in any build, thanks.

@dmytroxshevchuk
Copy link
Contributor Author

LGTM, please make sure there are no test failures in any build, thanks.

Looks like tests are working and not have any failures.

@dmytroxshevchuk
Copy link
Contributor Author

retest vsimage please

@dmytroxshevchuk
Copy link
Contributor Author

@lguohan I can`t restart CI using "retest vsimage please", "retest this please". How can I restart vsimage?

@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vadymhlushko-mlnx
Copy link
Contributor

retest vsimage please

1 similar comment
@liat-grozovik
Copy link
Collaborator

retest vsimage please

@anshuv-mfst anshuv-mfst added the YANG YANG model related changes label Feb 4, 2021
@lguohan lguohan added this to In progress in yang Feb 5, 2021
@liat-grozovik
Copy link
Collaborator

retest vsimage please

@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@akokhan
Copy link
Contributor

akokhan commented Feb 11, 2021

retest vsimage please

2 similar comments
@arlakshm
Copy link
Contributor

retest vsimage please

@liat-grozovik
Copy link
Collaborator

retest vsimage please

yang automation moved this from In progress to Review in progress Feb 23, 2021
@dmytroxshevchuk
Copy link
Contributor Author

dmytroxshevchuk commented Feb 24, 2021

@lguohan @liat-grozovik @praveen-li looks like test passed, please merge.

@zhenggen-xu
Copy link
Collaborator

@praveen-li can you sign off?

yang automation moved this from Review in progress to Reviewer approved Feb 24, 2021
@liat-grozovik liat-grozovik merged commit 3abd216 into sonic-net:master Feb 24, 2021
yang automation moved this from Reviewer approved to Done Feb 24, 2021
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
- Why I did it
Fix DPB crash caused of new entries in config db.
Details: sonic-net#6331

- How I did it
Updated crm yang model
shi-su added a commit that referenced this pull request Aug 25, 2021
Why I did it
Update FRR 7.2.1 head. The following is a list of new commits.

5ae667a1f Merge pull request #9335 from FRRouting/mergify/bp/stable/7.2/pr-9214
eb679e8a1 zebra: bugfix of error quit of zebra, due to no nexthop ACTIVE
80d2eaa98 Merge pull request #8886 from FRRouting/mergify/bp/stable/7.2/pr-8876
1eeab2c1e lib: remove pure attribute from functions that modify memory
eb00dc4ec Merge pull request #6944 from LabNConsulting/working/lb/7.2/valgrind-supp-libyang
b9d6d05bf bgpd: suppress new libyang_1.0 related loss reports
8c26a71eb Merge pull request #6562 from ton31337/fix/configuration_for_labeled_unicast_in_place_7.2
386a1719c bgpd: Make sure network/aggregate-address commands lay down under labeled safi
b01c8bf28 Merge pull request #6526 from ton31337/fix/set_ipv6_ll_if_global_zero_7.2
c382833e8 bgpd: Use IPv6 LL address as nexthop if global was set to ::/LL
99509b835 Merge pull request #6395 from opensourcerouting/7.2/init-config-perms
7eef8f7b1 build: use configfile mode in init script
4cbe07705 Merge pull request #6360 from opensourcerouting/7.2/fix-warnings
84bb11785 nhrpd: clean up SA warning
aac726476 nhrpd: be more careful with linked lists
3a4b6d654 debian: Fix spelling error
756c67c6c Merge pull request #6284 from opensourcerouting/7.2/gcc-10
65a116a64 Merge pull request #6354 from ton31337/fix/communities_bgpd_crash_7.2
f7a00fd67 bgpd: Check to ensure community attributes exist before freeing them
a960f99c2 vrrpd: fix build on Fedora Rawhide
d4caff99f babeld: GCC complaining about no return in non-void function
a014c27ae babeld: fix build on Fedora Rawhide
79ff55b5b bgpd: remove unused variable
ff343e588 pimd: Make frr able to be built by gcc 10
9a3cf1ba2 ldpd: remove multiple definitions of thread_master
a19515bfe ldpd: fix another linking issue with GCC-10
b4c8de38c tests: fix build with GCC 10
4f27e8c85 ldpd: Fix linking error on Fedora Rawhide with GCC 10

How I did it
Update FRR 7.2 pointer and create a tag frr-7.2.1-s4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
YANG YANG model related changes
Projects
yang
Done
Development

Successfully merging this pull request may close these issues.

None yet

8 participants