Reviewers: mp+212635_code.launchpad.net,

Message:
Please take a look.

Description:
Add networks params to AddService

Append includeNetworks and excludeNetworks to AddService
function parameters, and updates callers appropriately.
New AddTestingServiceWithNetworks created in a couple of
places and used for basic test.

https://code.launchpad.net/~gz/juju-core/addservice_using_networks/+merge/212635

(do not edit description out of merge proposal)


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

Affected files (+59, -34 lines):
   A [revision details]
   M cmd/juju/status_test.go
   M environs/jujutest/livetests.go
   M juju/conn_test.go
   M juju/deploy.go
   M juju/testing/conn.go
   M state/apiserver/charmrevisionupdater/testing/suite.go
   M state/apiserver/client/run_test.go
   M state/conn_test.go
   M state/export_test.go
   M state/service_test.go
   M state/state.go
   M state/state_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-20140324132340-bdfed5wwtmq8ws97
+New revision: <email address hidden>

Index: juju/conn_test.go
=== modified file 'juju/conn_test.go'
--- juju/conn_test.go	2014-03-17 22:42:51 +0000
+++ juju/conn_test.go	2014-03-25 14:51:19 +0000
@@ -400,7 +400,7 @@
  	curl := coretesting.Charms.ClonedURL(s.repo.Path, "quantal", "riak")
  	sch, err := s.conn.PutCharm(curl, s.repo, false)
  	c.Assert(err, gc.IsNil)
-	svc, err := s.conn.State.AddService("testriak", "user-admin", sch)
+	svc, err := s.conn.State.AddService("testriak", "user-admin", sch,  
[]string{}, []string{})
  	c.Assert(err, gc.IsNil)
  	units, err := juju.AddUnits(s.conn.State, svc, 2, "")
  	c.Assert(err, gc.IsNil)


Index: juju/deploy.go
=== modified file 'juju/deploy.go'
--- juju/deploy.go	2013-12-19 21:17:26 +0000
+++ juju/deploy.go	2014-03-25 14:51:19 +0000
@@ -49,7 +49,8 @@
  	}
  	// TODO(fwereade): transactional State.AddService including settings,  
constraints
  	// (minimumUnitCount, initialMachineIds?).
-	service, err := st.AddService(args.ServiceName, "user-admin", args.Charm)
+	// TODO(gz): pass through real includeNetworks and excludeNetworks vals
+	service, err := st.AddService(args.ServiceName, "user-admin", args.Charm,  
[]string{}, []string{})
  	if err != nil {
  		return nil, err
  	}


Index: state/service_test.go
=== modified file 'state/service_test.go'
--- state/service_test.go	2014-03-18 15:51:17 +0000
+++ state/service_test.go	2014-03-25 14:51:19 +0000
@@ -1433,8 +1433,18 @@
  func (s *ServiceSuite) TestNetworks(c *gc.C) {
  	service, err := s.State.Service(s.mysql.Name())
  	c.Assert(err, gc.IsNil)
-	includedNetworks, excludedNetworks, err := service.Networks()
-	c.Assert(err, gc.IsNil)
-	c.Check(includedNetworks, gc.DeepEquals, []string{})
-	c.Check(excludedNetworks, gc.DeepEquals, []string{})
+	includeNetworks, excludeNetworks, err := service.Networks()
+	c.Assert(err, gc.IsNil)
+	c.Check(includeNetworks, gc.DeepEquals, []string{})
+	c.Check(excludeNetworks, gc.DeepEquals, []string{})
+}
+
+func (s *ServiceSuite) TestNetworksOnService(c *gc.C) {
+	includeNetworks := []string{"yes", "on"}
+	excludeNetworks := []string{"no", "off"}
+	service := s.AddTestingServiceWithNetworks(c, "withnets", s.charm,  
includeNetworks, excludeNetworks)
+	haveIncludeNetworks, haveExcludeNetworks, err := service.Networks()
+	c.Assert(err, gc.IsNil)
+	c.Check(haveIncludeNetworks, gc.DeepEquals, includeNetworks)
+	c.Check(haveExcludeNetworks, gc.DeepEquals, excludeNetworks)
  }


Index: cmd/juju/status_test.go
=== modified file 'cmd/juju/status_test.go'
--- cmd/juju/status_test.go	2014-01-24 14:52:58 +0000
+++ cmd/juju/status_test.go	2014-03-25 14:51:19 +0000
@@ -1626,7 +1626,7 @@
  func (as addService) step(c *gc.C, ctx *context) {
  	ch, ok := ctx.charms[as.charm]
  	c.Assert(ok, gc.Equals, true)
-	_, err := ctx.st.AddService(as.name, "user-admin", ch)
+	_, err := ctx.st.AddService(as.name, "user-admin", ch, []string{},  
[]string{})
  	c.Assert(err, gc.IsNil)
  }



Index: environs/jujutest/livetests.go
=== modified file 'environs/jujutest/livetests.go'
--- environs/jujutest/livetests.go	2014-03-14 02:19:35 +0000
+++ environs/jujutest/livetests.go	2014-03-25 14:51:19 +0000
@@ -452,7 +452,7 @@
  	url := coretesting.Charms.ClonedURL(repoDir,  
mtools0.Version.Series, "dummy")
  	sch, err := conn.PutCharm(url, &charm.LocalRepository{repoDir}, false)
  	c.Assert(err, gc.IsNil)
-	svc, err := conn.State.AddService("dummy", "user-admin", sch)
+	svc, err := conn.State.AddService("dummy", "user-admin", sch, []string{},  
[]string{})
  	c.Assert(err, gc.IsNil)
  	units, err := juju.AddUnits(conn.State, svc, 1, "")
  	c.Assert(err, gc.IsNil)


Index: juju/testing/conn.go
=== modified file 'juju/testing/conn.go'
--- juju/testing/conn.go	2014-03-07 22:45:37 +0000
+++ juju/testing/conn.go	2014-03-25 14:51:19 +0000
@@ -293,8 +293,12 @@
  }

  func (s *JujuConnSuite) AddTestingService(c *gc.C, name string, ch  
*state.Charm) *state.Service {
+	return s.AddTestingServiceWithNetworks(c, name, ch, []string{},  
[]string{})
+}
+
+func (s *JujuConnSuite) AddTestingServiceWithNetworks(c *gc.C, name  
string, ch *state.Charm, includeNetworks, excludeNetworks []string)  
*state.Service {
  	c.Assert(s.State, gc.NotNil)
-	service, err := s.State.AddService(name, "user-admin", ch)
+	service, err := s.State.AddService(name, "user-admin", ch,  
includeNetworks, excludeNetworks)
  	c.Assert(err, gc.IsNil)
  	return service
  }


Index: state/apiserver/charmrevisionupdater/testing/suite.go
=== modified file 'state/apiserver/charmrevisionupdater/testing/suite.go'
--- state/apiserver/charmrevisionupdater/testing/suite.go	2014-01-17  
00:06:37 +0000
+++ state/apiserver/charmrevisionupdater/testing/suite.go	2014-03-25  
14:51:19 +0000
@@ -88,10 +88,10 @@
  }

  // AddService adds a service for the specified charm to state.
-func (s *CharmSuite) AddService(c *gc.C, charmName, serviceName string) {
+func (s *CharmSuite) AddService(c *gc.C, charmName, serviceName string,  
includeNetworks, excludeNetworks []string) {
  	ch, ok := s.charms[charmName]
  	c.Assert(ok, gc.Equals, true)
-	_, err := s.State.AddService(serviceName, "user-admin", ch)
+	_, err := s.State.AddService(serviceName, "user-admin", ch,  
includeNetworks, excludeNetworks)
  	c.Assert(err, gc.IsNil)
  }

@@ -127,12 +127,12 @@

  	// mysql is out of date
  	s.AddCharmWithRevision(c, "mysql", 22)
-	s.AddService(c, "mysql", "mysql")
+	s.AddService(c, "mysql", "mysql", []string{}, []string{})
  	s.AddUnit(c, "mysql", "1")

  	// wordpress is up to date
  	s.AddCharmWithRevision(c, "wordpress", 26)
-	s.AddService(c, "wordpress", "wordpress")
+	s.AddService(c, "wordpress", "wordpress", []string{}, []string{})
  	s.AddUnit(c, "wordpress", "2")
  	s.AddUnit(c, "wordpress", "2")
  	// wordpress/0 has a version, wordpress/1 is unknown
@@ -140,6 +140,6 @@

  	// varnish is a charm that does not have a version in the mock store.
  	s.AddCharmWithRevision(c, "varnish", 5)
-	s.AddService(c, "varnish", "varnish")
+	s.AddService(c, "varnish", "varnish", []string{}, []string{})
  	s.AddUnit(c, "varnish", "3")
  }


Index: state/conn_test.go
=== modified file 'state/conn_test.go'
--- state/conn_test.go	2014-03-17 06:05:54 +0000
+++ state/conn_test.go	2014-03-25 14:51:19 +0000
@@ -79,6 +79,10 @@
  	return state.AddTestingService(c, s.State, name, ch)
  }

+func (s *ConnSuite) AddTestingServiceWithNetworks(c *gc.C, name string, ch  
*state.Charm, includeNetworks, excludeNetworks []string) *state.Service {
+	return state.AddTestingServiceWithNetworks(c, s.State, name, ch,  
includeNetworks, excludeNetworks)
+}
+
  func (s *ConnSuite) AddSeriesCharm(c *gc.C, name, series string)  
*state.Charm {
  	return state.AddCustomCharm(c, s.State, name, "", "", series, -1)
  }


Index: state/export_test.go
=== modified file 'state/export_test.go'
--- state/export_test.go	2014-03-19 23:07:33 +0000
+++ state/export_test.go	2014-03-25 14:51:19 +0000
@@ -142,7 +142,11 @@
  }

  func AddTestingService(c *gc.C, st *State, name string, ch *Charm)  
*Service {
-	service, err := st.AddService(name, "user-admin", ch)
+	return AddTestingServiceWithNetworks(c, st, name, ch, []string{},  
[]string{})
+}
+
+func AddTestingServiceWithNetworks(c *gc.C, st *State, name string, ch  
*Charm, includeNetworks, excludeNetworks []string) *Service {
+	service, err := st.AddService(name, "user-admin", ch, includeNetworks,  
excludeNetworks)
  	c.Assert(err, gc.IsNil)
  	return service
  }


