Repository navigation
Conversation
a387998 to
0bf089f
Compare
f4c55e7 to
ce1c9a6
Compare
6bb9e48 to
ba22390
Compare
|
Note for the reviewers: with this commit we introduce a new dependency effect for unsupported next-hop configurations. If a referenced policy contains one, I want to raise this point as we might want to handle this differently. |
ba22390 to
a6d95f3
Compare
|
Caution No coverage for base commit 7fb91bd was found, so this report compares against its ancestor 10cc35f (1 commit earlier) Merging this branch changes the coverage (2 decrease, 4 increase)
Coverage detailsCompared to commit 10cc35f (run #40321897417)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files:
|
| return false | ||
| }, | ||
| }), | ||
| builder.WithPredicates(predicate.GenerationChangedPredicate{}), |
There was a problem hiding this comment.
Same as in the bgp controller. We favor more narrow predicates to only trigger, when field are changed, that are read during the reconcilation and therefore change the result of the reconcilation.
| current := new(v1alpha1.BGP) | ||
| g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(bgp), current)).To(Succeed()) | ||
| g.Expect(conditions.IsConfigured(current)).To(BeTrue()) | ||
| g.Expect(current.Generation).To(Equal(bgp.Generation)) |
There was a problem hiding this comment.
What is the reason we are checking the generation here?
| func TestNextHopSelfOnlyPolicy(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| change func(*v1alpha1.RoutingPolicy) |
There was a problem hiding this comment.
Again. Just make this a *v1alpha1.RoutingPolicy field. You trade a repetetive &v1alpha1.RoutingPolicy{...} (easier to follow) for callback-like abstraction.
| case *gpb.GetRequest: | ||
| path := req.GetPath()[0] | ||
| value := state | ||
| if path.Elem[len(path.Elem)-1].Name == "Dom-list" { |
There was a problem hiding this comment.
Why do we write a full gnmi server mock with xpath parsing? As with other tests, I would just remove this.
This commit enables introduces methods to control the next-hop for advertised routes. Vendors like Cisco support a `nexthop-self` knob in the peers address-family configuration. This is however not modeled in OpenConfig, for which the same can be achieved by configuring routing policies (and so for every vendor). As most vendors support this kind of knob, we add this into the API core type BGPPeer. We also add the SetNexhopAction to the RoutingPolicy statements. Cisco exposes the NX-OS peer address-family controls as a typed bitmask. We update provider to handle properly the comma-separated gNMI values returned by the device and avoid unnecessary device writes due to different order. Signed-off-by: Pujol <enric.pujol@sap.com>
a6d95f3 to
00008db
Compare
See commit message for design and implementation details.
Tested with a cisco nxos device: