Merge pull request #572 from stgraber/ovn

Re-organize the new (OVSDB) OVN/OVS functions

cmd/incusd/networks.go

@@ -899,8 +899,8 @@ func doNetworkGet(s *state.State, r *http.Request, allNodes bool, projectName st
 			return api.Network{}, fmt.Errorf("Failed to connect to OVS: %w", err)
 		}
 
-		exists, _ := vswitch.BridgeExists(apiNet.Name)
-		if exists {
+		_, err = vswitch.GetBridge(context.TODO(), apiNet.Name)
+		if err == nil {
 			apiNet.Type = "bridge"
 		} else {
 			apiNet.Type = "unknown"

internal/server/device/nic_ovn.go

@@ -631,7 +631,7 @@ func (d *nicOVN) Start() (*deviceConfig.RunConfig, error) {
 		return nil, fmt.Errorf("Failed to connect to OVS: %w", err)
 	}
 
-	chassisID, err := vswitch.ChassisID()
+	chassisID, err := vswitch.GetChassisID(context.TODO())
 	if err != nil {
 		return nil, fmt.Errorf("Failed getting OVS Chassis ID: %w", err)
 	}
@@ -1166,7 +1166,7 @@ func (d *nicOVN) setupHostNIC(hostName string, ovnPortName ovn.OVNSwitchPort, up
 		return nil, fmt.Errorf("Failed to connect to OVS: %w", err)
 	}
 
-	err = vswitch.BridgePortAdd(integrationBridge, hostName, true)
+	err = vswitch.CreateBridgePort(context.TODO(), integrationBridge, hostName, true)
 	if err != nil {
 		return nil, err
 	}

internal/server/network/driver_bridge.go

@@ -589,12 +589,12 @@ func (n *bridge) setup(oldConfig map[string]string) error {
 
 			// Add and configure the interface in one operation to reduce the number of executions and
 			// to avoid systemd-udevd from applying the default MACAddressPolicy=persistent policy.
-			err = vswitch.BridgeAdd(n.name, false, bridge.Address, bridge.MTU)
+			err = vswitch.CreateBridge(context.TODO(), n.name, false, bridge.Address, bridge.MTU)
 			if err != nil {
 				return err
 			}
 
-			revert.Add(func() { _ = vswitch.BridgeDelete(n.name) })
+			revert.Add(func() { _ = vswitch.DeleteBridge(context.Background(), n.name) })
 		} else {
 			// Add and configure the interface in one operation to reduce the number of executions and
 			// to avoid systemd-udevd from applying the default MACAddressPolicy=persistent policy.
@@ -1455,7 +1455,7 @@ func (n *bridge) Stop() error {
 			return err
 		}
 
-		err = vswitch.BridgeDelete(n.name)
+		err = vswitch.DeleteBridge(context.TODO(), n.name)
 		if err != nil {
 			return err
 		}

internal/server/network/driver_ovn.go

@@ -150,7 +150,7 @@ func (n *ovn) State() (*api.NetworkState, error) {
 		return nil, err
 	}
 
-	chassis, err := ovnsb.GetLogicalRouterPortActiveChassisHostname(n.getRouterExtPortName())
+	chassis, err := ovnsb.GetLogicalRouterPortActiveChassisHostname(context.TODO(), n.getRouterExtPortName())
 	if err != nil {
 		return nil, err
 	}
@@ -1359,13 +1359,13 @@ func (n *ovn) startUplinkPortBridgeNative(uplinkNet Network, bridgeDevice string
 		return fmt.Errorf("Failed to connect to OVS: %w", err)
 	}
 
-	err = vswitch.BridgeAdd(vars.ovsBridge, true, nil, 0)
+	err = vswitch.CreateBridge(context.TODO(), vars.ovsBridge, true, nil, 0)
 	if err != nil {
 		return fmt.Errorf("Failed to create uplink OVS bridge %q: %w", vars.ovsBridge, err)
 	}
 
 	// Connect OVS end veth interface to OVS bridge.
-	err = vswitch.BridgePortAdd(vars.ovsBridge, vars.ovsEnd, true)
+	err = vswitch.CreateBridgePort(context.TODO(), vars.ovsBridge, vars.ovsEnd, true)
 	if err != nil {
 		return fmt.Errorf("Failed to connect uplink veth interface %q to uplink OVS bridge %q: %w", vars.ovsEnd, vars.ovsBridge, err)
 	}
@@ -1475,8 +1475,10 @@ func (n *ovn) startUplinkPortPhysical(uplinkNet Network) error {
 		return fmt.Errorf("Failed to connect to OVS: %w", err)
 	}
 
-	isOVSBridge, _ := vswitch.BridgeExists(uplinkHostName)
-	if isOVSBridge {
+	_, err = vswitch.GetBridge(context.TODO(), uplinkHostName)
+	if err != nil && err != ovs.ErrNotFound {
+		return err
+	} else if err == nil {
 		return n.startUplinkPortBridgeOVS(uplinkNet, uplinkHostName)
 	}
 
@@ -1503,13 +1505,13 @@ func (n *ovn) startUplinkPortPhysical(uplinkNet Network) error {
 	}
 
 	// Create uplink OVS bridge if needed.
-	err = vswitch.BridgeAdd(vars.ovsBridge, true, nil, 0)
+	err = vswitch.CreateBridge(context.TODO(), vars.ovsBridge, true, nil, 0)
 	if err != nil {
 		return fmt.Errorf("Failed to create uplink OVS bridge %q: %w", vars.ovsBridge, err)
 	}
 
 	// Connect OVS end veth interface to OVS bridge.
-	err = vswitch.BridgePortAdd(vars.ovsBridge, uplinkHostName, true)
+	err = vswitch.CreateBridgePort(context.TODO(), vars.ovsBridge, uplinkHostName, true)
 	if err != nil {
 		return fmt.Errorf("Failed to connect uplink interface %q to uplink OVS bridge %q: %w", uplinkHostName, vars.ovsBridge, err)
 	}
@@ -1632,7 +1634,7 @@ func (n *ovn) deleteUplinkPortBridgeNative(uplinkNet Network) error {
 				return err
 			}
 
-			err = vswitch.BridgeDelete(vars.ovsBridge)
+			err = vswitch.DeleteBridge(context.TODO(), vars.ovsBridge)
 			if err != nil {
 				return err
 			}
@@ -1702,8 +1704,10 @@ func (n *ovn) deleteUplinkPortPhysical(uplinkNet Network) error {
 		return fmt.Errorf("Failed to connect to OVS: %w", err)
 	}
 
-	isOVSBridge, _ := vswitch.BridgeExists(uplinkHostName)
-	if isOVSBridge {
+	_, err = vswitch.GetBridge(context.TODO(), uplinkHostName)
+	if err != nil && err != ovs.ErrNotFound {
+		return err
+	} else if err == nil {
 		return n.deleteUplinkPortBridgeOVS(uplinkNet, uplinkHostName)
 	}
 
@@ -1727,7 +1731,7 @@ func (n *ovn) deleteUplinkPortPhysical(uplinkNet Network) error {
 				return err
 			}
 
-			err = vswitch.BridgeDelete(vars.ovsBridge)
+			err = vswitch.DeleteBridge(context.TODO(), vars.ovsBridge)
 			if err != nil {
 				return err
 			}
@@ -2632,7 +2636,7 @@ func (n *ovn) addChassisGroupEntry() error {
 		return fmt.Errorf("Failed to connect to OVS: %w", err)
 	}
 
-	chassisID, err := vswitch.ChassisID()
+	chassisID, err := vswitch.GetChassisID(context.TODO())
 	if err != nil {
 		return fmt.Errorf("Failed getting OVS Chassis ID: %w", err)
 	}
@@ -2678,7 +2682,7 @@ func (n *ovn) addChassisGroupEntry() error {
 		}
 	}
 
-	err = ovnnb.ChassisGroupChassisAdd(chassisGroupName, chassisID, priority)
+	err = ovnnb.SetChassisGroupPriority(context.TODO(), chassisGroupName, chassisID, priority)
 	if err != nil {
 		return fmt.Errorf("Failed adding OVS chassis %q with priority %d to chassis group %q: %w", chassisID, priority, chassisGroupName, err)
 	}
@@ -2701,7 +2705,7 @@ func (n *ovn) deleteChassisGroupEntry() error {
 		return fmt.Errorf("Failed to connect to OVS: %w", err)
 	}
 
-	chassisID, err := vswitch.ChassisID()
+	chassisID, err := vswitch.GetChassisID(context.TODO())
 	if err != nil {
 		return fmt.Errorf("Failed getting OVS Chassis ID: %w", err)
 	}

internal/server/network/network_utils_bridge.go

@@ -1,6 +1,7 @@
 package network
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"strings"
@@ -69,7 +70,7 @@ func AttachInterface(bridgeName string, devName string) error {
 			return fmt.Errorf("Failed to connect to OVS: %w", err)
 		}
 
-		err = vswitch.BridgePortAdd(bridgeName, devName, true)
+		err = vswitch.CreateBridgePort(context.TODO(), bridgeName, devName, true)
 		if err != nil {
 			return err
 		}

internal/server/network/ovn/ovn_nb_actions.go

@@ -1448,9 +1448,8 @@ func (o *NB) ChassisGroupDelete(haChassisGroupName OVNChassisGroup) error {
 	return nil
 }
 
-// ChassisGroupChassisAdd adds a chassis ID to an HA chassis group with the specified priority.
-func (o *NB) ChassisGroupChassisAdd(haChassisGroupName OVNChassisGroup, chassisID string, priority uint) error {
-	ctx := context.TODO()
+// SetChassisGroupPriority sets a given priority for the chassis ID in the chassis group..
+func (o *NB) SetChassisGroupPriority(ctx context.Context, haChassisGroupName OVNChassisGroup, chassisID string, priority uint) error {
 	operations := []ovsdb.Operation{}
 
 	// Get the chassis group.

internal/server/network/ovn/ovn_sb_actions.go

@@ -8,9 +8,7 @@ import (
 )
 
 // GetLogicalRouterPortActiveChassisHostname gets the hostname of the chassis managing the logical router port.
-func (o *SB) GetLogicalRouterPortActiveChassisHostname(ovnRouterPort OVNRouterPort) (string, error) {
-	ctx := context.TODO()
-
+func (o *SB) GetLogicalRouterPortActiveChassisHostname(ctx context.Context, ovnRouterPort OVNRouterPort) (string, error) {
 	// Look for the port binding.
 	pb := &ovnSB.PortBinding{
 		LogicalPort: fmt.Sprintf("cr-%s", ovnRouterPort),

internal/server/network/ovs/errors.go

@@ -0,0 +1,16 @@
+package ovs
+
+import (
+	"fmt"
+
+	ovsdbClient "github.com/ovn-org/libovsdb/client"
+)
+
+// ErrExists indicates that a DB record already exists.
+var ErrExists = fmt.Errorf("object already exists")
+
+// ErrNotFound indicates that a DB record doesn't exist.
+var ErrNotFound = ovsdbClient.ErrNotFound
+
+// ErrNotManaged indicates that a DB record wasn't created by Incus.
+var ErrNotManaged = fmt.Errorf("object not incus-managed")

internal/server/network/ovs/ovs_actions.go

@@ -28,27 +28,20 @@ func (o *VSwitch) Installed() bool {
 	return err == nil
 }
 
-// BridgeExists returns true if the bridge exists.
-func (o *VSwitch) BridgeExists(bridgeName string) (bool, error) {
-	ctx := context.TODO()
+// GetBridge returns a bridge entry.
+func (o *VSwitch) GetBridge(ctx context.Context, bridgeName string) (*ovsSwitch.Bridge, error) {
 	bridge := &ovsSwitch.Bridge{Name: bridgeName}
 
 	err := o.client.Get(ctx, bridge)
 	if err != nil {
-		if err == ovsdbClient.ErrNotFound {
-			return false, nil
-		}
-
-		return false, err
+		return nil, err
 	}
 
-	return true, nil
+	return bridge, nil
 }
 
-// BridgeAdd adds a new bridge.
-func (o *VSwitch) BridgeAdd(bridgeName string, mayExist bool, hwaddr net.HardwareAddr, mtu uint32) error {
-	ctx := context.TODO()
-
+// CreateBridge adds a new bridge.
+func (o *VSwitch) CreateBridge(ctx context.Context, bridgeName string, mayExist bool, hwaddr net.HardwareAddr, mtu uint32) error {
 	// Create interface.
 	iface := ovsSwitch.Interface{
 		UUID: "interface",
@@ -145,10 +138,8 @@ func (o *VSwitch) BridgeAdd(bridgeName string, mayExist bool, hwaddr net.Hardwar
 	return fmt.Errorf("Bridge interface failed to appear")
 }
 
-// BridgeDelete deletes a bridge.
-func (o *VSwitch) BridgeDelete(bridgeName string) error {
-	ctx := context.TODO()
-
+// DeleteBridge deletes a bridge.
+func (o *VSwitch) DeleteBridge(ctx context.Context, bridgeName string) error {
 	bridge := ovsSwitch.Bridge{
 		Name: bridgeName,
 	}
@@ -184,10 +175,8 @@ func (o *VSwitch) BridgeDelete(bridgeName string) error {
 	return nil
 }
 
-// BridgePortAdd adds a port to the bridge (if already attached does nothing).
-func (o *VSwitch) BridgePortAdd(bridgeName string, portName string, mayExist bool) error {
-	ctx := context.TODO()
-
+// CreateBridgePort adds a port to the bridge.
+func (o *VSwitch) CreateBridgePort(ctx context.Context, bridgeName string, portName string, mayExist bool) error {
 	// Get the bridge.
 	bridge := ovsSwitch.Bridge{
 		Name: bridgeName,
@@ -324,10 +313,8 @@ func (o *VSwitch) InterfaceAssociatedOVNSwitchPort(interfaceName string) (string
 	return strings.TrimSpace(ovnSwitchPort), nil
 }
 
-// ChassisID returns the local chassis ID.
-func (o *VSwitch) ChassisID() (string, error) {
-	ctx := context.TODO()
-
+// GetChassisID returns the local chassis ID.
+func (o *VSwitch) GetChassisID(ctx context.Context) (string, error) {
 	vSwitch := &ovsSwitch.OpenvSwitch{
 		UUID: o.rootUUID,
 	}
@@ -366,10 +353,8 @@ func (o *VSwitch) OVNEncapIP() (net.IP, error) {
 	return encapIP, nil
 }
 
-// OVNBridgeMappings gets the current OVN bridge mappings.
-func (o *VSwitch) OVNBridgeMappings(bridgeName string) ([]string, error) {
-	ctx := context.TODO()
-
+// GetOVNBridgeMappings gets the current OVN bridge mappings.
+func (o *VSwitch) GetOVNBridgeMappings(ctx context.Context, bridgeName string) ([]string, error) {
 	vSwitch := &ovsSwitch.OpenvSwitch{
 		UUID: o.rootUUID,
 	}
@@ -392,7 +377,7 @@ func (o *VSwitch) OVNBridgeMappingAdd(bridgeName string, providerName string) er
 	ovnBridgeMappingMutex.Lock()
 	defer ovnBridgeMappingMutex.Unlock()
 
-	mappings, err := o.OVNBridgeMappings(bridgeName)
+	mappings, err := o.GetOVNBridgeMappings(context.TODO(), bridgeName)
 	if err != nil {
 		return err
 	}
@@ -420,7 +405,7 @@ func (o *VSwitch) OVNBridgeMappingDelete(bridgeName string, providerName string)
 	ovnBridgeMappingMutex.Lock()
 	defer ovnBridgeMappingMutex.Unlock()
 
-	mappings, err := o.OVNBridgeMappings(bridgeName)
+	mappings, err := o.GetOVNBridgeMappings(context.TODO(), bridgeName)
 	if err != nil {
 		return err
 	}