juju-core

Merge lp:~dimitern/juju-core/350-api-service-deploy-with-networks into lp:~go-bot/juju-core/trunk

  1. 350-api-service-deploy-with-networks
  2. Merge into trunk
Proposed by Dimiter Naydenov
Status: Merged
Approved by: Dimiter Naydenov
Approved revision: no longer in the source branch.
Merged at revision: 2487
Proposed branch: lp:~dimitern/juju-core/350-api-service-deploy-with-networks
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 427 lines (+183/-49)
9 files modified
juju/conn_test.go (+19/-1)
juju/deploy.go (+21/-5)
state/api/client.go (+19/-2)
state/api/params/params.go (+4/-0)
state/apiserver/client/client.go (+15/-6)
state/apiserver/client/client_test.go (+45/-23)
state/apiserver/client/perm_test.go (+12/-0)
state/assign_test.go (+16/-1)
state/unit.go (+32/-11)
To merge this branch: bzr merge lp:~dimitern/juju-core/350-api-service-deploy-with-networks
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+211773@code.launchpad.net

Commit message

state/api: Add Client.ServiceDeployWithNetworks

This adds a new client API call: ServiceDeployWithNetworks,
which works exactly like ServiceDeploy, but takes two extra
arguments: IncludeNetworks and ExcludeNetworks.
These specify a list of VLANs/networks to enable or disable
at boot time for the machine being deployed, and also record
that into the service document.

AddUnits is also changed (and its tests) to respect networks
set on the service and pass them through MachineTemplate when
deploying or a new machine or container.

https://codereview.appspot.com/76910044/

R=gz, rogpeppe

Description of the change

state/api: Add Client.ServiceDeployWithNetworks

This adds a new client API call: ServiceDeployWithNetworks,
which works exactly like ServiceDeploy, but takes two extra
arguments: IncludeNetworks and ExcludeNetworks.
These specify a list of VLANs/networks to enable or disable
at boot time for the machine being deployed, and also record
that into the service document.

AddUnits is also changed (and its tests) to respect networks
set on the service and pass them through MachineTemplate when
deploying or a new machine or container.

https://codereview.appspot.com/76910044/

To post a comment you must log in.
Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Reviewers: mp+211773_code.launchpad.net,

Message:
Please take a look.

Description:
state/api: Add Client.ServiceDeployWithNetworks

This adds a new client API call: ServiceDeployWithNetworks,
which works exactly like ServiceDeploy, but takes two extra
arguments (required) IncludedNetworks and ExcludedNetworks.
These specify a list of VLANs/networks to enable or disable
at boot time for the machine being deployed, and also record
that into the service document.

Right now, the new API does not change the service and/or
machine documents to add the networks, because there's another
branch mgz is doing for that right now.

https://code.launchpad.net/~dimitern/juju-core/350-api-service-deploy-with-networks/+merge/211773

(do not edit description out of merge proposal)

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

Affected files (+162, -37 lines):
   A [revision details]
   M juju/deploy.go
   M state/api/client.go
   M state/api/params/params.go
   M state/apiserver/client/client.go
   M state/apiserver/client/client_test.go
   M state/apiserver/client/perm_test.go

Revision history for this message
William Reade (fwereade) wrote :

1) it's hard to approve this without an implementation in the background

