Merge lp:~jtv/juju-core/az-customdata into lp:~go-bot/juju-core/trunk

[Jeroen T. Vermeulen]

Implement custom data for Azure ("userdata").

Much of the code to generate cloudinit userdata is actually shared between
providers, hence the comments.

I also included a fix for a recent gwacl update. With this landed, devs will
have to update their copies of gwacl.

Implement custom data for Azure ("userdata").

Much of the code to generate cloudinit userdata is actually shared between
providers, hence the comments.

I also included a fix for a recent gwacl update. With this landed, devs will
have to update their copies of gwacl.

https://codereview.appspot.com/11158043/

[Jeroen T. Vermeulen (jtv)]

Reviewers: mp+174157_code.launchpad.net,

Message:
Please take a look.

Description:
Implement custom data for Azure ("userdata").

Much of the code to generate cloudinit userdata is actually shared
between
providers, hence the comments.

I also included a fix for a recent gwacl update. With this landed, devs
will
have to update their copies of gwacl.

https://code.launchpad.net/~jtv/juju-core/az-customdata/+merge/174157

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/11158043/

Affected files:
   A [revision details]
   A environs/azure/customdata.go
   A environs/azure/customdata_test.go
   M environs/azure/environ_test.go
   M environs/ec2/ec2.go
   M environs/maas/util.go
   M environs/openstack/provider.go

[Gavin Panella (allenap)]

LGTM

https://codereview.appspot.com/11158043/diff/1/environs/azure/customdata_test.go
File environs/azure/customdata_test.go (right):

https://codereview.appspot.com/11158043/diff/1/environs/azure/customdata_test.go#newcode70
environs/azure/customdata_test.go:70: c.Check(string(content),
gc.Matches, "(.|\n)*"+marker+"(.|\n)*")
This regex can also be written as "(?s).*" + marker + ".*". It's a
little more readable. It might also be worth using regexp.QuoteMeta() to
make it explicit that marker is literal string, not an expression.

https://codereview.appspot.com/11158043/

[Gavin Panella (allenap)]

review: Approve

[Jeroen T. Vermeulen (jtv)]

Much better, thanks.

https://codereview.appspot.com/11158043/

[Jeroen T. Vermeulen (jtv)]

Please take a look.

https://codereview.appspot.com/11158043/

[Raphaël Badin (rvb)]

On 2013/07/11 11:39:06, jtv.canonical wrote:
> Please take a look.

LGTM.

https://codereview.appspot.com/11158043/

[Go Bot (go-bot)]

The attempt to merge lp:~jtv/juju-core/az-customdata into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 2.485s
ok launchpad.net/juju-core/bzr 6.458s
ok launchpad.net/juju-core/cert 3.224s
ok launchpad.net/juju-core/charm 0.553s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.009s
ok launchpad.net/juju-core/cmd 0.240s
? launchpad.net/juju-core/cmd/builddb [no test files]
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 104.061s
ok launchpad.net/juju-core/cmd/jujud 42.723s
ok launchpad.net/juju-core/constraints 0.013s
ok launchpad.net/juju-core/container/lxc 0.324s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.366s
ok launchpad.net/juju-core/environs 1.770s
? launchpad.net/juju-core/environs/all [no test files]
# launchpad.net/juju-core/environs/azure
environs/azure/environ_test.go:608: unknown gwacl.HostedService field 'HostedServiceDescriptor' in struct literal
FAIL launchpad.net/juju-core/environs/azure [build failed]
ok launchpad.net/juju-core/environs/cloudinit 0.550s
ok launchpad.net/juju-core/environs/config 1.081s
ok launchpad.net/juju-core/environs/dummy 15.688s
ok launchpad.net/juju-core/environs/ec2 177.607s
ok launchpad.net/juju-core/environs/imagemetadata 0.266s
ok launchpad.net/juju-core/environs/instances 0.292s
ok launchpad.net/juju-core/environs/jujutest 0.204s
ok launchpad.net/juju-core/environs/local 1.182s
ok launchpad.net/juju-core/environs/localstorage 0.342s
ok launchpad.net/juju-core/environs/maas 8.823s
ok launchpad.net/juju-core/environs/openstack 8.702s
? launchpad.net/juju-core/environs/testing [no test files]
ok launchpad.net/juju-core/environs/tools 18.635s
? launchpad.net/juju-core/errors [no test files]
ok launchpad.net/juju-core/instance 0.007s
ok launchpad.net/juju-core/juju 12.085s
? launchpad.net/juju-core/juju/testing [no test files]
ok launchpad.net/juju-core/log 0.008s
ok launchpad.net/juju-core/log/syslog 0.010s
ok launchpad.net/juju-core/rpc 0.335s
ok launchpad.net/juju-core/rpc/jsoncodec 0.320s
ok launchpad.net/juju-core/schema 0.012s
ok launchpad.net/juju-core/state 61.116s
? launchpad.net/juju-core/state/api [no test files]
? launchpad.net/juju-core/state/api/common [no test files]
ok launchpad.net/juju-core/state/api/machineagent 1.476s
ok launchpad.net/juju-core/state/api/machiner 1.922s
ok launchpad.net/juju-core/state/api/params 0.021s
ok launchpad.net/juju-core/state/api/upgrader 2.775s
ok launchpad.net/juju-core/state/api/watcher 1.498s
ok launchpad.net/juju-core/state/apiserver 1.847s
ok launchpad.net/juju-core/state/apiserver/client 13.230s
ok launchpad.net/juju-core/state/apiserver/common 0.021s
ok launchpad.net/juju-core/state/apiserver/machine 3.547s
? launchpad.net/juju-core/state/apiserver/testing [no test files]
ok launchpad.net/juju-core/state/apiserver/upgrader 3.824s
ok launchpad.net/juju-core/state/multiwatcher 1.200s
ok...

=== added file 'environs/azure/customdata.go'
--- environs/azure/customdata.go	1970-01-01 00:00:00 +0000
+++ environs/azure/customdata.go	2013-07-11 11:40:35 +0000
@@ -0,0 +1,40 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package azure
+
+import (
+	"encoding/base64"
+	"fmt"
+
+	"launchpad.net/juju-core/environs/cloudinit"
+	"launchpad.net/juju-core/utils"
+)
+
+// userData returns a zipped cloudinit config.
+// TODO(bug 1199847): Some of this work can be shared between providers.
+func userData(cfg *cloudinit.MachineConfig) ([]byte, error) {
+	cloudcfg, err := cloudinit.New(cfg)
+	if err != nil {
+		return nil, fmt.Errorf("problem with cloudinit config: %v", err)
+	}
+	data, err := cloudcfg.Render()
+	if err != nil {
+		return nil, fmt.Errorf("problem with cloudinit config: %v", err)
+	}
+	cdata := utils.Gzip(data)
+	logger.Debugf("user data; %d bytes", len(cdata))
+	return cdata, nil
+}
+
+// makeCustomData produces custom data for Azure.  This is a base64-encoded
+// zipfile of cloudinit userdata.
+func makeCustomData(cfg *cloudinit.MachineConfig) (string, error) {
+	zipData, err := userData(cfg)
+	if err != nil {
+		return "", fmt.Errorf("failure while generating custom data: %v", err)
+	}
+	encodedData := base64.StdEncoding.EncodeToString(zipData)
+	logger.Debugf("base64-encoded custom data: %d bytes", len(encodedData))
+	return encodedData, nil
+}

=== added file 'environs/azure/customdata_test.go'
--- environs/azure/customdata_test.go	1970-01-01 00:00:00 +0000
+++ environs/azure/customdata_test.go	2013-07-11 11:40:35 +0000
@@ -0,0 +1,90 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package azure
+
+import (
+	"encoding/base64"
+
+	gc "launchpad.net/gocheck"
+
+	"launchpad.net/juju-core/environs/cloudinit"
+	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/state/api"
+	"launchpad.net/juju-core/testing"
+	"launchpad.net/juju-core/utils"
+)
+
+type CustomDataSuite struct{}
+
+var _ = gc.Suite(&CustomDataSuite{})
+
+// makeMachineConfig produces a valid cloudinit machine config.
+func makeMachineConfig(c *gc.C) *cloudinit.MachineConfig {
+	dir := c.MkDir()
+	machineID := "0"
+	return &cloudinit.MachineConfig{
+		MachineId:    machineID,
+		MachineNonce: "gxshasqlnng",
+		DataDir:      dir,
+		Tools:        &state.Tools{URL: "file://" + dir},
+		StateInfo: &state.Info{
+			CACert: []byte(testing.CACert),
+			Addrs:  []string{"127.0.0.1:123"},
+			Tag:    state.MachineTag(machineID),
+		},
+		APIInfo: &api.Info{
+			CACert: []byte(testing.CACert),
+			Addrs:  []string{"127.0.0.1:123"},
+			Tag:    state.MachineTag(machineID),
+		},
+	}
+}
+
+// makeBadMachineConfig produces a cloudinit machine config that cloudinit
+// will reject as invalid.
+func makeBadMachineConfig() *cloudinit.MachineConfig {
+	// As it happens, a default-initialized config is invalid.
+	return &cloudinit.MachineConfig{}
+}
+
+func (*CustomDataSuite) TestUserDataFailsOnBadData(c *gc.C) {
+	_, err := userData(makeBadMachineConfig())
+	c.Assert(err, gc.NotNil)
+	c.Check(err, gc.ErrorMatches, "problem with cloudinit config: .*")
+}
+
+func (*CustomDataSuite) TestUserDataProducesZipFile(c *gc.C) {
+	marker := "recognizable_text"
+	cfg := makeMachineConfig(c)
+	cfg.MachineNonce = marker
+
+	data, err := userData(cfg)
+	c.Assert(err, gc.IsNil)
+
+	content, err := utils.Gunzip(data)
+	c.Assert(err, gc.IsNil)
+	// The regex syntax is weird here.  It's a trick to work around
+	// gocheck's matcher prefixing "^" to the regex and suffixing "$".
+	// With this trick, we can scan for a pattern in a multi-line input.
+	c.Check(string(content), gc.Matches, "(?s).*"+marker+".*")
+}
+
+func (*CustomDataSuite) TestMakeCustomDataPropagatesError(c *gc.C) {
+	_, err := makeCustomData(makeBadMachineConfig())
+	c.Assert(err, gc.NotNil)
+	c.Check(err, gc.ErrorMatches, "failure while generating custom data: problem with cloudinit config: .*")
+}
+
+func (*CustomDataSuite) TestMakeCustomDataEncodesUserData(c *gc.C) {
+	cfg := makeMachineConfig(c)
+
+	encodedData, err := makeCustomData(cfg)
+	c.Assert(err, gc.IsNil)
+
+	data, err := base64.StdEncoding.DecodeString(encodedData)
+	c.Assert(err, gc.IsNil)
+	reference, err := userData(cfg)
+	c.Assert(err, gc.IsNil)
+	c.Check(data, gc.DeepEquals, reference)
+}

=== modified file 'environs/azure/environ_test.go'
--- environs/azure/environ_test.go	2013-07-10 05:37:46 +0000
+++ environs/azure/environ_test.go	2013-07-11 11:40:35 +0000
@@ -604,8 +604,8 @@
 }
 
 func makeAzureService(name string) (*gwacl.HostedService, *gwacl.HostedServiceDescriptor) {
-	service1 := &gwacl.HostedService{ServiceName: name}
 	service1Desc := &gwacl.HostedServiceDescriptor{ServiceName: name}
+	service1 := &gwacl.HostedService{HostedServiceDescriptor: *service1Desc}
 	return service1, service1Desc
 }
 
@@ -665,7 +665,7 @@
 	cleanup := setDummyStorage(c, env)
 	defer cleanup()
 
-	// Simulate 2 nodes corresponding to two Azure services.
+	// Simulate 2 instances corresponding to two Azure services.
 	prefix := env.getEnvPrefix()
 	service1Name := prefix + "service1"
 	service2Name := prefix + "service2"

=== modified file 'environs/ec2/ec2.go'
--- environs/ec2/ec2.go	2013-07-10 11:11:12 +0000
+++ environs/ec2/ec2.go	2013-07-11 11:40:35 +0000
@@ -302,6 +302,7 @@
 	})
 }
 
+// TODO(bug 1199847): Some of this work can be shared between providers.
 func (e *environ) userData(scfg *startInstanceParams, tools *state.Tools) ([]byte, error) {
 	mcfg := &cloudinit.MachineConfig{
 		MachineId:    scfg.machineId,

=== modified file 'environs/maas/util.go'
--- environs/maas/util.go	2013-06-16 23:07:00 +0000
+++ environs/maas/util.go	2013-07-11 11:40:35 +0000
@@ -35,6 +35,7 @@
 }
 
 // userData returns a zipped cloudinit config.
+// TODO(bug 1199847): Some of this work can be shared between providers.
 func userData(cfg *cloudinit.MachineConfig, scripts ...string) ([]byte, error) {
 	cloudcfg := cloudinit_core.New()
 	for _, script := range scripts {

=== modified file 'environs/openstack/provider.go'
--- environs/openstack/provider.go	2013-07-10 11:11:12 +0000
+++ environs/openstack/provider.go	2013-07-11 11:40:35 +0000
@@ -629,6 +629,7 @@
 	withPublicIP bool
 }
 
+// TODO(bug 1199847): Some of this work can be shared between providers.
 func (e *environ) userData(scfg *startInstanceParams, tools *state.Tools) ([]byte, error) {
 	mcfg := &cloudinit.MachineConfig{
 		MachineId:    scfg.machineId,