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

Example Declarative Configuration for establishing BGP session using GoBGP #19

Merged
merged 33 commits into from
Aug 30, 2022

Conversation

wenovus
Copy link
Contributor

@wenovus wenovus commented Aug 29, 2022

goBgpTask's doc comments discusses the design for declarative configuration for GoBGP. It is similar in idea to GoBGP's implementation for declarative configuration using its own config language (instead of OpenConfig/gNMI):
https://github.com/osrg/gobgp/blob/a5bb236b0d8d2a976bea8c62ffb2ab5c70ed6e9d/pkg/config/config.go#L367

Unfortunately I can't get the connection to be established yet. It only goes to "ACTIVE".

Incidentally, GoBGP actually uses an old version of OpenConfig for its configuration, although it uses its own custom Go binding instead of ygot based on viper. We may want to use this when we're actually implementing GoBGP to avoid re-inventing the wheel. For other protocols we will need our own implementation since GoBGP's implementation is very BGP specific. See the "Assumptions:" section below to see my concerns for why we might want something different.

wenovus and others added 27 commits March 4, 2022 12:05
This fake gNMI server simply mirrors whatever is set for its config leafs in its state leafs. It also has a mechanism for adding new "tasks", or go thread agents that can subscribe to particular values in ONDATRA's OpenConfig tree and write back values to it.

See gnmi/main/README.md for running/testing the server.
…ONDATRA.

Set's implementation is simplified due to removal of hacks to deal with
ONDATRA's idiosyncratic GoStructs not suitable for the reference
implementation, which has to work with them extensively.
ONDATRA's idiosyncratic GoStructs are not suitable for use with the reference implementation, because it prefers state over config, and uses shadow paths, which is a feature that has limited support, which in turn makes working with it awkward.
For a working test sample see
feature/bgp/bgp_base/tests/bgp_establish_test.go, note you will need
some changes in the Ondatra and KNE repos to get this working.
@wenovus wenovus requested a review from DanG100 August 29, 2022 23:26
@coveralls
Copy link

coveralls commented Aug 29, 2022

Pull Request Test Coverage Report for Build 2958845816

  • 8 of 22 (36.36%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.7%) to 70.624%

Changes Missing Coverage Covered Lines Changed/Added Lines %
gnmi/gnmit/gnmit.go 8 22 36.36%
Totals Coverage Status
Change from base Build 2952291712: -0.7%
Covered Lines: 702
Relevant Lines: 994

💛 - Coveralls

@wenovus
Copy link
Contributor Author

wenovus commented Aug 29, 2022

Workaround required to ignore staticcheck rules: golangci/golangci-lint#741

gnmi/fakedevice/gobgp.go Outdated Show resolved Hide resolved
gnmi/fakedevice/gobgp.go Show resolved Hide resolved
gnmi/fakedevice/gobgp.go Outdated Show resolved Hide resolved
@wenovus wenovus requested a review from DanG100 August 30, 2022 18:41
gnmi/fakedevice/gobgp.go Outdated Show resolved Hide resolved
@wenovus wenovus requested a review from DanG100 August 30, 2022 19:06
@wenovus wenovus merged commit c70e809 into main Aug 30, 2022
@wenovus wenovus deleted the gobgp branch August 30, 2022 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants