[lxc-devel] [lxd/master] Network: Adds persistent randomly generated MAC address for bridge interface
tomponline on Github
lxc-bot at linuxcontainers.org
Thu Jul 23 14:31:55 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1862 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200723/53c74347/attachment-0001.bin>
-------------- next part --------------
From 665725682880dd1b318ac640b72622eca706419a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 23 Jul 2020 14:50:10 +0100
Subject: [PATCH 01/11] lxd/networks: Fixes bug in doNetworkUpdate that
prevents removal of non-node specific keys
This was introduced when the PUT method was merged with the PATCH method.
However there is still sufficiently shared logic to justify putting in a small tweak to handle the different behaviour between PUT and PATCH.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/networks.go | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/lxd/networks.go b/lxd/networks.go
index 7b0b0a303e..0aca89662b 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -692,7 +692,7 @@ func networkPut(d *Daemon, r *http.Request) response.Response {
}
}
- return doNetworkUpdate(d, name, req, targetNode, isClusterNotification(r))
+ return doNetworkUpdate(d, name, req, targetNode, isClusterNotification(r), r.Method)
}
func networkPatch(d *Daemon, r *http.Request) response.Response {
@@ -701,7 +701,7 @@ func networkPatch(d *Daemon, r *http.Request) response.Response {
// doNetworkUpdate loads the current local network config, merges with the requested network config, validates
// and applies the changes. Will also notify other cluster nodes of non-node specific config if needed.
-func doNetworkUpdate(d *Daemon, name string, req api.NetworkPut, targetNode string, clusterNotification bool) response.Response {
+func doNetworkUpdate(d *Daemon, name string, req api.NetworkPut, targetNode string, clusterNotification bool, httpMethod string) response.Response {
// Load the local node-specific network.
n, err := network.LoadByName(d.State(), name)
if err != nil {
@@ -712,11 +712,23 @@ func doNetworkUpdate(d *Daemon, name string, req api.NetworkPut, targetNode stri
req.Config = map[string]string{}
}
- // Merge the current node-specific network config with the submitted config to allow validation.
- for k, v := range n.Config() {
- _, ok := req.Config[k]
- if !ok {
- req.Config[k] = v
+ if targetNode == "" && httpMethod != http.MethodPatch {
+ // If non-node specific config being updated via "put" method, then merge the current
+ // node-specific network config with the submitted config to allow validation.
+ // This allows removal of non-node specific keys when they are absent from request config.
+ for k, v := range n.Config() {
+ if shared.StringInSlice(k, db.NodeSpecificNetworkConfig) {
+ req.Config[k] = v
+ }
+ }
+ } else if httpMethod == http.MethodPatch {
+ // If config being updated via "patch" method, then merge all existing config with the keys that
+ // are present in the request config.
+ for k, v := range n.Config() {
+ _, ok := req.Config[k]
+ if !ok {
+ req.Config[k] = v
+ }
}
}
From 00808134dfa3361248561618b6700b101bff2a7f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 23 Jul 2020 11:03:19 +0100
Subject: [PATCH 02/11] lxd/network/driver/bridge: Consistent comment ending in
setup()
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/network/driver_bridge.go | 142 +++++++++++++++++------------------
1 file changed, 71 insertions(+), 71 deletions(-)
diff --git a/lxd/network/driver_bridge.go b/lxd/network/driver_bridge.go
index 8076cba649..d9f5c49c7b 100644
--- a/lxd/network/driver_bridge.go
+++ b/lxd/network/driver_bridge.go
@@ -357,7 +357,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return fmt.Errorf("Cannot start pending network")
}
- // Create directory
+ // Create directory.
if !shared.PathExists(shared.VarPath("networks", n.name)) {
err := os.MkdirAll(shared.VarPath("networks", n.name), 0711)
if err != nil {
@@ -365,7 +365,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Create the bridge interface
+ // Create the bridge interface.
if !n.isRunning() {
if n.config["bridge.driver"] == "openvswitch" {
ovs := openvswitch.NewOVS()
@@ -385,10 +385,10 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Get a list of tunnels
+ // Get a list of tunnels.
tunnels := n.getTunnels()
- // IPv6 bridge configuration
+ // IPv6 bridge configuration.
if !shared.StringInSlice(n.config["ipv6.address"], []string{"", "none"}) {
if !shared.PathExists("/proc/sys/net/ipv6") {
return fmt.Errorf("Network has ipv6.address but kernel IPv6 support is missing")
@@ -405,13 +405,13 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Get a list of interfaces
+ // Get a list of interfaces.
ifaces, err := net.Interfaces()
if err != nil {
return err
}
- // Cleanup any existing tunnel device
+ // Cleanup any existing tunnel device.
for _, iface := range ifaces {
if strings.HasPrefix(iface.Name, fmt.Sprintf("%s-", n.name)) {
_, err = shared.RunCommand("ip", "link", "del", "dev", iface.Name)
@@ -421,7 +421,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Set the MTU
+ // Set the MTU.
mtu := ""
if n.config["bridge.mtu"] != "" {
mtu = n.config["bridge.mtu"]
@@ -435,7 +435,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Attempt to add a dummy device to the bridge to force the MTU
+ // Attempt to add a dummy device to the bridge to force the MTU.
if mtu != "" && n.config["bridge.driver"] != "openvswitch" {
_, err = shared.RunCommand("ip", "link", "add", "dev", fmt.Sprintf("%s-mtu", n.name), "mtu", mtu, "type", "dummy")
if err == nil {
@@ -446,7 +446,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Now, set a default MTU
+ // Now, set a default MTU.
if mtu == "" {
mtu = "1500"
}
@@ -456,7 +456,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return err
}
- // Set the MAC address
+ // Set the MAC address.
if n.config["bridge.hwaddr"] != "" {
_, err = shared.RunCommand("ip", "link", "set", "dev", n.name, "address", n.config["bridge.hwaddr"])
if err != nil {
@@ -478,13 +478,13 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Bring it up
+ // Bring it up.
_, err = shared.RunCommand("ip", "link", "set", "dev", n.name, "up")
if err != nil {
return err
}
- // Add any listed existing external interface
+ // Add any listed existing external interface.
if n.config["bridge.external_interfaces"] != "" {
for _, entry := range strings.Split(n.config["bridge.external_interfaces"], ",") {
entry = strings.TrimSpace(entry)
@@ -532,7 +532,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return err
}
- // Flush all IPv4 addresses and routes
+ // Flush all IPv4 addresses and routes.
_, err = shared.RunCommand("ip", "-4", "addr", "flush", "dev", n.name, "scope", "global")
if err != nil {
return err
@@ -543,17 +543,17 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return err
}
- // Configure IPv4 firewall (includes fan)
+ // Configure IPv4 firewall (includes fan).
if n.config["bridge.mode"] == "fan" || !shared.StringInSlice(n.config["ipv4.address"], []string{"", "none"}) {
if n.HasDHCPv4() && n.hasIPv4Firewall() {
- // Setup basic iptables overrides for DHCP/DNS
+ // Setup basic iptables overrides for DHCP/DNS.
err = n.state.Firewall.NetworkSetupDHCPDNSAccess(n.name, 4)
if err != nil {
return err
}
}
- // Attempt a workaround for broken DHCP clients
+ // Attempt a workaround for broken DHCP clients.
if n.hasIPv4Firewall() {
err = n.state.Firewall.NetworkSetupDHCPv4Checksum(n.name)
if err != nil {
@@ -561,7 +561,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Allow forwarding
+ // Allow forwarding.
if n.config["bridge.mode"] == "fan" || n.config["ipv4.routing"] == "" || shared.IsTrue(n.config["ipv4.routing"]) {
err = util.SysctlSet("net/ipv4/ip_forward", "1")
if err != nil {
@@ -584,7 +584,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Start building process using subprocess package
+ // Start building process using subprocess package.
command := "dnsmasq"
dnsmasqCmd := []string{"--keep-in-foreground", "--strict-order", "--bind-interfaces",
"--except-interface=lo",
@@ -597,14 +597,14 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return err
}
- // --dhcp-rapid-commit option is only supported on >2.79
+ // --dhcp-rapid-commit option is only supported on >2.79.
minVer, _ := version.NewDottedVersion("2.79")
if dnsmasqVersion.Compare(minVer) > 0 {
dnsmasqCmd = append(dnsmasqCmd, "--dhcp-rapid-commit")
}
if !daemon.Debug {
- // --quiet options are only supported on >2.67
+ // --quiet options are only supported on >2.67.
minVer, _ := version.NewDottedVersion("2.67")
if err == nil && dnsmasqVersion.Compare(minVer) > 0 {
@@ -612,15 +612,15 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Configure IPv4
+ // Configure IPv4.
if !shared.StringInSlice(n.config["ipv4.address"], []string{"", "none"}) {
- // Parse the subnet
+ // Parse the subnet.
ip, subnet, err := net.ParseCIDR(n.config["ipv4.address"])
if err != nil {
return err
}
- // Update the dnsmasq config
+ // Update the dnsmasq config.
dnsmasqCmd = append(dnsmasqCmd, fmt.Sprintf("--listen-address=%s", ip.String()))
if n.HasDHCPv4() {
if !shared.StringInSlice("--dhcp-no-override", dnsmasqCmd) {
@@ -655,7 +655,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Add the address
+ // Add the address.
_, err = shared.RunCommand("ip", "-4", "addr", "add", "dev", n.name, n.config["ipv4.address"])
if err != nil {
return err
@@ -663,7 +663,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
// Configure NAT
if shared.IsTrue(n.config["ipv4.nat"]) {
- //If a SNAT source address is specified, use that, otherwise default to using MASQUERADE mode.
+ //If a SNAT source address is specified, use that, otherwise default to MASQUERADE mode.
var srcIP net.IP
if n.config["ipv4.nat.address"] != "" {
srcIP = net.ParseIP(n.config["ipv4.nat.address"])
@@ -682,7 +682,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Add additional routes
+ // Add additional routes.
if n.config["ipv4.routes"] != "" {
for _, route := range strings.Split(n.config["ipv4.routes"], ",") {
route = strings.TrimSpace(route)
@@ -715,7 +715,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return err
}
- // Flush all IPv6 addresses and routes
+ // Flush all IPv6 addresses and routes.
_, err = shared.RunCommand("ip", "-6", "addr", "flush", "dev", n.name, "scope", "global")
if err != nil {
return err
@@ -726,15 +726,15 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return err
}
- // Configure IPv6
+ // Configure IPv6.
if !shared.StringInSlice(n.config["ipv6.address"], []string{"", "none"}) {
- // Enable IPv6 for the subnet
+ // Enable IPv6 for the subnet.
err := util.SysctlSet(fmt.Sprintf("net/ipv6/conf/%s/disable_ipv6", n.name), "0")
if err != nil {
return err
}
- // Parse the subnet
+ // Parse the subnet.
ip, subnet, err := net.ParseCIDR(n.config["ipv6.address"])
if err != nil {
return err
@@ -745,18 +745,18 @@ func (n *bridge) setup(oldConfig map[string]string) error {
n.logger.Warn("IPv6 networks with a prefix larger than 64 aren't properly supported by dnsmasq")
}
- // Update the dnsmasq config
+ // Update the dnsmasq config.
dnsmasqCmd = append(dnsmasqCmd, []string{fmt.Sprintf("--listen-address=%s", ip.String()), "--enable-ra"}...)
if n.HasDHCPv6() {
if n.config["ipv6.firewall"] == "" || shared.IsTrue(n.config["ipv6.firewall"]) {
- // Setup basic iptables overrides for DHCP/DNS
+ // Setup basic iptables overrides for DHCP/DNS.
err = n.state.Firewall.NetworkSetupDHCPDNSAccess(n.name, 6)
if err != nil {
return err
}
}
- // Build DHCP configuration
+ // Build DHCP configuration.
if !shared.StringInSlice("--dhcp-no-override", dnsmasqCmd) {
dnsmasqCmd = append(dnsmasqCmd, []string{"--dhcp-no-override", "--dhcp-authoritative", fmt.Sprintf("--dhcp-leasefile=%s", shared.VarPath("networks", n.name, "dnsmasq.leases")), fmt.Sprintf("--dhcp-hostsfile=%s", shared.VarPath("networks", n.name, "dnsmasq.hosts"))}...)
}
@@ -782,15 +782,15 @@ func (n *bridge) setup(oldConfig map[string]string) error {
dnsmasqCmd = append(dnsmasqCmd, []string{"--dhcp-range", fmt.Sprintf("::,constructor:%s,ra-only", n.name)}...)
}
- // Allow forwarding
+ // Allow forwarding.
if n.config["ipv6.routing"] == "" || shared.IsTrue(n.config["ipv6.routing"]) {
- // Get a list of proc entries
+ // Get a list of proc entries.
entries, err := ioutil.ReadDir("/proc/sys/net/ipv6/conf/")
if err != nil {
return err
}
- // First set accept_ra to 2 for everything
+ // First set accept_ra to 2 for everything.
for _, entry := range entries {
content, err := ioutil.ReadFile(fmt.Sprintf("/proc/sys/net/ipv6/conf/%s/accept_ra", entry.Name()))
if err == nil && string(content) != "1\n" {
@@ -803,7 +803,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Then set forwarding for all of them
+ // Then set forwarding for all of them.
for _, entry := range entries {
err = util.SysctlSet(fmt.Sprintf("net/ipv6/conf/%s/forwarding", entry.Name()), "1")
if err != nil && !os.IsNotExist(err) {
@@ -826,13 +826,13 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Add the address
+ // Add the address.
_, err = shared.RunCommand("ip", "-6", "addr", "add", "dev", n.name, n.config["ipv6.address"])
if err != nil {
return err
}
- // Configure NAT
+ // Configure NAT.
if shared.IsTrue(n.config["ipv6.nat"]) {
var srcIP net.IP
if n.config["ipv6.nat.address"] != "" {
@@ -852,7 +852,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Add additional routes
+ // Add additional routes.
if n.config["ipv6.routes"] != "" {
for _, route := range strings.Split(n.config["ipv6.routes"], ",") {
route = strings.TrimSpace(route)
@@ -870,21 +870,21 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Configure the fan
+ // Configure the fan.
dnsClustered := false
dnsClusteredAddress := ""
var overlaySubnet *net.IPNet
if n.config["bridge.mode"] == "fan" {
tunName := fmt.Sprintf("%s-fan", n.name)
- // Parse the underlay
+ // Parse the underlay.
underlay := n.config["fan.underlay_subnet"]
_, underlaySubnet, err := net.ParseCIDR(underlay)
if err != nil {
return nil
}
- // Parse the overlay
+ // Parse the overlay.
overlay := n.config["fan.overlay_subnet"]
if overlay == "" {
overlay = "240.0.0.0/8"
@@ -895,7 +895,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return err
}
- // Get the address
+ // Get the address.
fanAddress, devName, devAddr, err := n.fanAddress(underlaySubnet, overlaySubnet)
if err != nil {
return err
@@ -906,17 +906,17 @@ func (n *bridge) setup(oldConfig map[string]string) error {
fanAddress = fmt.Sprintf("%s/24", addr[0])
}
- // Update the MTU based on overlay device (if available)
+ // Update the MTU based on overlay device (if available).
fanMtuInt, err := GetDevMTU(devName)
if err == nil {
- // Apply overhead
+ // Apply overhead.
if n.config["fan.type"] == "ipip" {
fanMtuInt = fanMtuInt - 20
} else {
fanMtuInt = fanMtuInt - 50
}
- // Apply changes
+ // Apply changes.
fanMtu := fmt.Sprintf("%d", fanMtuInt)
if fanMtu != mtu {
mtu = fanMtu
@@ -934,19 +934,19 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Parse the host subnet
+ // Parse the host subnet.
_, hostSubnet, err := net.ParseCIDR(fmt.Sprintf("%s/24", addr[0]))
if err != nil {
return err
}
- // Add the address
+ // Add the address.
_, err = shared.RunCommand("ip", "-4", "addr", "add", "dev", n.name, fanAddress)
if err != nil {
return err
}
- // Update the dnsmasq config
+ // Update the dnsmasq config.
expiry := "1h"
if n.config["ipv4.dhcp.expiry"] != "" {
expiry = n.config["ipv4.dhcp.expiry"]
@@ -959,7 +959,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
fmt.Sprintf("--dhcp-hostsfile=%s", shared.VarPath("networks", n.name, "dnsmasq.hosts")),
"--dhcp-range", fmt.Sprintf("%s,%s,%s", GetIP(hostSubnet, 2).String(), GetIP(hostSubnet, -2).String(), expiry)}...)
- // Setup the tunnel
+ // Setup the tunnel.
if n.config["fan.type"] == "ipip" {
_, err = shared.RunCommand("ip", "-4", "route", "flush", "dev", "tunl0")
if err != nil {
@@ -971,7 +971,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return err
}
- // Fails if the map is already set
+ // Fails if the map is already set.
shared.RunCommand("ip", "link", "change", "dev", "tunl0", "type", "ipip", "fan-map", fmt.Sprintf("%s:%s", overlay, underlay))
_, err = shared.RunCommand("ip", "route", "add", overlay, "dev", "tunl0", "src", addr[0])
@@ -1002,7 +1002,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Configure NAT
+ // Configure NAT.
if n.config["ipv4.nat"] == "" || shared.IsTrue(n.config["ipv4.nat"]) {
if n.config["ipv4.nat.order"] == "after" {
err = n.state.Firewall.NetworkSetupOutboundNAT(n.name, overlaySubnet, nil, true)
@@ -1017,7 +1017,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Setup clustered DNS
+ // Setup clustered DNS.
clusterAddress, err := node.ClusterAddress(n.state.Node)
if err != nil {
return err
@@ -1034,7 +1034,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
dnsClusteredAddress = strings.Split(fanAddress, "/")[0]
}
- // Configure tunnels
+ // Configure tunnels.
for _, tunnel := range tunnels {
getConfig := func(key string) string {
return n.config[fmt.Sprintf("tunnel.%s.%s", tunnel, key)]
@@ -1045,10 +1045,10 @@ func (n *bridge) setup(oldConfig map[string]string) error {
tunRemote := getConfig("remote")
tunName := fmt.Sprintf("%s-%s", n.name, tunnel)
- // Configure the tunnel
+ // Configure the tunnel.
cmd := []string{"ip", "link", "add", "dev", tunName}
if tunProtocol == "gre" {
- // Skip partial configs
+ // Skip partial configs.
if tunProtocol == "" || tunLocal == "" || tunRemote == "" {
continue
}
@@ -1058,7 +1058,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
tunGroup := getConfig("group")
tunInterface := getConfig("interface")
- // Skip partial configs
+ // Skip partial configs.
if tunProtocol == "" {
continue
}
@@ -1102,13 +1102,13 @@ func (n *bridge) setup(oldConfig map[string]string) error {
cmd = append(cmd, []string{"ttl", tunTTL}...)
}
- // Create the interface
+ // Create the interface.
_, err = shared.RunCommand(cmd[0], cmd[1:]...)
if err != nil {
return err
}
- // Bridge it and bring up
+ // Bridge it and bring up.
err = AttachInterface(n.name, tunName)
if err != nil {
return err
@@ -1131,7 +1131,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return err
}
- // Kill any existing dnsmasq and forkdns daemon for this network
+ // Kill any existing dnsmasq and forkdns daemon for this network.
err = dnsmasq.Kill(n.name, false)
if err != nil {
return err
@@ -1142,9 +1142,9 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return err
}
- // Configure dnsmasq
+ // Configure dnsmasq.
if n.config["bridge.mode"] == "fan" || !shared.StringInSlice(n.config["ipv4.address"], []string{"", "none"}) || !shared.StringInSlice(n.config["ipv6.address"], []string{"", "none"}) {
- // Setup the dnsmasq domain
+ // Setup the dnsmasq domain.
dnsDomain := n.config["dns.domain"]
if dnsDomain == "" {
dnsDomain = "lxd"
@@ -1167,12 +1167,12 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
dnsmasqCmd = append(dnsmasqCmd, fmt.Sprintf("--conf-file=%s", shared.VarPath("networks", n.name, "dnsmasq.raw")))
- // Attempt to drop privileges
+ // Attempt to drop privileges.
if n.state.OS.UnprivUser != "" {
dnsmasqCmd = append(dnsmasqCmd, []string{"-u", n.state.OS.UnprivUser}...)
}
- // Create DHCP hosts directory
+ // Create DHCP hosts directory.
if !shared.PathExists(shared.VarPath("networks", n.name, "dnsmasq.hosts")) {
err = os.MkdirAll(shared.VarPath("networks", n.name, "dnsmasq.hosts"), 0755)
if err != nil {
@@ -1180,13 +1180,13 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Check for dnsmasq
+ // Check for dnsmasq.
_, err := exec.LookPath("dnsmasq")
if err != nil {
return fmt.Errorf("dnsmasq is required for LXD managed bridges")
}
- // Update the static leases
+ // Update the static leases.
err = UpdateDNSMasqStatic(n.state, n.name)
if err != nil {
return err
@@ -1213,7 +1213,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
err = p.Save(shared.VarPath("networks", n.name, "dnsmasq.pid"))
if err != nil {
- // Kill Process if started, but could not save the file
+ // Kill Process if started, but could not save the file.
err2 := p.Stop()
if err != nil {
return fmt.Errorf("Could not kill subprocess while handling saving error: %s: %s", err, err2)
@@ -1222,9 +1222,9 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return fmt.Errorf("Failed to save subprocess details: %s", err)
}
- // Spawn DNS forwarder if needed (backgrounded to avoid deadlocks during cluster boot)
+ // Spawn DNS forwarder if needed (backgrounded to avoid deadlocks during cluster boot).
if dnsClustered {
- // Create forkdns servers directory
+ // Create forkdns servers directory.
if !shared.PathExists(shared.VarPath("networks", n.name, ForkdnsServersListPath)) {
err = os.MkdirAll(shared.VarPath("networks", n.name, ForkdnsServersListPath), 0755)
if err != nil {
@@ -1232,7 +1232,7 @@ func (n *bridge) setup(oldConfig map[string]string) error {
}
}
- // Create forkdns servers.conf file if doesn't exist
+ // Create forkdns servers.conf file if doesn't exist.
f, err := os.OpenFile(shared.VarPath("networks", n.name, ForkdnsServersListPath+"/"+ForkdnsServersListFile), os.O_RDONLY|os.O_CREATE, 0666)
if err != nil {
return err
From 785d1230c77659d80205816f020e7d512443ab97 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 23 Jul 2020 14:48:49 +0100
Subject: [PATCH 03/11] lxd/network/network/interface: fillConfig signature
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/network/network_interface.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/network/network_interface.go b/lxd/network/network_interface.go
index 1a558b380d..cedb68ef31 100644
--- a/lxd/network/network_interface.go
+++ b/lxd/network/network_interface.go
@@ -10,7 +10,7 @@ import (
type Network interface {
// Load.
init(state *state.State, id int64, name string, netType string, description string, config map[string]string, status string)
- fillConfig(*api.NetworksPost) error
+ fillConfig(config map[string]string) error
// Config.
Validate(config map[string]string) error
From 3955e2a5b2c6041c30d073c5dbb992c89b74360f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 23 Jul 2020 14:49:07 +0100
Subject: [PATCH 04/11] lxd/network/driver/common: Updates fillConfig signature
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/network/driver_common.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/network/driver_common.go b/lxd/network/driver_common.go
index b79735c336..bc26dcaed3 100644
--- a/lxd/network/driver_common.go
+++ b/lxd/network/driver_common.go
@@ -51,7 +51,7 @@ func (n *common) init(state *state.State, id int64, name string, netType string,
}
// fillConfig fills requested config with any default values, by default this is a no-op.
-func (n *common) fillConfig(req *api.NetworksPost) error {
+func (n *common) fillConfig(config map[string]string) error {
return nil
}
From 040a26c8b528b405f55a0157601023a91e13d725 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 23 Jul 2020 14:54:12 +0100
Subject: [PATCH 05/11] lxd/network/driver/bridge: Updates fillConfig signature
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/network/driver_bridge.go | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/lxd/network/driver_bridge.go b/lxd/network/driver_bridge.go
index d9f5c49c7b..373984f194 100644
--- a/lxd/network/driver_bridge.go
+++ b/lxd/network/driver_bridge.go
@@ -44,30 +44,30 @@ type bridge struct {
}
// fillConfig fills requested config with any default values.
-func (n *bridge) fillConfig(req *api.NetworksPost) error {
+func (n *bridge) fillConfig(config map[string]string) error {
// Set some default values where needed.
- if req.Config["bridge.mode"] == "fan" {
- if req.Config["fan.underlay_subnet"] == "" {
- req.Config["fan.underlay_subnet"] = "auto"
+ if config["bridge.mode"] == "fan" {
+ if config["fan.underlay_subnet"] == "" {
+ config["fan.underlay_subnet"] = "auto"
}
} else {
- if req.Config["ipv4.address"] == "" {
- req.Config["ipv4.address"] = "auto"
+ if config["ipv4.address"] == "" {
+ config["ipv4.address"] = "auto"
}
- if req.Config["ipv4.address"] == "auto" && req.Config["ipv4.nat"] == "" {
- req.Config["ipv4.nat"] = "true"
+ if config["ipv4.address"] == "auto" && config["ipv4.nat"] == "" {
+ config["ipv4.nat"] = "true"
}
- if req.Config["ipv6.address"] == "" {
+ if config["ipv6.address"] == "" {
content, err := ioutil.ReadFile("/proc/sys/net/ipv6/conf/default/disable_ipv6")
if err == nil && string(content) == "0\n" {
- req.Config["ipv6.address"] = "auto"
+ config["ipv6.address"] = "auto"
}
}
- if req.Config["ipv6.address"] == "auto" && req.Config["ipv6.nat"] == "" {
- req.Config["ipv6.nat"] = "true"
+ if config["ipv6.address"] == "auto" && config["ipv6.nat"] == "" {
+ config["ipv6.nat"] = "true"
}
}
From 93d4f48504ea45bcb40088c8dbc6249da94cecd8 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 23 Jul 2020 14:49:25 +0100
Subject: [PATCH 06/11] lxd/network/network/load: Updates FillConfig to use new
signature
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/network/network_load.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/network/network_load.go b/lxd/network/network_load.go
index 8638987cfd..0990e30662 100644
--- a/lxd/network/network_load.go
+++ b/lxd/network/network_load.go
@@ -56,7 +56,7 @@ func FillConfig(req *api.NetworksPost) error {
n := driverFunc()
n.init(nil, 0, req.Name, req.Type, req.Description, req.Config, "Unknown")
- err := n.fillConfig(req)
+ err := n.fillConfig(req.Config)
if err != nil {
return err
}
From ff5b56a32f7d5f22bfc8777987daad20afa6cb01 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 23 Jul 2020 14:56:56 +0100
Subject: [PATCH 07/11] lxd/network/driver/bridge: Fixes Update to regenerate
default values if missing
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/network/driver_bridge.go | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/lxd/network/driver_bridge.go b/lxd/network/driver_bridge.go
index 373984f194..2ba456aae3 100644
--- a/lxd/network/driver_bridge.go
+++ b/lxd/network/driver_bridge.go
@@ -1343,15 +1343,14 @@ func (n *bridge) Stop() error {
func (n *bridge) Update(newNetwork api.NetworkPut, targetNode string, clusterNotification bool) error {
n.logger.Debug("Update", log.Ctx{"clusterNotification": clusterNotification, "newNetwork": newNetwork})
- // When switching to a fan bridge, auto-detect the underlay if not specified.
- if newNetwork.Config["bridge.mode"] == "fan" {
- if newNetwork.Config["fan.underlay_subnet"] == "" {
- newNetwork.Config["fan.underlay_subnet"] = "auto"
- }
+ // Populate default values if they are missing.
+ err := n.fillConfig(newNetwork.Config)
+ if err != nil {
+ return err
}
// Populate auto fields.
- err := fillAuto(newNetwork.Config)
+ err = fillAuto(newNetwork.Config)
if err != nil {
return err
}
From 3239df52b087dc52c82430171872dbeff74fe7fd Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 23 Jul 2020 14:42:24 +0100
Subject: [PATCH 08/11] lxd/devices/device/utils/network: Removes
networkValidMAC
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/device_utils_network.go | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/lxd/device/device_utils_network.go b/lxd/device/device_utils_network.go
index 935edc043c..a875a2d131 100644
--- a/lxd/device/device_utils_network.go
+++ b/lxd/device/device_utils_network.go
@@ -5,7 +5,6 @@ import (
"encoding/hex"
"fmt"
"io/ioutil"
- "regexp"
"strconv"
"strings"
"sync"
@@ -543,20 +542,6 @@ func networkSetVethLimits(m deviceConfig.Device) error {
return nil
}
-// networkValidMAC validates an ethernet MAC address. e.g. "32:47:ae:06:22:f9".
-func networkValidMAC(value string) error {
- regexHwaddr, err := regexp.Compile("^([0-9a-fA-F]{2}:){5}[0-9a-fA-F]{2}$")
- if err != nil {
- return err
- }
-
- if regexHwaddr.MatchString(value) {
- return nil
- }
-
- return fmt.Errorf("Invalid value, must 6 bytes of lower case hex separated by colons")
-}
-
// networkValidGateway validates the gateway value.
func networkValidGateway(value string) error {
if shared.StringInSlice(value, []string{"none", "auto"}) {
From 6da7ec1fabda7430cf9c5f4705d9d0a5df432d72 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 23 Jul 2020 14:51:36 +0100
Subject: [PATCH 09/11] shared/instance: Adds IsNetworkMAC for use in network
and device packages
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
shared/instance.go | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/shared/instance.go b/shared/instance.go
index c0aab29572..1fec53e764 100644
--- a/shared/instance.go
+++ b/shared/instance.go
@@ -162,6 +162,20 @@ func IsRootDiskDevice(device map[string]string) bool {
return false
}
+// IsNetworkMAC validates an ethernet MAC address. e.g. "32:47:ae:06:22:f9".
+func IsNetworkMAC(value string) error {
+ regexHwaddr, err := regexp.Compile("^([0-9a-fA-F]{2}:){5}[0-9a-fA-F]{2}$")
+ if err != nil {
+ return err
+ }
+
+ if regexHwaddr.MatchString(value) {
+ return nil
+ }
+
+ return fmt.Errorf("Invalid value, must 6 bytes of lower case hex separated by colons")
+}
+
// IsNetworkAddress validates an IP (v4 or v6) address string. If string is empty, returns valid.
func IsNetworkAddress(value string) error {
if value == "" {
From f981caa07faffee7d828e03fb49590889926453e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 23 Jul 2020 14:42:47 +0100
Subject: [PATCH 10/11] lxd/device/nic: shared.IsNetworkMAC usage
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/device/nic.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/device/nic.go b/lxd/device/nic.go
index b50c19138f..d3c24820fd 100644
--- a/lxd/device/nic.go
+++ b/lxd/device/nic.go
@@ -13,7 +13,7 @@ func nicValidationRules(requiredFields []string, optionalFields []string) map[st
"network": shared.IsAny,
"mtu": shared.IsAny,
"vlan": networkValidVLAN,
- "hwaddr": networkValidMAC,
+ "hwaddr": shared.IsNetworkMAC,
"host_name": shared.IsAny,
"limits.ingress": shared.IsAny,
"limits.egress": shared.IsAny,
From 216aa7ae02742e64533f9e6092568ca548b7916d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Thu, 23 Jul 2020 14:57:51 +0100
Subject: [PATCH 11/11] lxd/network/driver/bridge: Adds volatile.bridge.hwaddr
key
- Generates persistent volatile MAC address (if no static hwaddr key set) on create & update of a network (but not start/restart).
- Does not generate volatile MAC on start/restart to avoid race conditions updating database when starting the network on multiple cluster nodes concurrently.
- Also ensures that existing behavior of random MAC on each start remains until the network is updated.
- Validates both hwaddr and volatile.bridge.hwaddr as valid MAC addresses.
Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
lxd/network/driver_bridge.go | 63 +++++++++++++++++++++++++++++++++---
1 file changed, 58 insertions(+), 5 deletions(-)
diff --git a/lxd/network/driver_bridge.go b/lxd/network/driver_bridge.go
index 2ba456aae3..3b9df0a308 100644
--- a/lxd/network/driver_bridge.go
+++ b/lxd/network/driver_bridge.go
@@ -19,6 +19,7 @@ import (
"github.com/lxc/lxd/lxd/cluster"
"github.com/lxc/lxd/lxd/daemon"
"github.com/lxc/lxd/lxd/dnsmasq"
+ "github.com/lxc/lxd/lxd/instance"
"github.com/lxc/lxd/lxd/network/openvswitch"
"github.com/lxc/lxd/lxd/node"
"github.com/lxc/lxd/lxd/revert"
@@ -43,6 +44,33 @@ type bridge struct {
common
}
+// getHwaddr retrieves existing static or volatile MAC address from config.
+func (n *bridge) getHwaddr(config map[string]string) string {
+ hwAddr := config["bridge.hwaddr"]
+ if hwAddr == "" {
+ hwAddr = config["volatile.bridge.hwaddr"]
+ }
+
+ return hwAddr
+}
+
+// fillHwaddr populates the volatile.bridge.hwaddr in config if it, nor bridge.hwaddr, are already set.
+// Returns MAC address generated if needed to, otherwise empty string.
+func (n *bridge) fillHwaddr(config map[string]string) (string, error) {
+ // If no existing MAC address, generate a new one and store in volatile.
+ if n.getHwaddr(config) == "" {
+ hwAddr, err := instance.DeviceNextInterfaceHWAddr()
+ if err != nil {
+ return "", errors.Wrapf(err, "Failed generating MAC address")
+ }
+
+ config["volatile.bridge.hwaddr"] = hwAddr
+ return config["volatile.bridge.hwaddr"], nil
+ }
+
+ return "", nil
+}
+
// fillConfig fills requested config with any default values.
func (n *bridge) fillConfig(config map[string]string) error {
// Set some default values where needed.
@@ -71,6 +99,13 @@ func (n *bridge) fillConfig(config map[string]string) error {
}
}
+ // If no static hwaddr specified generate a volatile one to store in DB record so that
+ // there are no races when starting the network at the same time on multiple cluster nodes.
+ _, err := n.fillHwaddr(config)
+ if err != nil {
+ return err
+ }
+
return nil
}
@@ -95,8 +130,21 @@ func (n *bridge) Validate(config map[string]string) error {
return nil
},
- "bridge.hwaddr": shared.IsAny,
- "bridge.mtu": shared.IsInt64,
+ "bridge.hwaddr": func(value string) error {
+ if value == "" {
+ return nil
+ }
+
+ return shared.IsNetworkMAC(value)
+ },
+ "volatile.bridge.hwaddr": func(value string) error {
+ if value == "" {
+ return nil
+ }
+
+ return shared.IsNetworkMAC(value)
+ },
+ "bridge.mtu": shared.IsInt64,
"bridge.mode": func(value string) error {
return shared.IsOneOf(value, []string{"standard", "fan"})
},
@@ -456,9 +504,14 @@ func (n *bridge) setup(oldConfig map[string]string) error {
return err
}
- // Set the MAC address.
- if n.config["bridge.hwaddr"] != "" {
- _, err = shared.RunCommand("ip", "link", "set", "dev", n.name, "address", n.config["bridge.hwaddr"])
+ // If static or persistent volatile MAC address present, use that.
+ // We do not generate missing persistent volatile MAC address at start time so as not to cause DB races
+ // when starting an existing network without volatile key in a cluster. This also allows the previous
+ // behavior for networks (i.e random MAC at start if not specified) until the network is next updated.
+ hwAddr := n.getHwaddr(n.config)
+ if hwAddr != "" {
+ // Set the MAC address on the bridge interface.
+ _, err = shared.RunCommand("ip", "link", "set", "dev", n.name, "address", hwAddr)
if err != nil {
return err
}
More information about the lxc-devel
mailing list