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

RT-1.27: fixed failed tests #3613

Merged
merged 14 commits into from
Jan 11, 2025
Merged

RT-1.27: fixed failed tests #3613

merged 14 commits into from
Jan 11, 2025

Conversation

singh-prem
Copy link
Contributor

This PR contains following:

  1. Changed bgp protocol instance name to default as table connection does not have a mechanism to pick a particular bgp instance.
  2. added sortDevicesByName function to sort emulated devices such that correct bgp neighbor gets emulated on correct otg port.
  3. Changed GetAll to LookupAll for prefixes learnt on OTG such that test does not fail fatally for cases where prefixes are not meant to be sent to otg.
  4. Added deviations and methods to configure default-reject route-policy, metric, attribute and skip subscription for table connection.

@singh-prem singh-prem requested review from dplore and a team as code owners November 29, 2024 11:30
@OpenConfigBot
Copy link

OpenConfigBot commented Nov 29, 2024 •
edited
Loading

Pull Request Functional Test Report for #3613 / 80975e2

Virtual Devices

Device Test Test Documentation Job Raw Log
Arista cEOS status
 RT-1.27: Static route to BGP redistribution
Cisco 8000E status
 RT-1.27: Static route to BGP redistribution
Cisco XRd status
 RT-1.27: Static route to BGP redistribution
Juniper ncPTX status
 RT-1.27: Static route to BGP redistribution
Nokia SR Linux status
 RT-1.27: Static route to BGP redistribution
Openconfig Lemming status
 RT-1.27: Static route to BGP redistribution

Hardware Devices

Device Test Test Documentation Raw Log
Arista 7808 status
 RT-1.27: Static route to BGP redistribution
Cisco 8808 status
 RT-1.27: Static route to BGP redistribution
Juniper PTX10008 status
 RT-1.27: Static route to BGP redistribution
Nokia 7250 IXR-10e status
 RT-1.27: Static route to BGP redistribution

Help

@coveralls
Copy link

coveralls commented Dec 7, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 12721746251

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 55.268%
Totals Coverage Status
Change from base Build 12719197464: 0.0%
Covered Lines: 1983
Relevant Lines: 3588
💛 - Coveralls

@dplore dplore self-assigned this Dec 7, 2024
@dplore dplore mentioned this pull request Dec 7, 2024
Closed
@ram-mac
Copy link
Contributor

ram-mac commented Dec 16, 2024

@singh-prem can you resolve the conflicts and also rebase the checks

@ram-mac ram-mac assigned singh-prem and unassigned dplore Dec 16, 2024
dplore
dplore requested changes Dec 17, 2024
View reviewed changes
@singh-prem singh-prem requested a review from a team December 20, 2024 12:17
@ram-mac
Copy link
Contributor

ram-mac commented Dec 27, 2024

There are some other changes for the same test - #3444
Can we check if the changes suggested in this PR are repeated in this PR?

@dplore
Copy link
Member

dplore commented Jan 3, 2025

There are some other changes for the same test - #3444 Can we check if the changes suggested in this PR are repeated in this PR?

We'll have to review these PR's one at a time unfortunately. I am giving this PR (#3613) priority for review first. After #3613 is merged, then #3444 will need to merge to mater and resolve and conflicts

@singh-prem singh-prem requested review from a team as code owners January 8, 2025 13:47
@singh-prem
Copy link
Contributor Author

@dplore @ram-mac apart from taking care of comments. I have changed static route configuration from Replace to Update, which required removing initial Delete of the existing static routes. Although it does not impact my run, I thought it will help to retain base configuration static routes. If you think, we should retain deleting existing static route, I can revert 2 lines.

@singh-prem
Copy link
Contributor Author

@singh-prem can you resolve the conflicts and also rebase the checks

done

@dplore
Copy link
Member

dplore commented Jan 8, 2025

/fptest virtual

dplore
dplore requested changes Jan 9, 2025
View reviewed changes
Copy link
Member

@dplore dplore left a comment

Choose a reason for hiding this comment

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

Thanks for all the changes. Two minor nits to fix static check issues, then this is ready

@dplore
Copy link
Member

dplore commented Jan 9, 2025

/fptest virtual

singh-prem and others added 2 commits January 9, 2025 11:55
@singh-prem @dplore
Update internal/cfgplugins/bgp_policy.go
7f353af 
function description change

Co-authored-by: Darren Loher <dloher@google.com>
@singh-prem @dplore
Update internal/cfgplugins/bgp_policy.go
02a190a 
function description change 2

Co-authored-by: Darren Loher <dloher@google.com>
@singh-prem
Copy link
Contributor Author

Thanks for all the changes. Two minor nits to fix static check issues, then this is ready

Thanks for detailed review. I have taken care of the two static check issues.

@ram-mac ram-mac merged commit a92713a into openconfig:main Jan 11, 2025
14 checks passed
ampattan pushed a commit to nokia/featureprofiles that referenced this pull request Jan 17, 2025
@singh-prem @dplore
@ram-mac @ampattan
RT-1.27: fixed failed tests (openconfig#3613)
1b3703a 
* intial code

* defined deviations

* updated deviation usage

* taken care of comments, also changed static route replace to update to preserve base routes

* added default bgp instance name in metadata file

* Update internal/cfgplugins/bgp_policy.go

function description change

Co-authored-by: Darren Loher <dloher@google.com>

* Update internal/cfgplugins/bgp_policy.go

function description change 2

Co-authored-by: Darren Loher <dloher@google.com>

* added bug info to the deviations

---------

Co-authored-by: Darren Loher <dloher@google.com>
Co-authored-by: Ram <rmachat@google.com>
alshabib pushed a commit to alshabib/featureprofiles that referenced this pull request Jan 19, 2025
@singh-prem @dplore
@ram-mac @alshabib
RT-1.27: fixed failed tests (openconfig#3613)
8a1af2f 
* intial code

* defined deviations

* updated deviation usage

* taken care of comments, also changed static route replace to update to preserve base routes

* added default bgp instance name in metadata file

* Update internal/cfgplugins/bgp_policy.go

function description change

Co-authored-by: Darren Loher <dloher@google.com>

* Update internal/cfgplugins/bgp_policy.go

function description change 2

Co-authored-by: Darren Loher <dloher@google.com>

* added bug info to the deviations

---------

Co-authored-by: Darren Loher <dloher@google.com>
Co-authored-by: Ram <rmachat@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LimeHat LimeHat LimeHat left review comments

@ram-mac ram-mac ram-mac left review comments

@dplore dplore dplore approved these changes

Assignees

@singh-prem singh-prem

Labels
cisco-pr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@singh-prem @OpenConfigBot @coveralls @ram-mac @dplore @LimeHat @arvbaska1