Merge lp:~gz/juju-core/add_startinstance_params into lp:~go-bot/juju-core/trunk

[Martin Packman]

environs: Added Networks to StartInstanceParams

Added to StartInstanceParams in environ/broker.go
Networks is a simple struct holding two str lists
defining which networks are to be included/excluded
for the started instance.

https://codereview.appspot.com/77490043/

R=natefinch

environs: Added Networks to StartInstanceParams

Added to StartInstanceParams in environ/broker.go
Networks is a simple struct holding two str lists
defining which networks are to be included/excluded
for the started instance.
Networks was defined in place for the moment.
Even though I added helpers to the ones already in
place (for Starting instances with constraints)
I did not add specific tests for the added struct.

https://codereview.appspot.com/77490043/

[Martin Packman (gz)]

Reviewers: mp+211588_code.launchpad.net,

Message:
Please take a look.

Description:
environs: Added Networks to StartInstanceParams

Added to StartInstanceParams in environ/broker.go
Networks is a simple struct holding two str lists
defining which networks are to be included/excluded
for the started instance.
Networks was defined in place for the moment.
Even though I added helpers to the ones already in
place (for Starting instances with constraints)
I did not add specific tests for the added struct.

https://code.launchpad.net/~gz/juju-core/add_startinstance_params/+merge/211588

(do not edit description out of merge proposal)

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

Affected files (+45, -6 lines):
   A [revision details]
   M environs/broker.go
   M juju/testing/instance.go
   M provider/common/bootstrap_test.go
   M provider/common/mock_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140318112939-onhi90o40412k44a
+New revision: <email address hidden>

Index: environs/broker.go
=== modified file 'environs/broker.go'
--- environs/broker.go 2014-03-13 05:09:14 +0000
+++ environs/broker.go 2014-03-18 17:18:22 +0000
@@ -10,6 +10,13 @@
   "launchpad.net/juju-core/tools"
  )

+// Networks holds network include/exclude
+// configuration
+type Networks struct {
+ IncludedNetworks []string
+ ExcludedNetworks []string
+}
+
  // StartInstanceParams holds parameters for the
  // InstanceBroker.StartInstace method.
  type StartInstanceParams struct {
@@ -17,6 +24,10 @@
   // the kind of instance to create.
   Constraints constraints.Value

+ // Networks holds 2 string slices indicating
+ // respectively included and excluded Networks
+ Networks Networks
+
   // Tools is a list of tools that may be used
   // to start a Juju agent on the machine.
   Tools tools.List

Index: juju/testing/instance.go
=== modified file 'juju/testing/instance.go'
--- juju/testing/instance.go 2014-03-13 05:09:14 +0000
+++ juju/testing/instance.go 2014-03-18 17:18:22 +0000
@@ -85,6 +85,31 @@
  ) (
   instance.Instance, *instance.HardwareCharacteristics, error,
  ) {
+ return StartInstanceWithConstraintsAndNetworks(env, machineId, cons,
environs.Networks{})
+}
+
+// AssertStartInstanceWithNetworks is a test helper function that starts an
+// instance with the given networks, and a plausible but invalid
+// configuration, and returns the result of Environ.StartInstance.
+func AssertStartInstanceWithNetworks(
+ c *gc.C, env environs.Environ, machineId string, cons constraints.Value,
nets environs.Networks,
+) (
+ instance.Instance, *instance.HardwareCharacteristics,
+) {
+ inst, hc, err := StartInstanceWithConstraintsAndNetworks(env, machineId,
cons, nets)
+ c.Assert(err, gc.IsNil)
+ return inst, hc
+}
+
+// StartInstanceWithNetworks is a test helper function that starts an
instance
+// with the given networks, and a plausible but invalid configuration, and
+// returns the result of Environ.StartInstance.
+func StartInstanceWithConstraintsAndNetwor...

[Nate Finch (natefinch)]

LGTM

https://codereview.appspot.com/77490043/

=== modified file 'environs/broker.go'
--- environs/broker.go	2014-03-13 05:09:14 +0000
+++ environs/broker.go	2014-03-18 17:27:01 +0000
@@ -10,6 +10,13 @@
 	"launchpad.net/juju-core/tools"
 )
 
