From bf75fc4e0c81f6c4cb7bb09bc982d26c723c5539 Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Mon, 22 Jul 2024 06:38:52 -0400 Subject: [PATCH 01/15] refactor(speaker): clean up the bgp part Signed-off-by: SkalaNetworks --- pkg/speaker/bgp.go | 155 ++++++++++++++++++++++++++++++++ pkg/speaker/config.go | 4 +- pkg/speaker/controller.go | 18 ++-- pkg/speaker/subnet.go | 183 ++------------------------------------ pkg/speaker/utils.go | 74 +++++++++++++++ 5 files changed, 253 insertions(+), 181 deletions(-) create mode 100644 pkg/speaker/bgp.go create mode 100644 pkg/speaker/utils.go diff --git a/pkg/speaker/bgp.go b/pkg/speaker/bgp.go new file mode 100644 index 00000000000..bd94b6c27de --- /dev/null +++ b/pkg/speaker/bgp.go @@ -0,0 +1,155 @@ +package speaker + +import ( + "context" + "fmt" + kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" + "github.com/kubeovn/kube-ovn/pkg/util" + bgpapi "github.com/osrg/gobgp/v3/api" + "github.com/osrg/gobgp/v3/pkg/packet/bgp" + "github.com/vishvananda/netlink" + "google.golang.org/protobuf/types/known/anypb" + "k8s.io/klog/v2" + "net" +) + +// addRoute adds a new route to advertise from our BGP speaker +func (c *Controller) addRoute(route string) error { + // Determine the Address Family Indicator (IPv6/IPv4) + routeAfi := bgpapi.Family_AFI_IP + if util.CheckProtocol(route) == kubeovnv1.ProtocolIPv6 { + routeAfi = bgpapi.Family_AFI_IP6 + } + + // Get NLRI and attributes to announce all the next hops possible + nlri, attrs, err := c.getNlriAndAttrs(route) + if err != nil { + return fmt.Errorf("failed to get NLRI and attributes: %w", nlri) + } + + // Announce every next hop we have + for _, attr := range attrs { + _, err = c.config.BgpServer.AddPath(context.Background(), &bgpapi.AddPathRequest{ + Path: &bgpapi.Path{ + Family: &bgpapi.Family{Afi: routeAfi, Safi: bgpapi.Family_SAFI_UNICAST}, + Nlri: nlri, + Pattrs: attr, + }, + }) + + if err != nil { + klog.Errorf("add path failed, %v", err) + return err + } + } + + return nil +} + +// delRoute removes a route we are currently advertising from our BGP speaker +func (c *Controller) delRoute(route string) error { + // Determine the Address Family Indicator (IPv6/IPv4) + routeAfi := bgpapi.Family_AFI_IP + if util.CheckProtocol(route) == kubeovnv1.ProtocolIPv6 { + routeAfi = bgpapi.Family_AFI_IP6 + } + + // Get NLRI and attributes to announce all the next hops possible + nlri, attrs, err := c.getNlriAndAttrs(route) + if err != nil { + return fmt.Errorf("failed to get NLRI and attributes: %w", nlri) + } + + // Withdraw every next hop we have + for _, attr := range attrs { + err = c.config.BgpServer.DeletePath(context.Background(), &bgpapi.DeletePathRequest{ + Path: &bgpapi.Path{ + Family: &bgpapi.Family{Afi: routeAfi, Safi: bgpapi.Family_SAFI_UNICAST}, + Nlri: nlri, + Pattrs: attr, + }, + }) + if err != nil { + klog.Errorf("del path failed, %v", err) + return err + } + } + return nil +} + +// getNlriAndAttrs returns the Network Layer Reachability Information (NLRI) and the BGP attributes for a particular route +func (c *Controller) getNlriAndAttrs(route string) (*anypb.Any, [][]*anypb.Any, error) { + // Should this route be advertised to IPv4 or IPv6 peers + // GoBGP supports extended-nexthop, we should be able to advertise IPv4 NRLI to IPv6 peer and the opposite to + // Is this check really necessary? + neighborAddresses := c.config.NeighborAddresses + if util.CheckProtocol(route) == kubeovnv1.ProtocolIPv6 { + neighborAddresses = c.config.NeighborIPv6Addresses + } + + // Get the route we're about to advertise and transform it to an NLRI + prefix, prefixLen, err := parseRoute(route) + if err != nil { + return nil, nil, fmt.Errorf("failed to parse route: %w", err) + } + + // Marshal the NLRI + nlri, err := anypb.New(&bgpapi.IPAddressPrefix{ + Prefix: prefix, + PrefixLen: prefixLen, + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to marshal NLRI: %w", err) + } + + // Create attributes for each neighbor to advertise the correct next hop + attrs := make([][]*anypb.Any, 0, len(neighborAddresses)) + for _, addr := range neighborAddresses { + a1, _ := anypb.New(&bgpapi.OriginAttribute{ + Origin: 0, + }) + a2, _ := anypb.New(&bgpapi.NextHopAttribute{ + NextHop: c.getNextHopAttribute(addr), + }) + attrs = append(attrs, []*anypb.Any{a1, a2}) + } + + return nlri, attrs, err +} + +// getNextHopAttribute returns the next hop we should advertise for a specific BGP neighbor +func (c *Controller) getNextHopAttribute(neighborAddress string) string { + nextHop := c.config.RouterID // If no route is found, fallback to router ID + + // Retrieve the route we use to speak to this neighbor and consider the source as next hop + routes, err := netlink.RouteGet(net.ParseIP(neighborAddress)) + if err == nil && len(routes) == 1 && routes[0].Src != nil { + nextHop = routes[0].Src.String() + } + + proto := util.CheckProtocol(nextHop) // Is next hop IPv4 or IPv6 + nextHopIP := net.ParseIP(nextHop) // Convert next hop to an IP + + // This takes care of a special case where the speaker might not be running in host mode + // If this happens, the nextHopIP will be the IP of a Pod (probably unreachable for the neighbours) + // For this case, the configuration allows for manually specifying the IPs to use as next hop (per protocol) + nodeIP := c.config.NodeIPs[proto] + if nodeIP != nil && nextHopIP != nil && nextHopIP.Equal(c.config.PodIPs[proto]) { + nextHop = nodeIP.String() + } + + return nextHop +} + +// getNextHopFromPathAttributes returns the next hop from BGP path attributes +func getNextHopFromPathAttributes(attrs []bgp.PathAttributeInterface) net.IP { + for _, attr := range attrs { + switch a := attr.(type) { + case *bgp.PathAttributeNextHop: + return a.Value + case *bgp.PathAttributeMpReachNLRI: + return a.Nexthop + } + } + return nil +} diff --git a/pkg/speaker/config.go b/pkg/speaker/config.go index 42df76fe0a8..2532cade11f 100644 --- a/pkg/speaker/config.go +++ b/pkg/speaker/config.go @@ -68,8 +68,8 @@ func ParseFlags() (*Configuration, error) { var ( argDefaultGracefulTime = pflag.Duration("graceful-restart-time", DefaultGracefulRestartTime, "BGP Graceful restart time according to RFC4724 3, maximum 4095s.") argGracefulRestartDeferralTime = pflag.Duration("graceful-restart-deferral-time", DefaultGracefulRestartDeferralTime, "BGP Graceful restart deferral time according to RFC4724 4.1, maximum 18h.") - argGracefulRestart = pflag.BoolP("graceful-restart", "", false, "Enables the BGP Graceful Restart so that routes are preserved on unexpected restarts") - argAnnounceClusterIP = pflag.BoolP("announce-cluster-ip", "", false, "The Cluster IP of the service to announce to the BGP peers.") + argGracefulRestart = pflag.BoolP("graceful-restart", "", false, "Enables the BGP Graceful Restart so that routes are preserved on unexpected restarts") + argAnnounceClusterIP = pflag.BoolP("announce-cluster-ip", "", false, "The Cluster IP of the service to announce to the BGP peers.") argGrpcHost = pflag.String("grpc-host", "127.0.0.1", "The host address for grpc to listen, default: 127.0.0.1") argGrpcPort = pflag.Uint32("grpc-port", DefaultBGPGrpcPort, "The port for grpc to listen, default:50051") argClusterAs = pflag.Uint32("cluster-as", DefaultBGPClusterAs, "The as number of container network, default 65000") diff --git a/pkg/speaker/controller.go b/pkg/speaker/controller.go index 1cc9030c8e4..1914c05596e 100644 --- a/pkg/speaker/controller.go +++ b/pkg/speaker/controller.go @@ -26,13 +26,18 @@ const controllerAgentName = "ovn-speaker" type Controller struct { config *Configuration - podsLister listerv1.PodLister - podsSynced cache.InformerSynced - subnetsLister kubeovnlister.SubnetLister - subnetSynced cache.InformerSynced + podsLister listerv1.PodLister + podsSynced cache.InformerSynced + + subnetsLister kubeovnlister.SubnetLister + subnetSynced cache.InformerSynced + servicesLister listerv1.ServiceLister servicesSynced cache.InformerSynced + eipLister kubeovnlister.IptablesEIPLister + eipSynced cache.InformerSynced + informerFactory kubeinformers.SharedInformerFactory kubeovnInformerFactory kubeovninformer.SharedInformerFactory recorder record.EventRecorder @@ -58,6 +63,7 @@ func NewController(config *Configuration) *Controller { podInformer := informerFactory.Core().V1().Pods() subnetInformer := kubeovnInformerFactory.Kubeovn().V1().Subnets() serviceInformer := informerFactory.Core().V1().Services() + eipInformer := kubeovnInformerFactory.Kubeovn().V1().IptablesEIPs() controller := &Controller{ config: config, @@ -68,6 +74,8 @@ func NewController(config *Configuration) *Controller { subnetSynced: subnetInformer.Informer().HasSynced, servicesLister: serviceInformer.Lister(), servicesSynced: serviceInformer.Informer().HasSynced, + eipLister: eipInformer.Lister(), + eipSynced: eipInformer.Informer().HasSynced, informerFactory: informerFactory, kubeovnInformerFactory: kubeovnInformerFactory, @@ -82,7 +90,7 @@ func (c *Controller) Run(stopCh <-chan struct{}) { c.informerFactory.Start(stopCh) c.kubeovnInformerFactory.Start(stopCh) - if !cache.WaitForCacheSync(stopCh, c.podsSynced, c.subnetSynced, c.servicesSynced) { + if !cache.WaitForCacheSync(stopCh, c.podsSynced, c.subnetSynced, c.servicesSynced, c.eipSynced) { util.LogFatalAndExit(nil, "failed to wait for caches to sync") return } diff --git a/pkg/speaker/subnet.go b/pkg/speaker/subnet.go index 47e8178f405..2261c7591f2 100644 --- a/pkg/speaker/subnet.go +++ b/pkg/speaker/subnet.go @@ -4,17 +4,13 @@ package speaker import ( "context" "fmt" - "net" - "strconv" "strings" bgpapi "github.com/osrg/gobgp/v3/api" bgpapiutil "github.com/osrg/gobgp/v3/pkg/apiutil" - "github.com/osrg/gobgp/v3/pkg/packet/bgp" + "github.com/vishvananda/netlink" "golang.org/x/sys/unix" - "google.golang.org/protobuf/types/known/anypb" - v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/klog/v2" @@ -23,32 +19,17 @@ import ( ) const ( - // "cluster" is the default policy + // announcePolicyCluster makes the Pod IPs/Subnet CIDRs be announced from every speaker, whether there's Pods + // that have that specific IP or that are part of the Subnet CIDR on that node. In other words, traffic may enter from + // any node hosting a speaker, and then be internally routed in the cluster to the actual Pod. In this configuration + // extra hops might be used. This is the default policy to Pods and Subnets. announcePolicyCluster = "cluster" - announcePolicyLocal = "local" + // announcePolicyLocal makes the Pod IPs be announced only from speakers on nodes that are actively hosting + // them. In other words, traffic will only enter from nodes hosting Pods marked as needing BGP advertisement, + // or Pods with an IP belonging to a Subnet marked as needing BGP advertisement. This makes the network path shorter. + announcePolicyLocal = "local" ) -func isPodAlive(p *v1.Pod) bool { - if p.Status.Phase == v1.PodSucceeded && p.Spec.RestartPolicy != v1.RestartPolicyAlways { - return false - } - - if p.Status.Phase == v1.PodFailed && p.Spec.RestartPolicy == v1.RestartPolicyNever { - return false - } - - if p.Status.Phase == v1.PodFailed && p.Status.Reason == "Evicted" { - return false - } - return true -} - -func isClusterIPService(svc *v1.Service) bool { - return svc.Spec.Type == v1.ServiceTypeClusterIP && - svc.Spec.ClusterIP != v1.ClusterIPNone && - len(svc.Spec.ClusterIP) != 0 -} - func (c *Controller) syncSubnetRoutes() { maskMap := map[string]int{kubeovnv1.ProtocolIPv4: 32, kubeovnv1.ProtocolIPv6: 128} bgpExpected, bgpExists := make(map[string][]string), make(map[string][]string) @@ -215,149 +196,3 @@ func (c *Controller) syncSubnetRoutes() { } } } - -func routeDiff(expected, exists []string) (toAdd, toDel []string) { - expectedMap, existsMap := map[string]bool{}, map[string]bool{} - for _, e := range expected { - expectedMap[e] = true - } - for _, e := range exists { - existsMap[e] = true - } - - for e := range expectedMap { - if !existsMap[e] { - toAdd = append(toAdd, e) - } - } - - for e := range existsMap { - if !expectedMap[e] { - toDel = append(toDel, e) - } - } - return toAdd, toDel -} - -func parseRoute(route string) (string, uint32, error) { - var prefixLen uint32 = 32 - prefix := route - if strings.Contains(route, "/") { - prefix = strings.Split(route, "/")[0] - strLen := strings.Split(route, "/")[1] - intLen, err := strconv.Atoi(strLen) - if err != nil { - return "", 0, err - } - prefixLen = uint32(intLen) - } - return prefix, prefixLen, nil -} - -func (c *Controller) addRoute(route string) error { - routeAfi := bgpapi.Family_AFI_IP - if util.CheckProtocol(route) == kubeovnv1.ProtocolIPv6 { - routeAfi = bgpapi.Family_AFI_IP6 - } - - nlri, attrs, err := c.getNlriAndAttrs(route) - if err != nil { - return err - } - for _, attr := range attrs { - _, err = c.config.BgpServer.AddPath(context.Background(), &bgpapi.AddPathRequest{ - Path: &bgpapi.Path{ - Family: &bgpapi.Family{Afi: routeAfi, Safi: bgpapi.Family_SAFI_UNICAST}, - Nlri: nlri, - Pattrs: attr, - }, - }) - if err != nil { - klog.Errorf("add path failed, %v", err) - return err - } - } - return nil -} - -func (c *Controller) getNlriAndAttrs(route string) (*anypb.Any, [][]*anypb.Any, error) { - neighborAddresses := c.config.NeighborAddresses - if util.CheckProtocol(route) == kubeovnv1.ProtocolIPv6 { - neighborAddresses = c.config.NeighborIPv6Addresses - } - - prefix, prefixLen, err := parseRoute(route) - if err != nil { - return nil, nil, err - } - nlri, _ := anypb.New(&bgpapi.IPAddressPrefix{ - Prefix: prefix, - PrefixLen: prefixLen, - }) - - attrs := make([][]*anypb.Any, 0, len(neighborAddresses)) - for _, addr := range neighborAddresses { - a1, _ := anypb.New(&bgpapi.OriginAttribute{ - Origin: 0, - }) - a2, _ := anypb.New(&bgpapi.NextHopAttribute{ - NextHop: c.getNextHopAttribute(addr), - }) - attrs = append(attrs, []*anypb.Any{a1, a2}) - } - - return nlri, attrs, err -} - -func (c *Controller) delRoute(route string) error { - routeAfi := bgpapi.Family_AFI_IP - if util.CheckProtocol(route) == kubeovnv1.ProtocolIPv6 { - routeAfi = bgpapi.Family_AFI_IP6 - } - - nlri, attrs, err := c.getNlriAndAttrs(route) - if err != nil { - return err - } - for _, attr := range attrs { - err = c.config.BgpServer.DeletePath(context.Background(), &bgpapi.DeletePathRequest{ - Path: &bgpapi.Path{ - Family: &bgpapi.Family{Afi: routeAfi, Safi: bgpapi.Family_SAFI_UNICAST}, - Nlri: nlri, - Pattrs: attr, - }, - }) - if err != nil { - klog.Errorf("del path failed, %v", err) - return err - } - } - return nil -} - -func getNextHopFromPathAttributes(attrs []bgp.PathAttributeInterface) net.IP { - for _, attr := range attrs { - switch a := attr.(type) { - case *bgp.PathAttributeNextHop: - return a.Value - case *bgp.PathAttributeMpReachNLRI: - return a.Nexthop - } - } - return nil -} - -func (c *Controller) getNextHopAttribute(neighborAddress string) string { - nextHop := c.config.RouterID - routes, err := netlink.RouteGet(net.ParseIP(neighborAddress)) - if err == nil && len(routes) == 1 && routes[0].Src != nil { - nextHop = routes[0].Src.String() - } - proto := util.CheckProtocol(nextHop) - nextHopIP := net.ParseIP(nextHop) - nodeIP := c.config.NodeIPs[proto] - if nodeIP != nil && nextHopIP != nil && nextHopIP.Equal(c.config.PodIPs[proto]) { - nextHop = nodeIP.String() - } - return nextHop -} diff --git a/pkg/speaker/utils.go b/pkg/speaker/utils.go new file mode 100644 index 00000000000..2b75212a2e9 --- /dev/null +++ b/pkg/speaker/utils.go @@ -0,0 +1,74 @@ +package speaker + +import ( + v1 "k8s.io/api/core/v1" + "strconv" + "strings" +) + +// isPodAlive returns whether a Pod is alive or not +func isPodAlive(p *v1.Pod) bool { + if p.Status.Phase == v1.PodSucceeded && p.Spec.RestartPolicy != v1.RestartPolicyAlways { + return false + } + + if p.Status.Phase == v1.PodFailed && p.Spec.RestartPolicy == v1.RestartPolicyNever { + return false + } + + if p.Status.Phase == v1.PodFailed && p.Status.Reason == "Evicted" { + return false + } + return true +} + +// isClusterIPService returns whether a Service is of type ClusterIP or not +func isClusterIPService(svc *v1.Service) bool { + return svc.Spec.Type == v1.ServiceTypeClusterIP && + svc.Spec.ClusterIP != v1.ClusterIPNone && + len(svc.Spec.ClusterIP) != 0 +} + +// routeDiff returns the routes that should be added and the routes that should be deleted +// after receiving the routes we except to advertise versus the route we are advertising +func routeDiff(expected, exists []string) (toAdd, toDel []string) { + expectedMap, existsMap := map[string]bool{}, map[string]bool{} + for _, e := range expected { + expectedMap[e] = true + } + for _, e := range exists { + existsMap[e] = true + } + + for e := range expectedMap { + if !existsMap[e] { + toAdd = append(toAdd, e) + } + } + + for e := range existsMap { + if !expectedMap[e] { + toDel = append(toDel, e) + } + } + + return toAdd, toDel +} + +// parseRoute returns the prefix and length of the prefix (in bits) by parsing the received route +// If no prefix is mentioned in the route (e.g 1.1.1.1 instead of 1.1.1.1/32), the prefix length +// is assumed to be 32 bits +func parseRoute(route string) (string, uint32, error) { + var prefixLen uint32 = 32 + prefix := route + if strings.Contains(route, "/") { + prefix = strings.Split(route, "/")[0] + strLen := strings.Split(route, "/")[1] + intLen, err := strconv.Atoi(strLen) + if err != nil { + return "", 0, err + } + prefixLen = uint32(intLen) + } + return prefix, prefixLen, nil +} From 221d51be31fbf1dcba45b70cde88c5f605249ab0 Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Mon, 22 Jul 2024 06:29:11 -0400 Subject: [PATCH 02/15] fix(bgpgw): conflict Signed-off-by: SkalaNetworks --- pkg/speaker/bgp.go | 92 +++++++++++++++++++++++++++++++++++++-- pkg/speaker/controller.go | 22 ++++++---- pkg/speaker/natgateway.go | 55 +++++++++++++++++++++++ pkg/speaker/subnet.go | 89 +++---------------------------------- pkg/speaker/utils.go | 44 +++++++++++++++++++ 5 files changed, 208 insertions(+), 94 deletions(-) create mode 100644 pkg/speaker/natgateway.go diff --git a/pkg/speaker/bgp.go b/pkg/speaker/bgp.go index bd94b6c27de..bb097ffccfa 100644 --- a/pkg/speaker/bgp.go +++ b/pkg/speaker/bgp.go @@ -6,13 +6,101 @@ import ( kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" "github.com/kubeovn/kube-ovn/pkg/util" bgpapi "github.com/osrg/gobgp/v3/api" + bgpapiutil "github.com/osrg/gobgp/v3/pkg/apiutil" "github.com/osrg/gobgp/v3/pkg/packet/bgp" "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" "google.golang.org/protobuf/types/known/anypb" "k8s.io/klog/v2" "net" ) +var ( + maskMap = map[string]int{kubeovnv1.ProtocolIPv4: 32, kubeovnv1.ProtocolIPv6: 128} +) + +// reconciliateRoutes configures the BGP speaker to announce only the routes we are expected to announce +// and to withdraw the ones that should not be announced anymore +func (c *Controller) reconciliateRoutes(expectedPrefixes prefixMap) error { + if len(c.config.NeighborAddresses) != 0 { + err := c.reconciliateIPFamily(kubeovnv1.ProtocolIPv4, expectedPrefixes) + if err != nil { + return fmt.Errorf("failed to reconciliate IPv4 routes: %w", err) + } + } + + if len(c.config.NeighborIPv6Addresses) != 0 { + err := c.reconciliateIPFamily(kubeovnv1.ProtocolIPv6, expectedPrefixes) + if err != nil { + return fmt.Errorf("failed to reconciliate IPv6 routes: %w", err) + } + } + + return nil +} + +// reconciliateIPFamily announces prefixes we are not currently announcing and withdraws prefixes we should +// not be announcing for a given IP family (IPv4/IPv6) +func (c *Controller) reconciliateIPFamily(ipFamily string, expectedPrefixes prefixMap) error { + // Get the address family associated with the Kube-OVN family + afi, err := kubeOvnFamilyToAFI(ipFamily) + if err != nil { + return fmt.Errorf("couldn't convert family to afi: %w", err) + } + + // Craft a BGP path listing request for this AFI + listPathRequest := &bgpapi.ListPathRequest{ + TableType: bgpapi.TableType_GLOBAL, + Family: &bgpapi.Family{Afi: afi, Safi: bgpapi.Family_SAFI_UNICAST}, + } + + // Anonymous function that stores the prefixes we are announcing for this AFI + var existingPrefixes []string + fn := func(d *bgpapi.Destination) { + for _, path := range d.Paths { + attrInterfaces, _ := bgpapiutil.UnmarshalPathAttributes(path.Pattrs) + nextHop := getNextHopFromPathAttributes(attrInterfaces) + klog.V(5).Infof("announcing route with prefix %s and nexthop: %s", d.Prefix, nextHop.String()) + + route, _ := netlink.RouteGet(nextHop) + if len(route) == 1 && route[0].Type == unix.RTN_LOCAL || nextHop.String() == c.config.RouterID { + existingPrefixes = append(existingPrefixes, d.Prefix) + return + } + } + } + + // Ask the BGP speaker what routes we're announcing for the IP family selected + if err := c.config.BgpServer.ListPath(context.Background(), listPathRequest, fn); err != nil { + return fmt.Errorf("failed to list existing %s routes: %w", ipFamily, err) + } + + klog.V(5).Infof("currently announcing %s routes: %v", ipFamily, existingPrefixes) + + // Announce routes we should be announcing and withdraw the ones that are no longer valid + c.announceAndWithdraw(routeDiff(expectedPrefixes[ipFamily], existingPrefixes)) + return nil +} + +// announceAndWithdraw commands the BGP speaker to start announcing new routes and to withdraw others +func (c *Controller) announceAndWithdraw(toAdd, toDel []string) { + // Announce routes that need to be added + klog.V(5).Infof("new routes we will announce: %v", toAdd) + for _, route := range toAdd { + if err := c.addRoute(route); err != nil { + klog.Error(err) + } + } + + // Withdraw routes that should be deleted + klog.V(5).Infof("announced routes we will withdraw: %v", toDel) + for _, route := range toDel { + if err := c.delRoute(route); err != nil { + klog.Error(err) + } + } +} + // addRoute adds a new route to advertise from our BGP speaker func (c *Controller) addRoute(route string) error { // Determine the Address Family Indicator (IPv6/IPv4) @@ -38,7 +126,6 @@ func (c *Controller) addRoute(route string) error { }) if err != nil { - klog.Errorf("add path failed, %v", err) return err } } @@ -70,7 +157,6 @@ func (c *Controller) delRoute(route string) error { }, }) if err != nil { - klog.Errorf("del path failed, %v", err) return err } } @@ -80,7 +166,7 @@ func (c *Controller) delRoute(route string) error { // getNlriAndAttrs returns the Network Layer Reachability Information (NLRI) and the BGP attributes for a particular route func (c *Controller) getNlriAndAttrs(route string) (*anypb.Any, [][]*anypb.Any, error) { // Should this route be advertised to IPv4 or IPv6 peers - // GoBGP supports extended-nexthop, we should be able to advertise IPv4 NRLI to IPv6 peer and the opposite to + // GoBGP supports extended-nexthop, we should be able to advertise IPv4 NLRI to IPv6 peer and the opposite to // Is this check really necessary? neighborAddresses := c.config.NeighborAddresses if util.CheckProtocol(route) == kubeovnv1.ProtocolIPv6 { diff --git a/pkg/speaker/controller.go b/pkg/speaker/controller.go index 1914c05596e..1b5389adee6 100644 --- a/pkg/speaker/controller.go +++ b/pkg/speaker/controller.go @@ -38,6 +38,9 @@ type Controller struct { eipLister kubeovnlister.IptablesEIPLister eipSynced cache.InformerSynced + natgatewayLister kubeovnlister.VpcNatGatewayLister + natgatewaySynced cache.InformerSynced + informerFactory kubeinformers.SharedInformerFactory kubeovnInformerFactory kubeovninformer.SharedInformerFactory recorder record.EventRecorder @@ -64,18 +67,21 @@ func NewController(config *Configuration) *Controller { subnetInformer := kubeovnInformerFactory.Kubeovn().V1().Subnets() serviceInformer := informerFactory.Core().V1().Services() eipInformer := kubeovnInformerFactory.Kubeovn().V1().IptablesEIPs() + natgatewayInformer := kubeovnInformerFactory.Kubeovn().V1().VpcNatGateways() controller := &Controller{ config: config, - podsLister: podInformer.Lister(), - podsSynced: podInformer.Informer().HasSynced, - subnetsLister: subnetInformer.Lister(), - subnetSynced: subnetInformer.Informer().HasSynced, - servicesLister: serviceInformer.Lister(), - servicesSynced: serviceInformer.Informer().HasSynced, - eipLister: eipInformer.Lister(), - eipSynced: eipInformer.Informer().HasSynced, + podsLister: podInformer.Lister(), + podsSynced: podInformer.Informer().HasSynced, + subnetsLister: subnetInformer.Lister(), + subnetSynced: subnetInformer.Informer().HasSynced, + servicesLister: serviceInformer.Lister(), + servicesSynced: serviceInformer.Informer().HasSynced, + eipLister: eipInformer.Lister(), + eipSynced: eipInformer.Informer().HasSynced, + natgatewayLister: natgatewayInformer.Lister(), + natgatewaySynced: natgatewayInformer.Informer().HasSynced, informerFactory: informerFactory, kubeovnInformerFactory: kubeovnInformerFactory, diff --git a/pkg/speaker/natgateway.go b/pkg/speaker/natgateway.go new file mode 100644 index 00000000000..00e78dccb74 --- /dev/null +++ b/pkg/speaker/natgateway.go @@ -0,0 +1,55 @@ +package speaker + +import ( + "fmt" + v1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" + "github.com/kubeovn/kube-ovn/pkg/util" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" +) + +const ( + HostnameEnvVariable = "HOSTNAME" +) + +// syncEIPRoutes retrieves all the EIPs attached to our GWs and starts announcing their route +func (c *Controller) syncEIPRoutes() error { + // Retrieve the name of our gateway + gatewayName := getGatewayName() + if gatewayName == "" { + return fmt.Errorf("failed to retrieve the name of the gateway, might not be running in a gateway pod") + } + + // Create label requirements to only get EIPs attached to our NAT GW + requirements, err := labels.NewRequirement(util.VpcNatGatewayLabel, selection.Equals, []string{gatewayName}) + if err != nil { + return fmt.Errorf("failed to create label selector requirement: %w", err) + } + + // Filter all EIPs attached to our NAT GW + eips, err := c.eipLister.List(labels.NewSelector().Add(*requirements)) + if err != nil { + return fmt.Errorf("failed to list EIPs attached to our GW: %w", err) + } + + return c.announceEIPs(eips) +} + +func (c *Controller) announceEIPs(eips []*v1.IptablesEIP) error { + expectedPrefixes := make(prefixMap) + for _, eip := range eips { + if !eip.Status.Ready { // Only announce EIPs marked as "ready" + continue + } + + if eip.Spec.V4ip != "" { // If we have an IPv4, add it to prefixes we should be announcing + addExpectedPrefix(eip.Spec.V4ip, expectedPrefixes) + } + + if eip.Spec.V4ip != "" { // If we have an IPv6, add it to prefixes we should be announcing + addExpectedPrefix(eip.Spec.V6ip, expectedPrefixes) + } + } + + return c.reconciliateRoutes(expectedPrefixes) +} diff --git a/pkg/speaker/subnet.go b/pkg/speaker/subnet.go index 2261c7591f2..a8428bac076 100644 --- a/pkg/speaker/subnet.go +++ b/pkg/speaker/subnet.go @@ -2,19 +2,11 @@ package speaker import ( - "context" - "fmt" "strings" - bgpapi "github.com/osrg/gobgp/v3/api" - bgpapiutil "github.com/osrg/gobgp/v3/pkg/apiutil" - - "github.com/vishvananda/netlink" - "golang.org/x/sys/unix" "k8s.io/apimachinery/pkg/labels" "k8s.io/klog/v2" - kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" "github.com/kubeovn/kube-ovn/pkg/util" ) @@ -31,8 +23,7 @@ const ( ) func (c *Controller) syncSubnetRoutes() { - maskMap := map[string]int{kubeovnv1.ProtocolIPv4: 32, kubeovnv1.ProtocolIPv6: 128} - bgpExpected, bgpExists := make(map[string][]string), make(map[string][]string) + bgpExpected := make(prefixMap) subnets, err := c.subnetsLister.List(labels.Everything()) if err != nil { @@ -54,8 +45,7 @@ func (c *Controller) syncSubnetRoutes() { for _, svc := range services { if svc.Annotations != nil && svc.Annotations[util.BgpAnnotation] == "true" && isClusterIPService(svc) { for _, clusterIP := range svc.Spec.ClusterIPs { - ipFamily := util.CheckProtocol(clusterIP) - bgpExpected[ipFamily] = append(bgpExpected[ipFamily], fmt.Sprintf("%s/%d", clusterIP, maskMap[ipFamily])) + addExpectedPrefix(clusterIP, bgpExpected) } } } @@ -120,79 +110,12 @@ func (c *Controller) syncSubnetRoutes() { } } - for ipFamily, ip := range ips { - bgpExpected[ipFamily] = append(bgpExpected[ipFamily], fmt.Sprintf("%s/%d", ip, maskMap[ipFamily])) + for _, ip := range ips { + addExpectedPrefix(ip, bgpExpected) } } - klog.V(5).Infof("expected announce ipv4 routes: %v, ipv6 routes: %v", bgpExpected[kubeovnv1.ProtocolIPv4], bgpExpected[kubeovnv1.ProtocolIPv6]) - - fn := func(d *bgpapi.Destination) { - for _, path := range d.Paths { - attrInterfaces, _ := bgpapiutil.UnmarshalPathAttributes(path.Pattrs) - nextHop := getNextHopFromPathAttributes(attrInterfaces) - klog.V(5).Infof("the route Prefix is %s, NextHop is %s", d.Prefix, nextHop.String()) - ipFamily := util.CheckProtocol(nextHop.String()) - route, _ := netlink.RouteGet(nextHop) - if len(route) == 1 && route[0].Type == unix.RTN_LOCAL || nextHop.String() == c.config.RouterID { - bgpExists[ipFamily] = append(bgpExists[ipFamily], d.Prefix) - return - } - } - } - - if len(c.config.NeighborAddresses) != 0 { - listPathRequest := &bgpapi.ListPathRequest{ - TableType: bgpapi.TableType_GLOBAL, - Family: &bgpapi.Family{Afi: bgpapi.Family_AFI_IP, Safi: bgpapi.Family_SAFI_UNICAST}, - } - if err := c.config.BgpServer.ListPath(context.Background(), listPathRequest, fn); err != nil { - klog.Errorf("failed to list exist route, %v", err) - return - } - - klog.V(5).Infof("exists ipv4 routes %v", bgpExists[kubeovnv1.ProtocolIPv4]) - toAdd, toDel := routeDiff(bgpExpected[kubeovnv1.ProtocolIPv4], bgpExists[kubeovnv1.ProtocolIPv4]) - klog.V(5).Infof("toAdd ipv4 routes %v", toAdd) - for _, route := range toAdd { - if err := c.addRoute(route); err != nil { - klog.Error(err) - } - } - - klog.V(5).Infof("toDel ipv4 routes %v", toDel) - for _, route := range toDel { - if err := c.delRoute(route); err != nil { - klog.Error(err) - } - } - } - - if len(c.config.NeighborIPv6Addresses) != 0 { - listIPv6PathRequest := &bgpapi.ListPathRequest{ - TableType: bgpapi.TableType_GLOBAL, - Family: &bgpapi.Family{Afi: bgpapi.Family_AFI_IP6, Safi: bgpapi.Family_SAFI_UNICAST}, - } - - if err := c.config.BgpServer.ListPath(context.Background(), listIPv6PathRequest, fn); err != nil { - klog.Errorf("failed to list exist route, %v", err) - return - } - - klog.V(5).Infof("exists ipv6 routes %v", bgpExists[kubeovnv1.ProtocolIPv6]) - toAdd, toDel := routeDiff(bgpExpected[kubeovnv1.ProtocolIPv6], bgpExists[kubeovnv1.ProtocolIPv6]) - klog.V(5).Infof("toAdd ipv6 routes %v", toAdd) - - for _, route := range toAdd { - if err := c.addRoute(route); err != nil { - klog.Error(err) - } - } - klog.V(5).Infof("toDel ipv6 routes %v", toDel) - for _, route := range toDel { - if err := c.delRoute(route); err != nil { - klog.Error(err) - } - } + if err := c.reconciliateRoutes(bgpExpected); err != nil { + klog.Errorf("failed to reconciliate routes: %s", err.Error()) } } diff --git a/pkg/speaker/utils.go b/pkg/speaker/utils.go index 2b75212a2e9..0c9a4ae7950 100644 --- a/pkg/speaker/utils.go +++ b/pkg/speaker/utils.go @@ -1,11 +1,26 @@ package speaker import ( + "fmt" + kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" + "github.com/kubeovn/kube-ovn/pkg/util" + bgpapi "github.com/osrg/gobgp/v3/api" v1 "k8s.io/api/core/v1" + "os" "strconv" "strings" ) +// prefixMap is a map associating an IP family (IPv4 or IPv6) and an IP +type prefixMap map[string][]string + +// addExpectedPrefix adds a new prefix to the list of expected prefixes we should be announcing +func addExpectedPrefix(ip string, expectedPrefixes prefixMap) { + ipFamily := util.CheckProtocol(ip) + prefix := fmt.Sprintf("%s/%d", ip, maskMap[ipFamily]) + expectedPrefixes[ipFamily] = append(expectedPrefixes[ipFamily], prefix) +} + // isPodAlive returns whether a Pod is alive or not func isPodAlive(p *v1.Pod) bool { if p.Status.Phase == v1.PodSucceeded && p.Spec.RestartPolicy != v1.RestartPolicyAlways { @@ -72,3 +87,32 @@ func parseRoute(route string) (string, uint32, error) { } return prefix, prefixLen, nil } + +// getGatewayName returns the name of the NAT GW hosting this speaker +func getGatewayName() string { + hostname := os.Getenv(HostnameEnvVariable) + hostnameSplit := strings.Split(hostname, "-") + splitLength := len(hostnameSplit) + + // The name of the GW is right before the index in the name of the pod + // For example: "vpc-nat-gw-gw1-0" is the name of the pod hosting the NAT GW "gw1" + if splitLength < 2 { + return "" + } + + return hostnameSplit[splitLength-2] +} + +// kubeOvnFamilyToAFI converts an IP family to its associated AFI +func kubeOvnFamilyToAFI(ipFamily string) (bgpapi.Family_Afi, error) { + var family bgpapi.Family_Afi + if ipFamily == kubeovnv1.ProtocolIPv6 { + family = bgpapi.Family_AFI_IP6 + } else if ipFamily == kubeovnv1.ProtocolIPv4 { + family = bgpapi.Family_AFI_IP + } else { + return bgpapi.Family_AFI_UNKNOWN, fmt.Errorf("ip family is invalid") + } + + return family, nil +} From eebe73990b8d0e5fe59168fbc66855fdad8936d3 Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Wed, 10 Jul 2024 10:24:46 -0400 Subject: [PATCH 03/15] feat(nat-gw): inject as env var the name of the gateway, prepare bgp speaker to integrate into the image Signed-off-by: SkalaNetworks --- pkg/controller/vpc_nat_gateway.go | 6 ++++++ pkg/speaker/bgp.go | 2 +- pkg/speaker/config.go | 5 ++++- pkg/speaker/controller.go | 13 ++++++++++++- pkg/speaker/natgateway.go | 13 ++++++++----- pkg/speaker/utils.go | 16 +++++----------- 6 files changed, 36 insertions(+), 19 deletions(-) diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go index fb13f8f3f61..5e618d1fb48 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -847,6 +847,12 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 Privileged: ptr.To(true), AllowPrivilegeEscalation: ptr.To(true), }, + Env: []corev1.EnvVar{ + { + Name: "GATEWAY_NAME", + Value: gw.Name, + }, + }, }, }, NodeSelector: selectors, diff --git a/pkg/speaker/bgp.go b/pkg/speaker/bgp.go index bb097ffccfa..225f68f9a73 100644 --- a/pkg/speaker/bgp.go +++ b/pkg/speaker/bgp.go @@ -70,7 +70,7 @@ func (c *Controller) reconciliateIPFamily(ipFamily string, expectedPrefixes pref } } - // Ask the BGP speaker what routes we're announcing for the IP family selected + // Ask the BGP speaker what route we're announcing for the IP family selected if err := c.config.BgpServer.ListPath(context.Background(), listPathRequest, fn); err != nil { return fmt.Errorf("failed to list existing %s routes: %w", ipFamily, err) } diff --git a/pkg/speaker/config.go b/pkg/speaker/config.go index 2532cade11f..ff4bc54dbf7 100644 --- a/pkg/speaker/config.go +++ b/pkg/speaker/config.go @@ -55,6 +55,7 @@ type Configuration struct { GracefulRestartTime time.Duration PassiveMode bool EbgpMultihopTTL uint8 + EIPAnnouncement bool NodeName string KubeConfigFile string @@ -83,8 +84,9 @@ func ParseFlags() (*Configuration, error) { argPprofPort = pflag.Uint32("pprof-port", DefaultPprofPort, "The port to get profiling data, default: 10667") argNodeName = pflag.String("node-name", os.Getenv(util.HostnameEnv), "Name of the node on which the speaker is running on.") argKubeConfigFile = pflag.String("kubeconfig", "", "Path to kubeconfig file with authorization and master location information. If not set use the inCluster token.") - argPassiveMode = pflag.BoolP("passivemode", "", false, "Set BGP Speaker to passive model,do not actively initiate connections to peers") + argPassiveMode = pflag.BoolP("passivemode", "", false, "Set BGP Speaker to passive model, do not actively initiate connections to peers") argEbgpMultihopTTL = pflag.Uint8("ebgp-multihop", DefaultEbgpMultiHop, "The TTL value of EBGP peer, default: 1") + argEIPAnnouncement = pflag.BoolP("eip-announcement", "", false, "Make the BGP speaker announce EIPs from gateways") ) klogFlags := flag.NewFlagSet("klog", flag.ExitOnError) klog.InitFlags(klogFlags) @@ -148,6 +150,7 @@ func ParseFlags() (*Configuration, error) { GracefulRestartTime: *argDefaultGracefulTime, PassiveMode: *argPassiveMode, EbgpMultihopTTL: *argEbgpMultihopTTL, + EIPAnnouncement: *argEIPAnnouncement, } if *argNeighborAddress != "" { diff --git a/pkg/speaker/controller.go b/pkg/speaker/controller.go index 1b5389adee6..62c0f7379ca 100644 --- a/pkg/speaker/controller.go +++ b/pkg/speaker/controller.go @@ -102,8 +102,19 @@ func (c *Controller) Run(stopCh <-chan struct{}) { } klog.Info("Started workers") - go wait.Until(c.syncSubnetRoutes, 5*time.Second, stopCh) + go wait.Until(c.Reconcile, 5*time.Second, stopCh) <-stopCh klog.Info("Shutting down workers") } + +func (c *Controller) Reconcile() { + if c.config.EIPAnnouncement { + err := c.syncEIPRoutes() + if err != nil { + klog.Errorf("failed to reconcile EIPs: %s", err.Error()) + } + } else { + c.syncSubnetRoutes() + } +} diff --git a/pkg/speaker/natgateway.go b/pkg/speaker/natgateway.go index 00e78dccb74..473938b131c 100644 --- a/pkg/speaker/natgateway.go +++ b/pkg/speaker/natgateway.go @@ -6,10 +6,7 @@ import ( "github.com/kubeovn/kube-ovn/pkg/util" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" -) - -const ( - HostnameEnvVariable = "HOSTNAME" + "k8s.io/klog/v2" ) // syncEIPRoutes retrieves all the EIPs attached to our GWs and starts announcing their route @@ -20,6 +17,8 @@ func (c *Controller) syncEIPRoutes() error { return fmt.Errorf("failed to retrieve the name of the gateway, might not be running in a gateway pod") } + klog.Infof("gw name is: %s", gatewayName) + // Create label requirements to only get EIPs attached to our NAT GW requirements, err := labels.NewRequirement(util.VpcNatGatewayLabel, selection.Equals, []string{gatewayName}) if err != nil { @@ -32,13 +31,17 @@ func (c *Controller) syncEIPRoutes() error { return fmt.Errorf("failed to list EIPs attached to our GW: %w", err) } + klog.Infof("%v", eips) + return c.announceEIPs(eips) } +// announceEIPs announce all the prefixes related to EIPs attached to a GW func (c *Controller) announceEIPs(eips []*v1.IptablesEIP) error { expectedPrefixes := make(prefixMap) for _, eip := range eips { - if !eip.Status.Ready { // Only announce EIPs marked as "ready" + // Only announce EIPs marked as "ready" and with the BGP annotation set to true + if eip.Annotations[util.BgpAnnotation] != "true" || !eip.Status.Ready { continue } diff --git a/pkg/speaker/utils.go b/pkg/speaker/utils.go index 0c9a4ae7950..e93bae7ffcd 100644 --- a/pkg/speaker/utils.go +++ b/pkg/speaker/utils.go @@ -11,6 +11,10 @@ import ( "strings" ) +const ( + GatewayNameEnvVariable = "GATEWAY_NAME" +) + // prefixMap is a map associating an IP family (IPv4 or IPv6) and an IP type prefixMap map[string][]string @@ -90,17 +94,7 @@ func parseRoute(route string) (string, uint32, error) { // getGatewayName returns the name of the NAT GW hosting this speaker func getGatewayName() string { - hostname := os.Getenv(HostnameEnvVariable) - hostnameSplit := strings.Split(hostname, "-") - splitLength := len(hostnameSplit) - - // The name of the GW is right before the index in the name of the pod - // For example: "vpc-nat-gw-gw1-0" is the name of the pod hosting the NAT GW "gw1" - if splitLength < 2 { - return "" - } - - return hostnameSplit[splitLength-2] + return os.Getenv(GatewayNameEnvVariable) } // kubeOvnFamilyToAFI converts an IP family to its associated AFI From a3719d9350f1546acb51506337a0b38ec1fd31a8 Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Fri, 12 Jul 2024 04:30:27 -0400 Subject: [PATCH 04/15] fix(merge): rebase Signed-off-by: SkalaNetworks --- pkg/controller/vpc_nat.go | 16 +++-- pkg/controller/vpc_nat_gateway.go | 87 +++++++++++++++++++++++++-- pkg/speaker/config.go | 6 +- pkg/speaker/controller.go | 4 +- pkg/speaker/{natgateway.go => eip.go} | 7 +-- pkg/speaker/utils.go | 6 +- pkg/util/const.go | 3 +- 7 files changed, 102 insertions(+), 27 deletions(-) rename pkg/speaker/{natgateway.go => eip.go} (89%) diff --git a/pkg/controller/vpc_nat.go b/pkg/controller/vpc_nat.go index 57b0eee160d..d56a32ab950 100644 --- a/pkg/controller/vpc_nat.go +++ b/pkg/controller/vpc_nat.go @@ -2,13 +2,14 @@ package controller import ( "fmt" - - "k8s.io/klog/v2" - "github.com/kubeovn/kube-ovn/pkg/util" + "k8s.io/klog/v2" ) -var vpcNatImage = "" +var ( + vpcNatImage = "" + vpcNatEnableBgpSpeaker = false +) func (c *Controller) resyncVpcNatImage() { cm, err := c.configMapsLister.ConfigMaps(c.config.PodNamespace).Get(util.VpcNatConfig) @@ -17,6 +18,7 @@ func (c *Controller) resyncVpcNatImage() { klog.Error(err) return } + image, exist := cm.Data["image"] if !exist { err = fmt.Errorf("%s should have image field", util.VpcNatConfig) @@ -24,4 +26,10 @@ func (c *Controller) resyncVpcNatImage() { return } vpcNatImage = image + + enableBgpSpeaker, exist := cm.Data["enableBgpSpeaker"] + if exist && enableBgpSpeaker == "true" { + klog.V(5).Infof("experimental BGP speaker enabled") + vpcNatEnableBgpSpeaker = true + } } diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go index 5e618d1fb48..09cf5e9f49b 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "maps" + "os" "reflect" "regexp" "slices" @@ -735,6 +736,39 @@ func (c *Controller) execNatGwRules(pod *corev1.Pod, operation string, rules []s return nil } +func (c *Controller) setNatGwInterface(annotations map[string]string, externalNetwork string, defaultSubnet *kubeovnv1.Subnet) { + nad := fmt.Sprintf("%s/%s, %s/%s", c.config.PodNamespace, externalNetwork, corev1.NamespaceDefault, nadName) + annotations[util.AttachmentNetworkAnnotation] = nad + + setNatGwRoute(annotations, defaultSubnet.Spec.Gateway) +} + +func setNatGwRoute(annotations map[string]string, subnetGw string) { + dst := os.Getenv("KUBERNETES_SERVICE_HOST") + + protocol := util.CheckProtocol(dst) + if !strings.ContainsRune(dst, '/') { + switch protocol { + case kubeovnv1.ProtocolIPv4: + dst = fmt.Sprintf("%s/32", dst) + case kubeovnv1.ProtocolIPv6: + dst = fmt.Sprintf("%s/128", dst) + } + } + for _, gw := range strings.Split(subnetGw, ",") { + if util.CheckProtocol(gw) == protocol { + routes := []request.Route{{Destination: dst, Gateway: gw}} + buf, err := json.Marshal(routes) + if err != nil { + klog.Errorf("failed to marshal routes %+v: %v", routes, err) + } else { + annotations[fmt.Sprintf(util.RoutesAnnotationTemplate, nadProvider)] = string(buf) + } + break + } + } +} + func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1.StatefulSet) (*v1.StatefulSet, error) { annotations := make(map[string]string, 7) if oldSts != nil && len(oldSts.Annotations) != 0 { @@ -747,6 +781,15 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 util.LogicalSwitchAnnotation: gw.Spec.Subnet, util.IPAddressAnnotation: gw.Spec.LanIP, } + + if vpcNatEnableBgpSpeaker { // Add an interface that can reach the API server + defaultSubnet, err := c.subnetsLister.Get(c.config.DefaultLogicalSwitch) + if err != nil { + return nil, fmt.Errorf("failed to get default subnet %s: %v", c.config.DefaultLogicalSwitch, err) + } + c.setNatGwInterface(podAnnotations, nadName, defaultSubnet) + } + for key, value := range podAnnotations { annotations[key] = value } @@ -782,6 +825,7 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 routes = append(routes, request.Route{Destination: cidrV6, Gateway: v6Gateway}) } } + if err = setPodRoutesAnnotation(annotations, util.OvnProvider, routes); err != nil { klog.Error(err) return nil, err @@ -820,6 +864,7 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 "app": name, util.VpcNatGatewayLabel: "true", } + sts := &v1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -847,12 +892,6 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 Privileged: ptr.To(true), AllowPrivilegeEscalation: ptr.To(true), }, - Env: []corev1.EnvVar{ - { - Name: "GATEWAY_NAME", - Value: gw.Name, - }, - }, }, }, NodeSelector: selectors, @@ -865,6 +904,42 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 }, }, } + + if vpcNatEnableBgpSpeaker { + containers := sts.Spec.Template.Spec.Containers + + sts.Spec.Template.Spec.ServiceAccountName = "vpc-nat-gw" + speakerContainer := corev1.Container{ + Name: "vpc-nat-gw-speaker", + Image: "superphenix.net/kubeovn:latest", + Command: []string{"/kube-ovn/kube-ovn-speaker"}, + ImagePullPolicy: corev1.PullIfNotPresent, + Env: []corev1.EnvVar{ + { + Name: util.GatewayNameEnv, + Value: gw.Name, + }, + { + Name: "POD_IP", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "status.podIP", + }, + }, + }, + }, + Args: []string{ + "--neighbor-address=100.127.4.161", + "--neighbor-as=65500", + "--cluster-as=65000", + "--nat-gw-mode", + "-v5", + }, + } + + sts.Spec.Template.Spec.Containers = append(containers, speakerContainer) + } + return sts, nil } diff --git a/pkg/speaker/config.go b/pkg/speaker/config.go index ff4bc54dbf7..66de90eeb4e 100644 --- a/pkg/speaker/config.go +++ b/pkg/speaker/config.go @@ -55,7 +55,7 @@ type Configuration struct { GracefulRestartTime time.Duration PassiveMode bool EbgpMultihopTTL uint8 - EIPAnnouncement bool + NatGwMode bool NodeName string KubeConfigFile string @@ -86,7 +86,7 @@ func ParseFlags() (*Configuration, error) { argKubeConfigFile = pflag.String("kubeconfig", "", "Path to kubeconfig file with authorization and master location information. If not set use the inCluster token.") argPassiveMode = pflag.BoolP("passivemode", "", false, "Set BGP Speaker to passive model, do not actively initiate connections to peers") argEbgpMultihopTTL = pflag.Uint8("ebgp-multihop", DefaultEbgpMultiHop, "The TTL value of EBGP peer, default: 1") - argEIPAnnouncement = pflag.BoolP("eip-announcement", "", false, "Make the BGP speaker announce EIPs from gateways") + argNatGwMode = pflag.BoolP("nat-gw-mode", "", false, "Make the BGP speaker announce EIPs from inside a NAT gateway, Pod IP/Service/Subnet announcements will be disabled") ) klogFlags := flag.NewFlagSet("klog", flag.ExitOnError) klog.InitFlags(klogFlags) @@ -150,7 +150,7 @@ func ParseFlags() (*Configuration, error) { GracefulRestartTime: *argDefaultGracefulTime, PassiveMode: *argPassiveMode, EbgpMultihopTTL: *argEbgpMultihopTTL, - EIPAnnouncement: *argEIPAnnouncement, + NatGwMode: *argNatGwMode, } if *argNeighborAddress != "" { diff --git a/pkg/speaker/controller.go b/pkg/speaker/controller.go index 62c0f7379ca..ff3315fdb07 100644 --- a/pkg/speaker/controller.go +++ b/pkg/speaker/controller.go @@ -1,6 +1,7 @@ package speaker import ( + "github.com/kubeovn/kube-ovn/pkg/util" "time" corev1 "k8s.io/api/core/v1" @@ -18,7 +19,6 @@ import ( kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" kubeovninformer "github.com/kubeovn/kube-ovn/pkg/client/informers/externalversions" kubeovnlister "github.com/kubeovn/kube-ovn/pkg/client/listers/kubeovn/v1" - "github.com/kubeovn/kube-ovn/pkg/util" ) const controllerAgentName = "ovn-speaker" @@ -109,7 +109,7 @@ func (c *Controller) Run(stopCh <-chan struct{}) { } func (c *Controller) Reconcile() { - if c.config.EIPAnnouncement { + if c.config.NatGwMode { err := c.syncEIPRoutes() if err != nil { klog.Errorf("failed to reconcile EIPs: %s", err.Error()) diff --git a/pkg/speaker/natgateway.go b/pkg/speaker/eip.go similarity index 89% rename from pkg/speaker/natgateway.go rename to pkg/speaker/eip.go index 473938b131c..e9a65798cb1 100644 --- a/pkg/speaker/natgateway.go +++ b/pkg/speaker/eip.go @@ -6,7 +6,6 @@ import ( "github.com/kubeovn/kube-ovn/pkg/util" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" - "k8s.io/klog/v2" ) // syncEIPRoutes retrieves all the EIPs attached to our GWs and starts announcing their route @@ -17,10 +16,8 @@ func (c *Controller) syncEIPRoutes() error { return fmt.Errorf("failed to retrieve the name of the gateway, might not be running in a gateway pod") } - klog.Infof("gw name is: %s", gatewayName) - // Create label requirements to only get EIPs attached to our NAT GW - requirements, err := labels.NewRequirement(util.VpcNatGatewayLabel, selection.Equals, []string{gatewayName}) + requirements, err := labels.NewRequirement(util.VpcNatGatewayNameLabel, selection.Equals, []string{gatewayName}) if err != nil { return fmt.Errorf("failed to create label selector requirement: %w", err) } @@ -31,8 +28,6 @@ func (c *Controller) syncEIPRoutes() error { return fmt.Errorf("failed to list EIPs attached to our GW: %w", err) } - klog.Infof("%v", eips) - return c.announceEIPs(eips) } diff --git a/pkg/speaker/utils.go b/pkg/speaker/utils.go index e93bae7ffcd..c2ee7458628 100644 --- a/pkg/speaker/utils.go +++ b/pkg/speaker/utils.go @@ -11,10 +11,6 @@ import ( "strings" ) -const ( - GatewayNameEnvVariable = "GATEWAY_NAME" -) - // prefixMap is a map associating an IP family (IPv4 or IPv6) and an IP type prefixMap map[string][]string @@ -94,7 +90,7 @@ func parseRoute(route string) (string, uint32, error) { // getGatewayName returns the name of the NAT GW hosting this speaker func getGatewayName() string { - return os.Getenv(GatewayNameEnvVariable) + return os.Getenv(util.GatewayNameEnv) } // kubeOvnFamilyToAFI converts an IP family to its associated AFI diff --git a/pkg/util/const.go b/pkg/util/const.go index b0d7006b609..c38411a1625 100644 --- a/pkg/util/const.go +++ b/pkg/util/const.go @@ -213,7 +213,8 @@ const ( InternalType = "internal-port" DpdkType = "dpdk-port" - HostnameEnv = "KUBE_NODE_NAME" + HostnameEnv = "KUBE_NODE_NAME" + GatewayNameEnv = "GATEWAY_NAME" MirrosRetryMaxTimes = 5 MirrosRetryInterval = 1 From d84718315f42c96a9e7037d29edc9c704ab11c06 Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Fri, 12 Jul 2024 06:05:08 -0400 Subject: [PATCH 05/15] fix(nat-gw): fix bgp speaker error handling Signed-off-by: SkalaNetworks --- pkg/controller/vpc_nat.go | 1 - pkg/speaker/bgp.go | 4 ++-- pkg/speaker/utils.go | 11 ++++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/controller/vpc_nat.go b/pkg/controller/vpc_nat.go index d56a32ab950..e4ea35cdd0f 100644 --- a/pkg/controller/vpc_nat.go +++ b/pkg/controller/vpc_nat.go @@ -29,7 +29,6 @@ func (c *Controller) resyncVpcNatImage() { enableBgpSpeaker, exist := cm.Data["enableBgpSpeaker"] if exist && enableBgpSpeaker == "true" { - klog.V(5).Infof("experimental BGP speaker enabled") vpcNatEnableBgpSpeaker = true } } diff --git a/pkg/speaker/bgp.go b/pkg/speaker/bgp.go index 225f68f9a73..4ff81025bd7 100644 --- a/pkg/speaker/bgp.go +++ b/pkg/speaker/bgp.go @@ -112,7 +112,7 @@ func (c *Controller) addRoute(route string) error { // Get NLRI and attributes to announce all the next hops possible nlri, attrs, err := c.getNlriAndAttrs(route) if err != nil { - return fmt.Errorf("failed to get NLRI and attributes: %w", nlri) + return fmt.Errorf("failed to get NLRI and attributes: %w", err) } // Announce every next hop we have @@ -144,7 +144,7 @@ func (c *Controller) delRoute(route string) error { // Get NLRI and attributes to announce all the next hops possible nlri, attrs, err := c.getNlriAndAttrs(route) if err != nil { - return fmt.Errorf("failed to get NLRI and attributes: %w", nlri) + return fmt.Errorf("failed to get NLRI and attributes: %w", err) } // Withdraw every next hop we have diff --git a/pkg/speaker/utils.go b/pkg/speaker/utils.go index c2ee7458628..dfa33c2234e 100644 --- a/pkg/speaker/utils.go +++ b/pkg/speaker/utils.go @@ -79,7 +79,7 @@ func parseRoute(route string) (string, uint32, error) { if strings.Contains(route, "/") { prefix = strings.Split(route, "/")[0] strLen := strings.Split(route, "/")[1] - intLen, err := strconv.Atoi(strLen) + intLen, err := strconv.ParseInt(strLen, 10, 32) if err != nil { return "", 0, err } @@ -96,11 +96,12 @@ func getGatewayName() string { // kubeOvnFamilyToAFI converts an IP family to its associated AFI func kubeOvnFamilyToAFI(ipFamily string) (bgpapi.Family_Afi, error) { var family bgpapi.Family_Afi - if ipFamily == kubeovnv1.ProtocolIPv6 { - family = bgpapi.Family_AFI_IP6 - } else if ipFamily == kubeovnv1.ProtocolIPv4 { + switch ipFamily { + case kubeovnv1.ProtocolIPv4: family = bgpapi.Family_AFI_IP - } else { + case kubeovnv1.ProtocolIPv6: + family = bgpapi.Family_AFI_IP6 + default: return bgpapi.Family_AFI_UNKNOWN, fmt.Errorf("ip family is invalid") } From eda5e2ff996493ea44e1a38b6e6aa5802eeab1da Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Fri, 12 Jul 2024 08:44:17 -0400 Subject: [PATCH 06/15] feat(natgw): make speaker image a parameter Signed-off-by: SkalaNetworks --- pkg/controller/vpc_nat.go | 15 ++++++++++++--- pkg/controller/vpc_nat_gateway.go | 6 +++--- pkg/speaker/bgp.go | 3 ++- pkg/speaker/controller.go | 3 ++- pkg/speaker/eip.go | 1 + pkg/speaker/utils.go | 7 ++++--- 6 files changed, 24 insertions(+), 11 deletions(-) diff --git a/pkg/controller/vpc_nat.go b/pkg/controller/vpc_nat.go index e4ea35cdd0f..28ad4681533 100644 --- a/pkg/controller/vpc_nat.go +++ b/pkg/controller/vpc_nat.go @@ -7,8 +7,9 @@ import ( ) var ( - vpcNatImage = "" - vpcNatEnableBgpSpeaker = false + vpcNatImage = "" + vpcNatGwEnableBgpSpeaker = false + vpcNatGwBgpSpeakerImage = "" ) func (c *Controller) resyncVpcNatImage() { @@ -27,8 +28,16 @@ func (c *Controller) resyncVpcNatImage() { } vpcNatImage = image + // Check BGP is enabled on the NAT GW, if yes, verify required parameters are present enableBgpSpeaker, exist := cm.Data["enableBgpSpeaker"] if exist && enableBgpSpeaker == "true" { - vpcNatEnableBgpSpeaker = true + vpcNatGwEnableBgpSpeaker = true + + vpcNatGwBgpSpeakerImage, exist = cm.Data["bgpSpeakerImage"] + if !exist { + err = fmt.Errorf("%s should have bgp speaker image field if bgp enabled", util.VpcNatConfig) + klog.Error(err) + return + } } } diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go index 09cf5e9f49b..0deed0d7fe4 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -782,7 +782,7 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 util.IPAddressAnnotation: gw.Spec.LanIP, } - if vpcNatEnableBgpSpeaker { // Add an interface that can reach the API server + if vpcNatGwEnableBgpSpeaker { // Add an interface that can reach the API server defaultSubnet, err := c.subnetsLister.Get(c.config.DefaultLogicalSwitch) if err != nil { return nil, fmt.Errorf("failed to get default subnet %s: %v", c.config.DefaultLogicalSwitch, err) @@ -905,13 +905,13 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 }, } - if vpcNatEnableBgpSpeaker { + if vpcNatGwEnableBgpSpeaker { containers := sts.Spec.Template.Spec.Containers sts.Spec.Template.Spec.ServiceAccountName = "vpc-nat-gw" speakerContainer := corev1.Container{ Name: "vpc-nat-gw-speaker", - Image: "superphenix.net/kubeovn:latest", + Image: vpcNatGwBgpSpeakerImage, Command: []string{"/kube-ovn/kube-ovn-speaker"}, ImagePullPolicy: corev1.PullIfNotPresent, Env: []corev1.EnvVar{ diff --git a/pkg/speaker/bgp.go b/pkg/speaker/bgp.go index 4ff81025bd7..9cdeb40819a 100644 --- a/pkg/speaker/bgp.go +++ b/pkg/speaker/bgp.go @@ -3,6 +3,8 @@ package speaker import ( "context" "fmt" + "net" + kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" "github.com/kubeovn/kube-ovn/pkg/util" bgpapi "github.com/osrg/gobgp/v3/api" @@ -12,7 +14,6 @@ import ( "golang.org/x/sys/unix" "google.golang.org/protobuf/types/known/anypb" "k8s.io/klog/v2" - "net" ) var ( diff --git a/pkg/speaker/controller.go b/pkg/speaker/controller.go index ff3315fdb07..efd32c27052 100644 --- a/pkg/speaker/controller.go +++ b/pkg/speaker/controller.go @@ -1,9 +1,10 @@ package speaker import ( - "github.com/kubeovn/kube-ovn/pkg/util" "time" + "github.com/kubeovn/kube-ovn/pkg/util" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" diff --git a/pkg/speaker/eip.go b/pkg/speaker/eip.go index e9a65798cb1..b0d6aaa665c 100644 --- a/pkg/speaker/eip.go +++ b/pkg/speaker/eip.go @@ -2,6 +2,7 @@ package speaker import ( "fmt" + v1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" "github.com/kubeovn/kube-ovn/pkg/util" "k8s.io/apimachinery/pkg/labels" diff --git a/pkg/speaker/utils.go b/pkg/speaker/utils.go index dfa33c2234e..7af4753c5fc 100644 --- a/pkg/speaker/utils.go +++ b/pkg/speaker/utils.go @@ -2,13 +2,14 @@ package speaker import ( "fmt" + "os" + "strconv" + "strings" + kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" "github.com/kubeovn/kube-ovn/pkg/util" bgpapi "github.com/osrg/gobgp/v3/api" v1 "k8s.io/api/core/v1" - "os" - "strconv" - "strings" ) // prefixMap is a map associating an IP family (IPv4 or IPv6) and an IP From 0499362f1f8b48ead2c428ac69f69a24cb1dfeb0 Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Fri, 12 Jul 2024 14:25:42 -0400 Subject: [PATCH 07/15] feat(nat-gw): make bgp speaker optional per gw Signed-off-by: SkalaNetworks --- charts/kube-ovn/templates/kube-ovn-crd.yaml | 9 +++++++++ dist/images/install.sh | 9 +++++++++ pkg/apis/kubeovn/v1/types.go | 6 ++++++ pkg/controller/vpc_nat_gateway.go | 20 +++++++++++--------- 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/charts/kube-ovn/templates/kube-ovn-crd.yaml b/charts/kube-ovn/templates/kube-ovn-crd.yaml index 565f54ebda6..d6ccd23afec 100644 --- a/charts/kube-ovn/templates/kube-ovn-crd.yaml +++ b/charts/kube-ovn/templates/kube-ovn-crd.yaml @@ -503,6 +503,15 @@ spec: type: string qosPolicy: type: string + bgpSpeaker: + type: object + properties: + enabled: + type: boolean + parameters: + type: array + items: + type: string tolerations: type: array items: diff --git a/dist/images/install.sh b/dist/images/install.sh index 6dc342a0d1a..cbc86509d6e 100755 --- a/dist/images/install.sh +++ b/dist/images/install.sh @@ -741,6 +741,15 @@ spec: type: string qosPolicy: type: string + bgpSpeaker: + type: object + properties: + enabled: + type: boolean + parameters: + type: array + items: + type: string tolerations: type: array items: diff --git a/pkg/apis/kubeovn/v1/types.go b/pkg/apis/kubeovn/v1/types.go index 5f2c25d8146..f43187daf2a 100644 --- a/pkg/apis/kubeovn/v1/types.go +++ b/pkg/apis/kubeovn/v1/types.go @@ -524,6 +524,12 @@ type VpcNatSpec struct { Tolerations []corev1.Toleration `json:"tolerations"` Affinity corev1.Affinity `json:"affinity"` QoSPolicy string `json:"qosPolicy"` + BgpSpeaker VpcBgpSpeaker `json:"bgpSpeaker"` +} + +type VpcBgpSpeaker struct { + Enabled bool `json:"enabled"` + Parameters []string `json:"parameters"` } type VpcNatStatus struct { diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go index 0deed0d7fe4..aea99f78cdb 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -782,7 +782,7 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 util.IPAddressAnnotation: gw.Spec.LanIP, } - if vpcNatGwEnableBgpSpeaker { // Add an interface that can reach the API server + if vpcNatGwEnableBgpSpeaker && gw.Spec.BgpSpeaker.Enabled { // Add an interface that can reach the API server defaultSubnet, err := c.subnetsLister.Get(c.config.DefaultLogicalSwitch) if err != nil { return nil, fmt.Errorf("failed to get default subnet %s: %v", c.config.DefaultLogicalSwitch, err) @@ -905,9 +905,17 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 }, } - if vpcNatGwEnableBgpSpeaker { + // BGP speaker for GWs must be enabled globally and for this specific instance + if vpcNatGwEnableBgpSpeaker && gw.Spec.BgpSpeaker.Enabled { containers := sts.Spec.Template.Spec.Containers + args := []string{ + "--nat-gw-mode", + "-v5", + } + + args = append(args, gw.Spec.BgpSpeaker.Parameters...) + sts.Spec.Template.Spec.ServiceAccountName = "vpc-nat-gw" speakerContainer := corev1.Container{ Name: "vpc-nat-gw-speaker", @@ -928,13 +936,7 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 }, }, }, - Args: []string{ - "--neighbor-address=100.127.4.161", - "--neighbor-as=65500", - "--cluster-as=65000", - "--nat-gw-mode", - "-v5", - }, + Args: args, } sts.Spec.Template.Spec.Containers = append(containers, speakerContainer) From 5cbd510f587b6a31714897862abfe0e99a7c7c03 Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Mon, 15 Jul 2024 03:43:07 -0400 Subject: [PATCH 08/15] fix: imports/uint parse Signed-off-by: SkalaNetworks --- pkg/controller/vpc_nat.go | 1 + pkg/speaker/bgp.go | 10 ++++------ pkg/speaker/controller.go | 3 +-- pkg/speaker/eip.go | 5 +++-- pkg/speaker/utils.go | 7 ++++--- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/controller/vpc_nat.go b/pkg/controller/vpc_nat.go index 28ad4681533..9cc7ed2d325 100644 --- a/pkg/controller/vpc_nat.go +++ b/pkg/controller/vpc_nat.go @@ -2,6 +2,7 @@ package controller import ( "fmt" + "github.com/kubeovn/kube-ovn/pkg/util" "k8s.io/klog/v2" ) diff --git a/pkg/speaker/bgp.go b/pkg/speaker/bgp.go index 9cdeb40819a..52231dbcffb 100644 --- a/pkg/speaker/bgp.go +++ b/pkg/speaker/bgp.go @@ -5,8 +5,6 @@ import ( "fmt" "net" - kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" - "github.com/kubeovn/kube-ovn/pkg/util" bgpapi "github.com/osrg/gobgp/v3/api" bgpapiutil "github.com/osrg/gobgp/v3/pkg/apiutil" "github.com/osrg/gobgp/v3/pkg/packet/bgp" @@ -14,12 +12,13 @@ import ( "golang.org/x/sys/unix" "google.golang.org/protobuf/types/known/anypb" "k8s.io/klog/v2" -) -var ( - maskMap = map[string]int{kubeovnv1.ProtocolIPv4: 32, kubeovnv1.ProtocolIPv6: 128} + kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" + "github.com/kubeovn/kube-ovn/pkg/util" ) +var maskMap = map[string]int{kubeovnv1.ProtocolIPv4: 32, kubeovnv1.ProtocolIPv6: 128} + // reconciliateRoutes configures the BGP speaker to announce only the routes we are expected to announce // and to withdraw the ones that should not be announced anymore func (c *Controller) reconciliateRoutes(expectedPrefixes prefixMap) error { @@ -125,7 +124,6 @@ func (c *Controller) addRoute(route string) error { Pattrs: attr, }, }) - if err != nil { return err } diff --git a/pkg/speaker/controller.go b/pkg/speaker/controller.go index efd32c27052..1fcbda7e66e 100644 --- a/pkg/speaker/controller.go +++ b/pkg/speaker/controller.go @@ -3,8 +3,6 @@ package speaker import ( "time" - "github.com/kubeovn/kube-ovn/pkg/util" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -20,6 +18,7 @@ import ( kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" kubeovninformer "github.com/kubeovn/kube-ovn/pkg/client/informers/externalversions" kubeovnlister "github.com/kubeovn/kube-ovn/pkg/client/listers/kubeovn/v1" + "github.com/kubeovn/kube-ovn/pkg/util" ) const controllerAgentName = "ovn-speaker" diff --git a/pkg/speaker/eip.go b/pkg/speaker/eip.go index b0d6aaa665c..8ff7993b967 100644 --- a/pkg/speaker/eip.go +++ b/pkg/speaker/eip.go @@ -3,10 +3,11 @@ package speaker import ( "fmt" - v1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" - "github.com/kubeovn/kube-ovn/pkg/util" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" + + v1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" + "github.com/kubeovn/kube-ovn/pkg/util" ) // syncEIPRoutes retrieves all the EIPs attached to our GWs and starts announcing their route diff --git a/pkg/speaker/utils.go b/pkg/speaker/utils.go index 7af4753c5fc..6f89c1054b9 100644 --- a/pkg/speaker/utils.go +++ b/pkg/speaker/utils.go @@ -6,10 +6,11 @@ import ( "strconv" "strings" - kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" - "github.com/kubeovn/kube-ovn/pkg/util" bgpapi "github.com/osrg/gobgp/v3/api" v1 "k8s.io/api/core/v1" + + kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" + "github.com/kubeovn/kube-ovn/pkg/util" ) // prefixMap is a map associating an IP family (IPv4 or IPv6) and an IP @@ -80,7 +81,7 @@ func parseRoute(route string) (string, uint32, error) { if strings.Contains(route, "/") { prefix = strings.Split(route, "/")[0] strLen := strings.Split(route, "/")[1] - intLen, err := strconv.ParseInt(strLen, 10, 32) + intLen, err := strconv.ParseUint(strLen, 10, 32) if err != nil { return "", 0, err } From e59763ce450c33f8609a4e3973cfd4cb35972f2f Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Mon, 15 Jul 2024 04:08:15 -0400 Subject: [PATCH 09/15] fix: linter Signed-off-by: SkalaNetworks --- pkg/controller/vpc_nat.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/controller/vpc_nat.go b/pkg/controller/vpc_nat.go index 9cc7ed2d325..5898e450ff5 100644 --- a/pkg/controller/vpc_nat.go +++ b/pkg/controller/vpc_nat.go @@ -3,8 +3,9 @@ package controller import ( "fmt" - "github.com/kubeovn/kube-ovn/pkg/util" "k8s.io/klog/v2" + + "github.com/kubeovn/kube-ovn/pkg/util" ) var ( From 561897d1975c2e15dc3504c4c5ca905356a65f8e Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Thu, 18 Jul 2024 03:19:55 -0400 Subject: [PATCH 10/15] chore(natgw): remove global flag to disable speaker in nat gateways Signed-off-by: SkalaNetworks --- pkg/controller/vpc_nat.go | 18 +++--------------- pkg/controller/vpc_nat_gateway.go | 9 +++++++-- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/pkg/controller/vpc_nat.go b/pkg/controller/vpc_nat.go index 5898e450ff5..7b456fa72a7 100644 --- a/pkg/controller/vpc_nat.go +++ b/pkg/controller/vpc_nat.go @@ -9,9 +9,8 @@ import ( ) var ( - vpcNatImage = "" - vpcNatGwEnableBgpSpeaker = false - vpcNatGwBgpSpeakerImage = "" + vpcNatImage = "" + vpcNatGwBgpSpeakerImage = "" ) func (c *Controller) resyncVpcNatImage() { @@ -30,16 +29,5 @@ func (c *Controller) resyncVpcNatImage() { } vpcNatImage = image - // Check BGP is enabled on the NAT GW, if yes, verify required parameters are present - enableBgpSpeaker, exist := cm.Data["enableBgpSpeaker"] - if exist && enableBgpSpeaker == "true" { - vpcNatGwEnableBgpSpeaker = true - - vpcNatGwBgpSpeakerImage, exist = cm.Data["bgpSpeakerImage"] - if !exist { - err = fmt.Errorf("%s should have bgp speaker image field if bgp enabled", util.VpcNatConfig) - klog.Error(err) - return - } - } + vpcNatGwBgpSpeakerImage = cm.Data["bgpSpeakerImage"] } diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go index aea99f78cdb..8650baebfff 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -782,7 +782,7 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 util.IPAddressAnnotation: gw.Spec.LanIP, } - if vpcNatGwEnableBgpSpeaker && gw.Spec.BgpSpeaker.Enabled { // Add an interface that can reach the API server + if gw.Spec.BgpSpeaker.Enabled { // Add an interface that can reach the API server defaultSubnet, err := c.subnetsLister.Get(c.config.DefaultLogicalSwitch) if err != nil { return nil, fmt.Errorf("failed to get default subnet %s: %v", c.config.DefaultLogicalSwitch, err) @@ -906,9 +906,14 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 } // BGP speaker for GWs must be enabled globally and for this specific instance - if vpcNatGwEnableBgpSpeaker && gw.Spec.BgpSpeaker.Enabled { + if gw.Spec.BgpSpeaker.Enabled { containers := sts.Spec.Template.Spec.Containers + // We need a speaker image configured in the NAT GW ConfigMap + if vpcNatGwBgpSpeakerImage == "" { + return nil, fmt.Errorf("%s should have bgp speaker image field if bgp enabled", util.VpcNatConfig) + } + args := []string{ "--nat-gw-mode", "-v5", From 35763387fb25cf87aec9e5daa100689fa2b18a98 Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Thu, 18 Jul 2024 06:09:31 -0400 Subject: [PATCH 11/15] feat(natgw): make bgp speaker configured by CRDs Signed-off-by: SkalaNetworks --- charts/kube-ovn/templates/kube-ovn-crd.yaml | 18 +++++- dist/images/install.sh | 18 +++++- pkg/apis/kubeovn/v1/types.go | 11 +++- pkg/controller/vpc_nat_gateway.go | 63 ++++++++++++++++++++- pkg/speaker/bgp.go | 16 +++--- pkg/speaker/config.go | 2 +- pkg/speaker/eip.go | 2 +- pkg/speaker/subnet.go | 4 +- 8 files changed, 115 insertions(+), 19 deletions(-) diff --git a/charts/kube-ovn/templates/kube-ovn-crd.yaml b/charts/kube-ovn/templates/kube-ovn-crd.yaml index d6ccd23afec..37ad6acfbfd 100644 --- a/charts/kube-ovn/templates/kube-ovn-crd.yaml +++ b/charts/kube-ovn/templates/kube-ovn-crd.yaml @@ -508,7 +508,23 @@ spec: properties: enabled: type: boolean - parameters: + asn: + type: integer + remoteAsn: + type: integer + neighbors: + type: array + items: + type: string + holdTime: + type: string + routerId: + type: string + password: + type: string + enableGracefulRestart: + type: boolean + extraArgs: type: array items: type: string diff --git a/dist/images/install.sh b/dist/images/install.sh index cbc86509d6e..70055457461 100755 --- a/dist/images/install.sh +++ b/dist/images/install.sh @@ -746,7 +746,23 @@ spec: properties: enabled: type: boolean - parameters: + asn: + type: integer + remoteAsn: + type: integer + neighbors: + type: array + items: + type: string + holdTime: + type: string + routerId: + type: string + password: + type: string + enableGracefulRestart: + type: boolean + extraArgs: type: array items: type: string diff --git a/pkg/apis/kubeovn/v1/types.go b/pkg/apis/kubeovn/v1/types.go index f43187daf2a..a2e951926e3 100644 --- a/pkg/apis/kubeovn/v1/types.go +++ b/pkg/apis/kubeovn/v1/types.go @@ -528,8 +528,15 @@ type VpcNatSpec struct { } type VpcBgpSpeaker struct { - Enabled bool `json:"enabled"` - Parameters []string `json:"parameters"` + Enabled bool `json:"enabled"` + ASN uint32 `json:"asn"` + RemoteASN uint32 `json:"remoteAsn"` + Neighbors []string `json:"neighbors"` + HoldTime metav1.Duration `json:"holdTime"` + RouterID string `json:"routerId"` + Password string `json:"password"` + EnableGracefulRestart bool `json:"enableGracefulRestart"` + ExtraArgs []string `json:"extraArgs"` } type VpcNatStatus struct { diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go index 8650baebfff..37d2b19eb45 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -915,11 +915,68 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 } args := []string{ - "--nat-gw-mode", - "-v5", + "--nat-gw-mode", // Force to run in NAT GW mode, we're not announcing Pod IPs or Services, only EIPs } - args = append(args, gw.Spec.BgpSpeaker.Parameters...) + speakerParams := gw.Spec.BgpSpeaker + + if speakerParams.RouterID != "" { // Override default auto-selected RouterID + args = append(args, fmt.Sprintf("--router-id=%s", speakerParams.RouterID)) + } + + if speakerParams.Password != "" { // Password for TCP MD5 BGP + args = append(args, fmt.Sprintf("--auth-password=%s", speakerParams.Password)) + } + + if speakerParams.EnableGracefulRestart { // Enable graceful restart + args = append(args, "--graceful-restart") + } + + if speakerParams.HoldTime != (metav1.Duration{}) { // Hold time + args = append(args, fmt.Sprintf("--holdtime=%s", speakerParams.HoldTime.Duration.String())) + } + + if speakerParams.ASN == 0 { // The ASN we use to speak + return nil, fmt.Errorf("ASN not set, but must be non-zero value") + } + + if speakerParams.RemoteASN == 0 { // The ASN we speak to + return nil, fmt.Errorf("remote ASN not set, but must be non-zero value") + } + + args = append(args, fmt.Sprintf("--cluster-as=%d", speakerParams.ASN)) + args = append(args, fmt.Sprintf("--neighbor-as=%d", speakerParams.RemoteASN)) + + if len(speakerParams.Neighbors) == 0 { + return nil, fmt.Errorf("no BGP neighbors specified") + } + + var neighIPv4 []string + var neighIPv6 []string + for _, neighbor := range speakerParams.Neighbors { + switch util.CheckProtocol(neighbor) { + case kubeovnv1.ProtocolIPv4: + neighIPv4 = append(neighIPv4, neighbor) + case kubeovnv1.ProtocolIPv6: + neighIPv6 = append(neighIPv6, neighbor) + } + } + + argNeighIPv4 := strings.Join(neighIPv4, ",") + argNeighIPv6 := strings.Join(neighIPv6, ",") + argNeighIPv4 = fmt.Sprintf("--neighbor-address=%s", argNeighIPv4) + argNeighIPv6 = fmt.Sprintf("--neighbor-ipv6-address=%s", argNeighIPv6) + + if len(neighIPv4) > 0 { + args = append(args, argNeighIPv4) + } + + if len(neighIPv6) > 0 { + args = append(args, argNeighIPv6) + } + + // Extra args to start the speaker with, for example, logging levels... + args = append(args, speakerParams.ExtraArgs...) sts.Spec.Template.Spec.ServiceAccountName = "vpc-nat-gw" speakerContainer := corev1.Container{ diff --git a/pkg/speaker/bgp.go b/pkg/speaker/bgp.go index 52231dbcffb..3ec6008bd37 100644 --- a/pkg/speaker/bgp.go +++ b/pkg/speaker/bgp.go @@ -19,29 +19,29 @@ import ( var maskMap = map[string]int{kubeovnv1.ProtocolIPv4: 32, kubeovnv1.ProtocolIPv6: 128} -// reconciliateRoutes configures the BGP speaker to announce only the routes we are expected to announce +// reconcileRoutes configures the BGP speaker to announce only the routes we are expected to announce // and to withdraw the ones that should not be announced anymore -func (c *Controller) reconciliateRoutes(expectedPrefixes prefixMap) error { +func (c *Controller) reconcileRoutes(expectedPrefixes prefixMap) error { if len(c.config.NeighborAddresses) != 0 { - err := c.reconciliateIPFamily(kubeovnv1.ProtocolIPv4, expectedPrefixes) + err := c.reconcileIPFamily(kubeovnv1.ProtocolIPv4, expectedPrefixes) if err != nil { - return fmt.Errorf("failed to reconciliate IPv4 routes: %w", err) + return fmt.Errorf("failed to reconcile IPv4 routes: %w", err) } } if len(c.config.NeighborIPv6Addresses) != 0 { - err := c.reconciliateIPFamily(kubeovnv1.ProtocolIPv6, expectedPrefixes) + err := c.reconcileIPFamily(kubeovnv1.ProtocolIPv6, expectedPrefixes) if err != nil { - return fmt.Errorf("failed to reconciliate IPv6 routes: %w", err) + return fmt.Errorf("failed to reconcile IPv6 routes: %w", err) } } return nil } -// reconciliateIPFamily announces prefixes we are not currently announcing and withdraws prefixes we should +// reconcileIPFamily announces prefixes we are not currently announcing and withdraws prefixes we should // not be announcing for a given IP family (IPv4/IPv6) -func (c *Controller) reconciliateIPFamily(ipFamily string, expectedPrefixes prefixMap) error { +func (c *Controller) reconcileIPFamily(ipFamily string, expectedPrefixes prefixMap) error { // Get the address family associated with the Kube-OVN family afi, err := kubeOvnFamilyToAFI(ipFamily) if err != nil { diff --git a/pkg/speaker/config.go b/pkg/speaker/config.go index 66de90eeb4e..49d2f69c58f 100644 --- a/pkg/speaker/config.go +++ b/pkg/speaker/config.go @@ -73,7 +73,7 @@ func ParseFlags() (*Configuration, error) { argAnnounceClusterIP = pflag.BoolP("announce-cluster-ip", "", false, "The Cluster IP of the service to announce to the BGP peers.") argGrpcHost = pflag.String("grpc-host", "127.0.0.1", "The host address for grpc to listen, default: 127.0.0.1") argGrpcPort = pflag.Uint32("grpc-port", DefaultBGPGrpcPort, "The port for grpc to listen, default:50051") - argClusterAs = pflag.Uint32("cluster-as", DefaultBGPClusterAs, "The as number of container network, default 65000") + argClusterAs = pflag.Uint32("cluster-as", DefaultBGPClusterAs, "The AS number of container network, default 65000") argRouterID = pflag.String("router-id", "", "The address for the speaker to use as router id, default the node ip") argNodeIPs = pflag.String("node-ips", "", "The comma-separated list of node IP addresses to use instead of the pod IP address for the next hop router IP address.") argNeighborAddress = pflag.String("neighbor-address", "", "Comma separated IPv4 router addresses the speaker connects to.") diff --git a/pkg/speaker/eip.go b/pkg/speaker/eip.go index 8ff7993b967..79c88376d15 100644 --- a/pkg/speaker/eip.go +++ b/pkg/speaker/eip.go @@ -51,5 +51,5 @@ func (c *Controller) announceEIPs(eips []*v1.IptablesEIP) error { } } - return c.reconciliateRoutes(expectedPrefixes) + return c.reconcileRoutes(expectedPrefixes) } diff --git a/pkg/speaker/subnet.go b/pkg/speaker/subnet.go index a8428bac076..100e9e418fb 100644 --- a/pkg/speaker/subnet.go +++ b/pkg/speaker/subnet.go @@ -115,7 +115,7 @@ func (c *Controller) syncSubnetRoutes() { } } - if err := c.reconciliateRoutes(bgpExpected); err != nil { - klog.Errorf("failed to reconciliate routes: %s", err.Error()) + if err := c.reconcileRoutes(bgpExpected); err != nil { + klog.Errorf("failed to reconcile routes: %s", err.Error()) } } From 66a6095a40ec044d5bc080dad888a7c8b1a9343d Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Sun, 21 Jul 2024 12:40:54 -0400 Subject: [PATCH 12/15] feat(natgw): throw error when nad undefined Signed-off-by: SkalaNetworks --- pkg/controller/controller.go | 2 +- pkg/controller/vpc_nat.go | 8 +++++++- pkg/controller/vpc_nat_gateway.go | 24 ++++++++++++++++++------ 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index ab81820b617..a202df48c96 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -1112,7 +1112,7 @@ func (c *Controller) startWorkers(ctx context.Context) { }, time.Second, ctx.Done()) go wait.Until(func() { - c.resyncVpcNatImage() + c.resyncVpcNatConfig() }, time.Second, ctx.Done()) go wait.Until(func() { diff --git a/pkg/controller/vpc_nat.go b/pkg/controller/vpc_nat.go index 7b456fa72a7..02182a9703f 100644 --- a/pkg/controller/vpc_nat.go +++ b/pkg/controller/vpc_nat.go @@ -11,9 +11,10 @@ import ( var ( vpcNatImage = "" vpcNatGwBgpSpeakerImage = "" + vpcNatApiNadProvider = "" ) -func (c *Controller) resyncVpcNatImage() { +func (c *Controller) resyncVpcNatConfig() { cm, err := c.configMapsLister.ConfigMaps(c.config.PodNamespace).Get(util.VpcNatConfig) if err != nil { err = fmt.Errorf("failed to get ovn-vpc-nat-config, %w", err) @@ -21,6 +22,7 @@ func (c *Controller) resyncVpcNatImage() { return } + // Image we're using to provision the NAT gateways image, exist := cm.Data["image"] if !exist { err = fmt.Errorf("%s should have image field", util.VpcNatConfig) @@ -29,5 +31,9 @@ func (c *Controller) resyncVpcNatImage() { } vpcNatImage = image + // Image for the BGP sidecar of the gateway (optional) vpcNatGwBgpSpeakerImage = cm.Data["bgpSpeakerImage"] + + // NetworkAttachmentDefinition for the BGP speaker to call the API server + vpcNatApiNadProvider = cm.Data["apiNadProvider"] } diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go index 37d2b19eb45..e8a49e9f1db 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -736,14 +736,14 @@ func (c *Controller) execNatGwRules(pod *corev1.Pod, operation string, rules []s return nil } -func (c *Controller) setNatGwInterface(annotations map[string]string, externalNetwork string, defaultSubnet *kubeovnv1.Subnet) { +func (c *Controller) setNatGwInterface(annotations map[string]string, externalNetwork string, defaultSubnet *kubeovnv1.Subnet) error { nad := fmt.Sprintf("%s/%s, %s/%s", c.config.PodNamespace, externalNetwork, corev1.NamespaceDefault, nadName) annotations[util.AttachmentNetworkAnnotation] = nad - setNatGwRoute(annotations, defaultSubnet.Spec.Gateway) + return setNatGwRoute(annotations, defaultSubnet.Spec.Gateway) } -func setNatGwRoute(annotations map[string]string, subnetGw string) { +func setNatGwRoute(annotations map[string]string, subnetGw string) error { dst := os.Getenv("KUBERNETES_SERVICE_HOST") protocol := util.CheckProtocol(dst) @@ -755,18 +755,27 @@ func setNatGwRoute(annotations map[string]string, subnetGw string) { dst = fmt.Sprintf("%s/128", dst) } } + + // Check the API NetworkAttachmentDefinition exists, otherwise we won't be able to attach + // the BGP speaker to a network that has access to the K8S apiserver (and won't be able to detect EIPs) + if vpcNatApiNadProvider == "" { + return fmt.Errorf("no NetworkAttachmentDefinition provided to access apiserver, check configmap ovn-vpc-nat-config and field 'apiNadProvider'") + } + for _, gw := range strings.Split(subnetGw, ",") { if util.CheckProtocol(gw) == protocol { routes := []request.Route{{Destination: dst, Gateway: gw}} buf, err := json.Marshal(routes) if err != nil { - klog.Errorf("failed to marshal routes %+v: %v", routes, err) + return fmt.Errorf("failed to marshal routes %+v: %v", routes, err) } else { - annotations[fmt.Sprintf(util.RoutesAnnotationTemplate, nadProvider)] = string(buf) + annotations[fmt.Sprintf(util.RoutesAnnotationTemplate, vpcNatApiNadProvider)] = string(buf) } break } } + + return nil } func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1.StatefulSet) (*v1.StatefulSet, error) { @@ -787,7 +796,10 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 if err != nil { return nil, fmt.Errorf("failed to get default subnet %s: %v", c.config.DefaultLogicalSwitch, err) } - c.setNatGwInterface(podAnnotations, nadName, defaultSubnet) + + if err := c.setNatGwInterface(podAnnotations, nadName, defaultSubnet); err != nil { + return nil, err + } } for key, value := range podAnnotations { From f6fbee538b15b03cc1184aded7bd9339d90fd603 Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Sun, 21 Jul 2024 13:13:33 -0400 Subject: [PATCH 13/15] feat(natgw): rename var Signed-off-by: SkalaNetworks --- pkg/controller/vpc_nat.go | 4 ++-- pkg/controller/vpc_nat_gateway.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/controller/vpc_nat.go b/pkg/controller/vpc_nat.go index 02182a9703f..7c76c319358 100644 --- a/pkg/controller/vpc_nat.go +++ b/pkg/controller/vpc_nat.go @@ -11,7 +11,7 @@ import ( var ( vpcNatImage = "" vpcNatGwBgpSpeakerImage = "" - vpcNatApiNadProvider = "" + vpcNatAPINadProvider = "" ) func (c *Controller) resyncVpcNatConfig() { @@ -35,5 +35,5 @@ func (c *Controller) resyncVpcNatConfig() { vpcNatGwBgpSpeakerImage = cm.Data["bgpSpeakerImage"] // NetworkAttachmentDefinition for the BGP speaker to call the API server - vpcNatApiNadProvider = cm.Data["apiNadProvider"] + vpcNatAPINadProvider = cm.Data["apiNadProvider"] } diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go index e8a49e9f1db..06acbf627d3 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -758,7 +758,7 @@ func setNatGwRoute(annotations map[string]string, subnetGw string) error { // Check the API NetworkAttachmentDefinition exists, otherwise we won't be able to attach // the BGP speaker to a network that has access to the K8S apiserver (and won't be able to detect EIPs) - if vpcNatApiNadProvider == "" { + if vpcNatAPINadProvider == "" { return fmt.Errorf("no NetworkAttachmentDefinition provided to access apiserver, check configmap ovn-vpc-nat-config and field 'apiNadProvider'") } @@ -768,9 +768,9 @@ func setNatGwRoute(annotations map[string]string, subnetGw string) error { buf, err := json.Marshal(routes) if err != nil { return fmt.Errorf("failed to marshal routes %+v: %v", routes, err) - } else { - annotations[fmt.Sprintf(util.RoutesAnnotationTemplate, vpcNatApiNadProvider)] = string(buf) } + + annotations[fmt.Sprintf(util.RoutesAnnotationTemplate, vpcNatAPINadProvider)] = string(buf) break } } From 4448dc9017696e2b1d7e08c4ca1a24ddf6b0fe56 Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Mon, 22 Jul 2024 06:50:50 -0400 Subject: [PATCH 14/15] chore(natgw): linter adjustments Signed-off-by: SkalaNetworks --- pkg/controller/vpc_nat_gateway.go | 12 ++++++------ pkg/speaker/eip.go | 3 ++- pkg/speaker/utils.go | 3 ++- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go index 06acbf627d3..794cc496c00 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -759,7 +759,7 @@ func setNatGwRoute(annotations map[string]string, subnetGw string) error { // Check the API NetworkAttachmentDefinition exists, otherwise we won't be able to attach // the BGP speaker to a network that has access to the K8S apiserver (and won't be able to detect EIPs) if vpcNatAPINadProvider == "" { - return fmt.Errorf("no NetworkAttachmentDefinition provided to access apiserver, check configmap ovn-vpc-nat-config and field 'apiNadProvider'") + return errors.New("no NetworkAttachmentDefinition provided to access apiserver, check configmap ovn-vpc-nat-config and field 'apiNadProvider'") } for _, gw := range strings.Split(subnetGw, ",") { @@ -767,7 +767,7 @@ func setNatGwRoute(annotations map[string]string, subnetGw string) error { routes := []request.Route{{Destination: dst, Gateway: gw}} buf, err := json.Marshal(routes) if err != nil { - return fmt.Errorf("failed to marshal routes %+v: %v", routes, err) + return fmt.Errorf("failed to marshal routes %+v: %w", routes, err) } annotations[fmt.Sprintf(util.RoutesAnnotationTemplate, vpcNatAPINadProvider)] = string(buf) @@ -794,7 +794,7 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 if gw.Spec.BgpSpeaker.Enabled { // Add an interface that can reach the API server defaultSubnet, err := c.subnetsLister.Get(c.config.DefaultLogicalSwitch) if err != nil { - return nil, fmt.Errorf("failed to get default subnet %s: %v", c.config.DefaultLogicalSwitch, err) + return nil, fmt.Errorf("failed to get default subnet %s: %w", c.config.DefaultLogicalSwitch, err) } if err := c.setNatGwInterface(podAnnotations, nadName, defaultSubnet); err != nil { @@ -949,18 +949,18 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 } if speakerParams.ASN == 0 { // The ASN we use to speak - return nil, fmt.Errorf("ASN not set, but must be non-zero value") + return nil, errors.New("ASN not set, but must be non-zero value") } if speakerParams.RemoteASN == 0 { // The ASN we speak to - return nil, fmt.Errorf("remote ASN not set, but must be non-zero value") + return nil, errors.New("remote ASN not set, but must be non-zero value") } args = append(args, fmt.Sprintf("--cluster-as=%d", speakerParams.ASN)) args = append(args, fmt.Sprintf("--neighbor-as=%d", speakerParams.RemoteASN)) if len(speakerParams.Neighbors) == 0 { - return nil, fmt.Errorf("no BGP neighbors specified") + return nil, errors.New("no BGP neighbors specified") } var neighIPv4 []string diff --git a/pkg/speaker/eip.go b/pkg/speaker/eip.go index 79c88376d15..b525c542910 100644 --- a/pkg/speaker/eip.go +++ b/pkg/speaker/eip.go @@ -1,6 +1,7 @@ package speaker import ( + "errors" "fmt" "k8s.io/apimachinery/pkg/labels" @@ -15,7 +16,7 @@ func (c *Controller) syncEIPRoutes() error { // Retrieve the name of our gateway gatewayName := getGatewayName() if gatewayName == "" { - return fmt.Errorf("failed to retrieve the name of the gateway, might not be running in a gateway pod") + return errors.New("failed to retrieve the name of the gateway, might not be running in a gateway pod") } // Create label requirements to only get EIPs attached to our NAT GW diff --git a/pkg/speaker/utils.go b/pkg/speaker/utils.go index 6f89c1054b9..22e9e6b7b1b 100644 --- a/pkg/speaker/utils.go +++ b/pkg/speaker/utils.go @@ -1,6 +1,7 @@ package speaker import ( + "errors" "fmt" "os" "strconv" @@ -104,7 +105,7 @@ func kubeOvnFamilyToAFI(ipFamily string) (bgpapi.Family_Afi, error) { case kubeovnv1.ProtocolIPv6: family = bgpapi.Family_AFI_IP6 default: - return bgpapi.Family_AFI_UNKNOWN, fmt.Errorf("ip family is invalid") + return bgpapi.Family_AFI_UNKNOWN, errors.New("ip family is invalid") } return family, nil From 2b841437daebf3ada9da3d853091ba41a0c7618e Mon Sep 17 00:00:00 2001 From: SkalaNetworks Date: Thu, 25 Jul 2024 04:00:51 -0400 Subject: [PATCH 15/15] feat(natgw): add new configmap option for nadname Signed-off-by: SkalaNetworks --- pkg/controller/vpc_nat.go | 6 +++++- pkg/controller/vpc_nat_gateway.go | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/pkg/controller/vpc_nat.go b/pkg/controller/vpc_nat.go index 7c76c319358..2cd93f698e0 100644 --- a/pkg/controller/vpc_nat.go +++ b/pkg/controller/vpc_nat.go @@ -11,6 +11,7 @@ import ( var ( vpcNatImage = "" vpcNatGwBgpSpeakerImage = "" + vpcNatAPINadName = "" vpcNatAPINadProvider = "" ) @@ -34,6 +35,9 @@ func (c *Controller) resyncVpcNatConfig() { // Image for the BGP sidecar of the gateway (optional) vpcNatGwBgpSpeakerImage = cm.Data["bgpSpeakerImage"] - // NetworkAttachmentDefinition for the BGP speaker to call the API server + // NetworkAttachmentDefinition name for the BGP speaker to call the API server + vpcNatAPINadName = cm.Data["apiNadName"] + + // NetworkAttachmentDefinition provider for the BGP speaker to call the API server vpcNatAPINadProvider = cm.Data["apiNadProvider"] } diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go index 794cc496c00..6e772485fd5 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -737,7 +737,11 @@ func (c *Controller) execNatGwRules(pod *corev1.Pod, operation string, rules []s } func (c *Controller) setNatGwInterface(annotations map[string]string, externalNetwork string, defaultSubnet *kubeovnv1.Subnet) error { - nad := fmt.Sprintf("%s/%s, %s/%s", c.config.PodNamespace, externalNetwork, corev1.NamespaceDefault, nadName) + if vpcNatAPINadName == "" { + return errors.New("no NetworkAttachmentDefinition provided to access apiserver, check configmap ovn-vpc-nat-config and field 'apiNadName'") + } + + nad := fmt.Sprintf("%s/%s, %s/%s", c.config.PodNamespace, externalNetwork, corev1.NamespaceDefault, vpcNatAPINadName) annotations[util.AttachmentNetworkAnnotation] = nad return setNatGwRoute(annotations, defaultSubnet.Spec.Gateway) @@ -759,7 +763,7 @@ func setNatGwRoute(annotations map[string]string, subnetGw string) error { // Check the API NetworkAttachmentDefinition exists, otherwise we won't be able to attach // the BGP speaker to a network that has access to the K8S apiserver (and won't be able to detect EIPs) if vpcNatAPINadProvider == "" { - return errors.New("no NetworkAttachmentDefinition provided to access apiserver, check configmap ovn-vpc-nat-config and field 'apiNadProvider'") + return errors.New("no NetworkAttachmentDefinition provided to access apiserver, check configmap ovn-vpc-nat-config and field 'apiNadName'") } for _, gw := range strings.Split(subnetGw, ",") {