2) should we be talking network tags?

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go#newcode243
state/apiserver/client/client.go:243: return c.serviceDeploy(&args, nil)
might be nicer to copy args into a ServiceDeployWithNetworks...

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go#newcode256
state/apiserver/client/client.go:256: func (c *Client)
serviceDeploy(args0 *params.ServiceDeploy, args1
*params.ServiceDeployWithNetworks) error {
...and just handle one arg type here?

https://codereview.appspot.com/76910044/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

https://codereview.appspot.com/76910044/diff/20001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/76910044/diff/20001/state/api/params/params.go#newcode155
state/api/params/params.go:155: type ServiceDeployWithNetworks struct {
I suggest that instead of making a new set of parameters, we could just
add the networks parameters to ServiceDeploy, and make
ServiceDeployWithNetworks an alias of Deploy.

We can document the new fields as recognised only in 1.18 (1.19? 1.20?)
API servers.

This means that clients that care that the networks fields are
recognised can use ServiceDeployWithNetworks, but existing clients can
continue to use Deploy, and eventually we should be able to deprecate
ServiceDeployWithNetworks.

This adds minimal clutter to the code, and provides the same
functionality AFAICS.

https://codereview.appspot.com/76910044/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/76910044/diff/20001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/76910044/diff/20001/state/api/params/params.go#newcode155
state/api/params/params.go:155: type ServiceDeployWithNetworks struct {
On 2014/03/20 14:06:23, rog wrote:
> I suggest that instead of making a new set of parameters, we could
 just add the
> networks parameters to ServiceDeploy, and make
 ServiceDeployWithNetworks an
> alias of Deploy.

> We can document the new fields as recognised only in 1.18 (1.19?
1.20?) API
> servers.

> This means that clients that care that the networks fields are
recognised can
> use ServiceDeployWithNetworks, but existing clients can continue to
use Deploy,
> and eventually we should be able to deprecate
ServiceDeployWithNetworks.

> This adds minimal clutter to the code, and provides the same
functionality
> AFAICS.

Done.

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go#newcode243
state/apiserver/client/client.go:243: return c.serviceDeploy(&args, nil)
On 2014/03/20 13:42:50, fwereade wrote:
> might be nicer to copy args into a ServiceDeployWithNetworks...

Done as rog suggested - ServiceDeployWithNetworks as an alias for
ServiceDeploy.

https://codereview.appspot.com/76910044/diff/20001/state/apiserver/client/client.go#newcode256
state/apiserver/client/client.go:256: func (c *Client)
serviceDeploy(args0 *params.ServiceDeploy, args1
*params.ServiceDeployWithNetworks) error {
On 2014/03/20 13:42:50, fwereade wrote:
> ...and just handle one arg type here?

Done differently.

https://codereview.appspot.com/76910044/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

much nicer, thanks.
LGTM with one suggestion below.

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go#newcode298
state/apiserver/client/client.go:298: if len(args.IncludedNetworks) == 0
&& len(args.ExcludedNetworks) == 0 {
I'd remove this and make the two commands exactly the same. Then clients
can use this call for all Deploy purposes if they wish.

https://codereview.appspot.com/76910044/

Revision history for this message
John A Meinel (jameinel) wrote :

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go#newcode298
state/apiserver/client/client.go:298: if len(args.IncludedNetworks) == 0
&& len(args.ExcludedNetworks) == 0 {
On 2014/03/20 15:45:03, rog wrote:
> I'd remove this and make the two commands exactly the same. Then
 clients can use
> this call for all Deploy purposes if they wish.

As William pointed out in IRC, I think the "ideal" would be to have API
versioning and then we'd just have ServiceDeploy that accepts
everything. However, I think lacking that I agree with Roger. I'd rather
have just 1 API that is always called.
(So we effectively have API versioning, just via a clumsy renaming of
the API each time.)

https://codereview.appspot.com/76910044/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go#newcode298
state/apiserver/client/client.go:298: if len(args.IncludedNetworks) == 0
&& len(args.ExcludedNetworks) == 0 {
On 2014/03/20 15:45:03, rog wrote:
> I'd remove this and make the two commands exactly the same. Then
 clients can use
> this call for all Deploy purposes if they wish.

Done.

https://codereview.appspot.com/76910044/diff/40001/state/apiserver/client/client.go#newcode298
state/apiserver/client/client.go:298: if len(args.IncludedNetworks) == 0
&& len(args.ExcludedNetworks) == 0 {
On 2014/03/20 20:38:22, jameinel wrote:
> On 2014/03/20 15:45:03, rog wrote:
> > I'd remove this and make the two commands exactly the same. Then
 clients can
> use
> > this call for all Deploy purposes if they wish.

> As William pointed out in IRC, I think the "ideal" would be to have
API
> versioning and then we'd just have ServiceDeploy that accepts
everything.
> However, I think lacking that I agree with Roger. I'd rather have just
1 API
> that is always called.
> (So we effectively have API versioning, just via a clumsy renaming of
the API
> each time.)

Done as rog suggested. Also, I'm not landing this until mgz's branch
lands. Then I'll integrate the two and drop the related TODOs.

https://codereview.appspot.com/76910044/

Revision history for this message
William Reade (fwereade) wrote :

On 2014/03/20 20:38:21, jameinel wrote:
> As William pointed out in IRC, I think the "ideal" would be to have
 API
> versioning and then we'd just have ServiceDeploy that accepts
 everything.
> However, I think lacking that I agree with Roger. I'd rather have just
 1 API
> that is always called.
> (So we effectively have API versioning, just via a clumsy renaming of
 the API
> each time.)

I can bear it on this occasion, but this is only suitable in the
ultra-short-term: it will now look as though the old version supports
networks, even though it doesn't (especially if we start autogenerating
docs etc).

https://codereview.appspot.com/76910044/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/76910044/

Revision history for this message
Martin Packman (gz) wrote :

LGTM.

https://codereview.appspot.com/76910044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'juju/conn_test.go'
--- juju/conn_test.go 2014-03-25 15:48:47 +0000
+++ juju/conn_test.go 2014-03-26 10:43:53 +0000
@@ -398,15 +398,30 @@
398 c.Assert(sch.Revision(), gc.Equals, rev+1)398 c.Assert(sch.Revision(), gc.Equals, rev+1)
399}399}
400400
401func (s *ConnSuite) assertAssignedMachineNetworks(c *gc.C, unit *state.Unit, expectInclude, expectExclude []string) {
402 machineId, err := unit.AssignedMachineId()
403 c.Assert(err, gc.IsNil)
404 machine, err := s.conn.State.Machine(machineId)
405 c.Assert(err, gc.IsNil)
406 include, exclude, err := machine.Networks()
407 c.Assert(err, gc.IsNil)
408 c.Assert(include, jc.DeepEquals, expectInclude)
409 c.Assert(exclude, jc.DeepEquals, expectExclude)
410}
411
401func (s *ConnSuite) TestAddUnits(c *gc.C) {412func (s *ConnSuite) TestAddUnits(c *gc.C) {
413 withNets := []string{"net1", "net2"}
414 withoutNets := []string{"net3", "net4"}
402 curl := coretesting.Charms.ClonedURL(s.repo.Path, "quantal", "riak")415 curl := coretesting.Charms.ClonedURL(s.repo.Path, "quantal", "riak")
403 sch, err := s.conn.PutCharm(curl, s.repo, false)416 sch, err := s.conn.PutCharm(curl, s.repo, false)
404 c.Assert(err, gc.IsNil)417 c.Assert(err, gc.IsNil)
405 svc, err := s.conn.State.AddService("testriak", "user-admin", sch, nil, nil)418 svc, err := s.conn.State.AddService("testriak", "user-admin", sch, withNets, withoutNets)
406 c.Assert(err, gc.IsNil)419 c.Assert(err, gc.IsNil)
407 units, err := juju.AddUnits(s.conn.State, svc, 2, "")420 units, err := juju.AddUnits(s.conn.State, svc, 2, "")
408 c.Assert(err, gc.IsNil)421 c.Assert(err, gc.IsNil)
409 c.Assert(units, gc.HasLen, 2)422 c.Assert(units, gc.HasLen, 2)
423 s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
424 s.assertAssignedMachineNetworks(c, units[1], withNets, withoutNets)
410425
411 id0, err := units[0].AssignedMachineId()426 id0, err := units[0].AssignedMachineId()
412 c.Assert(err, gc.IsNil)427 c.Assert(err, gc.IsNil)
@@ -419,16 +434,19 @@
419434
420 units, err = juju.AddUnits(s.conn.State, svc, 1, "0")435 units, err = juju.AddUnits(s.conn.State, svc, 1, "0")
421 c.Assert(err, gc.IsNil)436 c.Assert(err, gc.IsNil)
437 s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
422 id2, err := units[0].AssignedMachineId()438 id2, err := units[0].AssignedMachineId()
423 c.Assert(id2, gc.Equals, id0)439 c.Assert(id2, gc.Equals, id0)
424440
425 units, err = juju.AddUnits(s.conn.State, svc, 1, "lxc:0")441 units, err = juju.AddUnits(s.conn.State, svc, 1, "lxc:0")
426 c.Assert(err, gc.IsNil)442 c.Assert(err, gc.IsNil)
443 s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
427 id3, err := units[0].AssignedMachineId()444 id3, err := units[0].AssignedMachineId()
428 c.Assert(id3, gc.Equals, id0+"/lxc/0")445 c.Assert(id3, gc.Equals, id0+"/lxc/0")
429446
430 units, err = juju.AddUnits(s.conn.State, svc, 1, "lxc:"+id3)447 units, err = juju.AddUnits(s.conn.State, svc, 1, "lxc:"+id3)
431 c.Assert(err, gc.IsNil)448 c.Assert(err, gc.IsNil)
449 s.assertAssignedMachineNetworks(c, units[0], withNets, withoutNets)
432 id4, err := units[0].AssignedMachineId()450 id4, err := units[0].AssignedMachineId()
433 c.Assert(id4, gc.Equals, id0+"/lxc/0/lxc/0")451 c.Assert(id4, gc.Equals, id0+"/lxc/0/lxc/0")
434452
435453
=== modified file 'juju/deploy.go'
--- juju/deploy.go 2014-03-25 15:06:31 +0000
+++ juju/deploy.go 2014-03-26 10:43:53 +0000
@@ -28,6 +28,10 @@
28 // - a new container on an existing machine eg "lxc:1"28 // - a new container on an existing machine eg "lxc:1"
29 // Use string to avoid ambiguity around machine 0.29 // Use string to avoid ambiguity around machine 0.
30 ToMachineSpec string30 ToMachineSpec string
31 // IncludeNetworks holds a list of networks to start on boot.
32 IncludeNetworks []string
33 // ExcludeNetworks holds a list of networks to disable on boot.
34 ExcludeNetworks []string
31}35}
3236
33// DeployService takes a charm and various parameters and deploys it.37// DeployService takes a charm and various parameters and deploys it.
@@ -49,8 +53,13 @@
49 }53 }
50 // TODO(fwereade): transactional State.AddService including settings, constraints54 // TODO(fwereade): transactional State.AddService including settings, constraints
51 // (minimumUnitCount, initialMachineIds?).55 // (minimumUnitCount, initialMachineIds?).
52 // TODO(gz): pass through real includeNetworks and excludeNetworks vals56 service, err := st.AddService(
53 service, err := st.AddService(args.ServiceName, "user-admin", args.Charm, nil, nil)57 args.ServiceName,
58 "user-admin",
59 args.Charm,
60 args.IncludeNetworks,
61 args.ExcludeNetworks,
62 )
54 if err != nil {63 if err != nil {
55 return nil, err64 return nil, err
56 }65 }
@@ -81,6 +90,11 @@
81 units := make([]*state.Unit, n)90 units := make([]*state.Unit, n)
82 // Hard code for now till we implement a different approach.91 // Hard code for now till we implement a different approach.
83 policy := state.AssignCleanEmpty92 policy := state.AssignCleanEmpty
93 // All units should have the same networks as the service.
94 includeNetworks, excludeNetworks, err := svc.Networks()
95 if err != nil {
96 return nil, fmt.Errorf("cannot get service %q networks: %v", svc.Name(), err)
97 }
84 // TODO what do we do if we fail half-way through this process?98 // TODO what do we do if we fail half-way through this process?
85 for i := 0; i < n; i++ {99 for i := 0; i < n; i++ {
86 unit, err := svc.AddUnit()100 unit, err := svc.AddUnit()
@@ -116,9 +130,11 @@
116 // Create the new machine marked as dirty so that130 // Create the new machine marked as dirty so that
117 // nothing else will grab it before we assign the unit to it.131 // nothing else will grab it before we assign the unit to it.
118 template := state.MachineTemplate{132 template := state.MachineTemplate{
119 Series: unit.Series(),133 Series: unit.Series(),
120 Jobs: []state.MachineJob{state.JobHostUnits},134 Jobs: []state.MachineJob{state.JobHostUnits},
121 Dirty: true,135 Dirty: true,
136 IncludeNetworks: includeNetworks,
137 ExcludeNetworks: excludeNetworks,
122 }138 }
123 m, err = st.AddMachineInsideMachine(template, mid, containerType)139 m, err = st.AddMachineInsideMachine(template, mid, containerType)
124 } else {140 } else {
125141
=== modified file 'state/api/client.go'
--- state/api/client.go 2014-03-26 07:01:48 +0000
+++ state/api/client.go 2014-03-26 10:43:53 +0000
@@ -256,12 +256,29 @@
256 return c.call("ServiceUnexpose", params, nil)256 return c.call("ServiceUnexpose", params, nil)
257}257}
258258
259// ServiceDeployWithNetworks works exactly like ServiceDeploy, but
260// allows specifying networks to either include or exclude on the
261// machine where the charm is deployed.
262func (c *Client) ServiceDeployWithNetworks(charmURL string, serviceName string, numUnits int, configYAML string, cons constraints.Value, toMachineSpec string, includeNetworks, excludeNetworks []string) error {
263 params := params.ServiceDeploy{
264 ServiceName: serviceName,
265 CharmUrl: charmURL,
266 NumUnits: numUnits,
267 ConfigYAML: configYAML,
268 Constraints: cons,
269 ToMachineSpec: toMachineSpec,
270 IncludeNetworks: includeNetworks,
271 ExcludeNetworks: excludeNetworks,
272 }
273 return c.st.Call("Client", "", "ServiceDeployWithNetworks", params, nil)
274}
275
259// ServiceDeploy obtains the charm, either locally or from the charm store,276// ServiceDeploy obtains the charm, either locally or from the charm store,
260// and deploys it.277// and deploys it.
261func (c *Client) ServiceDeploy(charmUrl string, serviceName string, numUnits int, configYAML string, cons constraints.Value, toMachineSpec string) error {278func (c *Client) ServiceDeploy(charmURL string, serviceName string, numUnits int, configYAML string, cons constraints.Value, toMachineSpec string) error {
262 params := params.ServiceDeploy{279 params := params.ServiceDeploy{
263 ServiceName: serviceName,280 ServiceName: serviceName,
264 CharmUrl: charmUrl,281 CharmUrl: charmURL,
265 NumUnits: numUnits,282 NumUnits: numUnits,
266 ConfigYAML: configYAML,283 ConfigYAML: configYAML,
267 Constraints: cons,284 Constraints: cons,
268285
=== modified file 'state/api/params/params.go'
--- state/api/params/params.go 2014-03-26 07:01:48 +0000
+++ state/api/params/params.go 2014-03-26 10:43:53 +0000
@@ -149,6 +149,10 @@
149 ConfigYAML string // Takes precedence over config if both are present.149 ConfigYAML string // Takes precedence over config if both are present.
150 Constraints constraints.Value150 Constraints constraints.Value
151 ToMachineSpec string151 ToMachineSpec string
152 // The following fields are supported from 1.17.7 onwards and
153 // ignored before that.
154 IncludeNetworks []string
155 ExcludeNetworks []string
152}156}
153157
154// ServiceUpdate holds the parameters for making the ServiceUpdate call.158// ServiceUpdate holds the parameters for making the ServiceUpdate call.
155159
=== modified file 'state/apiserver/client/client.go'
--- state/apiserver/client/client.go 2014-03-26 07:01:48 +0000
+++ state/apiserver/client/client.go 2014-03-26 10:43:53 +0000
@@ -282,16 +282,25 @@
282282
283 _, err = juju.DeployService(c.api.state,283 _, err = juju.DeployService(c.api.state,
284 juju.DeployServiceParams{284 juju.DeployServiceParams{
285 ServiceName: args.ServiceName,285 ServiceName: args.ServiceName,
286 Charm: ch,286 Charm: ch,
287 NumUnits: args.NumUnits,287 NumUnits: args.NumUnits,
288 ConfigSettings: settings,288 ConfigSettings: settings,
289 Constraints: args.Constraints,289 Constraints: args.Constraints,
290 ToMachineSpec: args.ToMachineSpec,290 ToMachineSpec: args.ToMachineSpec,
291 IncludeNetworks: args.IncludeNetworks,
292 ExcludeNetworks: args.ExcludeNetworks,
291 })293 })
292 return err294 return err
293}295}
294296
297// ServiceDeployWithNetworks works exactly like ServiceDeploy, but
298// allows specifying networks to include or exclude on the machine
299// where the charm gets deployed.
300func (c *Client) ServiceDeployWithNetworks(args params.ServiceDeploy) error {
301 return c.ServiceDeploy(args)
302}
303
295// ServiceUpdate updates the service attributes, including charm URL,304// ServiceUpdate updates the service attributes, including charm URL,
296// minimum number of units, settings and constraints.305// minimum number of units, settings and constraints.
297// All parameters in params.ServiceUpdate except the service name are optional.306// All parameters in params.ServiceUpdate except the service name are optional.
298307
=== modified file 'state/apiserver/client/client_test.go'
--- state/apiserver/client/client_test.go 2014-03-26 07:01:48 +0000
+++ state/apiserver/client/client_test.go 2014-03-26 10:43:53 +0000
@@ -680,6 +680,50 @@
680 }680 }
681}681}
682682
683func (s *clientSuite) TestClientServiceDeployWithNetworks(c *gc.C) {
684 store, restore := makeMockCharmStore()
685 defer restore()
686 curl, bundle := addCharm(c, store, "dummy")
687 mem4g := constraints.MustParse("mem=4G")
688 err := s.APIState.Client().ServiceDeployWithNetworks(
689 curl.String(), "service", 3, "", mem4g, "", []string{"net1", "net2"}, []string{"net3"},
690 )
691 c.Assert(err, gc.IsNil)
692 service := s.assertPrincipalDeployed(c, "service", curl, false, bundle, mem4g)
693
694 include, exclude, err := service.Networks()
695 c.Assert(err, gc.IsNil)
696 c.Assert(include, gc.DeepEquals, []string{"net1", "net2"})
697 c.Assert(exclude, gc.DeepEquals, []string{"net3"})
698}
699
700func (s *clientSuite) assertPrincipalDeployed(c *gc.C, serviceName string, curl *charm.URL, forced bool, bundle charm.Charm, cons constraints.Value) *state.Service {
701 service, err := s.State.Service(serviceName)
702 c.Assert(err, gc.IsNil)
703 charm, force, err := service.Charm()
704 c.Assert(err, gc.IsNil)
705 c.Assert(force, gc.Equals, forced)
706 c.Assert(charm.URL(), gc.DeepEquals, curl)
707 c.Assert(charm.Meta(), gc.DeepEquals, bundle.Meta())
708 c.Assert(charm.Config(), gc.DeepEquals, bundle.Config())
709
710 serviceCons, err := service.Constraints()
711 c.Assert(err, gc.IsNil)
712 c.Assert(serviceCons, gc.DeepEquals, cons)
713 units, err := service.AllUnits()
714 c.Assert(err, gc.IsNil)
715 for _, unit := range units {
716 mid, err := unit.AssignedMachineId()
717 c.Assert(err, gc.IsNil)
718 machine, err := s.State.Machine(mid)
719 c.Assert(err, gc.IsNil)
720 machineCons, err := machine.Constraints()
721 c.Assert(err, gc.IsNil)
722 c.Assert(machineCons, gc.DeepEquals, cons)
723 }
724 return service
725}
726
683func (s *clientSuite) TestClientServiceDeployPrincipal(c *gc.C) {727func (s *clientSuite) TestClientServiceDeployPrincipal(c *gc.C) {
684 // TODO(fwereade): test ToMachineSpec directly on srvClient, when we728 // TODO(fwereade): test ToMachineSpec directly on srvClient, when we
685 // manage to extract it as a package and can thus do it conveniently.729 // manage to extract it as a package and can thus do it conveniently.
@@ -691,29 +735,7 @@
691 curl.String(), "service", 3, "", mem4g, "",735 curl.String(), "service", 3, "", mem4g, "",
692 )736 )
693 c.Assert(err, gc.IsNil)737 c.Assert(err, gc.IsNil)
694 service, err := s.State.Service("service")738 s.assertPrincipalDeployed(c, "service", curl, false, bundle, mem4g)
695 c.Assert(err, gc.IsNil)
696 charm, force, err := service.Charm()
697 c.Assert(err, gc.IsNil)
698 c.Assert(force, gc.Equals, false)
699 c.Assert(charm.URL(), gc.DeepEquals, curl)
700 c.Assert(charm.Meta(), gc.DeepEquals, bundle.Meta())
701 c.Assert(charm.Config(), gc.DeepEquals, bundle.Config())
702
703 cons, err := service.Constraints()
704 c.Assert(err, gc.IsNil)
705 c.Assert(cons, gc.DeepEquals, mem4g)
706 units, err := service.AllUnits()
707 c.Assert(err, gc.IsNil)
708 for _, unit := range units {
709 mid, err := unit.AssignedMachineId()
710 c.Assert(err, gc.IsNil)
711 machine, err := s.State.Machine(mid)
712 c.Assert(err, gc.IsNil)
713 cons, err := machine.Constraints()
714 c.Assert(err, gc.IsNil)
715 c.Assert(cons, gc.DeepEquals, mem4g)
716 }
717}739}
718740
719func (s *clientSuite) TestClientServiceDeploySubordinate(c *gc.C) {741func (s *clientSuite) TestClientServiceDeploySubordinate(c *gc.C) {
720742
=== modified file 'state/apiserver/client/perm_test.go'
--- state/apiserver/client/perm_test.go 2014-03-13 07:54:56 +0000
+++ state/apiserver/client/perm_test.go 2014-03-26 10:43:53 +0000
@@ -69,6 +69,10 @@
69 op: opClientServiceDeploy,69 op: opClientServiceDeploy,
70 allow: []string{"user-admin", "user-other"},70 allow: []string{"user-admin", "user-other"},
71}, {71}, {
72 about: "Client.ServiceDeployWithNetworks",
73 op: opClientServiceDeployWithNetworks,
74 allow: []string{"user-admin", "user-other"},
75}, {
72 about: "Client.ServiceUpdate",76 about: "Client.ServiceUpdate",
73 op: opClientServiceUpdate,77 op: opClientServiceUpdate,
74 allow: []string{"user-admin", "user-other"},78 allow: []string{"user-admin", "user-other"},
@@ -321,6 +325,14 @@
321 return func() {}, err325 return func() {}, err
322}326}
323327
328func opClientServiceDeployWithNetworks(c *gc.C, st *api.State, mst *state.State) (func(), error) {
329 err := st.Client().ServiceDeployWithNetworks("mad:bad/url-1", "x", 1, "", constraints.Value{}, "", nil, nil)
330 if err.Error() == `charm URL has invalid schema: "mad:bad/url-1"` {
331 err = nil
332 }
333 return func() {}, err
334}
335
324func opClientServiceUpdate(c *gc.C, st *api.State, mst *state.State) (func(), error) {336func opClientServiceUpdate(c *gc.C, st *api.State, mst *state.State) (func(), error) {
325 args := params.ServiceUpdate{337 args := params.ServiceUpdate{
326 ServiceName: "no-such-charm",338 ServiceName: "no-such-charm",
327339
=== modified file 'state/assign_test.go'
--- state/assign_test.go 2014-03-13 07:54:56 +0000
+++ state/assign_test.go 2014-03-26 10:43:53 +0000
@@ -28,7 +28,13 @@
2828
29func (s *AssignSuite) SetUpTest(c *gc.C) {29func (s *AssignSuite) SetUpTest(c *gc.C) {
30 s.ConnSuite.SetUpTest(c)30 s.ConnSuite.SetUpTest(c)
31 wordpress := s.AddTestingService(c, "wordpress", s.AddTestingCharm(c, "wordpress"))31 wordpress := s.AddTestingServiceWithNetworks(
32 c,
33 "wordpress",
34 s.AddTestingCharm(c, "wordpress"),
35 []string{"net1", "net2"},
36 []string{"net3", "net4"},
37 )
32 s.wordpress = wordpress38 s.wordpress = wordpress
33}39}
3440
@@ -312,12 +318,21 @@
312}318}
313319
314func (s *AssignSuite) assertAssignedUnit(c *gc.C, unit *state.Unit) string {320func (s *AssignSuite) assertAssignedUnit(c *gc.C, unit *state.Unit) string {
321 // Get service networks.
322 service, err := unit.Service()
323 c.Assert(err, gc.IsNil)
324 includeNetworks, excludeNetworks, err := service.Networks()
325 c.Assert(err, gc.IsNil)
315 // Check the machine on the unit is set.326 // Check the machine on the unit is set.
316 machineId, err := unit.AssignedMachineId()327 machineId, err := unit.AssignedMachineId()
317 c.Assert(err, gc.IsNil)328 c.Assert(err, gc.IsNil)
318 // Check that the principal is set on the machine.329 // Check that the principal is set on the machine.
319 machine, err := s.State.Machine(machineId)330 machine, err := s.State.Machine(machineId)
320 c.Assert(err, gc.IsNil)331 c.Assert(err, gc.IsNil)
332 include, exclude, err := machine.Networks()
333 c.Assert(err, gc.IsNil)
334 c.Assert(include, gc.DeepEquals, includeNetworks)
335 c.Assert(exclude, gc.DeepEquals, excludeNetworks)
321 machineUnits, err := machine.Units()336 machineUnits, err := machine.Units()
322 c.Assert(err, gc.IsNil)337 c.Assert(err, gc.IsNil)
323 c.Assert(machineUnits, gc.HasLen, 1)338 c.Assert(machineUnits, gc.HasLen, 1)
324339
=== modified file 'state/unit.go'
--- state/unit.go 2014-03-19 23:07:33 +0000
+++ state/unit.go 2014-03-26 10:43:53 +0000
@@ -991,11 +991,12 @@
991 return &cons, nil991 return &cons, nil
992}992}
993993
994// AssignToNewMachineOrContainer assigns the unit to a new machine, with constraints994// AssignToNewMachineOrContainer assigns the unit to a new machine,
995// determined according to the service and environment constraints at the time of unit creation.995// with constraints determined according to the service and
996// If a container is required, a clean, empty machine instance is required on which to create996// environment constraints at the time of unit creation. If a
997// the container. An existing clean, empty instance is first searched for, and if not found,997// container is required, a clean, empty machine instance is required
998// a new one is created.998// on which to create the container. An existing clean, empty instance
999// is first searched for, and if not found, a new one is created.
999func (u *Unit) AssignToNewMachineOrContainer() (err error) {1000func (u *Unit) AssignToNewMachineOrContainer() (err error) {
1000 defer assignContextf(&err, u, "new machine or container")1001 defer assignContextf(&err, u, "new machine or container")
1001 if u.doc.Principal != "" {1002 if u.doc.Principal != "" {
@@ -1026,10 +1027,20 @@
1026 } else if err != nil {1027 } else if err != nil {
1027 return err1028 return err
1028 }1029 }
1030 svc, err := u.Service()
1031 if err != nil {
1032 return err
1033 }
1034 includeNetworks, excludeNetworks, err := svc.Networks()
1035 if err != nil {
1036 return err
1037 }
1029 template := MachineTemplate{1038 template := MachineTemplate{
1030 Series: u.doc.Series,1039 Series: u.doc.Series,
1031 Constraints: *cons,1040 Constraints: *cons,
1032 Jobs: []MachineJob{JobHostUnits},1041 Jobs: []MachineJob{JobHostUnits},
1042 IncludeNetworks: includeNetworks,
1043 ExcludeNetworks: excludeNetworks,
1033 }1044 }
1034 err = u.assignToNewMachine(template, host.Id, *cons.Container)1045 err = u.assignToNewMachine(template, host.Id, *cons.Container)
1035 if err == machineNotCleanErr {1046 if err == machineNotCleanErr {
@@ -1059,10 +1070,20 @@
1059 if cons.HasContainer() {1070 if cons.HasContainer() {
1060 containerType = *cons.Container1071 containerType = *cons.Container
1061 }1072 }
1073 svc, err := u.Service()
1074 if err != nil {
1075 return err
1076 }
1077 includeNetworks, excludeNetworks, err := svc.Networks()
1078 if err != nil {
1079 return err
1080 }
1062 template := MachineTemplate{1081 template := MachineTemplate{
1063 Series: u.doc.Series,1082 Series: u.doc.Series,
1064 Constraints: *cons,1083 Constraints: *cons,
1065 Jobs: []MachineJob{JobHostUnits},1084 Jobs: []MachineJob{JobHostUnits},
1085 IncludeNetworks: includeNetworks,
1086 ExcludeNetworks: excludeNetworks,
1066 }1087 }
1067 return u.assignToNewMachine(template, "", containerType)1088 return u.assignToNewMachine(template, "", containerType)
1068}1089}

Subscribers

People subscribed via source and target branches

to status/vote changes: