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

Add unnumbered BGP peering for frrk8s backend #2605

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
E2E: add unnumbered BGP for frrk8s backend
Signed-off-by: karampok <karampok@gmail.com>
  • Loading branch information
karampok committed Jan 29, 2025
commit 44c774cea207b686aa78f89b1d0d518bae773570
2 changes: 1 addition & 1 deletion e2etest/bgptests/bgp.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ var _ = ginkgo.Describe("BGP", func() {

ginkgo.AfterEach(func() {
if ginkgo.CurrentSpecReport().Failed() {
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace)
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace, FRRContainers)
k8s.DumpInfo(Reporter, ginkgo.CurrentSpecReport().LeafNodeText)
}
err := k8s.DeleteNamespace(cs, testNamespace)
Expand Down
2 changes: 1 addition & 1 deletion e2etest/bgptests/bgp_multiprotocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var _ = ginkgo.Describe("BGP Multiprotocol", func() {

ginkgo.AfterEach(func() {
if ginkgo.CurrentSpecReport().Failed() {
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace)
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace, FRRContainers)
k8s.DumpInfo(Reporter, ginkgo.CurrentSpecReport().LeafNodeText)
}
err := k8s.DeleteNamespace(cs, testNamespace)
Expand Down
2 changes: 1 addition & 1 deletion e2etest/bgptests/bgppeer_selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var _ = ginkgo.Describe("BGP Peer Selector", func() {

ginkgo.AfterEach(func() {
if ginkgo.CurrentSpecReport().Failed() {
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace)
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace, FRRContainers)
k8s.DumpInfo(Reporter, ginkgo.CurrentSpecReport().LeafNodeText)
}
err := k8s.DeleteNamespace(cs, testNamespace)
Expand Down
2 changes: 1 addition & 1 deletion e2etest/bgptests/cordon_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ var _ = ginkgo.Describe("BGP Cordon Node", func() {

ginkgo.AfterEach(func() {
if ginkgo.CurrentSpecReport().Failed() {
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace)
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace, FRRContainers)
k8s.DumpInfo(Reporter, ginkgo.CurrentSpecReport().LeafNodeText)
}
err := k8s.DeleteNamespace(cs, testNamespace)
Expand Down
5 changes: 3 additions & 2 deletions e2etest/bgptests/dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,20 @@ import (
"github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"go.universe.tf/e2etest/pkg/frr"
frrcontainer "go.universe.tf/e2etest/pkg/frr/container"
"go.universe.tf/e2etest/pkg/metallb"
clientset "k8s.io/client-go/kubernetes"
)

func dumpBGPInfo(basePath, testName string, cs clientset.Interface, namespace string) {
func dumpBGPInfo(basePath, testName string, cs clientset.Interface, namespace string, containers []*frrcontainer.FRR) {
testPath := path.Join(basePath, strings.ReplaceAll(testName, " ", "-"))
err := os.Mkdir(testPath, 0755)
if err != nil && !errors.Is(err, os.ErrExist) {
fmt.Fprintf(os.Stderr, "failed to create test dir: %v\n", err)
return
}

for _, c := range FRRContainers {
for _, c := range containers {
dump, err := frr.RawDump(c, "/etc/frr/bgpd.conf", "/tmp/frr.log", "/etc/frr/daemons")
if err != nil {
ginkgo.GinkgoWriter.Printf("External frr dump for container %s failed %v", c.Name, err)
Expand Down
2 changes: 1 addition & 1 deletion e2etest/bgptests/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var _ = ginkgo.Describe("BGP metrics", func() {
testNamespace := ""
ginkgo.AfterEach(func() {
if ginkgo.CurrentSpecReport().Failed() {
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace)
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace, FRRContainers)
k8s.DumpInfo(Reporter, ginkgo.CurrentSpecReport().LeafNodeText)
}
err := k8s.DeleteNamespace(cs, testNamespace)
Expand Down
2 changes: 1 addition & 1 deletion e2etest/bgptests/node_selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ var _ = ginkgo.Describe("BGP Node Selector", func() {
}

if ginkgo.CurrentSpecReport().Failed() {
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace)
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace, FRRContainers)
k8s.DumpInfo(Reporter, ginkgo.CurrentSpecReport().LeafNodeText)
}
err := k8s.DeleteNamespace(cs, testNamespace)
Expand Down
200 changes: 200 additions & 0 deletions e2etest/bgptests/unnummbered.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
// SPDX-License-Identifier:Apache-2.0

package bgptests

import (
"context"
"fmt"
"net/netip"
"time"

"go.universe.tf/e2etest/pkg/config"
"go.universe.tf/e2etest/pkg/container"
"go.universe.tf/e2etest/pkg/executor"
"go.universe.tf/e2etest/pkg/frr"
frrconfig "go.universe.tf/e2etest/pkg/frr/config"
frrcontainer "go.universe.tf/e2etest/pkg/frr/container"
jigservice "go.universe.tf/e2etest/pkg/jigservice"
"go.universe.tf/e2etest/pkg/netdev"
testservice "go.universe.tf/e2etest/pkg/service"

"go.universe.tf/e2etest/pkg/k8s"
"go.universe.tf/e2etest/pkg/k8sclient"
"go.universe.tf/e2etest/pkg/metallb"
metallbv1beta1 "go.universe.tf/metallb/api/v1beta1"
metallbv1beta2 "go.universe.tf/metallb/api/v1beta2"

"github.com/google/go-cmp/cmp"
"github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clientset "k8s.io/client-go/kubernetes"
)

var FRRImage string
var _ = ginkgo.Describe("FRRK8S-MODE Unnumbered BGP", func() {
var (
testNamespace string
nodeWithP2PConnection corev1.Node
externalP2PContainer *frrcontainer.FRR

cs clientset.Interface
)

ginkgo.BeforeEach(func() {
ginkgo.By("Clearing any previous configuration")
err := ConfigUpdater.Clean()
Expect(err).NotTo(HaveOccurred())

cs = k8sclient.New()
allNodes, err := cs.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{})
Expect(err).NotTo(HaveOccurred())
nodeWithP2PConnection = allNodes.Items[0]

err = k8s.DeleteNamespace(k8sclient.New(), "unnumbered-bgp")
Expect(err).NotTo(HaveOccurred())
testNamespace, err = k8s.CreateTestNamespace(k8sclient.New(), "unnumbered-bgp")
Expect(err).NotTo(HaveOccurred())
ginkgo.DeferCleanup(func() {
err := k8s.DeleteNamespace(k8sclient.New(), testNamespace)
Expect(err).NotTo(HaveOccurred())
})
})

ginkgo.AfterEach(func() {
if ginkgo.CurrentSpecReport().Failed() {
k8s.DumpInfo(Reporter, ginkgo.CurrentSpecReport().LeafNodeText)
dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace, []*frrcontainer.FRR{externalP2PContainer})
}
})

ginkgo.DescribeTable("Session is established and route is advertised", func(prefixSend []string, iface string, tweak func(svc *corev1.Service)) {
rc := frrconfig.RouterConfigUnnumbered{
ASNLocal: metalLBASN,
ASNRemote: metalLBASN,
Hostname: "tor1",
Interface: iface,
RouterID: "1.1.1.1",
}

var err error
ginkgo.By(fmt.Sprintf("creating p2p %s:%s -- %s:external-p2p-container", nodeWithP2PConnection.Name, iface, iface))
externalP2PContainer, err = frrcontainer.CreateP2PPeerFor(nodeWithP2PConnection.Name, iface, FRRImage)
Expect(err).NotTo(HaveOccurred())
ginkgo.By(fmt.Sprintf("updating frrconfig to %s", externalP2PContainer.Name))
ginkgo.DeferCleanup(func() {
err := frrcontainer.Delete([]*frrcontainer.FRR{externalP2PContainer})
Expect(err).NotTo(HaveOccurred())
})

c, err := rc.Config()
Expect(err).NotTo(HaveOccurred())
err = externalP2PContainer.UpdateBGPConfigFile(c)
Expect(err).NotTo(HaveOccurred())

resources := config.Resources{
Peers: []metallbv1beta2.BGPPeer{
{
ObjectMeta: metav1.ObjectMeta{
Name: "tor",
Namespace: metallb.Namespace,
},
Spec: metallbv1beta2.BGPPeerSpec{
Interface: iface,
ASN: rc.ASNRemote,
MyASN: rc.ASNLocal,
BFDProfile: "simple",
},
},
},
BFDProfiles: []metallbv1beta1.BFDProfile{{
ObjectMeta: metav1.ObjectMeta{
Name: "simple",
},
},
},
Pools: []metallbv1beta1.IPAddressPool{
{
ObjectMeta: metav1.ObjectMeta{
Name: "bgp-test",
},
Spec: metallbv1beta1.IPAddressPoolSpec{
Addresses: prefixSend,
},
},
},
BGPAdvs: []metallbv1beta1.BGPAdvertisement{
{ObjectMeta: metav1.ObjectMeta{Name: "empty"}},
},
}

err = ConfigUpdater.Update(resources)
Expect(err).NotTo(HaveOccurred(), "apply the CR in k8s api failed")

nodeP2PContainer := executor.ForContainer(nodeWithP2PConnection.Name)
nodeLLA, err := netdev.LinkLocalAddressForDevice(nodeP2PContainer, iface)
Expect(err).NotTo(HaveOccurred())
ginkgo.By("validating the node and p2p container peered")
validateUnnumberedBGPPeering(externalP2PContainer, nodeLLA)

svc, _ := testservice.CreateWithBackend(cs, testNamespace, "unnumbered-lb", tweak)
ginkgo.By("checking the service gets an ip assigned")
for _, i := range svc.Status.LoadBalancer.Ingress {
ingressIP := jigservice.GetIngressPoint(&i)
err = config.ValidateIPInRange(resources.Pools, ingressIP)
Expect(err).NotTo(HaveOccurred())
}

ginkgo.By(fmt.Sprintf("validating the p2p peer %s received the routes from node", externalP2PContainer.Name))
validatePeerRoutesViaDevice(externalP2PContainer, iface, nodeLLA, prefixSend)

Copy link
Member

Choose a reason for hiding this comment

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

is it possible to check that the external client can reach the service?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could investigate but why do you think it worth? I mean having the route on the peer is all what metallb does, if works or not is not something that depends on our changes. Is not trivial because I will need to modify the routes in the host to return traffic in the right secondary interface

Copy link
Member

Choose a reason for hiding this comment

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

fair enough, we can keep it this way

},
ginkgo.Entry("IPV4", []string{"5.5.5.5/32"}, "net10", func(_ *corev1.Service) {}),
ginkgo.Entry("IPV6", []string{"5555::1/128"}, "net20", func(_ *corev1.Service) {}),
ginkgo.Entry("DUALSTACK", []string{"5.5.5.5/32", "5555::1/128"}, "net30",
func(svc *corev1.Service) { testservice.DualStack(svc) }),
)
})

func validateUnnumberedBGPPeering(peer *frrcontainer.FRR, nodeLLA string) {
ginkgo.By(fmt.Sprintf("validating BGP peering to %s", peer.Name))
Eventually(func() error {
neighbors, err := frr.NeighborsInfo(peer)
if err != nil {
return err
}
for _, n := range neighbors {
if n.BGPNeighborAddr == nodeLLA && n.Connected && n.BFDInfo.Status == "Up" {
return nil
}
}
return fmt.Errorf("no connected neighbor was found with %s", nodeLLA)
}, 2*time.Minute, 10*time.Second).ShouldNot(HaveOccurred(),
"timed out waiting to validate nodes peered with the frr instance")
}

// validatePeerRoutesViaDevice validates that the peer has BGP routes to the
// specified prefixes with the expected next-hop address on the specified device.
func validatePeerRoutesViaDevice(peer executor.Executor, dev, nextHop string, prefixes ...[]string) {
ginkgo.By(fmt.Sprintf("validating prefix %s to %s dev %s", prefixes, nextHop, dev))
Eventually(func() error {
nextHopAddr := netip.MustParseAddr(nextHop)
want := make(map[netip.Prefix]map[netip.Addr]struct{})
for _, prf := range prefixes {
for _, p := range prf {
want[netip.MustParsePrefix(p)] = map[netip.Addr]struct{}{nextHopAddr: {}}
}
}

got, err := container.BGPRoutes(peer, dev)
if err != nil {
return err
}
if !cmp.Equal(want, got) {
return fmt.Errorf("want %v\n got %v\n diff %v", want, got, cmp.Diff(want, got))
}
return nil
}, 30*time.Second, 5*time.Second).ShouldNot(HaveOccurred(), fmt.Sprintf("peer should have the routes %s", prefixes))
}
1 change: 1 addition & 0 deletions e2etest/e2etest_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ var _ = ginkgo.BeforeSuite(func() {
bgptests.FRRContainers, err = bgptests.HostContainerSetup(frrImage, hostBGPMode)
Expect(err).NotTo(HaveOccurred())
default:
bgptests.FRRImage = frrImage
bgptests.FRRContainers, err = bgptests.KindnetContainersSetup(cs, frrImage)
Expect(err).NotTo(HaveOccurred())
if withVRF {
Expand Down
4 changes: 4 additions & 0 deletions e2etest/go.work.sum
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,8 @@ github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zk
github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4=
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvlsiIGKtc+UG6U5vzxaoagmhXfyg=
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0/go.mod h1:QUyp042oQthUoa9bqDv0ER0wrtXnBruoNd7aNjkbP+k=
github.com/metallb/frr-k8s v0.0.0-20241217094531-80d4b7a5591b h1:3+sSwasbGnVDjF84TRsxlCjbgko9tkDbw/kaj+NKPD4=
github.com/metallb/frr-k8s v0.0.0-20241217094531-80d4b7a5591b/go.mod h1:gcOVS4IiQ7r6SRKc2HBgYll3Ku9SHEatxaqgNiuC/Tg=
github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg=
github.com/minio/asm2plan9s v0.0.0-20200509001527-cdd76441f9d8/go.mod h1:mC1jAcsrzbxHt8iiaC+zU4b1ylILSosueou12R++wfY=
github.com/minio/c2goasm v0.0.0-20190812172519-36a3d3bbc4f3/go.mod h1:RagcQ7I8IeTMnF8JTXieKnO4Z6JCsikNEzj0DwauVzE=
Expand Down Expand Up @@ -1567,6 +1569,7 @@ k8s.io/api v0.29.2/go.mod h1:sdIaaKuU7P44aoyyLlikSLayT6Vb7bvJNCX105xZXY0=
k8s.io/api v0.29.3/go.mod h1:y2yg2NTyHUUkIoTC+phinTnEa3KFM6RZ3szxt014a80=
k8s.io/api v0.30.2/go.mod h1:ULg5g9JvOev2dG0u2hig4Z7tQ2hHIuS+m8MNZ+X6EmI=
k8s.io/api v0.31.0/go.mod h1:0YiFF+JfFxMM6+1hQei8FY8M7s1Mth+z/q7eF1aJkTE=
k8s.io/api v0.31.1/go.mod h1:sbN1g6eY6XVLeqNsZGLnI5FwVseTrZX7Fv3O26rhAaI=
k8s.io/apiextensions-apiserver v0.28.3/go.mod h1:NE1XJZ4On0hS11aWWJUTNkmVB03j9LM7gJSisbRt8Lc=
k8s.io/apiextensions-apiserver v0.29.0/go.mod h1:TKmpy3bTS0mr9pylH0nOt/QzQRrW7/h7yLdRForMZwc=
k8s.io/apiextensions-apiserver v0.29.2/go.mod h1:aLfYjpA5p3OwtqNXQFkhJ56TB+spV8Gc4wfMhUA3/b8=
Expand All @@ -1579,6 +1582,7 @@ k8s.io/apimachinery v0.29.2/go.mod h1:6HVkd1FwxIagpYrHSwJlQqZI3G9LfYWRPAkUvLnXTK
k8s.io/apimachinery v0.29.3/go.mod h1:hx/S4V2PNW4OMg3WizRrHutyB5la0iCUbZym+W0EQIU=
k8s.io/apimachinery v0.30.2/go.mod h1:iexa2somDaxdnj7bha06bhb43Zpa6eWH8N8dbqVjTUc=
k8s.io/apimachinery v0.31.0/go.mod h1:rsPdaZJfTfLsNJSQzNHQvYoTmxhoOEofxtOsF3rtsMo=
k8s.io/apimachinery v0.31.1/go.mod h1:rsPdaZJfTfLsNJSQzNHQvYoTmxhoOEofxtOsF3rtsMo=
k8s.io/apiserver v0.28.3/go.mod h1:YIpM+9wngNAv8Ctt0rHG4vQuX/I5rvkEMtZtsxW2rNM=
k8s.io/apiserver v0.28.4 h1:BJXlaQbAU/RXYX2lRz+E1oPe3G3TKlozMMCZWu5GMgg=
k8s.io/apiserver v0.28.4/go.mod h1:Idq71oXugKZoVGUUL2wgBCTHbUR+FYTWa4rq9j4n23w=
Expand Down
9 changes: 8 additions & 1 deletion e2etest/pkg/container/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,14 @@ func BGPRoutes(exc executor.Executor, dev string) (map[netip.Prefix]map[netip.Ad
for _, r := range routes {
dst, err := netip.ParsePrefix(r.Dst)
if err != nil {
return nil, fmt.Errorf("invalid prefix %s: %w", r.Dst, err)
// `ip route` command does not print the prefix when /32 or /128
dst, err = netip.ParsePrefix(r.Dst + "/128")
if err != nil {
dst, err = netip.ParsePrefix(r.Dst + "/32")
if err != nil {
return nil, fmt.Errorf("invalid prefix %s: %w", r.Dst, err)
}
}
}

nextHops := make(map[netip.Addr]struct{})
Expand Down