FRR Mode: provide a way to append a piece of configuration

Currently it is not possible to run multiple FRR instances on the same node, due to port clashing in first instance. In order to provide a way to overcome this issue, we provide a non supported - use at your own risk way to append a fixed configuration right after the one generated by metallb. This can be used in short term to mitigate some of the issues reported. This solution will be deprecated in the long term, being replaced by a more cohesive one. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com>
pperiyasamy · Mar 21, 2023 · f389862 · f389862
1 parent 10cce38
commit f389862
Show file tree

Hide file tree

Showing 13 changed files with 155 additions and 1 deletion.
diff --git a/internal/bgp/bgp.go b/internal/bgp/bgp.go
@@ -78,4 +78,5 @@ type SessionParameters struct {
 type SessionManager interface {
 	NewSession(logger log.Logger, args SessionParameters) (Session, error)
 	SyncBFDProfiles(profiles map[string]*config.BFDProfile) error
+	SyncExtraInfo(extras string) error
 }
diff --git a/internal/bgp/frr/config.go b/internal/bgp/frr/config.go
@@ -31,6 +31,7 @@ type frrConfig struct {
 	Hostname    string
 	Routers     []*routerConfig
 	BFDProfiles []BFDProfile
+	ExtraConfig string
 }
 
 type reloadEvent struct {

diff --git a/internal/bgp/frr/frr.go b/internal/bgp/frr/frr.go
@@ -26,6 +26,7 @@ import (
 type sessionManager struct {
 	sessions     map[string]*session
 	bfdProfiles  []BFDProfile
+	extraConfig  string
 	reloadConfig chan reloadEvent
 	logLevel     string
 	sync.Mutex
@@ -153,6 +154,19 @@ func (sm *sessionManager) deleteSession(s *session) error {
 	return nil
 }
 
+func (sm *sessionManager) SyncExtraInfo(extraInfo string) error {
+	sm.Lock()
+	defer sm.Unlock()
+	sm.extraConfig = extraInfo
+	frrConfig, err := sm.createConfig()
+	if err != nil {
+		return err
+	}
+
+	sm.reloadConfig <- reloadEvent{config: frrConfig}
+	return nil
+}
+
 func (sm *sessionManager) SyncBFDProfiles(profiles map[string]*metallbconfig.BFDProfile) error {
 	sm.Lock()
 	defer sm.Unlock()
@@ -184,6 +198,7 @@ func (sm *sessionManager) createConfig() (*frrConfig, error) {
 		Hostname:    hostname,
 		Loglevel:    sm.logLevel,
 		BFDProfiles: sm.bfdProfiles,
+		ExtraConfig: sm.extraConfig,
 	}
 
 	type router struct {

diff --git a/internal/bgp/frr/frr_test.go b/internal/bgp/frr/frr_test.go
@@ -970,6 +970,38 @@ func TestTwoAdvertisementsTwoSessionsOneWithPeerSelector(t *testing.T) {
 	testCheckConfigFile(t)
 }
 
+func TestSingleSessionExtras(t *testing.T) {
+	testSetup(t)
+
+	l := log.NewNopLogger()
+	sessionManager := NewSessionManager(l, logging.LevelInfo)
+	defer close(sessionManager.reloadConfig)
+	err := sessionManager.SyncExtraInfo("# hello")
+	if err != nil {
+		t.Fatalf("Could not sync extra info")
+	}
+	session, err := sessionManager.NewSession(l,
+		bgp.SessionParameters{
+			PeerAddress:   "127.0.0.2:179",
+			SourceAddress: net.ParseIP("10.1.1.254"),
+			MyASN:         100,
+			RouterID:      net.ParseIP("10.1.1.254"),
+			PeerASN:       200,
+			HoldTime:      time.Second,
+			KeepAliveTime: time.Second,
+			Password:      "password",
+			CurrentNode:   "hostname",
+			EBGPMultiHop:  false,
+			SessionName:   "test-peer"})
+
+	if err != nil {
+		t.Fatalf("Could not create session: %s", err)
+	}
+	defer session.Close()
+
+	testCheckConfigFile(t)
+}
+
 func TestLoggingConfiguration(t *testing.T) {
 	testSetup(t)
 

diff --git a/internal/bgp/frr/templates/frr.tmpl b/internal/bgp/frr/templates/frr.tmpl
@@ -64,3 +64,7 @@ bfd
 {{- template "bfdprofile" dict "profile" . -}}
 {{- end }}
 {{- end }}
+
+{{- if .ExtraConfig }}
+{{ .ExtraConfig }}
+{{- end }}
diff --git a/internal/bgp/frr/testdata/TestSingleSessionExtras.golden b/internal/bgp/frr/testdata/TestSingleSessionExtras.golden
@@ -0,0 +1,40 @@
+log file /etc/frr/frr.log informational
+log timestamp precision 3
+hostname dummyhostname
+ip nht resolve-via-default
+ipv6 nht resolve-via-default
+route-map 127.0.0.2-in deny 20
+
+route-map 127.0.0.2-out permit 1
+  match ip address prefix-list 127.0.0.2-pl-ipv4
+route-map 127.0.0.2-out permit 2
+  match ipv6 address prefix-list 127.0.0.2-pl-ipv4
+
+
+ip prefix-list 127.0.0.2-pl-ipv4 deny any
+ipv6 prefix-list 127.0.0.2-pl-ipv4 deny any
+
+router bgp 100
+  no bgp ebgp-requires-policy
+  no bgp network import-check
+  no bgp default ipv4-unicast
+
+  bgp router-id 10.1.1.254
+  neighbor 127.0.0.2 remote-as 200
+  neighbor 127.0.0.2 port 179
+  neighbor 127.0.0.2 timers 1 1
+  neighbor 127.0.0.2 password password
+  neighbor 127.0.0.2 update-source 10.1.1.254
+
+  address-family ipv4 unicast
+    neighbor 127.0.0.2 activate
+    neighbor 127.0.0.2 route-map 127.0.0.2-in in
+    neighbor 127.0.0.2 route-map 127.0.0.2-out out
+  exit-address-family
+  address-family ipv6 unicast
+    neighbor 127.0.0.2 activate
+    neighbor 127.0.0.2 route-map 127.0.0.2-in in
+    neighbor 127.0.0.2 route-map 127.0.0.2-out out
+  exit-address-family
+
+# hello
diff --git a/internal/bgp/native/native.go b/internal/bgp/native/native.go
@@ -78,6 +78,13 @@ func (sm *sessionManager) SyncBFDProfiles(profiles map[string]*config.BFDProfile
 	return errors.New("bfd profiles not supported in native mode")
 }
 
+func (sm *sessionManager) SyncExtraInfo(extras string) error {
+	if extras != "" {
+		return errors.New("bgp extra info not supported in native mode")
+	}
+	return nil
+}
+
 // run tries to stay connected to the peer, and pumps route updates to it.
 func (s *session) run() {
 	defer stats.DeleteSession(s.PeerAddress)

diff --git a/internal/config/config.go b/internal/config/config.go
@@ -45,6 +45,7 @@ type ClusterResources struct {
 	PasswordSecrets    map[string]corev1.Secret          `json:"passwordsecrets"`
 	Nodes              []corev1.Node                     `json:"nodes"`
 	Namespaces         []corev1.Namespace                `json:"namespaces"`
+	BGPExtras          corev1.ConfigMap                  `json:"bgpextras"`
 }
 
 // Config is a parsed MetalLB configuration.
@@ -55,6 +56,8 @@ type Config struct {
 	Pools *Pools
 	// BFD profiles that can be used by peers.
 	BFDProfiles map[string]*BFDProfile
+	// Protocol dependent extra config. Currently used only by FRR
+	BGPExtras string
 }
 
 // Pools contains address pools and its namespace/service specific allocations.
@@ -76,6 +79,8 @@ const (
 	Layer2 Proto = "layer2"
 )
 
+const bgpExtrasField = "extras"
+
 var Protocols = []Proto{
 	BGP, Layer2,
 }
@@ -226,6 +231,11 @@ func For(resources ClusterResources, validate Validate) (*Config, error) {
 		return nil, err
 	}
 
+	cfg.BGPExtras = bgpExtrasFor(resources)
+	if err != nil {
+		return nil, err
+	}
+
 	err = validateConfig(cfg)
 	if err != nil {
 		return nil, err
@@ -351,6 +361,13 @@ func poolsFor(resources ClusterResources) (*Pools, error) {
 		ByServiceSelector: poolsByServiceSelector(pools)}, nil
 }
 
+func bgpExtrasFor(resources ClusterResources) string {
+	if resources.BGPExtras.Data == nil {
+		return ""
+	}
+	return resources.BGPExtras.Data[bgpExtrasField]
+}
+
 func communitiesFromCrs(cs []metallbv1beta1.Community) (map[string]uint32, error) {
 	communities := map[string]uint32{}
 	for _, c := range cs {

diff --git a/internal/k8s/controllers/config_controller.go b/internal/k8s/controllers/config_controller.go
@@ -26,6 +26,7 @@ import (
 	metallbv1beta2 "go.universe.tf/metallb/api/v1beta2"
 	"go.universe.tf/metallb/internal/config"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -36,6 +37,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/source"
 )
 
+const bgpExtrasConfigName = "bgpextras"
+
 type ConfigReconciler struct {
 	client.Client
 	Logger         log.Logger
@@ -116,6 +119,13 @@ var requestHandler = func(r *ConfigReconciler, ctx context.Context, req ctrl.Req
 		return ctrl.Result{}, err
 	}
 
+	var extrasMap corev1.ConfigMap
+	key := client.ObjectKey{Name: bgpExtrasConfigName, Namespace: r.Namespace}
+	if err := r.Get(ctx, key, &extrasMap); err != nil && !apierrors.IsNotFound(err) {
+		level.Error(r.Logger).Log("controller", "ConfigReconciler", "message", "failed to get the frr configmap", "error", err)
+		return ctrl.Result{}, err
+	}
+
 	resources := config.ClusterResources{
 		Pools:              ipAddressPools.Items,
 		Peers:              bgpPeers.Items,
@@ -127,6 +137,7 @@ var requestHandler = func(r *ConfigReconciler, ctx context.Context, req ctrl.Req
 		PasswordSecrets:    secrets,
 		Nodes:              nodes.Items,
 		Namespaces:         namespaces.Items,
+		BGPExtras:          extrasMap,
 	}
 
 	level.Debug(r.Logger).Log("controller", "ConfigReconciler", "metallb CRs and Secrets", dumpClusterResources(&resources))
@@ -138,6 +149,9 @@ var requestHandler = func(r *ConfigReconciler, ctx context.Context, req ctrl.Req
 		return ctrl.Result{}, nil
 	}
 
+	if cfg.BGPExtras != "" {
+		level.Info(r.Logger).Log("controller", "ConfigReconciler", "warning message", "BGP Extras provided, please note that this configuration is not supported and used at your own risk")
+	}
 	level.Debug(r.Logger).Log("controller", "ConfigReconciler", "rendered config", dumpConfig(cfg))
 	if r.currentConfig != nil && reflect.DeepEqual(r.currentConfig, cfg) {
 		level.Debug(r.Logger).Log("controller", "ConfigReconciler", "event", "configuration did not change, ignoring")
@@ -176,7 +190,7 @@ var requestHandler = func(r *ConfigReconciler, ctx context.Context, req ctrl.Req
 func (r *ConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	p := predicate.Funcs{
 		UpdateFunc: func(e event.UpdateEvent) bool {
-			return filterNodeEvent(e) && filterNamespaceEvent(e)
+			return filterNodeEvent(e) && filterNamespaceEvent(e) && filterConfigmapEvent(e)
 		},
 	}
 	return ctrl.NewControllerManagedBy(mgr).
@@ -190,6 +204,7 @@ func (r *ConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Watches(&source.Kind{Type: &metallbv1beta1.Community{}}, &handler.EnqueueRequestForObject{}).
 		Watches(&source.Kind{Type: &corev1.Secret{}}, &handler.EnqueueRequestForObject{}).
 		Watches(&source.Kind{Type: &corev1.Namespace{}}, &handler.EnqueueRequestForObject{}).
+		Watches(&source.Kind{Type: &corev1.ConfigMap{}}, &handler.EnqueueRequestForObject{}).
 		WithEventFilter(p).
 		Complete(r)
 }
@@ -225,6 +240,17 @@ func filterNamespaceEvent(e event.UpdateEvent) bool {
 	return true
 }
 
+func filterConfigmapEvent(e event.UpdateEvent) bool {
+	cm, ok := e.ObjectNew.(*corev1.ConfigMap)
+	if !ok {
+		return true
+	}
+	if cm.Name != bgpExtrasConfigName {
+		return false
+	}
+	return true
+}
+
 func (r *ConfigReconciler) getSecrets(ctx context.Context) (map[string]corev1.Secret, error) {
 	var secrets corev1.SecretList
 	if err := r.List(ctx, &secrets, client.InNamespace(r.Namespace)); err != nil {

diff --git a/internal/k8s/controllers/dump.go b/internal/k8s/controllers/dump.go
@@ -20,6 +20,7 @@ func dumpClusterResources(c *config.ClusterResources) string {
 		BGPAdvs:            c.BGPAdvs,
 		LegacyAddressPools: c.LegacyAddressPools,
 		Communities:        c.Communities,
+		BGPExtras:          c.BGPExtras,
 	}
 	withNoSecret.PasswordSecrets = make(map[string]corev1.Secret)
 	for k, s := range c.PasswordSecrets {

diff --git a/internal/k8s/k8s.go b/internal/k8s/k8s.go
@@ -142,6 +142,7 @@ func New(cfg *Config) (*Client, error) {
 				&metallbv1beta2.BGPPeer{}:          namespaceSelector,
 				&metallbv1beta1.Community{}:        namespaceSelector,
 				&corev1.Secret{}:                   namespaceSelector,
+				&corev1.ConfigMap{}:                namespaceSelector,
 			},
 		}),
 	})

diff --git a/speaker/bgp_controller.go b/speaker/bgp_controller.go
@@ -98,6 +98,11 @@ newPeers:
 	if err != nil {
 		return errors.Wrap(err, "failed to sync bfd profiles")
 	}
+	err = c.sessionManager.SyncExtraInfo(cfg.BGPExtras)
+	if err != nil {
+		return errors.Wrap(err, "failed to sync extra info")
+	}
+
 	return c.syncPeers(l)
 }
 

diff --git a/speaker/bgp_controller_test.go b/speaker/bgp_controller_test.go
@@ -126,6 +126,10 @@ func (f *fakeBGPSessionManager) SyncBFDProfiles(profiles map[string]*config.BFDP
 	return nil
 }
 
+func (f *fakeBGPSessionManager) SyncExtraInfo(extra string) error {
+	return nil
+}
+
 func (f *fakeBGPSessionManager) Ads() map[string][]*bgp.Advertisement {
 	ret := map[string][]*bgp.Advertisement{}