From 977c225fefc9b16bdfd34842c2aa67db6d1363b6 Mon Sep 17 00:00:00 2001 From: SkalaNetworks <127797154+SkalaNetworks@users.noreply.github.com> Date: Wed, 21 Aug 2024 10:25:54 +0200 Subject: [PATCH] feat(bgp): logger & enhance nadProvider config (#4352) Signed-off-by: SkalaNetworks --- pkg/controller/vpc_nat.go | 4 -- pkg/controller/vpc_nat_gateway.go | 70 +++++++++++++++++++++---------- pkg/speaker/config.go | 20 ++++++++- pkg/speaker/logger.go | 41 ++++++++++++++++++ 4 files changed, 108 insertions(+), 27 deletions(-) create mode 100644 pkg/speaker/logger.go diff --git a/pkg/controller/vpc_nat.go b/pkg/controller/vpc_nat.go index 2cd93f698e0..8c367adc077 100644 --- a/pkg/controller/vpc_nat.go +++ b/pkg/controller/vpc_nat.go @@ -11,7 +11,6 @@ import ( var ( vpcNatImage = "" vpcNatGwBgpSpeakerImage = "" - vpcNatAPINadName = "" vpcNatAPINadProvider = "" ) @@ -35,9 +34,6 @@ func (c *Controller) resyncVpcNatConfig() { // Image for the BGP sidecar of the gateway (optional) vpcNatGwBgpSpeakerImage = cm.Data["bgpSpeakerImage"] - // 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 9c9d51b2040..3c6b5e54b55 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -742,18 +742,37 @@ 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) error { - if vpcNatAPINadName == "" { - return errors.New("no NetworkAttachmentDefinition provided to access apiserver, check configmap ovn-vpc-nat-config and field 'apiNadName'") +// setNatGwAPIAccess adds an interface with API access to the NAT gateway and attaches the standard externalNetwork to the gateway. +// This interface is backed by a NetworkAttachmentDefinition (NAD) with a provider corresponding +// to one that is configured on a subnet part of the default VPC (the K8S apiserver runs in the default VPC) +func (c *Controller) setNatGwAPIAccess(annotations map[string]string, externalNetwork string) error { + // Check the NetworkAttachmentDefinition provider exists, must be user-configured + if vpcNatAPINadProvider == "" { + return errors.New("no NetworkAttachmentDefinition provided to access apiserver, check configmap ovn-vpc-nat-config and field 'apiNadProvider'") + } + + // Subdivide provider so we can infer the name of the NetworkAttachmentDefinition + providerSplit := strings.Split(vpcNatAPINadProvider, ".") + if len(providerSplit) != 3 || providerSplit[2] != util.OvnProvider { + return fmt.Errorf("name of the provider must have syntax 'name.namespace.ovn', got %s", vpcNatAPINadProvider) } - nad := fmt.Sprintf("%s/%s, %s/%s", c.config.PodNamespace, externalNetwork, corev1.NamespaceDefault, vpcNatAPINadName) - annotations[util.AttachmentNetworkAnnotation] = nad + // Extract the name of the provider and its namespace + name, namespace := providerSplit[0], providerSplit[1] + + // Craft the name of the NAD for the externalNetwork and the apiNetwork + externalNetworkAttachment := fmt.Sprintf("%s/%s", c.config.PodNamespace, externalNetwork) + apiNetworkAttachment := fmt.Sprintf("%s/%s", namespace, name) + + // Attach the NADs to the Pod by adding them to the special annotation + attachmentAnnotation := fmt.Sprintf("%s, %s", externalNetworkAttachment, apiNetworkAttachment) + annotations[util.AttachmentNetworkAnnotation] = attachmentAnnotation - return setNatGwRoute(annotations, defaultSubnet.Spec.Gateway) + // Set the network route to the API, so we can reach it + return c.setNatGwAPIRoute(annotations, namespace, name) } -func setNatGwRoute(annotations map[string]string, subnetGw string) error { +func (c *Controller) setNatGwAPIRoute(annotations map[string]string, nadNamespace, nadName string) error { dst := os.Getenv("KUBERNETES_SERVICE_HOST") protocol := util.CheckProtocol(dst) @@ -766,13 +785,20 @@ 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 'apiNadName'") + // Retrieve every subnet on the cluster + subnets, err := c.subnetsLister.List(labels.Everything()) + if err != nil { + return fmt.Errorf("failed to list subnets: %w", err) } - for _, gw := range strings.Split(subnetGw, ",") { + // Retrieve the subnet connected to the NAD, this subnet should be in the VPC of the API + apiSubnet, err := c.findSubnetByNetworkAttachmentDefinition(nadNamespace, nadName, subnets) + if err != nil { + return fmt.Errorf("failed to find api subnet using the nad %s/%s: %w", nadNamespace, nadName, err) + } + + // Craft the route to reach the API from the subnet we've just retrieved + for _, gw := range strings.Split(apiSubnet.Spec.Gateway, ",") { if util.CheckProtocol(gw) == protocol { routes := []request.Route{{Destination: dst, Gateway: gw}} buf, err := json.Marshal(routes) @@ -793,21 +819,19 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 if oldSts != nil && len(oldSts.Annotations) != 0 { annotations = maps.Clone(oldSts.Annotations) } - nadName := util.GetNatGwExternalNetwork(gw.Spec.ExternalSubnets) + + externalNetworkNad := util.GetNatGwExternalNetwork(gw.Spec.ExternalSubnets) podAnnotations := map[string]string{ util.VpcNatGatewayAnnotation: gw.Name, - util.AttachmentNetworkAnnotation: fmt.Sprintf("%s/%s", c.config.PodNamespace, nadName), + util.AttachmentNetworkAnnotation: fmt.Sprintf("%s/%s", c.config.PodNamespace, externalNetworkNad), util.LogicalSwitchAnnotation: gw.Spec.Subnet, util.IPAddressAnnotation: gw.Spec.LanIP, } - 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: %w", c.config.DefaultLogicalSwitch, err) - } - - if err := c.setNatGwInterface(podAnnotations, nadName, defaultSubnet); err != nil { + // Add an interface that can reach the API server, we need access to it to probe Kube-OVN resources + if gw.Spec.BgpSpeaker.Enabled { + if err := c.setNatGwAPIAccess(podAnnotations, externalNetworkNad); err != nil { + klog.Error(err) return nil, err } } @@ -853,7 +877,7 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 return nil, err } - subnet, err := c.findSubnetByNetworkAttachmentDefinition(c.config.PodNamespace, nadName, subnets) + subnet, err := c.findSubnetByNetworkAttachmentDefinition(c.config.PodNamespace, externalNetworkNad, subnets) if err != nil { klog.Error(err) return nil, err @@ -981,6 +1005,8 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1 neighIPv4 = append(neighIPv4, neighbor) case kubeovnv1.ProtocolIPv6: neighIPv6 = append(neighIPv6, neighbor) + default: + return nil, fmt.Errorf("unsupported protocol for peer %s", neighbor) } } diff --git a/pkg/speaker/config.go b/pkg/speaker/config.go index 0bcd9bb20cc..4bb77ee99ba 100644 --- a/pkg/speaker/config.go +++ b/pkg/speaker/config.go @@ -11,6 +11,7 @@ import ( "time" api "github.com/osrg/gobgp/v3/api" + bgplog "github.com/osrg/gobgp/v3/pkg/log" "github.com/osrg/gobgp/v3/pkg/packet/bgp" gobgp "github.com/osrg/gobgp/v3/pkg/server" "github.com/spf13/pflag" @@ -244,10 +245,21 @@ func (config *Configuration) checkGracefulRestartOptions() error { func (config *Configuration) initBgpServer() error { maxSize := 256 << 20 var listenPort int32 = -1 + + // Set logger options for GoBGP based on klog's verbosity + var logger bgpLogger + if klog.V(3).Enabled() { + logger.SetLevel(bgplog.TraceLevel) + } else { + logger.SetLevel(bgplog.InfoLevel) + } + grpcOpts := []grpc.ServerOption{grpc.MaxRecvMsgSize(maxSize), grpc.MaxSendMsgSize(maxSize)} s := gobgp.NewBgpServer( gobgp.GrpcListenAddress(fmt.Sprintf("%s:%d", config.GrpcHost, config.GrpcPort)), - gobgp.GrpcOption(grpcOpts)) + gobgp.GrpcOption(grpcOpts), + gobgp.LoggerOption(logger), + ) go s.Serve() peersMap := map[api.Family_Afi][]string{ @@ -258,6 +270,12 @@ func (config *Configuration) initBgpServer() error { if config.PassiveMode { listenPort = bgp.BGP_PORT } + + klog.Infof("Starting bgp server with asn %d, routerId %s on port %d", + config.ClusterAs, + config.RouterID, + listenPort) + if err := s.StartBgp(context.Background(), &api.StartBgpRequest{ Global: &api.Global{ Asn: config.ClusterAs, diff --git a/pkg/speaker/logger.go b/pkg/speaker/logger.go new file mode 100644 index 00000000000..fa4d632e8e1 --- /dev/null +++ b/pkg/speaker/logger.go @@ -0,0 +1,41 @@ +package speaker + +import ( + bgplog "github.com/osrg/gobgp/v3/pkg/log" + "k8s.io/klog/v2" +) + +// bgpLogger is a struct implementing the GoBGP logger interface +// This is useful to inject our custom klog logger into the GoBGP speaker +type bgpLogger struct{} + +func (k bgpLogger) Panic(msg string, fields bgplog.Fields) { + klog.Fatalf("%s %v", msg, fields) +} + +func (k bgpLogger) Fatal(msg string, fields bgplog.Fields) { + klog.Fatalf("%s %v", msg, fields) +} + +func (k bgpLogger) Error(msg string, fields bgplog.Fields) { + klog.Errorf("%s %v", msg, fields) +} + +func (k bgpLogger) Warn(msg string, fields bgplog.Fields) { + klog.Warningf("%s %v", msg, fields) +} + +func (k bgpLogger) Info(msg string, fields bgplog.Fields) { + klog.Infof("%s %v", msg, fields) +} + +func (k bgpLogger) Debug(msg string, fields bgplog.Fields) { + klog.V(5).Infof("%s %v", msg, fields) +} + +func (k bgpLogger) SetLevel(_ bgplog.LogLevel) { +} + +func (k bgpLogger) GetLevel() bgplog.LogLevel { + return 0 +}