From 1a290c29d7e18b4660b57d7d94f973f7b119d740 Mon Sep 17 00:00:00 2001 From: astoycos Date: Tue, 7 Dec 2021 15:47:44 -0500 Subject: [PATCH] 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 --- .../hybrid-overlay/pkg/controller/master.go | 3 +- go-controller/pkg/ovn/libovsdbops/datapath.go | 44 ++++++++++++++ .../pkg/ovn/libovsdbops/mac_binding.go | 29 +++++++++ go-controller/pkg/ovn/libovsdbops/model.go | 5 +- go-controller/pkg/util/ovn.go | 59 +++++-------------- 5 files changed, 92 insertions(+), 48 deletions(-) create mode 100644 go-controller/pkg/ovn/libovsdbops/datapath.go create mode 100644 go-controller/pkg/ovn/libovsdbops/mac_binding.go diff --git a/go-controller/hybrid-overlay/pkg/controller/master.go b/go-controller/hybrid-overlay/pkg/controller/master.go index b4036e23c7e..a87a2b3696d 100644 --- a/go-controller/hybrid-overlay/pkg/controller/master.go +++ b/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) } } diff --git a/go-controller/pkg/ovn/libovsdbops/datapath.go b/go-controller/pkg/ovn/libovsdbops/datapath.go new file mode 100644 index 00000000000..427bad8818d --- /dev/null +++ b/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) +} diff --git a/go-controller/pkg/ovn/libovsdbops/mac_binding.go b/go-controller/pkg/ovn/libovsdbops/mac_binding.go new file mode 100644 index 00000000000..e5d34efa2f4 --- /dev/null +++ b/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 +} diff --git a/go-controller/pkg/ovn/libovsdbops/model.go b/go-controller/pkg/ovn/libovsdbops/model.go index 6b5bd27918e..c4d74723608 100644 --- a/go-controller/pkg/ovn/libovsdbops/model.go +++ b/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{ diff --git a/go-controller/pkg/util/ovn.go b/go-controller/pkg/util/ovn.go index 0827f299542..d3bd6e79f6f 100644 --- a/go-controller/pkg/util/ovn.go +++ b/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 -}