Move ifdown/ifup logic to the interface definition file.

cmd/jujud/main_nix.go

@@ -7,6 +7,9 @@ package main
 
 import (
 	"os"
+
+	// this is here to make godeps output the same on all OSes.
+	_ "bitbucket.org/kardianos/service"
 )
 
 func main() {

dependencies.tsv

@@ -1,3 +1,5 @@
+bitbucket.org/kardianos/osext	hg	5d3ddcf53a508cc2f7404eaebf546ef2cb5cdb6e	12
+bitbucket.org/kardianos/service	hg	d2fd4f8afd23d9b5d57250b8d3f0bc0c8bf363ec	58
 code.google.com/p/go.crypto	hg	aa2644fe4aa50e3b38d75187b4799b1f0c9ddcef	212
 code.google.com/p/go.net	hg	c17ad62118ea511e1051721b429779fa40bddc74	116
 github.com/bmizerany/pat	git	48be7df2c27e1cec821a3284a683ce6ef90d9052	
@@ -19,8 +21,8 @@ github.com/juju/testing	git	503e61bd033592d7b6003174389f68454deb7b7a
 github.com/juju/txn	git	ee0346875f2ae9a21442f3ff64409f750f37afbc	
 github.com/juju/utils	git	28f5ce7a725633883b280766a4f835277cde79cb	
 gopkg.in/juju/charm.v3	git	eb7dae8ed9dfa44b89e4e8eee6e21f80e31a3b69	
-gopkg.in/natefinch/npipe.v2	git	e562d4ae5c2f838f9e7e406f7d9890d5b02467a9	
 gopkg.in/mgo.v2	git	dc255bb679efa273b6544a03261c4053505498a4	
+gopkg.in/natefinch/npipe.v2	git	e562d4ae5c2f838f9e7e406f7d9890d5b02467a9	
 gopkg.in/yaml.v1	git	1418a9bc452f9cf4efa70307cafcb10743e64a56	
 launchpad.net/gnuflag	bzr	roger.peppe@canonical.com-20140716064605-pk32dnmfust02yab	13
 launchpad.net/goamz	bzr	martin.packman@canonical.com-20140813150539-umttn7s536u85eiz	49
@@ -29,5 +31,4 @@ launchpad.net/golxc	bzr	ian.booth@canonical.com-20140730020217-xa1jyuytye6g1qie
 launchpad.net/gomaasapi	bzr	andrew.wilkins@canonical.com-20140728022143-mth8hx6p0i546y0w	52
 launchpad.net/goose	bzr	tarmac-20140702055133-xpuj6gm4pa0f3a0e	127
 launchpad.net/gwacl	bzr	andrew.wilkins@canonical.com-20140624093241-rlqsuf7pqxnrztgw	236
-launchpad.net/tomb	bzr	gustavo@niemeyer.net-20130531003818-70ikdgklbxopn8x4	17
-bitbucket.org/kardianos/service	hg	d2fd4f8afd23d9b5d57250b8d3f0bc0c8bf363ec	
+launchpad.net/tomb	bzr	gustavo@niemeyer.net-20130531003818-70ikdgklbxopn8x4	17

dependencies_test.go

@@ -4,8 +4,13 @@
 package juju_test
 
 import (
-	"go/build"
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
 	"io/ioutil"
+	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -21,15 +26,8 @@ type dependenciesTest struct{}
 
 var _ = gc.Suite(&dependenciesTest{})
 
-func projectRoot(c *gc.C) string {
-	p, err := build.Import("github.com/juju/juju", "", build.FindOnly)
-	c.Assert(err, gc.IsNil)
-	return p.Dir
-}
-
 func (*dependenciesTest) TestDependenciesTsvFormat(c *gc.C) {
-	filename := filepath.Join(projectRoot(c), "dependencies.tsv")
-	content, err := ioutil.ReadFile(filename)
+	content, err := ioutil.ReadFile("dependencies.tsv")
 	c.Assert(err, gc.IsNil)
 
 	for _, line := range strings.Split(string(content), "\n") {
@@ -40,3 +38,93 @@ func (*dependenciesTest) TestDependenciesTsvFormat(c *gc.C) {
 		c.Assert(segments, gc.HasLen, 4)
 	}
 }
+
+func (*dependenciesTest) TestGodepsIsRight(c *gc.C) {
+	f, err := os.Open("dependencies.tsv")
+	c.Assert(err, gc.IsNil)
+	defer f.Close()
+
+	gopath := os.Getenv("GOPATH")
+	gopath = strings.Split(gopath, fmt.Sprintf("%v", os.PathListSeparator))[0]
+
+	godeps, err := exec.LookPath("godeps")
+	if err != nil {
+		godeps = filepath.Join(gopath, "bin", "godeps")
+		if _, err := os.Stat(godeps); err != nil {
+			c.Skip("Godeps not found in $PATH or $GOPATH/bin")
+		}
+	}
+
+	cmd := exec.Command(godeps, "-t", "github.com/juju/juju/...")
+	output, err := cmd.StdoutPipe()
+	stderr, err := cmd.StderrPipe()
+	c.Assert(err, gc.IsNil)
+	err = cmd.Start()
+	c.Assert(err, gc.IsNil)
+	defer func() {
+		out, err := ioutil.ReadAll(stderr)
+		if err2 := cmd.Wait(); err2 != nil {
+			if err != nil {
+				c.Fatalf("Error running godeps: %s", err2)
+			}
+			c.Fatal(string(out))
+		}
+	}()
+
+	if err := diff(output, f); err != nil {
+		c.Fatal(err)
+	}
+}
+
+func diff(exp, act io.Reader) error {
+	// this whole monstrosity loops through the file contents and ensures each
+	// line is the same as in the godeps output.
+	actscan := bufio.NewScanner(act)
+	outscan := bufio.NewScanner(exp)
+	for actscan.Scan() {
+		if !outscan.Scan() {
+			// Scanning godeps output ended before the file contents ended.
+
+			if err := outscan.Err(); err != nil {
+				return fmt.Errorf("Error reading from godeps output: %v", err)
+			}
+
+			// Since there's no error, this means there are dependencies in the
+			// file that aren't actual dependnecies.
+			errs := []string{
+				"dependencies.tsv contains entries not reported by godeps: "}
+			errs = append(errs, actscan.Text())
+			for actscan.Scan() {
+				errs = append(errs, actscan.Text())
+			}
+			if err := actscan.Err(); err != nil {
+				return fmt.Errorf("Error reading from dependencies.tsv: %v", err)
+			}
+			return errors.New(strings.Join(errs, "\n"))
+		}
+		output := outscan.Text()
+		filedata := actscan.Text()
+		if output != filedata {
+			return fmt.Errorf("Godeps output does not match dependencies.tsv:\ngodeps: %s\n  .tsv: %s", output, filedata)
+		}
+	}
+	if err := actscan.Err(); err != nil {
+		return fmt.Errorf("Error reading from dependencies.tsv: %v", err)
+	}
+
+	if outscan.Scan() {
+		// dependencies.tsv scan ended before godeps output ended.
+		// This means there are dependendencies missing in the file.
+		errs := []string{
+			"Godeps reports dependencies not contained in dependencies.tsv: "}
+		errs = append(errs, outscan.Text())
+		for outscan.Scan() {
+			errs = append(errs, outscan.Text())
+		}
+		if err := outscan.Err(); err != nil {
+			return fmt.Errorf("Error reading from godeps output: %v", err)
+		}
+		return errors.New(strings.Join(errs, "\n"))
+	}
+	return nil
+}

provider/maas/environ.go

@@ -4,13 +4,15 @@
 package maas
 
 import (
+	"bytes"
 	"encoding/base64"
 	"encoding/xml"
 	"fmt"
 	"net"
 	"net/url"
 	"strings"
 	"sync"
+	"text/template"
 	"time"
 
 	"github.com/juju/errors"
@@ -330,38 +332,68 @@ func (environ *maasEnviron) startNode(node gomaasapi.MAASObject, series string,
 	return err
 }
 
+// replaceInterface replaces all the ocurrences of the given interface on the provided command
+// template
+func replaceInterface(command string, iface string) (string, error) {
+	var cmd bytes.Buffer
+
+	tpl := template.New("")
+	tpl, err := tpl.Parse(command)
+
+	if err != nil {
+		return "", err
+	}
+
+	// data := struct {
+	// 	Interface string
+	// }{
+	// 	iface,
+	// }
+
+	err = tpl.Execute(&cmd, struct{ NetworkInterface string }{iface})
+
+	if err != nil {
+		return "", err
+	}
+
+	return cmd.String(), nil
+}
+
 // restoreInterfacesFiles returns a string representing the upstart command to
 // revert MAAS changes to interfaces file.
-func restoreInterfacesFiles(iface string) string {
-	return fmt.Sprintf(`mkdir -p etc/network/interfaces.d
-cat > /etc/network/interfaces.d/%s.cfg << EOF
+func restoreInterfacesFiles(iface string) (string, error) {
+	return replaceInterface(`mkdir -p etc/network/interfaces.d
+cat > /etc/network/interfaces.d/{{ .NetworkInterface }}.cfg << EOF
 # The primary network interface
-auto %s
-iface %s inet dhcp
+auto {{ .NetworkInterface }}
+iface {{ .NetworkInterface }} inet dhcp
 EOF
-sed -i '/auto %s/{N;s/auto %s\niface %s inet dhcp//}' /etc/network/interfaces
+sed -i '/auto {{ .NetworkInterface }}/{N;s/auto {{ .NetworkInterface }}\niface {{ .NetworkInterface }} inet dhcp//}' /etc/network/interfaces
 cat >> /etc/network/interfaces << EOF
 # Source interfaces
 # Please check /etc/network/interfaces.d before changing this file
 # as interfaces may have been defined in /etc/network/interfaces.d
 # NOTE: the primary ethernet device is defined in
-# /etc/network/interfaces.d/%s.cfg
+# /etc/network/interfaces.d/{{ .NetworkInterface }}.cfg
 # See LP: #1262951
 source /etc/network/interfaces.d/*.cfg
 EOF
-`, iface, iface, iface, iface, iface, iface, iface)
+`, iface)
 }
 
 // createBridgeNetwork returns a string representing the upstart command to
 // create a bridged interface.
-func createBridgeNetwork(iface string) string {
-	return fmt.Sprintf(`cat > /etc/network/interfaces.d/br0.cfg << EOF
+func createBridgeNetwork(iface string) (string, error) {
+	return replaceInterface(`cat > /etc/network/interfaces.d/br0.cfg << EOF
 auto br0
 iface br0 inet dhcp
-  bridge_ports %s
+  pre-up ifconfig {{ .NetworkInterface }} down
+  pre-up brctl addbr br0
+  pre-up brctl addif br0 {{ .NetworkInterface }}
+  pre-up ifconfig {{ .NetworkInterface }} up
 EOF
-sed -i 's/iface %s inet dhcp/iface %s inet manual/' /etc/network/interfaces.d/%s.cfg
-`, iface, iface, iface, iface)
+sed -i 's/iface {{ .NetworkInterface }} inet dhcp/iface {{ .NetworkInterface }} inet manual/' /etc/network/interfaces.d/{{ .NetworkInterface }}.cfg
+`, iface)
 }
 
 var unsupportedConstraints = []string{
@@ -545,12 +577,24 @@ func newCloudinitConfig(hostname, iface, series string) (*cloudinit.Config, erro
 	case version.Ubuntu:
 		cloudcfg.SetAptUpdate(true)
 		cloudcfg.AddPackage("bridge-utils")
+
+		bridgeNetwork, err := createBridgeNetwork(iface)
+
+		if err != nil {
+			return nil, err
+		}
+
+		ifacesFiles, err := restoreInterfacesFiles(iface)
+
+		if err != nil {
+			return nil, err
+		}
+
 		cloudcfg.AddScripts(
 			"set -xe",
 			runCmd,
-			fmt.Sprintf("ifdown %s", iface),
-			restoreInterfacesFiles(iface),
-			createBridgeNetwork(iface),
+			ifacesFiles,
+			bridgeNetwork,
 			"ifup br0",
 		)
 	}

provider/maas/environ_test.go

@@ -197,7 +197,6 @@ func (*environSuite) TestNewEnvironSetsConfig(c *gc.C) {
 var expectedCloudinitConfig = []interface{}{
 	"set -xe",
 	"mkdir -p '/var/lib/juju'\ninstall -m 755 /dev/null '/var/lib/juju/MAASmachine.txt'\nprintf '%s\\n' ''\"'\"'hostname: testing.invalid\n'\"'\"'' > '/var/lib/juju/MAASmachine.txt'",
-	"ifdown eth0",
 	`mkdir -p etc/network/interfaces.d
 cat > /etc/network/interfaces.d/eth0.cfg << EOF
 # The primary network interface
@@ -218,7 +217,10 @@ EOF
 	`cat > /etc/network/interfaces.d/br0.cfg << EOF
 auto br0
 iface br0 inet dhcp
-  bridge_ports eth0
+  pre-up ifconfig eth0 down
+  pre-up brctl addbr br0
+  pre-up brctl addif br0 eth0
+  pre-up ifconfig eth0 up
 EOF
 sed -i 's/iface eth0 inet dhcp/iface eth0 inet manual/' /etc/network/interfaces.d/eth0.cfg
 `,