fix(evpn): addressing reviews second attempt

Signed-off-by: Dimitrios Markou <dimitrios.markou@ericsson.com>

pkg/frr/frr.go

@@ -32,22 +32,39 @@ import (
 // frrComp string constant
 const frrComp string = "frr"
 
-// runningFrrConfFile holds the running configuration of FRR daemon
-const runningFrrConfFile = "/etc/frr/frr.conf"
-
-// basicFrrConfFile holds the basic/initial configuration of FRR daemon
-const basicFrrConfFile = "/etc/frr/frr-basic.conf"
-
-// backupFrrConfFile holds the backup configuration the current running config of FRR daemon
-const backupFrrConfFile = "/etc/frr/frr.conf.bak"
-
+// replayThreshold time threshold for replay
 const replayThreshold = 64 * time.Second
 
 // ModulefrrHandler empty structure
 type ModulefrrHandler struct{}
 
 // ModuleFrrActionHandler empty structure
-type ModuleFrrActionHandler struct{}
+type ModuleFrrActionHandler struct {
+	// runningFrrConfFile holds the running configuration of FRR daemon
+	runningFrrConfFile string
+	// basicFrrConfFile holds the basic/initial configuration of FRR daemon
+	basicFrrConfFile string
+	// backupFrrConfFile holds the backup configuration the current running config of FRR daemon
+	backupFrrConfFile string
+}
+
+// NewModuleFrrActionHandler initializes a default ModuleFrrActionHandler
+func NewModuleFrrActionHandler() *ModuleFrrActionHandler {
+	return &ModuleFrrActionHandler{
+		runningFrrConfFile: "/etc/frr/frr.conf",
+		basicFrrConfFile:   "/etc/frr/frr-basic.conf",
+		backupFrrConfFile:  "/etc/frr/frr.conf.bak",
+	}
+}
+
+// NewModuleFrrActionHandlerWithArgs initializes a ModuleFrrActionHandler
+func NewModuleFrrActionHandlerWithArgs(runningFrrConfFile, basicFrrConfFile, backupFrrConfFile string) *ModuleFrrActionHandler {
+	return &ModuleFrrActionHandler{
+		runningFrrConfFile: runningFrrConfFile,
+		basicFrrConfFile:   basicFrrConfFile,
+		backupFrrConfFile:  backupFrrConfFile,
+	}
+}
 
 // HandleEvent handles the events
 func (h *ModulefrrHandler) HandleEvent(eventType string, objectData *eventbus.ObjectData) {
@@ -68,13 +85,13 @@ func (h *ModuleFrrActionHandler) HandleAction(actionType string, actionData *act
 	switch actionType {
 	case "preReplay":
 		log.Printf("Module FRR received %s\n", actionType)
-		handlePreReplay(actionData)
+		h.handlePreReplay(actionData)
 	default:
 		log.Printf("error: Unknown action type %s", actionType)
 	}
 }
 
-func handlePreReplay(actionData *actionbus.ActionData) {
+func (h *ModuleFrrActionHandler) handlePreReplay(actionData *actionbus.ActionData) {
 	var deferErr error
 
 	defer func() {
@@ -84,22 +101,22 @@ func handlePreReplay(actionData *actionbus.ActionData) {
 	}()
 
 	// Backup the current running config
-	deferErr = os.Rename(runningFrrConfFile, backupFrrConfFile)
+	deferErr = os.Rename(h.runningFrrConfFile, h.backupFrrConfFile)
 	if deferErr != nil {
 		log.Printf("FRR: handlePreReplay(): Failed to backup running config of FRR: %s\n", deferErr)
 		return
 	}
 
 	// Create a new running config based on the basic/initial FRR config
-	input, deferErr := os.ReadFile(basicFrrConfFile)
+	input, deferErr := os.ReadFile(h.basicFrrConfFile)
 	if deferErr != nil {
-		log.Printf("FRR: handlePreReplay(): Failed to read content of %s: %s\n", basicFrrConfFile, deferErr)
+		log.Printf("FRR: handlePreReplay(): Failed to read content of %s: %s\n", h.basicFrrConfFile, deferErr)
 		return
 	}
 
-	deferErr = os.WriteFile(runningFrrConfFile, input, 0600)
+	deferErr = os.WriteFile(h.runningFrrConfFile, input, 0600)
 	if deferErr != nil {
-		log.Printf("FRR: handlePreReplay(): Failed to write content to %s: %s\n", runningFrrConfFile, deferErr)
+		log.Printf("FRR: handlePreReplay(): Failed to write content to %s: %s\n", h.runningFrrConfFile, deferErr)
 		return
 	}
 
@@ -122,9 +139,9 @@ func handlePreReplay(actionData *actionbus.ActionData) {
 		return
 	}
 
-	deferErr = os.Chown(runningFrrConfFile, uid, gid)
+	deferErr = os.Chown(h.runningFrrConfFile, uid, gid)
 	if deferErr != nil {
-		log.Printf("FRR: handlePreReplay(): Failed to chown of %s to frr:frr : %s\n", runningFrrConfFile, deferErr)
+		log.Printf("FRR: handlePreReplay(): Failed to chown of %s to frr:frr : %s\n", h.runningFrrConfFile, deferErr)
 		return
 	}
 
@@ -368,7 +385,7 @@ func subscribeInfradb(config *config.Config) {
 			}
 		}
 	}
-	ab.StartSubscriber(frrComp, "preReplay", &ModuleFrrActionHandler{})
+	ab.StartSubscriber(frrComp, "preReplay", NewModuleFrrActionHandler())
 }
 
 // ctx variable of type context

pkg/infradb/subscriberframework/actionbus/actionbus.go

@@ -51,7 +51,7 @@ func (a *ActionBus) StartSubscriber(moduleName, actionType string, actionHandler
 				case action := <-subscriber.Ch:
 					log.Printf("\nSubscriber %s for %s received \n", moduleName, actionType)
 
-					handlerKey := utils.ConcatenateModuleNameWithType(moduleName, actionType)
+					handlerKey := utils.ComposeHandlerName(moduleName, actionType)
 					if handler, ok := a.actionHandlers[handlerKey]; ok {
 						if actionData, ok := action.(*ActionData); ok {
 							handler.HandleAction(actionType, actionData)
@@ -98,7 +98,7 @@ func (a *ActionBus) Subscribe(moduleName, actionType string, actionHandler Actio
 
 	a.subscribers[actionType] = append(a.subscribers[actionType], subscriber)
 
-	handlerKey := utils.ConcatenateModuleNameWithType(moduleName, actionType)
+	handlerKey := utils.ComposeHandlerName(moduleName, actionType)
 	a.actionHandlers[handlerKey] = actionHandler
 
 	log.Printf("Subscriber %s registered for action %s\n", moduleName, actionType)
@@ -107,8 +107,8 @@ func (a *ActionBus) Subscribe(moduleName, actionType string, actionHandler Actio
 
 // GetSubscribers api is used to fetch the list of subscribers registered with given actionType
 func (a *ActionBus) GetSubscribers(actionType string) []*Subscriber {
-	a.subscriberL.Lock()
-	defer a.subscriberL.Unlock()
+	a.subscriberL.RLock()
+	defer a.subscriberL.RUnlock()
 
 	return a.subscribers[actionType]
 }

pkg/infradb/subscriberframework/eventbus/eventbus.go

@@ -56,7 +56,7 @@ func (e *EventBus) StartSubscriber(moduleName, eventType string, priority int, e
 				case event := <-subscriber.Ch:
 					log.Printf("\nSubscriber %s for %s received \n", moduleName, eventType)
 
-					handlerKey := utils.ConcatenateModuleNameWithType(moduleName, eventType)
+					handlerKey := utils.ComposeHandlerName(moduleName, eventType)
 					if handler, ok := e.eventHandlers[handlerKey]; ok {
 						if objectData, ok := event.(*ObjectData); ok {
 							handler.HandleEvent(eventType, objectData)
@@ -99,7 +99,7 @@ func (e *EventBus) Subscribe(moduleName, eventType string, priority int, eventHa
 
 	e.subscribers[eventType] = append(e.subscribers[eventType], subscriber)
 
-	handlerKey := utils.ConcatenateModuleNameWithType(moduleName, eventType)
+	handlerKey := utils.ComposeHandlerName(moduleName, eventType)
 	e.eventHandlers[handlerKey] = eventHandler
 
 	// Sort subscribers based on priority
@@ -142,7 +142,7 @@ func (e *EventBus) UnsubscribeModule(moduleName string) bool {
 					sub.Quit <- true
 					e.subscribers[eventName] = append(subs[:i], subs[i+1:]...)
 
-					handlerKey := utils.ConcatenateModuleNameWithType(moduleName, eventName)
+					handlerKey := utils.ComposeHandlerName(moduleName, eventName)
 					e.eventHandlers[handlerKey] = nil
 					log.Printf("\n Module %s is unsubscribed for event %s", sub.Name, eventName)
 				}

pkg/utils/helpers.go

@@ -84,7 +84,7 @@ func CheckReplayThreshold(currentTimer, replayThreshold time.Duration) bool {
 	return (currentTimer > replayThreshold)
 }
 
-// ConcatenateModuleNameWithType this function concatenates the module name and type
-func ConcatenateModuleNameWithType(moduleName, kindOfType string) string {
+// ComposeHandlerName this function concatenates the module name and type
+func ComposeHandlerName(moduleName, kindOfType string) string {
 	return moduleName + "." + kindOfType
 }