+// Networks holds network include/exclude
+// configuration
+type Networks struct {
+	IncludedNetworks []string
+	ExcludedNetworks []string
+}
+
 // StartInstanceParams holds parameters for the
 // InstanceBroker.StartInstace method.
 type StartInstanceParams struct {
@@ -17,6 +24,10 @@
 	// the kind of instance to create.
 	Constraints constraints.Value
 
+	// Networks holds 2 string slices indicating
+	// respectively included and excluded Networks
+	Networks Networks
+
 	// Tools is a list of tools that may be used
 	// to start a Juju agent on the machine.
 	Tools tools.List

=== modified file 'juju/testing/instance.go'
--- juju/testing/instance.go	2014-03-13 05:09:14 +0000
+++ juju/testing/instance.go	2014-03-18 17:27:01 +0000
@@ -85,6 +85,31 @@
 ) (
 	instance.Instance, *instance.HardwareCharacteristics, error,
 ) {
+	return StartInstanceWithConstraintsAndNetworks(env, machineId, cons, environs.Networks{})
+}
+
+// AssertStartInstanceWithNetworks is a test helper function that starts an
+// instance with the given networks, and a plausible but invalid
+// configuration, and returns the result of Environ.StartInstance.
+func AssertStartInstanceWithNetworks(
+	c *gc.C, env environs.Environ, machineId string, cons constraints.Value, nets environs.Networks,
+) (
+	instance.Instance, *instance.HardwareCharacteristics,
+) {
+	inst, hc, err := StartInstanceWithConstraintsAndNetworks(env, machineId, cons, nets)
+	c.Assert(err, gc.IsNil)
+	return inst, hc
+}
+
+// StartInstanceWithNetworks is a test helper function that starts an instance
+// with the given networks, and a plausible but invalid configuration, and
+// returns the result of Environ.StartInstance.
+func StartInstanceWithConstraintsAndNetworks(
+	env environs.Environ, machineId string, cons constraints.Value,
+	nets environs.Networks,
+) (
+	instance.Instance, *instance.HardwareCharacteristics, error,
+) {
 	series := env.Config().DefaultSeries()
 	agentVersion, ok := env.Config().AgentVersion()
 	if !ok {
@@ -100,6 +125,7 @@
 	machineConfig := environs.NewMachineConfig(machineId, machineNonce, stateInfo, apiInfo)
 	return env.StartInstance(environs.StartInstanceParams{
 		Constraints:   cons,
+		Networks:      nets,
 		Tools:         possibleTools,
 		MachineConfig: machineConfig,
 	})

=== modified file 'provider/common/bootstrap_test.go'
--- provider/common/bootstrap_test.go	2014-03-13 07:54:56 +0000
+++ provider/common/bootstrap_test.go	2014-03-18 17:27:01 +0000
@@ -93,7 +93,7 @@
 	checkCons := constraints.MustParse("mem=8G")
 
 	startInstance := func(
-		cons constraints.Value, possibleTools tools.List, mcfg *cloudinit.MachineConfig,
+		cons constraints.Value, _ environs.Networks, possibleTools tools.List, mcfg *cloudinit.MachineConfig,
 	) (
 		instance.Instance, *instance.HardwareCharacteristics, error,
 	) {
@@ -118,7 +118,7 @@
 	stor := &mockStorage{Storage: innerStorage}
 
 	startInstance := func(
-		_ constraints.Value, _ tools.List, _ *cloudinit.MachineConfig,
+		_ constraints.Value, _ environs.Networks, _ tools.List, _ *cloudinit.MachineConfig,
 	) (
 		instance.Instance, *instance.HardwareCharacteristics, error,
 	) {
@@ -151,7 +151,7 @@
 	stor := &mockStorage{Storage: innerStorage}
 
 	startInstance := func(
-		_ constraints.Value, _ tools.List, _ *cloudinit.MachineConfig,
+		_ constraints.Value, _ environs.Networks, _ tools.List, _ *cloudinit.MachineConfig,
 	) (
 		instance.Instance, *instance.HardwareCharacteristics, error,
 	) {
@@ -193,7 +193,7 @@
 
 	checkURL := ""
 	startInstance := func(
-		_ constraints.Value, _ tools.List, mcfg *cloudinit.MachineConfig,
+		_ constraints.Value, _ environs.Networks, _ tools.List, mcfg *cloudinit.MachineConfig,
 	) (
 		instance.Instance, *instance.HardwareCharacteristics, error,
 	) {

=== modified file 'provider/common/mock_test.go'
--- provider/common/mock_test.go	2014-03-13 05:09:14 +0000
+++ provider/common/mock_test.go	2014-03-18 17:27:01 +0000
@@ -17,7 +17,7 @@
 )
 
 type allInstancesFunc func() ([]instance.Instance, error)
-type startInstanceFunc func(constraints.Value, tools.List, *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, error)
+type startInstanceFunc func(constraints.Value, environs.Networks, tools.List, *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, error)
 type stopInstancesFunc func([]instance.Instance) error
 type getToolsSourcesFunc func() ([]simplestreams.DataSource, error)
 type configFunc func() *config.Config
@@ -46,7 +46,7 @@
 	return env.allInstances()
 }
 func (env *mockEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, error) {
-	return env.startInstance(args.Constraints, args.Tools, args.MachineConfig)
+	return env.startInstance(args.Constraints, args.Networks, args.Tools, args.MachineConfig)
 }
 
 func (env *mockEnviron) StopInstances(instances []instance.Instance) error {