[lxc-devel] [lxd/master] Network: OVN use DB transactions when allocating external IP on parent network
tomponline on Github
lxc-bot at linuxcontainers.org
Mon Aug 17 13:51:26 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 829 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200817/8a2254ab/attachment.bin>
-------------- next part --------------
From 3d8cf6fdae11c5c9b3fda5c7b553a4e0aea4a0df Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 17 Aug 2020 14:43:37 +0100
Subject: [PATCH 1/5] lxd/db/networks: Separates network type and status
conversion into separate functions
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/networks.go | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/lxd/db/networks.go b/lxd/db/networks.go
index 88a3e175fb..9e60c995c8 100644
--- a/lxd/db/networks.go
+++ b/lxd/db/networks.go
@@ -356,6 +356,20 @@ func (c *Cluster) getNetwork(name string, onlyCreated bool) (int64, *api.Network
network.Description = description.String
network.Config = config
+ // Populate Status and Type fields by converting from DB values.
+ networkFillStatus(&network, state)
+ networkFillType(&network, netType)
+
+ nodes, err := c.networkNodes(id)
+ if err != nil {
+ return -1, nil, err
+ }
+ network.Locations = nodes
+
+ return id, &network, nil
+}
+
+func networkFillStatus(network *api.Network, state int) {
switch state {
case networkPending:
network.Status = api.NetworkStatusPending
@@ -366,7 +380,9 @@ func (c *Cluster) getNetwork(name string, onlyCreated bool) (int64, *api.Network
default:
network.Status = api.NetworkStatusUnknown
}
+}
+func networkFillType(network *api.Network, netType NetworkType) {
switch netType {
case NetworkTypeBridge:
network.Type = "bridge"
@@ -379,14 +395,6 @@ func (c *Cluster) getNetwork(name string, onlyCreated bool) (int64, *api.Network
default:
network.Type = "" // Unknown
}
-
- nodes, err := c.networkNodes(id)
- if err != nil {
- return -1, nil, err
- }
- network.Locations = nodes
-
- return id, &network, nil
}
// Return the names of the nodes the given network is defined on.
From 984b076140a02a12dad74a520a3c69a4ac040082 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 17 Aug 2020 14:44:53 +0100
Subject: [PATCH 2/5] lxd/db/networks: Adds ClusterTx.GetNonPendingNetworks
function
Returns non-pending networks and their configs using a transaction connection.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/networks.go | 54 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/lxd/db/networks.go b/lxd/db/networks.go
index 9e60c995c8..c1bb3f82af 100644
--- a/lxd/db/networks.go
+++ b/lxd/db/networks.go
@@ -66,6 +66,60 @@ func (c *ClusterTx) GetNonPendingNetworkIDs() (map[string]int64, error) {
return ids, nil
}
+// GetNonPendingNetworks returns a map of api.Network associated to network ID.
+//
+// Pending networks are skipped.
+func (c *ClusterTx) GetNonPendingNetworks() (map[int64]api.Network, error) {
+ stmt, err := c.tx.Prepare("SELECT id, name, description, type, state FROM networks WHERE state != ?")
+ if err != nil {
+ return nil, err
+ }
+ defer stmt.Close()
+
+ rows, err := stmt.Query(networkPending)
+ if err != nil {
+ return nil, err
+ }
+ defer rows.Close()
+
+ networks := make(map[int64]api.Network)
+
+ for i := 0; rows.Next(); i++ {
+ var networkID int64
+ var networkType NetworkType
+ var networkState int
+ var network api.Network
+
+ err := rows.Scan(&networkID, &network.Name, &network.Description, &networkType, &networkState)
+ if err != nil {
+ return nil, err
+ }
+
+ // Populate Status and Type fields by converting from DB values.
+ networkFillStatus(&network, networkState)
+ networkFillType(&network, networkType)
+
+ networks[networkID] = network
+ }
+ err = rows.Err()
+ if err != nil {
+ return nil, err
+ }
+
+ // Populate config.
+ for networkID, network := range networks {
+ networkConfig, err := query.SelectConfig(c.tx, "networks_config", "network_id=? AND (node_id=? OR node_id IS NULL)", networkID, c.nodeID)
+ if err != nil {
+ return nil, err
+ }
+
+ network.Config = networkConfig
+ networks[networkID] = network
+ }
+
+ return networks, nil
+}
+
// GetNetworkID returns the ID of the network with the given name.
func (c *ClusterTx) GetNetworkID(name string) (int64, error) {
stmt := "SELECT id FROM networks WHERE name=?"
From 304a344deff1eeeac1f7e150e7fec45616b9e732 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 17 Aug 2020 14:45:53 +0100
Subject: [PATCH 3/5] lxd/db/networks: Adds ClusterTx.UpdateNetwork function
And updates existing UpdateNetwork function to use it.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/db/networks.go | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/lxd/db/networks.go b/lxd/db/networks.go
index c1bb3f82af..2755eb35d9 100644
--- a/lxd/db/networks.go
+++ b/lxd/db/networks.go
@@ -312,6 +312,26 @@ func (c *ClusterTx) networkState(name string, state int) error {
return nil
}
+// UpdateNetwork updates the network with the given ID.
+func (c *ClusterTx) UpdateNetwork(id int64, description string, config map[string]string) error {
+ err := updateNetworkDescription(c.tx, id, description)
+ if err != nil {
+ return err
+ }
+
+ err = clearNetworkConfig(c.tx, id, c.nodeID)
+ if err != nil {
+ return err
+ }
+
+ err = networkConfigAdd(c.tx, id, c.nodeID, config)
+ if err != nil {
+ return err
+ }
+
+ return nil
+}
+
// GetNetworks returns the names of existing networks.
func (c *Cluster) GetNetworks() ([]string, error) {
return c.networks("")
@@ -603,17 +623,7 @@ func (c *Cluster) UpdateNetwork(name, description string, config map[string]stri
}
err = c.Transaction(func(tx *ClusterTx) error {
- err = updateNetworkDescription(tx.tx, id, description)
- if err != nil {
- return err
- }
-
- err = clearNetworkConfig(tx.tx, id, c.nodeID)
- if err != nil {
- return err
- }
-
- err = networkConfigAdd(tx.tx, id, c.nodeID, config)
+ err = tx.UpdateNetwork(id, description, config)
if err != nil {
return err
}
From 05af94244e81f3c8b2312515f53e819529f078a6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 17 Aug 2020 14:46:54 +0100
Subject: [PATCH 4/5] lxd/network/driver/ovn: Use DB transactions to safely
allocate OVN external IPs on parent network
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/network/driver_ovn.go | 94 ++++++++++++++++++++++-----------------
1 file changed, 53 insertions(+), 41 deletions(-)
diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 9d0cca9a26..fc49a5ce5a 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -15,6 +15,7 @@ import (
"github.com/pkg/errors"
"github.com/lxc/lxd/lxd/cluster"
+ "github.com/lxc/lxd/lxd/db"
"github.com/lxc/lxd/lxd/dnsmasq"
"github.com/lxc/lxd/lxd/locking"
"github.com/lxc/lxd/lxd/network/openvswitch"
@@ -288,58 +289,74 @@ func (n *ovn) setupParentPortBridge(parentNet Network, routerMAC net.HardwareAdd
v.routerExtGwIPv6 = parentIPv6
}
- // Retrieve and parse existing allocated IPs for this network on the parent network.
+ // Parse existing allocated IPs for this network on the parent network (if not set yet, will be nil).
routerExtPortIPv4 := net.ParseIP(n.config[ovnVolatileParentIPv4])
routerExtPortIPv6 := net.ParseIP(n.config[ovnVolatileParentIPv6])
// Decide whether we need to allocate new IP(s) and go to the expense of retrieving all allocated IPs.
if (parentIPv4Net != nil && routerExtPortIPv4 == nil) || (parentIPv6Net != nil && routerExtPortIPv6 == nil) {
- allAllocatedIPv4, allAllocatedIPv6, err := n.parentAllAllocatedIPs(parentNet.Name())
- if err != nil {
- return nil, errors.Wrapf(err, "Failed to get all allocated IPs for parent")
- }
-
- if parentIPv4Net != nil && routerExtPortIPv4 == nil {
- ipRanges, err := parseIPRanges(parentNetConf["ipv4.ovn.ranges"], parentNet.DHCPv4Subnet())
- if err != nil {
- return nil, errors.Wrapf(err, "Failed to parse parent IPv4 OVN ranges")
- }
-
- routerExtPortIPv4, err = n.parentAllocateIP(ipRanges, allAllocatedIPv4)
+ err := n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
+ allAllocatedIPv4, allAllocatedIPv6, err := n.parentAllAllocatedIPs(tx, parentNet.Name())
if err != nil {
- return nil, errors.Wrapf(err, "Failed to allocate parent IPv4 address")
+ return errors.Wrapf(err, "Failed to get all allocated IPs for parent")
}
- n.config[ovnVolatileParentIPv4] = routerExtPortIPv4.String()
- }
+ if parentIPv4Net != nil && routerExtPortIPv4 == nil {
+ if parentNetConf["ipv4.ovn.ranges"] == "" {
+ return fmt.Errorf(`Missing required "ipv4.ovn.ranges" config key on parent network`)
+ }
- if parentIPv6Net != nil && routerExtPortIPv6 == nil {
- // If IPv6 OVN ranges are specified by the parent, allocate from them.
- if parentNetConf["ipv6.ovn.ranges"] != "" {
- ipRanges, err := parseIPRanges(parentNetConf["ipv6.ovn.ranges"], parentNet.DHCPv6Subnet())
+ ipRanges, err := parseIPRanges(parentNetConf["ipv4.ovn.ranges"], parentNet.DHCPv4Subnet())
if err != nil {
- return nil, errors.Wrapf(err, "Failed to parse parent IPv6 OVN ranges")
+ return errors.Wrapf(err, "Failed to parse parent IPv4 OVN ranges")
}
- routerExtPortIPv6, err = n.parentAllocateIP(ipRanges, allAllocatedIPv6)
+ routerExtPortIPv4, err = n.parentAllocateIP(ipRanges, allAllocatedIPv4)
if err != nil {
- return nil, errors.Wrapf(err, "Failed to allocate parent IPv6 address")
+ return errors.Wrapf(err, "Failed to allocate parent IPv4 address")
}
- } else {
- // Otherwise use EUI64 derived from MAC address.
- routerExtPortIPv6, err = eui64.ParseMAC(parentIPv6Net.IP, routerMAC)
- if err != nil {
- return nil, err
+ n.config[ovnVolatileParentIPv4] = routerExtPortIPv4.String()
+ }
+
+ if parentIPv6Net != nil && routerExtPortIPv6 == nil {
+ // If IPv6 OVN ranges are specified by the parent, allocate from them.
+ if parentNetConf["ipv6.ovn.ranges"] != "" {
+ ipRanges, err := parseIPRanges(parentNetConf["ipv6.ovn.ranges"], parentNet.DHCPv6Subnet())
+ if err != nil {
+ return errors.Wrapf(err, "Failed to parse parent IPv6 OVN ranges")
+ }
+
+ routerExtPortIPv6, err = n.parentAllocateIP(ipRanges, allAllocatedIPv6)
+ if err != nil {
+ return errors.Wrapf(err, "Failed to allocate parent IPv6 address")
+ }
+
+ } else {
+ // Otherwise use EUI64 derived from MAC address.
+ routerExtPortIPv6, err = eui64.ParseMAC(parentIPv6Net.IP, routerMAC)
+ if err != nil {
+ return err
+ }
}
+
+ n.config[ovnVolatileParentIPv6] = routerExtPortIPv6.String()
}
- n.config[ovnVolatileParentIPv6] = routerExtPortIPv6.String()
- }
+ networkID, err := tx.GetNetworkID(n.name)
+ if err != nil {
+ return errors.Wrapf(err, "Failed to get network ID for network %q", n.name)
+ }
- err = n.state.Cluster.UpdateNetwork(n.name, n.description, n.config)
+ err = tx.UpdateNetwork(networkID, n.description, n.config)
+ if err != nil {
+ return errors.Wrapf(err, "Failed saving allocated parent network IPs")
+ }
+
+ return nil
+ })
if err != nil {
- return nil, errors.Wrapf(err, "Failed saving allocated parent network IPs")
+ return nil, err
}
}
@@ -364,9 +381,9 @@ func (n *ovn) setupParentPortBridge(parentNet Network, routerMAC net.HardwareAdd
}
// parentAllAllocatedIPs gets a list of all IPv4 and IPv6 addresses allocated to OVN networks connected to parent.
-func (n *ovn) parentAllAllocatedIPs(parentNetName string) ([]net.IP, []net.IP, error) {
- // Get a list of managed networks.
- networks, err := n.state.Cluster.GetNonPendingNetworks()
+func (n *ovn) parentAllAllocatedIPs(tx *db.ClusterTx, parentNetName string) ([]net.IP, []net.IP, error) {
+ // Get all managed networks.
+ networks, err := tx.GetNonPendingNetworks()
if err != nil {
return nil, nil, err
}
@@ -374,12 +391,7 @@ func (n *ovn) parentAllAllocatedIPs(parentNetName string) ([]net.IP, []net.IP, e
v4IPs := make([]net.IP, 0)
v6IPs := make([]net.IP, 0)
- for _, name := range networks {
- _, netInfo, err := n.state.Cluster.GetNetworkInAnyState(name)
- if err != nil {
- return nil, nil, err
- }
-
+ for _, netInfo := range networks {
if netInfo.Type != "ovn" || netInfo.Config["parent"] != parentNetName {
continue
}
From 245443955eabede3bf8a5c91e20c4514363bf116 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 17 Aug 2020 14:47:23 +0100
Subject: [PATCH 5/5] lxd/network/driver/ovn: Include last IP in OVN range for
allocatable IPs
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/network/driver_ovn.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index fc49a5ce5a..cabe01f2f8 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -436,7 +436,7 @@ func (n *ovn) parentAllocateIP(ipRanges []*shared.IPRange, allAllocated []net.IP
// Iterate through IPs in range, return the first unallocated one found.
for {
- if startBig.Cmp(endBig) >= 0 {
+ if startBig.Cmp(endBig) > 0 {
break
}
More information about the lxc-devel
mailing list