Remove final sbctl calls from Hybrid Overlay

Remove the final sbctl calls regarding mac_bindings
from the hybrid overlay code and convert to libovsdb

Ensure we index mac_binding by both possible options,
the `logical_port` OR `ip` columns

Signed-off-by: astoycos <[email protected]>

go-controller/hybrid-overlay/pkg/controller/master.go

@@ -32,6 +32,7 @@ type MasterController struct {
 	nodeEventHandler informer.EventHandler
 	modelClient      libovsdbops.ModelClient
 	nbClient         client.Client
+	sbClient         client.Client
 }
 
 // NewMaster a new master controller that listens for node events
@@ -374,7 +375,7 @@ func (m *MasterController) setupHybridLRPolicySharedGw(nodeSubnets []*net.IPNet,
 
 		if len(logicalRouterPolicyRes) == 0 {
 			logicalPort := ovntypes.RouterToSwitchPrefix + nodeName
-			if err := util.CreateMACBinding(logicalPort, ovntypes.OVNClusterRouter, portMac, drIP); err != nil {
+			if err := util.CreateMACBinding(m.sbClient, logicalPort, ovntypes.OVNClusterRouter, portMac, drIP); err != nil {
 				return fmt.Errorf("failed to create MAC Binding for hybrid overlay: %v", err)
 			}
 		}

go-controller/pkg/ovn/libovsdbops/datapath.go

@@ -0,0 +1,44 @@
+package libovsdbops
+
+import (
+	"context"
+	"fmt"
+
+	libovsdbclient "github.com/ovn-org/libovsdb/client"
+
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/sbdb"
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
+)
+
+func findDatapathByPredicate(sbClient libovsdbclient.Client, lookupFcn func(dp *sbdb.DatapathBinding) bool) (*sbdb.DatapathBinding, error) {
+	ctx, cancel := context.WithTimeout(context.Background(), types.OVSDBTimeout)
+	defer cancel()
+	datapathBindings := []sbdb.DatapathBinding{}
+	err := sbClient.WhereCache(lookupFcn).List(ctx, &datapathBindings)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(datapathBindings) != 1 {
+		return nil, fmt.Errorf("multiple datapath bindings found for a given lookup function")
+	}
+
+	return &datapathBindings[0], nil
+}
+
+func FindDatapathByExternalIDs(sbClient libovsdbclient.Client, externalIDs map[string]string) (*sbdb.DatapathBinding, error) {
+	datapathLookupFcn := func(dp *sbdb.DatapathBinding) bool {
+		datapathMatch := false
+		for k, v := range externalIDs {
+			if itemVal, ok := dp.ExternalIDs[k]; ok {
+				if itemVal == v {
+					datapathMatch = true
+				}
+			}
+		}
+
+		return datapathMatch
+	}
+
+	return findDatapathByPredicate(sbClient, datapathLookupFcn)
+}

go-controller/pkg/ovn/libovsdbops/mac_binding.go

@@ -0,0 +1,29 @@
+package libovsdbops
+
+import (
+	libovsdbclient "github.com/ovn-org/libovsdb/client"
+
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/sbdb"
+)
+
+// Create or UpdateMacBinding either create a new mac binding or ensure the existing one has the
+// correct datapath, logical_port, ip, and mac column entries
+func CreateOrUpdateMacBinding(sbClient libovsdbclient.Client, mb *sbdb.MACBinding) error {
+	opModel := OperationModel{
+		// no predicate needed, ip and logical_port columns are indexes
+		Model: mb,
+		OnModelUpdates: []interface{}{
+			&mb.Datapath,
+			&mb.LogicalPort,
+			&mb.IP,
+			&mb.MAC,
+		},
+	}
+
+	m := NewModelClient(sbClient)
+	if _, err := m.CreateOrUpdate(opModel); err != nil {
+		return err
+	}
+
+	return nil
+}

go-controller/pkg/ovn/libovsdbops/model.go

@@ -190,8 +190,9 @@ func copyIndexes(model model.Model) model.Model {
 		}
 	case *sbdb.MACBinding:
 		return &sbdb.MACBinding{
-			UUID: t.UUID,
-			IP:   t.IP,
+			UUID:        t.UUID,
+			LogicalPort: t.LogicalPort,
+			IP:          t.IP,
 		}
 	case *sbdb.Encap:
 		return &sbdb.Encap{

go-controller/pkg/util/ovn.go

@@ -6,64 +6,33 @@ package util
 import (
 	"fmt"
 	"net"
-	"strings"
-	"time"
 
-	ovntypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
+	libovsdbclient "github.com/ovn-org/libovsdb/client"
 
-	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/klog/v2"
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/ovn/libovsdbops"
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/sbdb"
 )
 
 // CreateMACBinding Creates MAC binding in OVN SBDB
-func CreateMACBinding(logicalPort, datapathName string, portMAC net.HardwareAddr, nextHop net.IP) error {
-	datapath, err := GetDatapathUUID(datapathName)
+func CreateMACBinding(sbClient libovsdbclient.Client, logicalPort, datapathName string, portMAC net.HardwareAddr, nextHop net.IP) error {
+	datapath, err := libovsdbops.FindDatapathByExternalIDs(sbClient, map[string]string{"name": datapathName})
 	if err != nil {
 		return err
 	}
 
-	// Check if exact match already exists
-	stdout, stderr, err := RunOVNSbctl("--data=bare", "--no-heading", "--columns=_uuid", "find", "MAC_Binding",
-		"logical_port="+logicalPort,
-		fmt.Sprintf(`mac="%s"`, portMAC),
-		"datapath="+datapath,
-		fmt.Sprintf("ip=\"%s\"", nextHop))
-	if err != nil {
-		return fmt.Errorf("failed to check existence of MAC_Binding entry of (%s, %s, %s, %s)"+
-			"stderr: %q, error: %v", datapath, logicalPort, portMAC, nextHop, stderr, err)
-	}
-	if stdout != "" {
-		klog.Infof("The MAC_Binding entry of (%s, %s, %s, %s) exists with uuid %s",
-			datapath, logicalPort, portMAC, nextHop, stdout)
-		return nil
+	// find Create mac_binding if needed
+	mb := &sbdb.MACBinding{
+		LogicalPort: logicalPort,
+		MAC:         string(portMAC),
+		Datapath:    datapath.UUID,
+		IP:          string(nextHop),
 	}
 
-	// Create new binding
-	_, stderr, err = RunOVNSbctl("create", "mac_binding", "datapath="+datapath, fmt.Sprintf("ip=\"%s\"", nextHop),
-		"logical_port="+logicalPort, fmt.Sprintf(`mac="%s"`, portMAC))
+	err = libovsdbops.CreateOrUpdateMacBinding(sbClient, mb)
 	if err != nil {
-		return fmt.Errorf("failed to create a MAC_Binding entry of (%s, %s, %s, %s) "+
-			"stderr: %q, error: %v", datapath, logicalPort, portMAC, nextHop, stderr, err)
+		return fmt.Errorf("failed to create/update MAC_Binding entry of (%s, %s, %s, %s)"+
+			"error: %v", datapath.UUID, logicalPort, portMAC, nextHop, err)
 	}
 
 	return nil
 }
-
-// GetDatapathUUID returns the OVN SBDB UUID for a datapath
-func GetDatapathUUID(datapathName string) (string, error) {
-	// Get datapath from southbound, depending on startup this may take some time, so
-	// wait a bit for northd to create the cluster router's datapath in southbound
-	var datapath string
-	err := wait.PollImmediate(time.Second, 30*time.Second, func() (bool, error) {
-		datapath, _, _ = RunOVNSbctl("--data=bare", "--no-heading", "--columns=_uuid", "find", "datapath",
-			"external_ids:name="+datapathName)
-		datapath = strings.TrimSuffix(datapath, "\n")
-		// Ignore errors; can't easily detect which are transient or fatal
-		return datapath != "", nil
-	})
-	if err != nil {
-		return "", fmt.Errorf("failed to get the datapath UUID of %s from OVN SB "+
-			"stdout: %q, error: %v", ovntypes.OVNClusterRouter, datapath, err)
-	}
-	return datapath, nil
-}