Index: state/apiserver/client/run_test.go
=== modified file 'state/apiserver/client/run_test.go'
--- state/apiserver/client/run_test.go	2014-03-13 07:54:56 +0000
+++ state/apiserver/client/run_test.go	2014-03-25 14:51:19 +0000
@@ -76,16 +76,16 @@

  func (s *runSuite) TestGetAllUnitNames(c *gc.C) {
  	charm := s.AddTestingCharm(c, "dummy")
-	magic, err := s.State.AddService("magic", "user-admin", charm)
+	magic, err := s.State.AddService("magic", "user-admin", charm,  
[]string{}, []string{})
  	s.addUnit(c, magic)
  	s.addUnit(c, magic)

-	notAssigned, err := s.State.AddService("not-assigned", "user-admin",  
charm)
+	notAssigned, err := s.State.AddService("not-assigned", "user-admin",  
charm, []string{}, []string{})
  	c.Assert(err, gc.IsNil)
  	_, err = notAssigned.AddUnit()
  	c.Assert(err, gc.IsNil)

-	_, err = s.State.AddService("no-units", "user-admin", charm)
+	_, err = s.State.AddService("no-units", "user-admin", charm, []string{},  
[]string{})
  	c.Assert(err, gc.IsNil)

  	for i, test := range []struct {
@@ -241,7 +241,7 @@
  	s.addMachineWithAddress(c, "10.3.2.1")

  	charm := s.AddTestingCharm(c, "dummy")
-	magic, err := s.State.AddService("magic", "user-admin", charm)
+	magic, err := s.State.AddService("magic", "user-admin", charm,  
[]string{}, []string{})
  	s.addUnit(c, magic)
  	s.addUnit(c, magic)



Index: state/state.go
=== modified file 'state/state.go'
--- state/state.go	2014-03-24 10:22:17 +0000
+++ state/state.go	2014-03-25 14:51:19 +0000
@@ -907,7 +907,7 @@
  // AddService creates a new service, running the supplied charm, with the
  // supplied name (which must be unique). If the charm defines peer  
relations,
  // they will be created automatically.
-func (st *State) AddService(name, ownerTag string, ch *Charm) (service  
*Service, err error) {
+func (st *State) AddService(name, ownerTag string, ch *Charm,  
includeNetworks, excludeNetworks []string) (service *Service, err error) {
  	defer utils.ErrorContextf(&err, "cannot add service %q", name)
  	kind, ownerId, err := names.ParseTag(ownerTag, names.UserTagKind)
  	if err != nil || kind != names.UserTagKind {
@@ -951,7 +951,7 @@
  	ops := []txn.Op{
  		env.assertAliveOp(),
  		createConstraintsOp(st, svc.globalKey(), constraints.Value{}),
-		createNetworksOp(st, svc.globalKey(), []string{}, []string{}),
+		createNetworksOp(st, svc.globalKey(), includeNetworks, excludeNetworks),
  		createSettingsOp(st, svc.settingsKey(), nil),
  		{
  			C:      st.users.Name,


Index: state/state_test.go
=== modified file 'state/state_test.go'
--- state/state_test.go	2014-03-20 01:23:05 +0000
+++ state/state_test.go	2014-03-25 14:51:19 +0000
@@ -964,19 +964,19 @@

  func (s *StateSuite) TestAddService(c *gc.C) {
  	charm := s.AddTestingCharm(c, "dummy")
-	_, err := s.State.AddService("haha/borken", "user-admin", charm)
+	_, err := s.State.AddService("haha/borken", "user-admin", charm,  
[]string{}, []string{})
  	c.Assert(err, gc.ErrorMatches, `cannot add service "haha/borken": invalid  
name`)
  	_, err = s.State.Service("haha/borken")
  	c.Assert(err, gc.ErrorMatches, `"haha/borken" is not a valid service  
name`)

  	// set that a nil charm is handled correctly
-	_, err = s.State.AddService("umadbro", "user-admin", nil)
+	_, err = s.State.AddService("umadbro", "user-admin", nil, []string{},  
[]string{})
  	c.Assert(err, gc.ErrorMatches, `cannot add service "umadbro": charm is  
nil`)

-	wordpress, err := s.State.AddService("wordpress", "user-admin", charm)
+	wordpress, err := s.State.AddService("wordpress", "user-admin", charm,  
[]string{}, []string{})
  	c.Assert(err, gc.IsNil)
  	c.Assert(wordpress.Name(), gc.Equals, "wordpress")
-	mysql, err := s.State.AddService("mysql", "user-admin", charm)
+	mysql, err := s.State.AddService("mysql", "user-admin", charm,  
[]string{}, []string{})
  	c.Assert(err, gc.IsNil)
  	c.Assert(mysql.Name(), gc.Equals, "mysql")

@@ -1003,7 +1003,7 @@
  	c.Assert(err, gc.IsNil)
  	err = env.Destroy()
  	c.Assert(err, gc.IsNil)
-	_, err = s.State.AddService("s1", "user-admin", charm)
+	_, err = s.State.AddService("s1", "user-admin", charm, []string{},  
[]string{})
  	c.Assert(err, gc.ErrorMatches, `cannot add service "s1": environment is  
no longer alive`)
  }

@@ -1018,7 +1018,7 @@
  		c.Assert(env.Life(), gc.Equals, state.Alive)
  		c.Assert(env.Destroy(), gc.IsNil)
  	}).Check()
-	_, err = s.State.AddService("s1", "user-admin", charm)
+	_, err = s.State.AddService("s1", "user-admin", charm, []string{},  
[]string{})
  	c.Assert(err, gc.ErrorMatches, `cannot add service "s1": environment is  
no longer alive`)
  }

@@ -1030,19 +1030,19 @@

  func (s *StateSuite) TestAddServiceNoTag(c *gc.C) {
  	charm := s.AddTestingCharm(c, "dummy")
-	_, err := s.State.AddService("wordpress", state.AdminUser, charm)
+	_, err := s.State.AddService("wordpress", state.AdminUser, charm,  
[]string{}, []string{})
  	c.Assert(err, gc.ErrorMatches, "cannot add service \"wordpress\": Invalid  
ownertag admin")
  }

  func (s *StateSuite) TestAddServiceNotUserTag(c *gc.C) {
  	charm := s.AddTestingCharm(c, "dummy")
-	_, err := s.State.AddService("wordpress", "machine-3", charm)
+	_, err := s.State.AddService("wordpress", "machine-3", charm, []string{},  
[]string{})
  	c.Assert(err, gc.ErrorMatches, "cannot add service \"wordpress\": Invalid  
ownertag machine-3")
  }

  func (s *StateSuite) TestAddServiceNonExistentUser(c *gc.C) {
  	charm := s.AddTestingCharm(c, "dummy")
-	_, err := s.State.AddService("wordpress", "user-notAuser", charm)
+	_, err := s.State.AddService("wordpress", "user-notAuser", charm,  
[]string{}, []string{})
  	c.Assert(err, gc.ErrorMatches, "cannot add service \"wordpress\": user  
notAuser doesn't exist")
  }

@@ -1053,13 +1053,13 @@
  	c.Assert(len(services), gc.Equals, 0)

  	// Check that after adding services the result is ok.
-	_, err = s.State.AddService("wordpress", "user-admin", charm)
+	_, err = s.State.AddService("wordpress", "user-admin", charm, []string{},  
[]string{})
  	c.Assert(err, gc.IsNil)
  	services, err = s.State.AllServices()
  	c.Assert(err, gc.IsNil)
  	c.Assert(len(services), gc.Equals, 1)

-	_, err = s.State.AddService("mysql", "user-admin", charm)
+	_, err = s.State.AddService("mysql", "user-admin", charm, []string{},  
[]string{})
  	c.Assert(err, gc.IsNil)
  	services, err = s.State.AllServices()
  	c.Assert(err, gc.IsNil)
@@ -2524,7 +2524,7 @@
  	// Add a service and 4 units: one with a different version, one
  	// with an empty version, one with the current version, and one
  	// with the new version.
-	service, err := s.State.AddService("wordpress", "user-admin",  
s.AddTestingCharm(c, "wordpress"))
+	service, err := s.State.AddService("wordpress", "user-admin",  
s.AddTestingCharm(c, "wordpress"), []string{}, []string{})
  	c.Assert(err, gc.IsNil)
  	unit0, err := service.AddUnit()
  	c.Assert(err, gc.IsNil)
@@ -2574,7 +2574,7 @@
  	// Add a machine and a unit with the current version.
  	machine, err := s.State.AddMachine("series", state.JobHostUnits)
  	c.Assert(err, gc.IsNil)
-	service, err := s.State.AddService("wordpress", "user-admin",  
s.AddTestingCharm(c, "wordpress"))
+	service, err := s.State.AddService("wordpress", "user-admin",  
s.AddTestingCharm(c, "wordpress"), []string{}, []string{})
  	c.Assert(err, gc.IsNil)
  	unit, err := service.AddUnit()
  	c.Assert(err, gc.IsNil)