Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

provider/maas: persist bridge script #6599

Merged
merged 3 commits into from Nov 24, 2016
Merged

provider/maas: persist bridge script #6599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
905cbce
provider/maas: persist bridge script
Nov 22, 2016
7f78ca7
Reinstate commented out tests
Nov 23, 2016
83717e8
Review comments
Nov 24, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 0 additions & 3 deletions provider/maas/Makefile
View file
Open in desktop
Expand Up @@ -8,10 +8,7 @@ bridgescript.go: add-juju-bridge.py Makefile
$(RM) $@
echo -n '// This file is auto generated. Edits will be lost.\n\n' >> $@
echo -n 'package maas\n\n' >> $@
echo -n '//go:generate make -q\n\n' >> $@
echo -n 'import "path"\n\n' >> $@
echo -n 'const bridgeScriptName = "add-juju-bridge.py"\n\n' >> $@
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove this line?
Don't we need "make" in order to build this .go file to include the .py file as a string?

I suppose we don't need "import path", but I think we do want to keep the "//go:generate" line. (I believe it tells 'go build' to run 'make -q' to regenerate this file).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was never used. We don't have a top-down rule that runs go generate. If I change the script I have (for ever) just run make in the provider/maas directory, then commit those changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The go generate mechanism never materialised in the build because, for a long time, we were stuck on versions of go that did not support go generate.

echo -n 'var bridgeScriptPath = path.Join("/var/tmp", bridgeScriptName)\n\n' >> $@
echo -n "const bridgeScriptPython = \`" >> $@
cat add-juju-bridge.py >> $@
echo -n '`\n' >> $@
Expand Down
6 changes: 0 additions & 6 deletions provider/maas/bridgescript.go
View file
Open in desktop
Expand Up @@ -2,14 +2,8 @@

package maas

//go:generate make -q

import "path"

const bridgeScriptName = "add-juju-bridge.py"

var bridgeScriptPath = path.Join("/var/tmp", bridgeScriptName)

const bridgeScriptPython = `#!/usr/bin/env python

# Copyright 2015 Canonical Ltd.
Expand Down
31 changes: 21 additions & 10 deletions provider/maas/environ.go
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"net/http"
"net/url"
"path"
"regexp"
"strconv"
"strings"
Expand All @@ -31,6 +32,7 @@ import (
"github.com/juju/juju/environs/storage"
"github.com/juju/juju/environs/tags"
"github.com/juju/juju/instance"
"github.com/juju/juju/juju/paths"
"github.com/juju/juju/network"
"github.com/juju/juju/provider/common"
"github.com/juju/juju/state/multiwatcher"
Expand Down Expand Up @@ -1318,9 +1320,10 @@ func (environ *maasEnviron) selectNode2(args selectNodeArgs) (maasInstance, erro
return inst, nil
}

// setupJujuNetworking returns a string representing the script to run
// in order to prepare the Juju-specific networking config on a node.
func setupJujuNetworking(interfacesToBridge []string) string {
// bridgeScriptWrapperForCloudInit returns a string representing the script
// to run in order to prepare the Juju-specific networking config on a
// node during cloud-init.
func bridgeScriptWrapperForCloudInit(bridgeScriptPath string, interfacesToBridge []string) string {
// For ubuntu series < xenial we prefer python2 over python3
// as we don't want to invalidate lots of testing against
// known cloud-image contents. A summary of Ubuntu releases
Expand All @@ -1337,8 +1340,6 @@ func setupJujuNetworking(interfacesToBridge []string) string {
// going forward: python 3 only

return fmt.Sprintf(`
trap 'rm -f %[1]q' EXIT

