bgpv1: component test framework #25362

harsimran-pabla · 2023-05-10T15:14:38Z

Introducing bgp control plane component tests. This change introduces test harness and basic BGP tests related to podCIDR advertisements, lb service advertisements and neighbor events.

Motivation of this change

Increase test coverage around boundaries of subcomponents

For eg, we do not test complete flow of change from getting Kubernetes event and then validating that it is reflected in the peering router.
Unit tests today exists for
-- Controller : This is limited to that policy change calls correct reconciler methods.
-- Reconcilers : Tests validate the diffs are generated correctly and we are inspecting gobgp directly for validation. Direct inspection of gobgp might not be suitable here in long term, if another underlying routing agent is used.

Validate and make BGP Peering part of CI testing.

This is mainly for neighbour configuration : like timers, graceful restart, authentication etc. And later on for more advanced BGP as well ( Routing policies and filters )
During new feature development, we have to validate the expected behaviour and add tests for it. It is currently not possible with unit tests, as we only configure local gobgp and we have to validate the behaviour separately via container labs to test actual peering behaviour. Which is again not coded anywhere and we do not repeat those manual tests as we continue to modify the code ( I would say this is the biggest reason ).
Also having this tests as part of CI, will ensure that there are no regressions introduced to feature behavior as we make changes.

Types of test cases to add

This component level test is mainly cover high level BGP features

So, what I see following tests need to be added

Pod CIDR advertisements
LB Service advertisements
Neighbor behavior
-- Authentication
-- Timers
-- Graceful restart
Some special test cases
-- Restarts
-- Any customer reported bugs and issues with order of operations.

pkg/bgpv1/test/adverts_test.go

pkg/bgpv1/test/links.go

pkg/bgpv1/gobgp/logger.go

YutaroHayakawa

Now looks good to me 👍

One non-blocking comment: I'd prefer putting all context in the PR description into the commit message. So that we don't have to jump to GitHub every time.

harsimran-pabla · 2023-05-18T10:49:27Z

/test

Pass log parameters to gobgp logger instead of hardcoding them in log methods. This is done so we can reuse the logger and pass on different log fields. Signed-off-by: harsimran pabla <hpabla@isovalent.com>

Introducing bgp control plane component tests. This change introduces test harness and basic BGP tests related to podCIDR advertisements, lb service advertisements and neighbor events. Motivation of this change is to increase test coverage around boundaries of BGP subsystem and also to validate actual peering state at high level for primary BGP features. Types of test cases to add here would range from exposed BGP features like route advertisements and various neighbor configuration settings. Signed-off-by: harsimran pabla <hpabla@isovalent.com>

harsimran-pabla · 2023-05-18T18:32:49Z

/test

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 10, 2023

harsimran-pabla added the release-note/misc This PR makes changes that have no direct user impact. label May 10, 2023

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 10, 2023

harsimran-pabla added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. area/bgp labels May 10, 2023

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 10, 2023

harsimran-pabla force-pushed the bgp_component_tests branch 4 times, most recently from 70634af to c0cc7bc Compare May 11, 2023 14:47

harsimran-pabla marked this pull request as ready for review May 11, 2023 15:20

harsimran-pabla requested a review from a team as a code owner May 11, 2023 15:20

harsimran-pabla requested a review from dylandreimerink May 11, 2023 15:20

harsimran-pabla force-pushed the bgp_component_tests branch 2 times, most recently from 554582a to 515310a Compare May 11, 2023 18:01