Fixed lp:1576674: Refactored network config in userdata for containers

apiserver/uniter/uniter.go

@@ -1585,7 +1585,7 @@ func (u *UniterAPIV3) getOneNetworkConfig(canAccess common.AuthFunc, unitTagArg,
 		)
 
 		privateAddress, err := machine.PrivateAddress()
-		if err != nil && !network.IsNoAddressError(err) {
+		if err != nil {
 			return nil, errors.Annotatef(err, "getting machine %q preferred private address", machineID)
 		}
 
@@ -1613,13 +1613,13 @@ func (u *UniterAPIV3) getOneNetworkConfig(canAccess common.AuthFunc, unitTagArg,
 
 	for _, addr := range addresses {
 		subnet, err := addr.Subnet()
-		if err != nil {
-			return nil, errors.Annotatef(err, "cannot get subnet for address %q", addr)
-		}
-		if subnet == nil {
-			logger.Debugf("skipping %s: not linked to a known subnet", addr)
+		if errors.IsNotFound(err) {
+			logger.Debugf("skipping %s: not linked to a known subnet (%v)", addr, err)
 			continue
+		} else if err != nil {
+			return nil, errors.Annotatef(err, "cannot get subnet for address %q", addr)
 		}
+
 		if space := subnet.SpaceName(); space != boundSpace {
 			logger.Debugf("skipping %s: want bound to space %q, got space %q", addr, boundSpace, space)
 			continue

cloudconfig/containerinit/container_userdata.go

@@ -10,11 +10,11 @@ import (
 	"io/ioutil"
 	"path/filepath"
 	"strings"
-	"text/template"
 
 	"github.com/juju/errors"
 	"github.com/juju/loggo"
 	"github.com/juju/utils/proxy"
+	"github.com/juju/utils/set"
 
 	"github.com/juju/juju/cloudconfig"
 	"github.com/juju/juju/cloudconfig/cloudinit"
@@ -58,90 +58,123 @@ func WriteCloudInitFile(directory string, userData []byte) (string, error) {
 	return userDataFilename, nil
 }
 
-// networkConfigTemplate defines how to render /etc/network/interfaces
-// file for a container with one or more NICs.
-const networkConfigTemplate = `
-# loopback interface
-auto lo
-iface lo inet loopback{{define "static"}}
-{{.InterfaceName | printf "# interface %q"}}{{if not .NoAutoStart}}
-auto {{.InterfaceName}}{{end}}
-iface {{.InterfaceName}} inet manual{{if gt (len .DNSServers) 0}}
-    dns-nameservers{{range $dns := .DNSServers}} {{$dns.Value}}{{end}}{{end}}{{if gt (len .DNSSearch) 0}}
-    dns-search {{.DNSSearch}}{{end}}
-    pre-up ip address add {{.Address.Value}}/32 dev {{.InterfaceName}} &> /dev/null || true
-    up ip route replace {{.GatewayAddress.Value}} dev {{.InterfaceName}}
-    up ip route replace default via {{.GatewayAddress.Value}}
-    down ip route del default via {{.GatewayAddress.Value}} &> /dev/null || true
-    down ip route del {{.GatewayAddress.Value}} dev {{.InterfaceName}} &> /dev/null || true
-    post-down ip address del {{.Address.Value}}/32 dev {{.InterfaceName}} &> /dev/null || true
-{{end}}{{define "dhcp"}}
-{{.InterfaceName | printf "# interface %q"}}{{if not .NoAutoStart}}
-auto {{.InterfaceName}}{{end}}
-iface {{.InterfaceName}} inet dhcp
-{{end}}{{range $nic := . }}{{if eq $nic.ConfigType "static"}}
-{{template "static" $nic}}{{else}}{{template "dhcp" $nic}}{{end}}{{end}}`
-
-// multiBridgeNetworkConfigTemplate defines how to render /etc/network/interfaces
-// file for a multi-NIC container.
-const multiBridgeNetworkConfigTemplate = `
-auto lo
-iface lo inet loopback
-{{range $nic := .}}{{template "single" $nic}}{{end}}
-{{define "single"}}{{if not .NoAutoStart}}
-auto {{.InterfaceName}}{{end}}
-iface {{.InterfaceName}} inet manual{{if .DNSServers}}
-  dns-nameservers{{range $srv := .DNSServers}} {{$srv.Value}}{{end}}{{end}}{{if .DNSSearchDomains}}
-  dns-search{{range $dom := .DNSSearchDomains}} {{$dom}}{{end}}{{end}}
-  pre-up ip address add {{.CIDRAddress}} dev {{.InterfaceName}} || true
-  up ip route replace {{.CIDR}} dev {{.InterfaceName}} || true
-  down ip route del {{.CIDR}} dev {{.InterfaceName}} || true
-  post-down address del {{.CIDRAddress}} dev {{.InterfaceName}} || true{{if .GatewayAddress.Value}}
-  up ip route replace default via {{.GatewayAddress.Value}} || true
-  down ip route del default via {{.GatewayAddress.Value}} || true{{end}}
-{{end}}`
-
-var networkInterfacesFile = "/etc/network/interfaces.d/00-juju.cfg"
-
-// GenerateNetworkConfig renders a network config for one or more
-// network interfaces, using the given non-nil networkConfig
-// containing a non-empty Interfaces field.
+var networkInterfacesFile = "/etc/network/interfaces"
+
+// GenerateNetworkConfig renders a network config for one or more network
+// interfaces, using the given non-nil networkConfig containing a non-empty
+// Interfaces field.
 func GenerateNetworkConfig(networkConfig *container.NetworkConfig) (string, error) {
 	if networkConfig == nil || len(networkConfig.Interfaces) == 0 {
-		// Don't generate networking config.
 		logger.Tracef("no network config to generate")
 		return "", nil
 	}
 	logger.Debugf("generating network config from %#v", *networkConfig)
 
-	// Copy the InterfaceInfo before modifying the original.
-	interfacesCopy := make([]network.InterfaceInfo, len(networkConfig.Interfaces))
-	copy(interfacesCopy, networkConfig.Interfaces)
-	for i, info := range interfacesCopy {
-		if info.MACAddress != "" {
-			info.MACAddress = ""
+	prepared := PrepareNetworkConfigFromInterfaces(networkConfig.Interfaces)
+
+	var output bytes.Buffer
+	gatewayWritten := false
+	for _, name := range prepared.InterfaceNames {
+		output.WriteString("\n")
+		if name == "lo" {
+			output.WriteString("auto ")
+			autoStarted := strings.Join(prepared.AutoStarted, " ")
+			output.WriteString(autoStarted + "\n\n")
+			output.WriteString("iface lo inet loopback\n")
+
+			dnsServers := strings.Join(prepared.DNSServers, " ")
+			if dnsServers != "" {
+				output.WriteString("  dns-nameservers ")
+				output.WriteString(dnsServers + "\n")
+			}
+
+			dnsSearchDomains := strings.Join(prepared.DNSSearchDomains, " ")
+			if dnsSearchDomains != "" {
+				output.WriteString("  dns-search ")
+				output.WriteString(dnsSearchDomains + "\n")
+			}
+			continue
 		}
-		if info.InterfaceName != "eth0" {
-			info.GatewayAddress = network.Address{}
+
+		address, hasAddress := prepared.NameToAddress[name]
+		if !hasAddress {
+			output.WriteString("iface " + name + " inet manual\n")
+			continue
 		}
-		interfacesCopy[i] = info
-	}
 
-	// Render the config first.
-	tmpl, err := template.New("config").Parse(multiBridgeNetworkConfigTemplate)
-	if err != nil {
-		return "", errors.Annotate(err, "cannot parse network config template")
+		output.WriteString("iface " + name + " inet static\n")
+		output.WriteString("  address " + address + "\n")
+		if !gatewayWritten && prepared.GatewayAddress != "" {
+			output.WriteString("  gateway " + prepared.GatewayAddress + "\n")
+			gatewayWritten = true // write it only once
+		}
 	}
 
-	var buf bytes.Buffer
-	if err := tmpl.Execute(&buf, interfacesCopy); err != nil {
-		return "", errors.Annotate(err, "cannot render network config")
+	generatedConfig := output.String()
+	logger.Debugf("generated network config:\n%s", generatedConfig)
+
+	return generatedConfig, nil
+}
+
+// PreparedConfig holds all the necessary information to render a persistent
+// network config to a file.
+type PreparedConfig struct {
+	InterfaceNames   []string
+	AutoStarted      []string
+	DNSServers       []string
+	DNSSearchDomains []string
+	NameToAddress    map[string]string
+	GatewayAddress   string
+}
+
+// PrepareNetworkConfigFromInterfaces collects the necessary information to
+// render a persistent network config from the given slice of
+// network.InterfaceInfo. The result always includes the loopback interface.
+func PrepareNetworkConfigFromInterfaces(interfaces []network.InterfaceInfo) *PreparedConfig {
+	dnsServers := set.NewStrings()
+	dnsSearchDomains := set.NewStrings()
+	gatewayAddress := ""
+	namesInOrder := make([]string, 1, len(interfaces)+1)
+	nameToAddress := make(map[string]string)
+
+	// Always include the loopback.
+	namesInOrder[0] = "lo"
+	autoStarted := set.NewStrings("lo")
+
+	for _, info := range interfaces {
+		if !info.NoAutoStart {
+			autoStarted.Add(info.InterfaceName)
+		}
+
+		if cidr := info.CIDRAddress(); cidr != "" {
+			nameToAddress[info.InterfaceName] = cidr
+		}
+
+		for _, dns := range info.DNSServers {
+			dnsServers.Add(dns.Value)
+		}
+
+		dnsSearchDomains = dnsSearchDomains.Union(set.NewStrings(info.DNSSearchDomains...))
+
+		if info.InterfaceName == "eth0" && gatewayAddress == "" {
+			// Only set gateway once for the primary NIC.
+			gatewayAddress = info.GatewayAddress.Value
+		}
+
+		namesInOrder = append(namesInOrder, info.InterfaceName)
 	}
 
-	generatedConfig := buf.String()
-	logger.Debugf("generated network config from %#v\nusing%#v:\n%s", interfacesCopy, networkConfig.Interfaces, generatedConfig)
+	prepared := &PreparedConfig{
+		InterfaceNames:   namesInOrder,
+		NameToAddress:    nameToAddress,
+		AutoStarted:      autoStarted.SortedValues(),
+		DNSServers:       dnsServers.SortedValues(),
+		DNSSearchDomains: dnsSearchDomains.SortedValues(),
+		GatewayAddress:   gatewayAddress,
+	}
 
-	return generatedConfig, nil
+	logger.Debugf("prepared network config for rendering: %+v", prepared)
+	return prepared
 }
 
 // newCloudInitConfigWithNetworks creates a cloud-init config which

cloudconfig/containerinit/container_userdata_test.go

@@ -5,6 +5,7 @@
 package containerinit_test
 
 import (
+	"fmt"
 	"path/filepath"
 	"strings"
 	stdtesting "testing"
@@ -67,35 +68,20 @@ func (s *UserDataSuite) SetUpTest(c *gc.C) {
 		NoAutoStart:   true,
 	}}
 	s.expectedNetConfig = `
-auto lo
-iface lo inet loopback
+auto eth0 eth1 lo
 
-auto eth0
-iface eth0 inet manual
-  dns-nameservers ns1.invalid ns2.invalid
-  dns-search foo bar
-  pre-up ip address add 0.1.2.3/24 dev eth0 || true
-  up ip route replace 0.1.2.0/24 dev eth0 || true
-  down ip route del 0.1.2.0/24 dev eth0 || true
-  post-down address del 0.1.2.3/24 dev eth0 || true
-  up ip route replace default via 0.1.2.1 || true
-  down ip route del default via 0.1.2.1 || true
-
-auto eth1
-iface eth1 inet manual
+iface lo inet loopback
   dns-nameservers ns1.invalid ns2.invalid
-  dns-search foo bar
-  pre-up ip address add 0.1.2.4/24 dev eth1 || true
-  up ip route replace 0.1.2.0/24 dev eth1 || true
-  down ip route del 0.1.2.0/24 dev eth1 || true
-  post-down address del 0.1.2.4/24 dev eth1 || true
+  dns-search bar foo
 
-iface eth2 inet manual
-  pre-up ip address add  dev eth2 || true
-  up ip route replace  dev eth2 || true
-  down ip route del  dev eth2 || true
-  post-down address del  dev eth2 || true
+iface eth0 inet static
+  address 0.1.2.3/24
+  gateway 0.1.2.1
 
+iface eth1 inet static
+  address 0.1.2.4/24
+
+iface eth2 inet manual
 `
 	s.PatchValue(containerinit.NetworkInterfacesFile, s.networkInterfacesFile)
 }
@@ -125,18 +111,32 @@ func (s *UserDataSuite) TestNewCloudInitConfigWithNetworks(c *gc.C) {
 	// dropping the last new line and using unindented blank lines.
 	lines := strings.Split(s.expectedNetConfig, "\n")
 	indentedNetConfig := strings.Join(lines[:len(lines)-2], "\n  ")
-	indentedNetConfig = strings.Replace(indentedNetConfig, "\n  \n", "\n\n", -1) + "\n"
+	indentedNetConfig = strings.Replace(indentedNetConfig, "\n  \n", "\n\n", -1)
 	expected := `
 #cloud-config
 bootcmd:
-- install -D -m 644 /dev/null '`[1:] + s.networkInterfacesFile + `'
+- install -D -m 644 /dev/null '%[1]s'
 - |-
-  printf '%s\n' '` + indentedNetConfig + `
-  ' > '` + s.networkInterfacesFile + `'
+  printf '%%s\n' '
+  auto eth0 eth1 lo
+
+  iface lo inet loopback
+    dns-nameservers ns1.invalid ns2.invalid
+    dns-search bar foo
+
+  iface eth0 inet static
+    address 0.1.2.3/24
+    gateway 0.1.2.1
+
+  iface eth1 inet static
+    address 0.1.2.4/24
+
+  iface eth2 inet manual
+  ' > '%[1]s'
 runcmd:
 - ifup -a || true
-`
-	assertUserData(c, cloudConf, expected)
+`[1:]
+	assertUserData(c, cloudConf, fmt.Sprintf(expected, s.networkInterfacesFile))
 }
 
 func (s *UserDataSuite) TestNewCloudInitConfigWithNetworksNoConfig(c *gc.C) {

provider/maas/add-juju-bridge.py

@@ -164,51 +164,45 @@ def bridge(self, prefix, bridge_name, add_auto_stanza):
             return self._bridge_device(bridge_name)
 
     def _bridge_device(self, bridge_name):
-        s1 = IfaceStanza(self.name, self.family, "manual", [])
-        s2 = AutoStanza(bridge_name)
+        stanzas = []
+        stanzas.append(AutoStanza(bridge_name))
         options = list(self.options)
         options.append("bridge_ports {}".format(self.name))
-        s3 = IfaceStanza(bridge_name, self.family, self.method, options)
-        return [s1, s2, s3]
+        stanzas.append(IfaceStanza(bridge_name, self.family, self.method, options))
+        return stanzas
 
     def _bridge_vlan(self, bridge_name, add_auto_stanza):
         stanzas = []
-        s1 = IfaceStanza(self.name, self.family, "manual", self.options)
-        stanzas.append(s1)
         if add_auto_stanza:
             stanzas.append(AutoStanza(bridge_name))
-        options = [x for x in self.options if not x.startswith("vlan")]
+        options = [x for x in self.options if not x.startswith("vlan_id")]
         options.append("bridge_ports {}".format(self.name))
-        s3 = IfaceStanza(bridge_name, self.family, self.method, options)
-        stanzas.append(s3)
+        stanzas.append(IfaceStanza(bridge_name, self.family, self.method, options))
         return stanzas
 
     def _bridge_alias(self, add_auto_stanza):
         stanzas = []
         if add_auto_stanza:
             stanzas.append(AutoStanza(self.name))
-        s1 = IfaceStanza(self.name, self.family, self.method, list(self.options))
-        stanzas.append(s1)
+        stanzas.append(IfaceStanza(self.name, self.family, self.method, list(self.options)))
         return stanzas
 
     def _bridge_bond(self, bridge_name, add_auto_stanza):
         stanzas = []
         if add_auto_stanza:
             stanzas.append(AutoStanza(self.name))
-        s1 = IfaceStanza(self.name, self.family, "manual", list(self.options))
-        s2 = AutoStanza(bridge_name)
+        stanzas.append(IfaceStanza(self.name, self.family, "manual", list(self.options)))
         options = [x for x in self.options if not x.startswith("bond")]
         options.append("bridge_ports {}".format(self.name))
-        s3 = IfaceStanza(bridge_name, self.family, self.method, options)
-        stanzas.extend([s1, s2, s3])
+        stanzas.append(AutoStanza(bridge_name))
+        stanzas.append(IfaceStanza(bridge_name, self.family, self.method, options))
         return stanzas
 
     def _bridge_unchanged(self, add_auto_stanza):
         stanzas = []
         if add_auto_stanza:
             stanzas.append(AutoStanza(self.name))
-        s1 = IfaceStanza(self.name, self.family, self.method, list(self.options))
-        stanzas.append(s1)
+        stanzas.append(IfaceStanza(self.name, self.family, self.method, list(self.options)))
         return stanzas