Merge pull request ovn-org#3694 from jordigilh/fix/add_controller_nam…

…e_apb_tests Fix libovsdbops panic when testing apb external controller due to empty controllerName value
jcaamano · Jun 22, 2023 · fd9088f · fd9088f
2 parents 89ef49b + ee6a035
commit fd9088f
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 14 deletions.
diff --git a/go-controller/pkg/ovn/controller/apbroute/external_controller_policy_test.go b/go-controller/pkg/ovn/controller/apbroute/external_controller_policy_test.go
@@ -56,14 +56,14 @@ func initController(k8sObjects, routePolicyObjects []runtime.Object) {
 	iFactory, err = factory.NewMasterWatchFactory(&util.OVNMasterClientset{KubeClient: fakeClient})
 	Expect(err).NotTo(HaveOccurred())
 	iFactory.Start()
-	externalController, err = NewExternalMasterController(controllerName, fakeClient,
+	externalController, err = NewExternalMasterController(fakeClient,
 		fakeRouteClient,
 		stopChan,
 		iFactory.PodCoreInformer(),
 		iFactory.NamespaceInformer(),
 		iFactory.NodeCoreInformer().Lister(),
 		nbClient,
-		addressset.NewFakeAddressSetFactory(controllerName))
+		addressset.NewFakeAddressSetFactory(apbControllerName))
 	Expect(err).NotTo(HaveOccurred())
 	mgr = externalController.mgr
 	go func() {

diff --git a/go-controller/pkg/ovn/controller/apbroute/master_controller.go b/go-controller/pkg/ovn/controller/apbroute/master_controller.go
@@ -34,12 +34,9 @@ import (
 )
 
 const (
-	resyncInterval = 0
-	maxRetries     = 15
-)
-
-var (
-	controllerName string
+	resyncInterval    = 0
+	maxRetries        = 15
+	apbControllerName = "apb-external-route-controller"
 )
 
 // Admin Policy Based Route services
@@ -78,7 +75,6 @@ type ExternalGatewayMasterController struct {
 }
 
 func NewExternalMasterController(
-	parentControllerName string,
 	client kubernetes.Interface,
 	apbRoutePolicyClient adminpolicybasedrouteclient.Interface,
 	stopCh <-chan struct{},
@@ -89,7 +85,6 @@ func NewExternalMasterController(
 	addressSetFactory addressset.AddressSetFactory,
 ) (*ExternalGatewayMasterController, error) {
 
-	controllerName = parentControllerName
 	routePolicyInformer := adminpolicybasedrouteinformer.NewSharedInformerFactory(apbRoutePolicyClient, resyncInterval)
 	externalRouteInformer := routePolicyInformer.K8s().V1().AdminPolicyBasedExternalRoutes()
 	externalGWCache := make(map[ktypes.NamespacedName]*ExternalRouteInfo)
@@ -101,6 +96,7 @@ func NewExternalMasterController(
 		addressSetFactory: addressSetFactory,
 		externalGWCache:   externalGWCache,
 		exGWCacheMutex:    exGWCacheMutex,
+		controllerName:    apbControllerName,
 	}
 
 	c := &ExternalGatewayMasterController{

diff --git a/go-controller/pkg/ovn/controller/apbroute/network_client.go b/go-controller/pkg/ovn/controller/apbroute/network_client.go
@@ -44,6 +44,7 @@ type northBoundClient struct {
 	addressSetFactory addressset.AddressSetFactory
 	externalGWCache   map[ktypes.NamespacedName]*ExternalRouteInfo
 	exGWCacheMutex    *sync.RWMutex
+	controllerName    string
 }
 
 type conntrackClient struct {
@@ -76,7 +77,7 @@ func (nb *northBoundClient) delAllHybridRoutePolicies() error {
 
 	// nuke all the address-sets.
 	// if we fail to remove LRP's above, we don't attempt to remove ASes due to dependency constraints.
-	predicateIDs := libovsdbops.NewDbObjectIDs(libovsdbops.AddressSetHybridNodeRoute, controllerName, nil)
+	predicateIDs := libovsdbops.NewDbObjectIDs(libovsdbops.AddressSetHybridNodeRoute, nb.controllerName, nil)
 	asPred := libovsdbops.GetPredicate[*nbdb.AddressSet](predicateIDs, nil)
 	err = libovsdbops.DeleteAddressSetsWithPredicate(nb.nbClient, asPred)
 	if err != nil {
@@ -261,7 +262,7 @@ func (nb *northBoundClient) addGWRoutesForPod(gateways []*gatewayInfo, podIfAddr
 func (nb *northBoundClient) addHybridRoutePolicyForPod(podIP net.IP, node string) error {
 	if config.Gateway.Mode == config.GatewayModeLocal {
 		// Add podIP to the node's address_set.
-		asIndex := getHybridRouteAddrSetDbIDs(node, controllerName)
+		asIndex := getHybridRouteAddrSetDbIDs(node, nb.controllerName)
 		as, err := nb.addressSetFactory.EnsureAddressSet(asIndex)
 		if err != nil {
 			return fmt.Errorf("cannot ensure that addressSet for node %s exists %v", node, err)
@@ -534,7 +535,7 @@ func (nb *northBoundClient) deleteLogicalRouterStaticRoute(podIP, mask, gw, gr s
 func (nb *northBoundClient) delHybridRoutePolicyForPod(podIP net.IP, node string) error {
 	if config.Gateway.Mode == config.GatewayModeLocal {
 		// Delete podIP from the node's address_set.
-		asIndex := getHybridRouteAddrSetDbIDs(node, controllerName)
+		asIndex := getHybridRouteAddrSetDbIDs(node, nb.controllerName)
 		as, err := nb.addressSetFactory.EnsureAddressSet(asIndex)
 		if err != nil {
 			return fmt.Errorf("cannot Ensure that addressSet for node %s exists %v", node, err)

diff --git a/go-controller/pkg/ovn/default_network_controller.go b/go-controller/pkg/ovn/default_network_controller.go
@@ -168,7 +168,6 @@ func newDefaultNetworkControllerCommon(cnci *CommonNetworkControllerInfo,
 		zoneChassisHandler = zoneic.NewZoneChassisHandler(cnci.sbClient)
 	}
 	apbExternalRouteController, err := apbroutecontroller.NewExternalMasterController(
-		DefaultNetworkControllerName,
 		cnci.client,
 		cnci.kube.APBRouteClient,
 		defaultStopChan,