if [ -x /usr/bin/python2 ]; then
juju_networking_preferred_python_binary=/usr/bin/python2
elif [ -x /usr/bin/python3 ]; then
Expand Down Expand Up @@ -1373,10 +1374,6 @@ fi`,
)
}

func renderEtcNetworkInterfacesScript(interfacesToBridge ...string) string {
return setupJujuNetworking(interfacesToBridge)
}

// newCloudinitConfig creates a cloudinit.Config structure suitable as a base
// for initialising a MAAS node.
func (environ *maasEnviron) newCloudinitConfig(hostname, forSeries string, interfacesToBridge []string) (cloudinit.CloudConfig, error) {
Expand Down Expand Up @@ -1409,9 +1406,15 @@ func (environ *maasEnviron) newCloudinitConfig(hostname, forSeries string, inter
)
break
}

bridgeScriptPath, err := bridgeScriptPathForSeries(forSeries)
if err != nil {
return nil, errors.Trace(err)
}

cloudcfg.AddPackage("bridge-utils")
cloudcfg.AddBootTextFile(bridgeScriptPath, bridgeScriptPython, 0755)
cloudcfg.AddScripts(setupJujuNetworking(interfacesToBridge))
cloudcfg.AddScripts(bridgeScriptWrapperForCloudInit(bridgeScriptPath, interfacesToBridge))
}
return cloudcfg, nil
}
Expand Down Expand Up @@ -2414,3 +2417,11 @@ func (env *maasEnviron) releaseContainerAddresses2(macAddresses []string) error
}
return nil
}

func bridgeScriptPathForSeries(series string) (string, error) {
dataDir, err := paths.DataDir(series)
if err != nil {
return "", err
}
return path.Join(dataDir, bridgeScriptName), nil
}
13 changes: 10 additions & 3 deletions provider/maas/environ_test.go
View file
Open in desktop
Expand Up @@ -146,7 +146,10 @@ func (*environSuite) TestNewCloudinitConfig(c *gc.C) {
cfg := getSimpleTestConfig(c, nil)
env, err := maas.NewEnviron(getSimpleCloudSpec(), cfg)
c.Assert(err, jc.ErrorIsNil)
modifyNetworkScript := maas.RenderEtcNetworkInterfacesScript("eth0", "eth1")
var path string
path, err = maas.BridgeScriptPathForSeries("quantal")
c.Assert(err, jc.ErrorIsNil)
modifyNetworkScript := maas.BridgeScriptWrapperForCloudInit(path, []string{"eth0", "eth1"})
script := expectedCloudinitConfig
script = append(script, modifyNetworkScript)
cloudcfg, err := maas.NewCloudinitConfig(env, "testing.invalid", "quantal", []string{"eth0", "eth1"})
Expand All @@ -169,13 +172,17 @@ func (*environSuite) TestNewCloudinitConfigWithDisabledNetworkManagement(c *gc.C
}

func (*environSuite) TestRenderEtcNetworkInterfacesScriptMultipleNames(c *gc.C) {
script := maas.RenderEtcNetworkInterfacesScript("eth0", "eth0:1", "eth2", "eth1")
path, err := maas.BridgeScriptPathForSeries("quantal")
c.Assert(err, jc.ErrorIsNil)
script := maas.BridgeScriptWrapperForCloudInit(path, []string{"eth0", "eth0:1", "eth2", "eth1"})
c.Check(script, jc.Contains, `--interfaces-to-bridge="eth0 eth0:1 eth2 eth1"`)
c.Check(script, jc.Contains, `--bridge-prefix="br-"`)
}

func (*environSuite) TestRenderEtcNetworkInterfacesScriptSingleName(c *gc.C) {
script := maas.RenderEtcNetworkInterfacesScript("eth0")
path, err := maas.BridgeScriptPathForSeries("quantal")
c.Assert(err, jc.ErrorIsNil)
script := maas.BridgeScriptWrapperForCloudInit(path, []string{"eth0"})
c.Check(script, jc.Contains, `--interfaces-to-bridge="eth0"`)
c.Check(script, jc.Contains, `--bridge-prefix="br-"`)
}
Expand Down
6 changes: 3 additions & 3 deletions provider/maas/export_test.go
View file
Open in desktop
Expand Up @@ -11,7 +11,9 @@ import (
)

var (
ShortAttempt = &shortAttempt
ShortAttempt = &shortAttempt
BridgeScriptWrapperForCloudInit = bridgeScriptWrapperForCloudInit
BridgeScriptPathForSeries = bridgeScriptPathForSeries
)

func GetMAASClient(env environs.Environ) *gomaasapi.MAASObject {
Expand All @@ -21,5 +23,3 @@ func GetMAASClient(env environs.Environ) *gomaasapi.MAASObject {
func NewCloudinitConfig(env environs.Environ, hostname, series string, interfacesToBridge []string) (cloudinit.CloudConfig, error) {
return env.(*maasEnviron).newCloudinitConfig(hostname, series, interfacesToBridge)
}

var RenderEtcNetworkInterfacesScript = renderEtcNetworkInterfacesScript