juju-core

Merge lp:~dimitern/juju-core/070-deployer-client-facade into lp:~go-bot/juju-core/trunk

  1. 070-deployer-client-facade
  2. Merge into trunk
Proposed by Dimiter Naydenov
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: 1478
Proposed branch: lp:~dimitern/juju-core/070-deployer-client-facade
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 648 lines (+482/-21)
13 files modified
state/api/deployer/deployer.go (+61/-0)
state/api/deployer/deployer_test.go (+244/-0)
state/api/deployer/machine.go (+40/-0)
state/api/deployer/unit.go (+85/-0)
state/api/machiner/machiner.go (+1/-1)
state/api/state.go (+6/-0)
state/api/upgrader/upgrader_test.go (+1/-1)
state/api/watcher/watcher.go (+12/-16)
state/apiserver/root.go (+11/-0)
state/machine.go (+5/-0)
state/machine_test.go (+2/-0)
state/unit.go (+8/-3)
state/unit_test.go (+6/-0)
To merge this branch: bzr merge lp:~dimitern/juju-core/070-deployer-client-facade
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+174973@code.launchpad.net

Commit message

state/api: Deployer client-side facade.

Implemented all the necessary API calls for the
deployer worker. This is the last step needed
before the deployer can be refactored to use
only the API to talk to state.

Also fixed the StringsWatcher client implementation
(I initially got it wrong, but since nobody was using
it yet it got undetected).

Few other minor issues discovered and fixed in state,
adding tests as needed.

https://codereview.appspot.com/11342043/

R=fwereade, themue

Description of the change

state/api: Deployer client-side facade.

Implemented all the necessary API calls for the
deployer worker. This is the last step needed
before the deployer can be refactored to use
only the API to talk to state.

Also fixed the StringsWatcher client implementation
(I initially got it wrong, but since nobody was using
it yet it got undetected).

Few other minor issues discovered and fixed in state,
adding tests as needed.

https://codereview.appspot.com/11342043/

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

Reviewers: mp+174973_code.launchpad.net,

Message:
Please take a look.

Description:
state/api: Deployer client-side facade.

Implemented all the necessary API calls for the
deployer worker. This is the last step needed
before the deployer can be refactored to use
only the API to talk to state.

Also fixed the StringsWatcher client implementation
(I initially got it wrong, but since nobody was using
it yet it got undetected).

Few other minor issues discovered and fixed in state,
adding tests as needed.

https://code.launchpad.net/~dimitern/juju-core/070-deployer-client-facade/+merge/174973

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   A state/api/deployer/deployer.go
   A state/api/deployer/deployer_test.go
   A state/api/deployer/machine.go
   A state/api/deployer/unit.go
   M state/api/machiner/machiner.go
   M state/api/state.go
   M state/api/upgrader/upgrader_test.go
   M state/api/watcher/watcher.go
   M state/apiserver/root.go
   M state/machine.go
   M state/machine_test.go
   M state/unit.go
   M state/unit_test.go

Revision history for this message
Frank Mueller (themue) wrote :

LGTM, only some comments.

https://codereview.appspot.com/11342043/diff/1/state/api/deployer/deployer_test.go
File state/api/deployer/deployer_test.go (right):

https://codereview.appspot.com/11342043/diff/1/state/api/deployer/deployer_test.go#newcode53
state/api/deployer/deployer_test.go:53: s.stateAPI = s.OpenAPIAs(c,
s.machine.Tag(), "test-password")
Stumbled upon this for the first time. You call it "login" in the
comment. So maybe LoginAsToAPI() would be better. But that's only an
idea.

https://codereview.appspot.com/11342043/diff/1/state/api/watcher/watcher.go
File state/api/watcher/watcher.go (right):

https://codereview.appspot.com/11342043/diff/1/state/api/watcher/watcher.go#newcode179
state/api/watcher/watcher.go:179: stringWatcherId string
It's StringsWatcher and StringsWatchId, so plural strings. Would name
the field here stringsWatcherId too.

https://codereview.appspot.com/11342043/diff/1/state/apiserver/root.go
File state/apiserver/root.go (right):

https://codereview.appspot.com/11342043/diff/1/state/apiserver/root.go#newcode89
state/apiserver/root.go:89: // TODO: There is no direct test for this
// TODO(dimitern) ...

https://codereview.appspot.com/11342043/diff/1/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/11342043/diff/1/state/machine.go#newcode163
state/machine.go:163: if !strings.HasPrefix(tag, machineTagPrefix) {
How does code calling this function react when "" is returned? Maybe
it's better to return an error too.

MachineIdFromTag(tag string) (string, error)

https://codereview.appspot.com/11342043/diff/1/state/unit.go
File state/unit.go (right):

https://codereview.appspot.com/11342043/diff/1/state/unit.go#newcode635
state/unit.go:635: if !strings.HasPrefix(tag, unitTagPrefix) {
Same as said for machine here.

https://codereview.appspot.com/11342043/

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

LGTM. I can see arguments both ways for the *FromTag functions... I
think it's probably good enough that they return things that are never
valid ids/names.

https://codereview.appspot.com/11342043/diff/1/state/api/deployer/deployer_test.go
File state/api/deployer/deployer_test.go (right):

https://codereview.appspot.com/11342043/diff/1/state/api/deployer/deployer_test.go#newcode135
state/api/deployer/deployer_test.go:135: c.Assert(err, gc.IsNil)
Removal shouldn't be necessary to see the change, fwiw.

https://codereview.appspot.com/11342043/diff/1/state/api/watcher/watcher.go
File state/api/watcher/watcher.go (right):

https://codereview.appspot.com/11342043/diff/1/state/api/watcher/watcher.go#newcode179
state/api/watcher/watcher.go:179: stringWatcherId string
On 2013/07/16 12:20:37, mue wrote:
> It's StringsWatcher and StringsWatchId, so plural strings. Would name
 the field
> here stringsWatcherId too.

+1

https://codereview.appspot.com/11342043/

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

Please take a look.

https://codereview.appspot.com/11342043/diff/1/state/api/deployer/deployer_test.go
File state/api/deployer/deployer_test.go (right):

https://codereview.appspot.com/11342043/diff/1/state/api/deployer/deployer_test.go#newcode53
state/api/deployer/deployer_test.go:53: s.stateAPI = s.OpenAPIAs(c,
s.machine.Tag(), "test-password")
On 2013/07/16 12:20:37, mue wrote:
> Stumbled upon this for the first time. You call it "login" in the
 comment. So
> maybe LoginAsToAPI() would be better. But that's only an idea.

The idea AFAIU is that you always need to login when you open a
connection the the API server, hence the name of the function.

https://codereview.appspot.com/11342043/diff/1/state/api/deployer/deployer_test.go#newcode135
state/api/deployer/deployer_test.go:135: c.Assert(err, gc.IsNil)
On 2013/07/16 13:00:06, fwereade wrote:
> Removal shouldn't be necessary to see the change, fwiw.

Good point, will leave only the EnsureDead call.

https://codereview.appspot.com/11342043/diff/1/state/api/watcher/watcher.go
File state/api/watcher/watcher.go (right):

https://codereview.appspot.com/11342043/diff/1/state/api/watcher/watcher.go#newcode179
state/api/watcher/watcher.go:179: stringWatcherId string
On 2013/07/16 12:20:37, mue wrote:
> It's StringsWatcher and StringsWatchId, so plural strings. Would name
 the field
> here stringsWatcherId too.

Done.

https://codereview.appspot.com/11342043/diff/1/state/apiserver/root.go
File state/apiserver/root.go (right):

https://codereview.appspot.com/11342043/diff/1/state/apiserver/root.go#newcode89
state/apiserver/root.go:89: // TODO: There is no direct test for this
On 2013/07/16 12:20:37, mue wrote:
> // TODO(dimitern) ...

Done.

https://codereview.appspot.com/11342043/diff/1/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/11342043/diff/1/state/machine.go#newcode163
state/machine.go:163: if !strings.HasPrefix(tag, machineTagPrefix) {
On 2013/07/16 12:20:37, mue wrote:
> How does code calling this function react when "" is returned? Maybe
 it's better
> to return an error too.

> MachineIdFromTag(tag string) (string, error)

I didn't want to change the signature, but I added a test for that. I'll
add a TODO for your suggestion as a possible future improvement.

https://codereview.appspot.com/11342043/diff/1/state/unit.go
File state/unit.go (right):

https://codereview.appspot.com/11342043/diff/1/state/unit.go#newcode635
state/unit.go:635: if !strings.HasPrefix(tag, unitTagPrefix) {
On 2013/07/16 12:20:37, mue wrote:
> Same as said for machine here.

Same answer :)

https://codereview.appspot.com/11342043/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (17.6 KiB)

The attempt to merge lp:~dimitern/juju-core/070-deployer-client-facade into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 3.692s
ok launchpad.net/juju-core/bzr 11.178s
ok launchpad.net/juju-core/cert 3.268s
ok launchpad.net/juju-core/charm 0.558s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.009s
ok launchpad.net/juju-core/cmd 0.233s
? 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 190.088s
ok launchpad.net/juju-core/cmd/jujud 63.840s
ok launchpad.net/juju-core/constraints 0.014s
ok launchpad.net/juju-core/container/lxc 0.358s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.326s
ok launchpad.net/juju-core/environs 1.801s
? launchpad.net/juju-core/environs/all [no test files]
ok launchpad.net/juju-core/environs/azure 2.950s
ok launchpad.net/juju-core/environs/cloudinit 0.562s
ok launchpad.net/juju-core/environs/config 1.129s
ok launchpad.net/juju-core/environs/dummy 21.849s
ok launchpad.net/juju-core/environs/ec2 175.539s
ok launchpad.net/juju-core/environs/imagemetadata 0.375s
ok launchpad.net/juju-core/environs/instances 0.284s
ok launchpad.net/juju-core/environs/jujutest 0.272s
ok launchpad.net/juju-core/environs/local 1.186s
ok launchpad.net/juju-core/environs/localstorage 0.232s
ok launchpad.net/juju-core/environs/maas 3.337s
ok launchpad.net/juju-core/environs/openstack 9.028s
ok launchpad.net/juju-core/environs/testing 0.005s
ok launchpad.net/juju-core/environs/tools 20.990s
? launchpad.net/juju-core/errors [no test files]
ok launchpad.net/juju-core/instance 0.007s
ok launchpad.net/juju-core/juju 20.328s
? 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.273s
ok launchpad.net/juju-core/rpc/jsoncodec 0.220s
ok launchpad.net/juju-core/schema 0.012s
ok launchpad.net/juju-core/state 182.705s
? 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/deployer 6.581s
ok launchpad.net/juju-core/state/api/machineagent 2.641s
ok launchpad.net/juju-core/state/api/machiner 4.796s
ok launchpad.net/juju-core/state/api/params 0.023s
ok launchpad.net/juju-core/state/api/upgrader 9.312s
ok launchpad.net/juju-core/state/api/watcher 2.702s
ok launchpad.net/juju-core/state/apiserver 2.786s
ok launchpad.net/juju-core/state/apiserver/client 41.817s
ok launchpad.net/juju-core/state/apiserver/common 0.021s
ok launchpad.net/juju-core/state/apiserver/deployer 13.085s
ok launchpad.net/juju-core/state/apiserver/machine 27.977s
? launchpad.net/juju-core/state/apiserver/testing [no test files]
ok launchpad.net/juju-core/state/apiserver/upgrader 14.218s
ok launchpad.net/juju-core/state/multiwatcher 1.134s
ok launchpad.net/juju-core/state/...

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (7.9 KiB)

The attempt to merge lp:~dimitern/juju-core/070-deployer-client-facade into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 3.052s
ok launchpad.net/juju-core/bzr 9.935s
ok launchpad.net/juju-core/cert 3.051s
ok launchpad.net/juju-core/charm 0.587s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.009s
ok launchpad.net/juju-core/cmd 0.219s
? 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 205.614s
ok launchpad.net/juju-core/cmd/jujud 85.595s
ok launchpad.net/juju-core/constraints 0.030s
ok launchpad.net/juju-core/container/lxc 0.762s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.576s
ok launchpad.net/juju-core/environs 8.475s
? launchpad.net/juju-core/environs/all [no test files]
ok launchpad.net/juju-core/environs/azure 4.098s
ok launchpad.net/juju-core/environs/cloudinit 0.711s
ok launchpad.net/juju-core/environs/config 1.981s
ok launchpad.net/juju-core/environs/dummy 27.859s
ok launchpad.net/juju-core/environs/ec2 176.994s
ok launchpad.net/juju-core/environs/imagemetadata 0.294s
ok launchpad.net/juju-core/environs/instances 0.261s
ok launchpad.net/juju-core/environs/jujutest 0.282s
ok launchpad.net/juju-core/environs/local 1.235s
ok launchpad.net/juju-core/environs/localstorage 0.249s
ok launchpad.net/juju-core/environs/maas 3.311s
ok launchpad.net/juju-core/environs/openstack 9.103s
ok launchpad.net/juju-core/environs/testing 0.005s
ok launchpad.net/juju-core/environs/tools 20.188s
? launchpad.net/juju-core/errors [no test files]
ok launchpad.net/juju-core/instance 0.007s
ok launchpad.net/juju-core/juju 15.262s
? 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.301s
ok launchpad.net/juju-core/rpc/jsoncodec 0.235s
ok launchpad.net/juju-core/schema 0.011s
ok launchpad.net/juju-core/state 81.228s
? 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/deployer 4.506s
ok launchpad.net/juju-core/state/api/machineagent 2.037s
ok launchpad.net/juju-core/state/api/machiner 2.393s
ok launchpad.net/juju-core/state/api/params 0.022s
ok launchpad.net/juju-core/state/api/upgrader 3.226s
ok launchpad.net/juju-core/state/api/watcher 1.638s

----------------------------------------------------------------------
FAIL: server_test.go:30: serverSuite.TestStop

[LOG] 55.40365 INFO juju environs/testing: uploading FAKE tools 1.11.3-precise-amd64
[LOG] 55.40375 INFO juju environs: reading tools with major version 1
[LOG] 55.40377 DEBUG juju environs/tools: reading v1.* tools
[LOG] 55.40379 INFO juju environs: falling back to public bucket
[LOG] 55.40382 DEBUG juju environs/tools: reading v1.* tools
[LOG] 55.40386 DEBUG juju environs/tools: found 1.11.3-p...

Read more...

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

FWIW, I don't think you fixed the potential event-skipping in
StringsWatcher. Followup please, though, rather than adding to this.

https://codereview.appspot.com/11342043/

Revision history for this message
Go Bot (go-bot) wrote :

The attempt to merge lp:~dimitern/juju-core/070-deployer-client-facade into lp:juju-core failed. Below is the output from the failed tests.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added directory 'state/api/deployer'
2=== added file 'state/api/deployer/deployer.go'
3--- state/api/deployer/deployer.go 1970-01-01 00:00:00 +0000
4+++ state/api/deployer/deployer.go 2013-07-16 14:21:27 +0000
5@@ -0,0 +1,61 @@
6+// Copyright 2012, 2013 Canonical Ltd.
7+// Licensed under the AGPLv3, see LICENCE file for details.
8+
9+package deployer
10+
11+import (
12+ "fmt"
13+
14+ "launchpad.net/juju-core/state/api/common"
15+ "launchpad.net/juju-core/state/api/params"
16+)
17+
18+// Deployer provides access to the Deployer API facade.
19+type Deployer struct {
20+ caller common.Caller
21+}
22+
23+// New creates a new client-side Deployer facade.
24+func New(caller common.Caller) *Deployer {
25+ return &Deployer{caller}
26+}
27+
28+// unitLife requests the lifecycle of the given unit from the server.
29+func (d *Deployer) unitLife(tag string) (params.Life, error) {
30+ var result params.LifeResults
31+ args := params.Entities{
32+ Entities: []params.Entity{{Tag: tag}},
33+ }
34+ err := d.caller.Call("Deployer", "", "Life", args, &result)
35+ if err != nil {
36+ return "", err
37+ }
38+ if len(result.Results) != 1 {
39+ return "", fmt.Errorf("expected one result, got %d", len(result.Results))
40+ }
41+ if err := result.Results[0].Error; err != nil {
42+ return "", err
43+ }
44+ return result.Results[0].Life, nil
45+}
46+
47+// Deployer provides access to methods of a state.Unit through the facade.
48+func (d *Deployer) Unit(tag string) (*Unit, error) {
49+ life, err := d.unitLife(tag)
50+ if err != nil {
51+ return nil, err
52+ }
53+ return &Unit{
54+ tag: tag,
55+ life: life,
56+ dstate: d,
57+ }, nil
58+}
59+
60+// Machine provides access to methods of a state.Machine through the facade.
61+func (d *Deployer) Machine(tag string) (*Machine, error) {
62+ return &Machine{
63+ tag: tag,
64+ dstate: d,
65+ }, nil
66+}
67
68=== added file 'state/api/deployer/deployer_test.go'
69--- state/api/deployer/deployer_test.go 1970-01-01 00:00:00 +0000
70+++ state/api/deployer/deployer_test.go 2013-07-16 14:21:27 +0000
71@@ -0,0 +1,244 @@
72+// Copyright 2012, 2013 Canonical Ltd.
73+// Licensed under the AGPLv3, see LICENCE file for details.
74+
75+package deployer_test
76+
77+import (
78+ stdtesting "testing"
79+
80+ gc "launchpad.net/gocheck"
81+
82+ "launchpad.net/juju-core/juju/testing"
83+ "launchpad.net/juju-core/state"
84+ "launchpad.net/juju-core/state/api"
85+ "launchpad.net/juju-core/state/api/deployer"
86+ "launchpad.net/juju-core/state/api/params"
87+ statetesting "launchpad.net/juju-core/state/testing"
88+ coretesting "launchpad.net/juju-core/testing"
89+)
90+
91+func TestAll(t *stdtesting.T) {
92+ coretesting.MgoTestPackage(t)
93+}
94+
95+type deployerSuite struct {
96+ testing.JujuConnSuite
97+
98+ stateAPI *api.State
99+
100+ // These are raw State objects. Use them for setup and assertions, but
101+ // should never be touched by the API calls themselves
102+ machine *state.Machine
103+ service0 *state.Service
104+ service1 *state.Service
105+ principal *state.Unit
106+ subordinate *state.Unit
107+
108+ deployer *deployer.Deployer
109+}
110+
111+var _ = gc.Suite(&deployerSuite{})
112+
113+func (s *deployerSuite) SetUpTest(c *gc.C) {
114+ s.JujuConnSuite.SetUpTest(c)
115+
116+ // Create a machine to work with.
117+ var err error
118+ s.machine, err = s.State.AddMachine("series", state.JobHostUnits)
119+ c.Assert(err, gc.IsNil)
120+ err = s.machine.SetPassword("test-password")
121+ c.Assert(err, gc.IsNil)
122+
123+ // Login as the machine agent of the created machine.
124+ s.stateAPI = s.OpenAPIAs(c, s.machine.Tag(), "test-password")
125+ c.Assert(s.stateAPI, gc.NotNil)
126+
127+ // Create the needed services and relate them.
128+ s.service0, err = s.State.AddService("mysql", s.AddTestingCharm(c, "mysql"))
129+ c.Assert(err, gc.IsNil)
130+ s.service1, err = s.State.AddService("logging", s.AddTestingCharm(c, "logging"))
131+ c.Assert(err, gc.IsNil)
132+ eps, err := s.State.InferEndpoints([]string{"mysql", "logging"})
133+ c.Assert(err, gc.IsNil)
134+ rel, err := s.State.AddRelation(eps...)
135+ c.Assert(err, gc.IsNil)
136+
137+ // Create principal and subordinate units and assign them.
138+ s.principal, err = s.service0.AddUnit()
139+ c.Assert(err, gc.IsNil)
140+ err = s.principal.AssignToMachine(s.machine)
141+ c.Assert(err, gc.IsNil)
142+ relUnit, err := rel.Unit(s.principal)
143+ c.Assert(err, gc.IsNil)
144+ err = relUnit.EnterScope(nil)
145+ c.Assert(err, gc.IsNil)
146+ s.subordinate, err = s.service1.Unit("logging/0")
147+ c.Assert(err, gc.IsNil)
148+
149+ // Create the deployer facade.
150+ s.deployer, err = s.stateAPI.Deployer()
151+ c.Assert(err, gc.IsNil)
152+ c.Assert(s.deployer, gc.NotNil)
153+}
154+
155+func (s *deployerSuite) TearDownTest(c *gc.C) {
156+ if s.stateAPI != nil {
157+ err := s.stateAPI.Close()
158+ c.Check(err, gc.IsNil)
159+ }
160+ s.JujuConnSuite.TearDownTest(c)
161+}
162+
163+// Note: This is really meant as a unit-test, this isn't a test that
164+// should need all of the setup we have for this test suite
165+func (s *deployerSuite) TestNew(c *gc.C) {
166+ deployer := deployer.New(s.stateAPI)
167+ c.Assert(deployer, gc.NotNil)
168+}
169+
170+func (s *deployerSuite) assertUnauthorized(c *gc.C, err error) {
171+ c.Assert(err, gc.ErrorMatches, "permission denied")
172+ c.Assert(params.ErrCode(err), gc.Equals, params.CodeUnauthorized)
173+}
174+
175+func (s *deployerSuite) TestWatchUnitsWrongMachine(c *gc.C) {
176+ // Try with a non-existent machine tag.
177+ machine, err := s.deployer.Machine("machine-42")
178+ c.Assert(err, gc.IsNil)
179+ w, err := machine.WatchUnits()
180+ s.assertUnauthorized(c, err)
181+ c.Assert(w, gc.IsNil)
182+
183+ // Try it with an invalid tag format.
184+ machine, err = s.deployer.Machine("foo")
185+ c.Assert(err, gc.IsNil)
186+ w, err = machine.WatchUnits()
187+ s.assertUnauthorized(c, err)
188+ c.Assert(w, gc.IsNil)
189+}
190+
191+func (s *deployerSuite) TestWatchUnits(c *gc.C) {
192+ machine, err := s.deployer.Machine(s.machine.Tag())
193+ c.Assert(err, gc.IsNil)
194+ w, err := machine.WatchUnits()
195+ c.Assert(err, gc.IsNil)
196+ defer statetesting.AssertStop(c, w)
197+ wc := statetesting.NewStringsWatcherC(c, s.BackingState, w)
198+
199+ // Initial event.
200+ wc.AssertOneChange("mysql/0", "logging/0")
201+
202+ // Change something other than the lifecycle and make sure it's
203+ // not detected.
204+ err = s.subordinate.SetPassword("foo")
205+ c.Assert(err, gc.IsNil)
206+ wc.AssertNoChange()
207+
208+ // Make the subordinate dead and check it's detected.
209+ err = s.subordinate.EnsureDead()
210+ c.Assert(err, gc.IsNil)
211+ wc.AssertOneChange("logging/0")
212+
213+ statetesting.AssertStop(c, w)
214+ wc.AssertClosed()
215+}
216+
217+func (s *deployerSuite) TestUnit(c *gc.C) {
218+ // Try getting a missing unit and an invalid tag.
219+ unit, err := s.deployer.Unit("unit-foo-42")
220+ s.assertUnauthorized(c, err)
221+ c.Assert(unit, gc.IsNil)
222+ unit, err = s.deployer.Unit("42")
223+ s.assertUnauthorized(c, err)
224+ c.Assert(unit, gc.IsNil)
225+
226+ // Try getting a unit we're not responsible for.
227+ // First create a new machine and deploy another unit there.
228+ machine, err := s.State.AddMachine("series", state.JobHostUnits)
229+ c.Assert(err, gc.IsNil)
230+ principal1, err := s.service0.AddUnit()
231+ c.Assert(err, gc.IsNil)
232+ err = principal1.AssignToMachine(machine)
233+ c.Assert(err, gc.IsNil)
234+ unit, err = s.deployer.Unit(principal1.Tag())
235+ s.assertUnauthorized(c, err)
236+ c.Assert(unit, gc.IsNil)
237+
238+ // Get the principal and subordinate we're responsible for.
239+ unit, err = s.deployer.Unit(s.principal.Tag())
240+ c.Assert(err, gc.IsNil)
241+ c.Assert(unit.Name(), gc.Equals, "mysql/0")
242+ unit, err = s.deployer.Unit(s.subordinate.Tag())
243+ c.Assert(err, gc.IsNil)
244+ c.Assert(unit.Name(), gc.Equals, "logging/0")
245+}
246+
247+func (s *deployerSuite) TestUnitLifeRefresh(c *gc.C) {
248+ unit, err := s.deployer.Unit(s.subordinate.Tag())
249+ c.Assert(err, gc.IsNil)
250+
251+ c.Assert(unit.Life(), gc.Equals, params.Alive)
252+
253+ // Now make it dead and check again, then refresh and check.
254+ err = s.subordinate.EnsureDead()
255+ c.Assert(err, gc.IsNil)
256+ err = s.subordinate.Refresh()
257+ c.Assert(err, gc.IsNil)
258+ c.Assert(s.subordinate.Life(), gc.Equals, state.Dead)
259+ c.Assert(unit.Life(), gc.Equals, params.Alive)
260+ err = unit.Refresh()
261+ c.Assert(err, gc.IsNil)
262+ c.Assert(unit.Life(), gc.Equals, params.Dead)
263+}
264+
265+func (s *deployerSuite) TestUnitRemove(c *gc.C) {
266+ unit, err := s.deployer.Unit(s.principal.Tag())
267+ c.Assert(err, gc.IsNil)
268+
269+ // It fails because the entity is still alive.
270+ // And EnsureDead will fail because there is a subordinate.
271+ err = unit.Remove()
272+ c.Assert(err, gc.ErrorMatches, `cannot remove entity "unit-mysql-0": still alive`)
273+ c.Assert(params.ErrCode(err), gc.Equals, "")
274+
275+ // With the subordinate it also fails due to it being alive.
276+ unit, err = s.deployer.Unit(s.subordinate.Tag())
277+ c.Assert(err, gc.IsNil)
278+ err = unit.Remove()
279+ c.Assert(err, gc.ErrorMatches, `cannot remove entity "unit-logging-0": still alive`)
280+ c.Assert(params.ErrCode(err), gc.Equals, "")
281+
282+ // Make it dead first and try again.
283+ err = s.subordinate.EnsureDead()
284+ c.Assert(err, gc.IsNil)
285+ err = unit.Remove()
286+ c.Assert(err, gc.IsNil)
287+
288+ // Verify it's gone.
289+ err = unit.Refresh()
290+ s.assertUnauthorized(c, err)
291+ unit, err = s.deployer.Unit(s.subordinate.Tag())
292+ s.assertUnauthorized(c, err)
293+ c.Assert(unit, gc.IsNil)
294+}
295+
296+func (s *deployerSuite) TestUnitSetPassword(c *gc.C) {
297+ unit, err := s.deployer.Unit(s.principal.Tag())
298+ c.Assert(err, gc.IsNil)
299+
300+ // Change the principal's password and verify.
301+ err = unit.SetPassword("foobar")
302+ c.Assert(err, gc.IsNil)
303+ err = s.principal.Refresh()
304+ c.Assert(err, gc.IsNil)
305+ c.Assert(s.principal.PasswordValid("foobar"), gc.Equals, true)
306+
307+ // Then the subordinate.
308+ unit, err = s.deployer.Unit(s.subordinate.Tag())
309+ c.Assert(err, gc.IsNil)
310+ err = unit.SetPassword("phony")
311+ c.Assert(err, gc.IsNil)
312+ err = s.subordinate.Refresh()
313+ c.Assert(err, gc.IsNil)
314+ c.Assert(s.subordinate.PasswordValid("phony"), gc.Equals, true)
315+}
316
317=== added file 'state/api/deployer/machine.go'
318--- state/api/deployer/machine.go 1970-01-01 00:00:00 +0000
319+++ state/api/deployer/machine.go 2013-07-16 14:21:27 +0000
320@@ -0,0 +1,40 @@
321+// Copyright 2012, 2013 Canonical Ltd.
322+// Licensed under the AGPLv3, see LICENCE file for details.
323+
324+package deployer
325+
326+import (
327+ "fmt"
328+
329+ "launchpad.net/juju-core/state/api/params"
330+ "launchpad.net/juju-core/state/api/watcher"
331+)
332+
333+// Machine represents a juju machine as seen by the deployer worker.
334+type Machine struct {
335+ tag string
336+ dstate *Deployer
337+}
338+
339+// WatchUnits starts a StringsWatcher to watch all units deployed to
340+// the machine, in order to track which ones should be deployed or
341+// recalled.
342+func (m *Machine) WatchUnits() (*watcher.StringsWatcher, error) {
343+ var results params.StringsWatchResults
344+ args := params.Entities{
345+ Entities: []params.Entity{{Tag: m.tag}},
346+ }
347+ err := m.dstate.caller.Call("Deployer", "", "WatchUnits", args, &results)
348+ if err != nil {
349+ return nil, err
350+ }
351+ if len(results.Results) != 1 {
352+ return nil, fmt.Errorf("expected one result, got %d", len(results.Results))
353+ }
354+ result := results.Results[0]
355+ if result.Error != nil {
356+ return nil, result.Error
357+ }
358+ w := watcher.NewStringsWatcher(m.dstate.caller, result)
359+ return w, nil
360+}
361
362=== added file 'state/api/deployer/unit.go'
363--- state/api/deployer/unit.go 1970-01-01 00:00:00 +0000
364+++ state/api/deployer/unit.go 2013-07-16 14:21:27 +0000
365@@ -0,0 +1,85 @@
366+// Copyright 2012, 2013 Canonical Ltd.
367+// Licensed under the AGPLv3, see LICENCE file for details.
368+
369+package deployer
370+
371+import (
372+ "strings"
373+
374+ "launchpad.net/juju-core/state/api/params"
375+)
376+
377+// Unit represents a juju unit as seen by the deployer worker.
378+type Unit struct {
379+ tag string
380+ life params.Life
381+ dstate *Deployer
382+}
383+
384+// Tag returns the unit's tag.
385+func (u *Unit) Tag() string {
386+ return u.tag
387+}
388+
389+const unitTagPrefix = "unit-"
390+
391+// Name returns the unit's name.
392+func (u *Unit) Name() string {
393+ if !strings.HasPrefix(u.tag, unitTagPrefix) {
394+ return ""
395+ }
396+ // Strip off the "unit-" prefix.
397+ name := u.tag[len(unitTagPrefix):]
398+ // Put the slashes back.
399+ name = strings.Replace(name, "-", "/", -1)
400+ return name
401+}
402+
403+// Life returns the unit's lifecycle value.
404+func (u *Unit) Life() params.Life {
405+ return u.life
406+}
407+
408+// Refresh updates the cached local copy of the unit's data.
409+func (u *Unit) Refresh() error {
410+ life, err := u.dstate.unitLife(u.tag)
411+ if err != nil {
412+ return err
413+ }
414+ u.life = life
415+ return nil
416+}
417+
418+// Remove removes the unit from state, calling EnsureDead first, then Remove.
419+// It will fail if the unit is not present.
420+func (u *Unit) Remove() error {
421+ var result params.ErrorResults
422+ args := params.Entities{
423+ Entities: []params.Entity{{Tag: u.tag}},
424+ }
425+ err := u.dstate.caller.Call("Deployer", "", "Remove", args, &result)
426+ if err != nil {
427+ return err
428+ }
429+ if len(result.Errors) > 0 && result.Errors[0] != nil {
430+ return result.Errors[0]
431+ }
432+ return nil
433+}
434+
435+func (u *Unit) SetPassword(password string) error {
436+ var result params.ErrorResults
437+ args := params.PasswordChanges{
438+ Changes: []params.PasswordChange{
439+ {Tag: u.tag, Password: password},
440+ },
441+ }
442+ err := u.dstate.caller.Call("Deployer", "", "SetPasswords", args, &result)
443+ if err != nil {
444+ return err
445+ }
446+ if len(result.Errors) > 0 && result.Errors[0] != nil {
447+ return result.Errors[0]
448+ }
449+ return nil
450+}
451
452=== modified file 'state/api/machiner/machiner.go'
453--- state/api/machiner/machiner.go 2013-07-09 13:06:05 +0000
454+++ state/api/machiner/machiner.go 2013-07-16 14:21:27 +0000
455@@ -15,7 +15,7 @@
456 caller common.Caller
457 }
458
459-// Machiner returns a version of the state that provides functionality
460+// NewState returns a version of the state that provides functionality
461 // required by the machiner worker.
462 func NewState(caller common.Caller) *State {
463 return &State{caller}
464
465=== modified file 'state/api/state.go'
466--- state/api/state.go 2013-06-26 12:51:39 +0000
467+++ state/api/state.go 2013-07-16 14:21:27 +0000
468@@ -4,6 +4,7 @@
469 package api
470
471 import (
472+ "launchpad.net/juju-core/state/api/deployer"
473 "launchpad.net/juju-core/state/api/machineagent"
474 "launchpad.net/juju-core/state/api/machiner"
475 "launchpad.net/juju-core/state/api/params"
476@@ -42,3 +43,8 @@
477 func (st *State) Upgrader() (*upgrader.Upgrader, error) {
478 return upgrader.New(st), nil
479 }
480+
481+// Deployer returns access to the Deployer API
482+func (st *State) Deployer() (*deployer.Deployer, error) {
483+ return deployer.New(st), nil
484+}
485
486=== modified file 'state/api/upgrader/upgrader_test.go'
487--- state/api/upgrader/upgrader_test.go 2013-07-10 17:23:31 +0000
488+++ state/api/upgrader/upgrader_test.go 2013-07-16 14:21:27 +0000
489@@ -145,8 +145,8 @@
490
491 func (s *upgraderSuite) TestWatchAPIVersion(c *C) {
492 w, err := s.upgrader.WatchAPIVersion(s.rawMachine.Tag())
493+ c.Assert(err, IsNil)
494 defer statetesting.AssertStop(c, w)
495- c.Assert(err, IsNil)
496 wc := statetesting.NewNotifyWatcherC(c, s.BackingState, w)
497 // Initial event
498 wc.AssertOneChange()
499
500=== modified file 'state/api/watcher/watcher.go'
501--- state/api/watcher/watcher.go 2013-07-10 17:23:31 +0000
502+++ state/api/watcher/watcher.go 2013-07-16 14:21:27 +0000
503@@ -175,34 +175,30 @@
504 // The content of the changes is a list of strings.
505 type StringsWatcher struct {
506 commonWatcher
507- caller common.Caller
508- watchCall string
509- out chan []string
510+ caller common.Caller
511+ stringsWatcherId string
512+ out chan []string
513 }
514
515-func NewStringsWatcher(caller common.Caller, watchCall string) *StringsWatcher {
516+func NewStringsWatcher(caller common.Caller, result params.StringsWatchResult) *StringsWatcher {
517 w := &StringsWatcher{
518- caller: caller,
519- watchCall: watchCall,
520- out: make(chan []string),
521+ caller: caller,
522+ stringsWatcherId: result.StringsWatcherId,
523+ out: make(chan []string),
524 }
525 go func() {
526 defer w.tomb.Done()
527 defer close(w.out)
528- w.tomb.Kill(w.loop())
529+ w.tomb.Kill(w.loop(result.Changes))
530 }()
531 return w
532 }
533
534-func (w *StringsWatcher) loop() error {
535- var result params.StringsWatchResult
536- if err := w.caller.Call("State", "", w.watchCall, nil, &result); err != nil {
537- return err
538- }
539- changes := result.Changes
540+func (w *StringsWatcher) loop(initialChanges []string) error {
541+ changes := initialChanges
542 w.newResult = func() interface{} { return new(params.StringsWatchResult) }
543- w.call = func(request string, newResult interface{}) error {
544- return w.caller.Call("StringsWatcher", result.StringsWatcherId, request, nil, newResult)
545+ w.call = func(request string, result interface{}) error {
546+ return w.caller.Call("StringsWatcher", w.stringsWatcherId, request, nil, &result)
547 }
548 w.commonWatcher.init()
549 go w.commonLoop()
550
551=== modified file 'state/apiserver/root.go'
552--- state/apiserver/root.go 2013-07-11 09:45:36 +0000
553+++ state/apiserver/root.go 2013-07-16 14:21:27 +0000
554@@ -7,6 +7,7 @@
555 "launchpad.net/juju-core/state"
556 "launchpad.net/juju-core/state/apiserver/client"
557 "launchpad.net/juju-core/state/apiserver/common"
558+ "launchpad.net/juju-core/state/apiserver/deployer"
559 "launchpad.net/juju-core/state/apiserver/machine"
560 "launchpad.net/juju-core/state/apiserver/upgrader"
561 "launchpad.net/juju-core/state/multiwatcher"
562@@ -81,6 +82,16 @@
563 return machine.NewAgentAPI(r.srv.state, r)
564 }
565
566+// Deployer returns an object that provides access to the Deployer API facade.
567+// The id argument is reserved for future use and must be empty.
568+func (r *srvRoot) Deployer(id string) (*deployer.DeployerAPI, error) {
569+ if id != "" {
570+ // TODO(dimitern): There is no direct test for this
571+ return nil, common.ErrBadId
572+ }
573+ return deployer.NewDeployerAPI(r.srv.state, r.resources, r)
574+}
575+
576 // Upgrader returns an object that provides access to the Upgrader API facade.
577 // The id argument is reserved for future use and must be empty.
578 func (r *srvRoot) Upgrader(id string) (*upgrader.UpgraderAPI, error) {
579
580=== modified file 'state/machine.go'
581--- state/machine.go 2013-07-09 10:32:23 +0000
582+++ state/machine.go 2013-07-16 14:21:27 +0000
583@@ -160,6 +160,11 @@
584
585 // MachineIdFromTag returns the machine id that was used to create the tag.
586 func MachineIdFromTag(tag string) string {
587+ // TODO(dimitern): Possibly change this to return (string, error),
588+ // so the case below can be reported.
589+ if !strings.HasPrefix(tag, machineTagPrefix) {
590+ return ""
591+ }
592 // Strip off the "machine-" prefix.
593 id := tag[len(machineTagPrefix):]
594 // Put the slashes back.
595
596=== modified file 'state/machine_test.go'
597--- state/machine_test.go 2013-07-09 10:32:23 +0000
598+++ state/machine_test.go 2013-07-16 14:21:27 +0000
599@@ -255,6 +255,8 @@
600 // Check reversability.
601 nested := "2/kvm/0/lxc/3"
602 c.Assert(state.MachineIdFromTag(state.MachineTag(nested)), Equals, nested)
603+ // Try with an invalid tag format.
604+ c.Assert(state.MachineIdFromTag("foo"), Equals, "")
605 }
606
607 func (s *MachineSuite) TestSetMongoPassword(c *C) {
608
609=== modified file 'state/unit.go'
610--- state/unit.go 2013-07-15 23:00:25 +0000
611+++ state/unit.go 2013-07-16 14:21:27 +0000
612@@ -632,11 +632,16 @@
613
614 // UnitNameFromTag returns the unit name that was used to create the tag.
615 func UnitNameFromTag(tag string) string {
616+ // TODO(dimitern): Possibly change this to return (string, error),
617+ // so the case below can be reported.
618+ if !strings.HasPrefix(tag, unitTagPrefix) {
619+ return ""
620+ }
621 // Strip off the "unit-" prefix.
622- id := tag[len(unitTagPrefix):]
623+ name := tag[len(unitTagPrefix):]
624 // Put the slashes back.
625- id = strings.Replace(id, "-", "/", -1)
626- return id
627+ name = strings.Replace(name, "-", "/", -1)
628+ return name
629 }
630
631 // Tag returns a name identifying the unit that is safe to use
632
633=== modified file 'state/unit_test.go'
634--- state/unit_test.go 2013-07-09 10:32:23 +0000
635+++ state/unit_test.go 2013-07-16 14:21:27 +0000
636@@ -43,6 +43,12 @@
637 c.Assert(err, checkers.Satisfies, errors.IsNotFoundError)
638 }
639
640+func (s *UnitSuite) TestUnitNameFromTag(c *C) {
641+ // Try both valid and invalid tag formats.
642+ c.Assert(state.UnitNameFromTag("unit-wordpress-0"), Equals, "wordpress/0")
643+ c.Assert(state.UnitNameFromTag("foo"), Equals, "")
644+}
645+
646 func (s *UnitSuite) TestService(c *C) {
647 svc, err := s.unit.Service()
648 c.Assert(err, IsNil)

Subscribers

People subscribed via source and target branches

to status/vote changes: