juju-core

Merge lp:~wallyworld/juju-core/machineswithtransienterrors-api into lp:~go-bot/juju-core/trunk

  1. machineswithtransienterrors-api
  2. Merge into trunk
Proposed by Ian Booth
Status: Merged
Approved by: Ian Booth
Approved revision: no longer in the source branch.
Merged at revision: 2483
Proposed branch: lp:~wallyworld/juju-core/machineswithtransienterrors-api
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 707 lines (+361/-34)
17 files modified
state/api/client.go (+14/-0)
state/api/machiner/machine.go (+1/-1)
state/api/params/internal.go (+7/-4)
state/api/provisioner/machine.go (+3/-3)
state/api/provisioner/provisioner.go (+23/-0)
state/api/provisioner/provisioner_test.go (+50/-2)
state/api/uniter/unit.go (+1/-1)
state/apiserver/client/client.go (+18/-4)
state/apiserver/client/client_test.go (+15/-0)
state/apiserver/common/setstatus.go (+50/-0)
state/apiserver/common/setstatus_test.go (+66/-2)
state/apiserver/machine/machiner_test.go (+1/-1)
state/apiserver/provisioner/provisioner.go (+36/-1)
state/apiserver/provisioner/provisioner_test.go (+68/-13)
state/apiserver/uniter/uniter_test.go (+1/-1)
state/interface.go (+6/-0)
worker/provisioner/provisioner_task.go (+1/-1)
To merge this branch: bzr merge lp:~wallyworld/juju-core/machineswithtransienterrors-api
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+212749@code.launchpad.net

Commit message

New APIs for provisioner retries.

Add new APIs to support allowing the provisioner to retry
machines with errors. A machine in error status can be
retried if its status data has transient=true.

1. MachinesWithTransientErrors is called by the provisioner
to find machines which can be retried.
2. ResolveProvisioningError is a client API which allows
the user to manually mark a machine as resolved.

https://codereview.appspot.com/80230043/

Description of the change

New APIs for provisioner retries.

Add new APIs to support allowing the provisioner to retry
machines with errors. A machine in error status can be
retried if its status data has transient=true.

1. MachinesWithTransientErrors is called by the provisioner
to find machines which can be retried.
2. ResolveProvisioningError is a client API which allows
the user to manually mark a machine as resolved.

Existing param structs - SetEntityStatus and SetStatus - were
moved from internal.go to params.go and renamed since they
are now used by the client.

https://codereview.appspot.com/80230043/

To post a comment you must log in.
Revision history for this message
Ian Booth (wallyworld) wrote :

Reviewers: mp+212749_code.launchpad.net,

Message:
Please take a look.

Description:
New APIs for provisioner retries.

Add new APIs to support allowing the provisioner to retry
machines with errors. A machine in error status can be
retried if its status data has transient=true.

1. MachinesWithTransientErrors is called by the provisioner
to find machines which can be retried.
2. ResolveProvisioningError is a client API which allows
the user to manually mark a machine as resolved.

Existing param structs - SetEntityStatus and SetStatus - were
moved from internal.go to params.go and renamed since they
are now used by the client.

https://code.launchpad.net/~wallyworld/juju-core/machineswithtransienterrors-api/+merge/212749

(do not edit description out of merge proposal)

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

Affected files (+379, -43 lines):
   A [revision details]
   M state/api/client.go
   M state/api/machiner/machine.go
   M state/api/params/internal.go
   M state/api/params/params.go
   M state/api/provisioner/machine.go
   M state/api/provisioner/provisioner.go
   M state/api/provisioner/provisioner_test.go
   M state/api/uniter/unit.go
   M state/apiserver/client/client.go
   M state/apiserver/client/client_test.go
   M state/apiserver/common/setstatus.go
   M state/apiserver/common/setstatus_test.go
   M state/apiserver/machine/machiner_test.go
   M state/apiserver/provisioner/provisioner.go
   M state/apiserver/provisioner/provisioner_test.go
   M state/apiserver/uniter/uniter_test.go
   M state/interface.go
   M state/machine_test.go
   M worker/provisioner/provisioner_task.go

Revision history for this message
Andrew Wilkins (axwalk) wrote :

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

https://codereview.appspot.com/80230043/diff/1/state/api/client.go#newcode141
state/api/client.go:141: Entities: []params.EntityStatus{{Tag: machine,
Data: params.StatusData{"transient": true}}},
This is a bit too leaky. The client API should be taking (machine) tags
and setting the transient=true status on the server side, otherwise
we're stuck with this implementation detail forever.

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

https://codereview.appspot.com/80230043/diff/1/state/apiserver/provisioner/provisioner.go#newcode239
state/apiserver/provisioner/provisioner.go:239: canAccess :=
canAccessFunc(machine.Tag())
Should do the access check first to avoid an unnecessary mongo trip.

https://codereview.appspot.com/80230043/diff/1/state/apiserver/provisioner/provisioner.go#newcode243
state/apiserver/provisioner/provisioner.go:243: if err == nil ||
!canAccess {
This is confusing, but I think you just want
     if err == nil && !canAccess {
         err = common.ErrPerm
     }
... right?

https://codereview.appspot.com/80230043/

Revision history for this message
Ian Booth (wallyworld) wrote :

Please take a look.

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

https://codereview.appspot.com/80230043/diff/1/state/api/client.go#newcode141
state/api/client.go:141: Entities: []params.EntityStatus{{Tag: machine,
Data: params.StatusData{"transient": true}}},
On 2014/03/26 02:54:08, axw wrote:
> This is a bit too leaky. The client API should be taking (machine)
 tags and
> setting the transient=true status on the server side, otherwise we're
 stuck with
> this implementation detail forever.

I didn't want to encode the ResolveProvisioningError API as gospel in
case we decided not to go with it long term, and thought a generic
UpdateMachineStatus API would be generically useful. The implementation
detail is behind a client fasçade. But it is leaky I agree in terms of
being stuck supporting this client version. I'll change it.

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

https://codereview.appspot.com/80230043/diff/1/state/apiserver/provisioner/provisioner.go#newcode239
state/apiserver/provisioner/provisioner.go:239: canAccess :=
canAccessFunc(machine.Tag())
On 2014/03/26 02:54:08, axw wrote:
> Should do the access check first to avoid an unnecessary mongo trip.

Done.

https://codereview.appspot.com/80230043/diff/1/state/apiserver/provisioner/provisioner.go#newcode243
state/apiserver/provisioner/provisioner.go:243: if err == nil ||
!canAccess {
On 2014/03/26 02:54:08, axw wrote:
> This is confusing, but I think you just want
> if err == nil && !canAccess {
> err = common.ErrPerm
> }
> ... right?

Changed to due above refactoring

https://codereview.appspot.com/80230043/

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Thanks for the Client change. Sorry, I missed a couple of things the
first time round.

https://codereview.appspot.com/80230043/diff/20001/state/api/provisioner/provisioner_test.go
File state/api/provisioner/provisioner_test.go (right):

https://codereview.appspot.com/80230043/diff/20001/state/api/provisioner/provisioner_test.go#newcode139
state/api/provisioner/provisioner_test.go:139: c.Assert(info[0].Error,
gc.ErrorMatches, "permission denied")
This seems like a weird situation to be in. If there's a machine that
the caller is not allowed to read, I think it should just be excluded.

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

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode244
state/apiserver/provisioner/provisioner.go:244: if canAccess && err ==
nil {
err == nil implies canAccess (otherwise it'd be ErrPerm), so just "if
err == nil"

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode249
state/apiserver/provisioner/provisioner.go:249: if transient, ok :=
result.Data["transient"]; !ok || !transient.(bool) {
There's a potential type assertion panic here, if something sets
transient to something other than a bool. Not sure if we want to be
bothered working around that.

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode254
state/apiserver/provisioner/provisioner.go:254: if err == nil {
It doesn't seem right to me that MachinesWithTransientErrors should be
returning errors for individual machines. Machine that are inaccessible
simply shouldn't show up, I think. The caller isn't asking about them
specifically.

https://codereview.appspot.com/80230043/

Revision history for this message
Ian Booth (wallyworld) wrote :

Please take a look.

https://codereview.appspot.com/80230043/diff/20001/state/api/provisioner/provisioner_test.go
File state/api/provisioner/provisioner_test.go (right):

https://codereview.appspot.com/80230043/diff/20001/state/api/provisioner/provisioner_test.go#newcode139
state/api/provisioner/provisioner_test.go:139: c.Assert(info[0].Error,
gc.ErrorMatches, "permission denied")
On 2014/03/26 04:57:01, axw wrote:
> This seems like a weird situation to be in. If there's a machine that
 the caller
> is not allowed to read, I think it should just be excluded.

Done.

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

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode244
state/apiserver/provisioner/provisioner.go:244: if canAccess && err ==
nil {
On 2014/03/26 04:57:01, axw wrote:
> err == nil implies canAccess (otherwise it'd be ErrPerm), so just "if
 err ==
> nil"

Done.

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode249
state/apiserver/provisioner/provisioner.go:249: if transient, ok :=
result.Data["transient"]; !ok || !transient.(bool) {
On 2014/03/26 04:57:01, axw wrote:
> There's a potential type assertion panic here, if something sets
 transient to
> something other than a bool. Not sure if we want to be bothered
 working around
> that.

I thought about it but forgot. Fixed.

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode254
state/apiserver/provisioner/provisioner.go:254: if err == nil {
On 2014/03/26 04:57:01, axw wrote:
> It doesn't seem right to me that MachinesWithTransientErrors should be
 returning
> errors for individual machines. Machine that are inaccessible simply
 shouldn't
> show up, I think. The caller isn't asking about them specifically.

Hmmmm. I think that's a valid point. I've changed it.

https://codereview.appspot.com/80230043/

Revision history for this message
Andrew Wilkins (axwalk) wrote :

On 2014/03/26 05:17:59, wallyworld wrote:
> Please take a look.

https://codereview.appspot.com/80230043/diff/20001/state/api/provisioner/provisioner_test.go
> File state/api/provisioner/provisioner_test.go (right):

https://codereview.appspot.com/80230043/diff/20001/state/api/provisioner/provisioner_test.go#newcode139
> state/api/provisioner/provisioner_test.go:139: c.Assert(info[0].Error,
> gc.ErrorMatches, "permission denied")
> On 2014/03/26 04:57:01, axw wrote:
> > This seems like a weird situation to be in. If there's a machine
 that the
> caller
> > is not allowed to read, I think it should just be excluded.

> Done.

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go
> File state/apiserver/provisioner/provisioner.go (right):

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode244
> state/apiserver/provisioner/provisioner.go:244: if canAccess && err ==
 nil {
> On 2014/03/26 04:57:01, axw wrote:
> > err == nil implies canAccess (otherwise it'd be ErrPerm), so just
 "if err ==
> > nil"

> Done.

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode249
> state/apiserver/provisioner/provisioner.go:249: if transient, ok :=
> result.Data["transient"]; !ok || !transient.(bool) {
> On 2014/03/26 04:57:01, axw wrote:
> > There's a potential type assertion panic here, if something sets
 transient to
> > something other than a bool. Not sure if we want to be bothered
 working around
> > that.

> I thought about it but forgot. Fixed.

https://codereview.appspot.com/80230043/diff/20001/state/apiserver/provisioner/provisioner.go#newcode254
> state/apiserver/provisioner/provisioner.go:254: if err == nil {
> On 2014/03/26 04:57:01, axw wrote:
> > It doesn't seem right to me that MachinesWithTransientErrors should
 be
> returning
> > errors for individual machines. Machine that are inaccessible simply
 shouldn't
> > show up, I think. The caller isn't asking about them specifically.

> Hmmmm. I think that's a valid point. I've changed it.

Thanks, LGTM

https://codereview.appspot.com/80230043/

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

The attempt to merge lp:~wallyworld/juju-core/machineswithtransienterrors-api into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.013s
ok launchpad.net/juju-core/agent 1.103s
ok launchpad.net/juju-core/agent/mongo 0.536s
ok launchpad.net/juju-core/agent/tools 0.206s
ok launchpad.net/juju-core/bzr 5.054s
ok launchpad.net/juju-core/cert 2.984s
ok launchpad.net/juju-core/charm 0.443s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.033s
ok launchpad.net/juju-core/cloudinit/sshinit 0.811s
ok launchpad.net/juju-core/cmd 0.155s
ok launchpad.net/juju-core/cmd/charm-admin 0.722s
? 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 196.023s
ok launchpad.net/juju-core/cmd/jujud 64.041s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 8.457s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.160s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.021s
ok launchpad.net/juju-core/container 0.044s
ok launchpad.net/juju-core/container/factory 0.050s
ok launchpad.net/juju-core/container/kvm 0.217s
ok launchpad.net/juju-core/container/kvm/mock 0.044s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.290s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.250s
ok launchpad.net/juju-core/environs 2.390s
ok launchpad.net/juju-core/environs/bootstrap 10.380s
ok launchpad.net/juju-core/environs/cloudinit 0.469s
ok launchpad.net/juju-core/environs/config 2.283s
ok launchpad.net/juju-core/environs/configstore 0.033s
ok launchpad.net/juju-core/environs/filestorage 0.027s
ok launchpad.net/juju-core/environs/httpstorage 0.689s
ok launchpad.net/juju-core/environs/imagemetadata 0.431s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.050s
ok launchpad.net/juju-core/environs/jujutest 0.186s
ok launchpad.net/juju-core/environs/manual 12.385s
ok launchpad.net/juju-core/environs/simplestreams 0.267s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 0.869s
ok launchpad.net/juju-core/environs/storage 0.758s
ok launchpad.net/juju-core/environs/sync 42.734s
ok launchpad.net/juju-core/environs/testing 0.130s
ok launchpad.net/juju-core/environs/tools 4.933s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.011s
ok launchpad.net/juju-core/instance 0.017s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 19.305s
ok launchpad.net/juju-core/juj...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'state/api/client.go'
2--- state/api/client.go 2014-03-24 13:27:29 +0000
3+++ state/api/client.go 2014-03-26 05:17:40 +0000
4@@ -138,6 +138,20 @@
5 return c.call("Resolved", p, nil)
6 }
7
8+// ResolveProvisioningError updates the provisioning status of a machine allowing the
9+// provisioner to retry.
10+func (c *Client) ResolveProvisioningError(machine string) error {
11+ p := params.Entities{
12+ Entities: []params.Entity{{Tag: machine}},
13+ }
14+ var results params.ErrorResults
15+ err := c.st.Call("Client", "", "ResolveProvisioningError", p, &results)
16+ if err != nil {
17+ return err
18+ }
19+ return results.OneError()
20+}
21+
22 // PublicAddress returns the public address of the specified
23 // machine or unit.
24 func (c *Client) PublicAddress(target string) (string, error) {
25
26=== modified file 'state/api/machiner/machine.go'
27--- state/api/machiner/machine.go 2014-03-24 13:44:31 +0000
28+++ state/api/machiner/machine.go 2014-03-26 05:17:40 +0000
29@@ -42,7 +42,7 @@
30 func (m *Machine) SetStatus(status params.Status, info string, data params.StatusData) error {
31 var result params.ErrorResults
32 args := params.SetStatus{
33- Entities: []params.SetEntityStatus{
34+ Entities: []params.EntityStatus{
35 {Tag: m.tag, Status: status, Info: info, Data: data},
36 },
37 }
38
39=== modified file 'state/api/params/internal.go'
40--- state/api/params/internal.go 2014-03-21 11:52:30 +0000
41+++ state/api/params/internal.go 2014-03-26 05:17:40 +0000
42@@ -315,25 +315,28 @@
43 Machines []MachineSetProvisioned
44 }
45
46-// SetEntityStatus holds an entity tag, status and extra info.
47-type SetEntityStatus struct {
48+// EntityStatus holds an entity tag, status and extra info.
49+type EntityStatus struct {
50 Tag string
51 Status Status
52 Info string
53 Data StatusData
54 }
55
56-// SetStatus holds the parameters for making a SetStatus call.
57+// SetStatus holds the parameters for making a SetStatus/UpdateStatus call.
58 type SetStatus struct {
59- Entities []SetEntityStatus
60+ Entities []EntityStatus
61 }
62
63 // StatusResult holds an entity status, extra information, or an
64 // error.
65 type StatusResult struct {
66 Error *Error
67+ Id string
68+ Life Life
69 Status Status
70 Info string
71+ Data StatusData
72 }
73
74 // StatusResults holds multiple status results.
75
76=== modified file 'state/api/provisioner/machine.go'
77--- state/api/provisioner/machine.go 2014-03-24 13:27:29 +0000
78+++ state/api/provisioner/machine.go 2014-03-26 05:17:40 +0000
79@@ -56,11 +56,11 @@
80 }
81
82 // SetStatus sets the status of the machine.
83-func (m *Machine) SetStatus(status params.Status, info string) error {
84+func (m *Machine) SetStatus(status params.Status, info string, data params.StatusData) error {
85 var result params.ErrorResults
86 args := params.SetStatus{
87- Entities: []params.SetEntityStatus{
88- {Tag: m.tag, Status: status, Info: info},
89+ Entities: []params.EntityStatus{
90+ {Tag: m.tag, Status: status, Info: info, Data: data},
91 },
92 }
93 err := m.st.call("SetStatus", args, &result)
94
95=== modified file 'state/api/provisioner/provisioner.go'
96--- state/api/provisioner/provisioner.go 2014-03-21 18:40:43 +0000
97+++ state/api/provisioner/provisioner.go 2014-03-26 05:17:40 +0000
98@@ -6,6 +6,7 @@
99 import (
100 "fmt"
101
102+ "launchpad.net/juju-core/names"
103 "launchpad.net/juju-core/state/api/base"
104 "launchpad.net/juju-core/state/api/common"
105 "launchpad.net/juju-core/state/api/params"
106@@ -107,3 +108,25 @@
107 err = st.call("ContainerConfig", nil, &result)
108 return result, err
109 }
110+
111+// MachinesWithTransientErrors returns a slice of machines and corresponding status information
112+// for those machines which have transient provisioning errors.
113+func (st *State) MachinesWithTransientErrors() ([]*Machine, []params.StatusResult, error) {
114+ var results params.StatusResults
115+ err := st.call("MachinesWithTransientErrors", nil, &results)
116+ if err != nil {
117+ return nil, nil, err
118+ }
119+ machines := make([]*Machine, len(results.Results))
120+ for i, status := range results.Results {
121+ if status.Error != nil {
122+ continue
123+ }
124+ machines[i] = &Machine{
125+ tag: names.MachineTag(status.Id),
126+ life: status.Life,
127+ st: st,
128+ }
129+ }
130+ return machines, results.Results, nil
131+}
132
133=== modified file 'state/api/provisioner/provisioner_test.go'
134--- state/api/provisioner/provisioner_test.go 2014-03-21 18:18:08 +0000
135+++ state/api/provisioner/provisioner_test.go 2014-03-26 05:17:40 +0000
136@@ -88,13 +88,61 @@
137 c.Assert(status, gc.Equals, params.StatusPending)
138 c.Assert(info, gc.Equals, "")
139
140- err = apiMachine.SetStatus(params.StatusStarted, "blah")
141+ err = apiMachine.SetStatus(params.StatusStarted, "blah", nil)
142 c.Assert(err, gc.IsNil)
143
144 status, info, err = apiMachine.Status()
145 c.Assert(err, gc.IsNil)
146 c.Assert(status, gc.Equals, params.StatusStarted)
147 c.Assert(info, gc.Equals, "blah")
148+ _, _, data, err := s.machine.Status()
149+ c.Assert(err, gc.IsNil)
150+ c.Assert(data, gc.HasLen, 0)
151+}
152+
153+func (s *provisionerSuite) TestGetSetStatusWithData(c *gc.C) {
154+ apiMachine, err := s.provisioner.Machine(s.machine.Tag())
155+ c.Assert(err, gc.IsNil)
156+
157+ err = apiMachine.SetStatus(params.StatusError, "blah", params.StatusData{"foo": "bar"})
158+ c.Assert(err, gc.IsNil)
159+
160+ status, info, err := apiMachine.Status()
161+ c.Assert(err, gc.IsNil)
162+ c.Assert(status, gc.Equals, params.StatusError)
163+ c.Assert(info, gc.Equals, "blah")
164+ _, _, data, err := s.machine.Status()
165+ c.Assert(err, gc.IsNil)
166+ c.Assert(data, gc.DeepEquals, params.StatusData{"foo": "bar"})
167+}
168+
169+func (s *provisionerSuite) TestMachinesWithTransientErrors(c *gc.C) {
170+ machine, err := s.State.AddMachine("quantal", state.JobHostUnits)
171+ c.Assert(err, gc.IsNil)
172+ err = machine.SetStatus(params.StatusError, "blah", params.StatusData{"transient": true})
173+ c.Assert(err, gc.IsNil)
174+ password, err := utils.RandomPassword()
175+ c.Assert(err, gc.IsNil)
176+ err = machine.SetPassword(password)
177+ c.Assert(err, gc.IsNil)
178+ err = machine.SetProvisioned("i-manager", "fake_nonce", nil)
179+ c.Assert(err, gc.IsNil)
180+ st := s.OpenAPIAsMachine(c, machine.Tag(), password, "fake_nonce")
181+ c.Assert(s.st, gc.NotNil)
182+ p := st.Provisioner()
183+
184+ machines, info, err := p.MachinesWithTransientErrors()
185+ c.Assert(err, gc.IsNil)
186+ c.Assert(machines, gc.HasLen, 1)
187+ c.Assert(machines[0].Id(), gc.Equals, "1")
188+ c.Assert(info, gc.HasLen, 1)
189+ c.Assert(info[0], gc.DeepEquals, params.StatusResult{
190+ Id: "1",
191+ Life: "alive",
192+ Status: "error",
193+ Info: "blah",
194+ Data: params.StatusData{"transient": true},
195+ })
196 }
197
198 func (s *provisionerSuite) TestEnsureDeadAndRemove(c *gc.C) {
199@@ -254,7 +302,7 @@
200
201 // Change something other than the containers and make sure it's
202 // not detected.
203- err = apiMachine.SetStatus(params.StatusStarted, "not really")
204+ err = apiMachine.SetStatus(params.StatusStarted, "not really", nil)
205 c.Assert(err, gc.IsNil)
206 wc.AssertNoChange()
207
208
209=== modified file 'state/api/uniter/unit.go'
210--- state/api/uniter/unit.go 2014-03-21 18:40:43 +0000
211+++ state/api/uniter/unit.go 2014-03-26 05:17:40 +0000
212@@ -58,7 +58,7 @@
213 func (u *Unit) SetStatus(status params.Status, info string, data params.StatusData) error {
214 var result params.ErrorResults
215 args := params.SetStatus{
216- Entities: []params.SetEntityStatus{
217+ Entities: []params.EntityStatus{
218 {Tag: u.tag, Status: status, Info: info, Data: data},
219 },
220 }
221
222=== modified file 'state/apiserver/client/client.go'
223--- state/apiserver/client/client.go 2014-03-20 22:35:39 +0000
224+++ state/apiserver/client/client.go 2014-03-26 05:17:40 +0000
225@@ -38,6 +38,8 @@
226 resources *common.Resources
227 client *Client
228 dataDir string
229+ // statusSetter provides common methods for updating an entity's provisioning status.
230+ statusSetter *common.StatusSetter
231 }
232
233 // Client serves client-specific API methods.
234@@ -48,10 +50,11 @@
235 // NewAPI creates a new instance of the Client API.
236 func NewAPI(st *state.State, resources *common.Resources, authorizer common.Authorizer, datadir string) *API {
237 r := &API{
238- state: st,
239- auth: authorizer,
240- resources: resources,
241- dataDir: datadir,
242+ state: st,
243+ auth: authorizer,
244+ resources: resources,
245+ dataDir: datadir,
246+ statusSetter: common.NewStatusSetter(st, common.AuthAlways(true)),
247 }
248 r.client = &Client{
249 api: r,
250@@ -947,3 +950,14 @@
251 }
252 return charm.Quote(fmt.Sprintf("%s-%d-%s", name, revision, uuid)), nil
253 }
254+
255+// ResolveProvisioningError marks a provisioning error as transient on the machines.
256+func (c *Client) ResolveProvisioningError(p params.Entities) (params.ErrorResults, error) {
257+ entityStatus := make([]params.EntityStatus, len(p.Entities))
258+ for i, entity := range p.Entities {
259+ entityStatus[i] = params.EntityStatus{Tag: entity.Tag, Data: params.StatusData{"transient": true}}
260+ }
261+ return c.api.statusSetter.UpdateStatus(params.SetStatus{
262+ Entities: entityStatus,
263+ })
264+}
265
266=== modified file 'state/apiserver/client/client_test.go'
267--- state/apiserver/client/client_test.go 2014-03-24 20:08:45 +0000
268+++ state/apiserver/client/client_test.go 2014-03-26 05:17:40 +0000
269@@ -2004,3 +2004,18 @@
270 func getArchiveName(bundleURL *url.URL) string {
271 return strings.TrimPrefix(bundleURL.RequestURI(), "/dummyenv/private/")
272 }
273+
274+func (s *clientSuite) TestResolveProvisioningError(c *gc.C) {
275+ machine, err := s.State.AddMachine("quantal", state.JobHostUnits)
276+ c.Assert(err, gc.IsNil)
277+ err = machine.SetStatus(params.StatusError, "error", nil)
278+ c.Assert(err, gc.IsNil)
279+ err = s.APIState.Client().ResolveProvisioningError(machine.Tag())
280+ c.Assert(err, gc.IsNil)
281+
282+ status, info, data, err := machine.Status()
283+ c.Assert(err, gc.IsNil)
284+ c.Assert(status, gc.Equals, params.StatusError)
285+ c.Assert(info, gc.Equals, "error")
286+ c.Assert(data["transient"], gc.Equals, true)
287+}
288
289=== modified file 'state/apiserver/common/setstatus.go'
290--- state/apiserver/common/setstatus.go 2014-01-20 21:00:43 +0000
291+++ state/apiserver/common/setstatus.go 2014-03-26 05:17:40 +0000
292@@ -58,3 +58,53 @@
293 }
294 return result, nil
295 }
296+
297+func (s *StatusSetter) updateEntityStatusData(tag string, data params.StatusData) error {
298+ entity0, err := s.st.FindEntity(tag)
299+ if err != nil {
300+ return err
301+ }
302+ statusGetter, ok := entity0.(state.StatusGetter)
303+ if !ok {
304+ return NotSupportedError(tag, "getting status")
305+ }
306+ existingStatus, existingInfo, existingData, err := statusGetter.Status()
307+ if err != nil {
308+ return err
309+ }
310+ newData := existingData
311+ if newData == nil {
312+ newData = data
313+ } else {
314+ for k, v := range data {
315+ newData[k] = v
316+ }
317+ }
318+ entity, ok := entity0.(state.StatusSetter)
319+ if !ok {
320+ return NotSupportedError(tag, "updating status")
321+ }
322+ return entity.SetStatus(existingStatus, existingInfo, newData)
323+}
324+
325+// UpdateStatus updates the status data of each given entity.
326+func (s *StatusSetter) UpdateStatus(args params.SetStatus) (params.ErrorResults, error) {
327+ result := params.ErrorResults{
328+ Results: make([]params.ErrorResult, len(args.Entities)),
329+ }
330+ if len(args.Entities) == 0 {
331+ return result, nil
332+ }
333+ canModify, err := s.getCanModify()
334+ if err != nil {
335+ return params.ErrorResults{}, err
336+ }
337+ for i, arg := range args.Entities {
338+ err := ErrPerm
339+ if canModify(arg.Tag) {
340+ err = s.updateEntityStatusData(arg.Tag, arg.Data)
341+ }
342+ result.Results[i].Error = ServerError(err)
343+ }
344+ return result, nil
345+}
346
347=== modified file 'state/apiserver/common/setstatus_test.go'
348--- state/apiserver/common/setstatus_test.go 2014-01-20 21:29:08 +0000
349+++ state/apiserver/common/setstatus_test.go 2014-03-26 05:17:40 +0000
350@@ -34,6 +34,17 @@
351 return s.err
352 }
353
354+func (s *fakeStatusSetter) Status() (status params.Status, info string, data params.StatusData, err error) {
355+ return s.status, s.info, s.data, nil
356+}
357+
358+func (s *fakeStatusSetter) UpdateStatus(data params.StatusData) error {
359+ for k, v := range data {
360+ s.data[k] = v
361+ }
362+ return s.err
363+}
364+
365 func (*statusSetterSuite) TestSetStatus(c *gc.C) {
366 st := &fakeState{
367 entities: map[string]entityWithError{
368@@ -55,7 +66,7 @@
369 }
370 s := common.NewStatusSetter(st, getCanModify)
371 args := params.SetStatus{
372- Entities: []params.SetEntityStatus{
373+ Entities: []params.EntityStatus{
374 {"x0", params.StatusStarted, "bar", nil},
375 {"x1", params.StatusStopped, "", nil},
376 {"x2", params.StatusPending, "not really", nil},
377@@ -91,7 +102,7 @@
378 }
379 s := common.NewStatusSetter(&fakeState{}, getCanModify)
380 args := params.SetStatus{
381- Entities: []params.SetEntityStatus{{"x0", "", "", nil}},
382+ Entities: []params.EntityStatus{{"x0", "", "", nil}},
383 }
384 _, err := s.SetStatus(args)
385 c.Assert(err, gc.ErrorMatches, "pow")
386@@ -106,3 +117,56 @@
387 c.Assert(err, gc.IsNil)
388 c.Assert(result.Results, gc.HasLen, 0)
389 }
390+
391+func (*statusSetterSuite) TestUpdateStatus(c *gc.C) {
392+ st := &fakeState{
393+ entities: map[string]entityWithError{
394+ "x0": &fakeStatusSetter{status: params.StatusPending, info: "blah", err: fmt.Errorf("x0 fails")},
395+ "x1": &fakeStatusSetter{status: params.StatusStarted, info: "foo", data: params.StatusData{"foo": "blah"}},
396+ "x2": &fakeStatusSetter{status: params.StatusError, info: "some info"},
397+ "x3": &fakeStatusSetter{fetchError: "x3 error"},
398+ "x4": &fakeStatusSetter{status: params.StatusStopped, info: ""},
399+ },
400+ }
401+ getCanModify := func() (common.AuthFunc, error) {
402+ return func(tag string) bool {
403+ switch tag {
404+ case "x0", "x1", "x2", "x3":
405+ return true
406+ }
407+ return false
408+ }, nil
409+ }
410+ s := common.NewStatusSetter(st, getCanModify)
411+ args := params.SetStatus{
412+ Entities: []params.EntityStatus{
413+ {Tag: "x0", Data: nil},
414+ {Tag: "x1", Data: nil},
415+ {Tag: "x2", Data: params.StatusData{"foo": "bar"}},
416+ {Tag: "x3", Data: params.StatusData{"foo": "bar"}},
417+ {Tag: "x4", Data: params.StatusData{"foo": "bar"}},
418+ {Tag: "x5", Data: nil},
419+ },
420+ }
421+ result, err := s.UpdateStatus(args)
422+ c.Assert(err, gc.IsNil)
423+ c.Assert(result, gc.DeepEquals, params.ErrorResults{
424+ Results: []params.ErrorResult{
425+ {&params.Error{Message: "x0 fails"}},
426+ {nil},
427+ {nil},
428+ {&params.Error{Message: "x3 error"}},
429+ {apiservertesting.ErrUnauthorized},
430+ {apiservertesting.ErrUnauthorized},
431+ },
432+ })
433+ get := func(tag string) *fakeStatusSetter {
434+ return st.entities[tag].(*fakeStatusSetter)
435+ }
436+ c.Assert(get("x1").status, gc.Equals, params.StatusStarted)
437+ c.Assert(get("x1").info, gc.Equals, "foo")
438+ c.Assert(get("x1").data, gc.DeepEquals, params.StatusData{"foo": "blah"})
439+ c.Assert(get("x2").status, gc.Equals, params.StatusError)
440+ c.Assert(get("x2").info, gc.Equals, "some info")
441+ c.Assert(get("x2").data, gc.DeepEquals, params.StatusData{"foo": "bar"})
442+}
443
444=== modified file 'state/apiserver/machine/machiner_test.go'
445--- state/apiserver/machine/machiner_test.go 2014-01-20 21:00:43 +0000
446+++ state/apiserver/machine/machiner_test.go 2014-03-26 05:17:40 +0000
447@@ -57,7 +57,7 @@
448 c.Assert(err, gc.IsNil)
449
450 args := params.SetStatus{
451- Entities: []params.SetEntityStatus{
452+ Entities: []params.EntityStatus{
453 {Tag: "machine-1", Status: params.StatusError, Info: "not really"},
454 {Tag: "machine-0", Status: params.StatusStopped, Info: "foobar"},
455 {Tag: "machine-42", Status: params.StatusStarted, Info: "blah"},
456
457=== modified file 'state/apiserver/provisioner/provisioner.go'
458--- state/apiserver/provisioner/provisioner.go 2014-03-20 13:57:50 +0000
459+++ state/apiserver/provisioner/provisioner.go 2014-03-26 05:17:40 +0000
460@@ -214,13 +214,48 @@
461 machine, err := p.getMachine(canAccess, entity.Tag)
462 if err == nil {
463 r := &result.Results[i]
464- r.Status, r.Info, _, err = machine.Status()
465+ r.Status, r.Info, r.Data, err = machine.Status()
466 }
467 result.Results[i].Error = common.ServerError(err)
468 }
469 return result, nil
470 }
471
472+// MachinesWithTransientErrors returns status data for machines with provisioning
473+// errors which are transient.
474+func (p *ProvisionerAPI) MachinesWithTransientErrors() (params.StatusResults, error) {
475+ results := params.StatusResults{}
476+ canAccessFunc, err := p.getAuthFunc()
477+ if err != nil {
478+ return results, err
479+ }
480+ // TODO (wallyworld) - add state.State API for more efficient machines query
481+ machines, err := p.st.AllMachines()
482+ if err != nil {
483+ return results, err
484+ }
485+ for _, machine := range machines {
486+ if !canAccessFunc(machine.Tag()) {
487+ continue
488+ }
489+ result := params.StatusResult{}
490+ if result.Status, result.Info, result.Data, err = machine.Status(); err != nil {
491+ continue
492+ }
493+ if result.Status != params.StatusError {
494+ continue
495+ }
496+ // Transient errors are marked as such in the status data.
497+ if transient, ok := result.Data["transient"].(bool); !ok || !transient {
498+ continue
499+ }
500+ result.Id = machine.Id()
501+ result.Life = params.Life(machine.Life().String())
502+ results.Results = append(results.Results, result)
503+ }
504+ return results, nil
505+}
506+
507 // Series returns the deployed series for each given machine entity.
508 func (p *ProvisionerAPI) Series(args params.Entities) (params.StringResults, error) {
509 result := params.StringResults{
510
511=== modified file 'state/apiserver/provisioner/provisioner_test.go'
512--- state/apiserver/provisioner/provisioner_test.go 2014-03-18 02:36:58 +0000
513+++ state/apiserver/provisioner/provisioner_test.go 2014-03-26 05:17:40 +0000
514@@ -57,7 +57,7 @@
515 if withStateServer {
516 s.machines = append(s.machines, testing.AddStateServerMachine(c, s.State))
517 }
518- for i := 0; i < 3; i++ {
519+ for i := 0; i < 4; i++ {
520 machine, err := s.State.AddMachine("quantal", state.JobHostUnits)
521 c.Check(err, gc.IsNil)
522 s.machines = append(s.machines, machine)
523@@ -119,6 +119,7 @@
524 {Tag: s.machines[0].Tag(), Password: "xxx0-1234567890123457890"},
525 {Tag: s.machines[1].Tag(), Password: "xxx1-1234567890123457890"},
526 {Tag: s.machines[2].Tag(), Password: "xxx2-1234567890123457890"},
527+ {Tag: s.machines[3].Tag(), Password: "xxx3-1234567890123457890"},
528 {Tag: "machine-42", Password: "foo"},
529 {Tag: "unit-foo-0", Password: "zzz"},
530 {Tag: "service-bar", Password: "abc"},
531@@ -131,6 +132,7 @@
532 {nil},
533 {nil},
534 {nil},
535+ {nil},
536 {apiservertesting.NotFoundError("machine 42")},
537 {apiservertesting.ErrUnauthorized},
538 {apiservertesting.ErrUnauthorized},
539@@ -317,8 +319,9 @@
540 c.Assert(err, gc.IsNil)
541
542 args := params.SetStatus{
543- Entities: []params.SetEntityStatus{
544- {Tag: s.machines[0].Tag(), Status: params.StatusError, Info: "not really"},
545+ Entities: []params.EntityStatus{
546+ {Tag: s.machines[0].Tag(), Status: params.StatusError, Info: "not really",
547+ Data: params.StatusData{"foo": "bar"}},
548 {Tag: s.machines[1].Tag(), Status: params.StatusStopped, Info: "foobar"},
549 {Tag: s.machines[2].Tag(), Status: params.StatusStarted, Info: "again"},
550 {Tag: "machine-42", Status: params.StatusStarted, Info: "blah"},
551@@ -339,9 +342,58 @@
552 })
553
554 // Verify the changes.
555- s.assertStatus(c, 0, params.StatusError, "not really")
556- s.assertStatus(c, 1, params.StatusStopped, "foobar")
557- s.assertStatus(c, 2, params.StatusStarted, "again")
558+ s.assertStatus(c, 0, params.StatusError, "not really", params.StatusData{"foo": "bar"})
559+ s.assertStatus(c, 1, params.StatusStopped, "foobar", params.StatusData{})
560+ s.assertStatus(c, 2, params.StatusStarted, "again", params.StatusData{})
561+}
562+
563+func (s *withoutStateServerSuite) TestMachinesWithTransientErrors(c *gc.C) {
564+ err := s.machines[0].SetStatus(params.StatusStarted, "blah", nil)
565+ c.Assert(err, gc.IsNil)
566+ err = s.machines[1].SetStatus(params.StatusError, "transient error",
567+ params.StatusData{"transient": true, "foo": "bar"})
568+ c.Assert(err, gc.IsNil)
569+ err = s.machines[2].SetStatus(params.StatusError, "error", params.StatusData{"transient": false})
570+ c.Assert(err, gc.IsNil)
571+ err = s.machines[3].SetStatus(params.StatusError, "error", nil)
572+ c.Assert(err, gc.IsNil)
573+
574+ result, err := s.provisioner.MachinesWithTransientErrors()
575+ c.Assert(err, gc.IsNil)
576+ c.Assert(result, gc.DeepEquals, params.StatusResults{
577+ Results: []params.StatusResult{
578+ {Id: "1", Life: "alive", Status: "error", Info: "transient error",
579+ Data: params.StatusData{"transient": true, "foo": "bar"}},
580+ },
581+ })
582+}
583+
584+func (s *withoutStateServerSuite) TestMachinesWithTransientErrorsPermission(c *gc.C) {
585+ // Machines where there's permission issues are omitted.
586+ anAuthorizer := s.authorizer
587+ anAuthorizer.MachineAgent = true
588+ anAuthorizer.EnvironManager = false
589+ anAuthorizer.Tag = "machine-1"
590+ aProvisioner, err := provisioner.NewProvisionerAPI(s.State, s.resources,
591+ anAuthorizer)
592+ err = s.machines[0].SetStatus(params.StatusStarted, "blah", nil)
593+ c.Assert(err, gc.IsNil)
594+ err = s.machines[1].SetStatus(params.StatusError, "transient error",
595+ params.StatusData{"transient": true, "foo": "bar"})
596+ c.Assert(err, gc.IsNil)
597+ err = s.machines[2].SetStatus(params.StatusError, "error", params.StatusData{"transient": false})
598+ c.Assert(err, gc.IsNil)
599+ err = s.machines[3].SetStatus(params.StatusError, "error", nil)
600+ c.Assert(err, gc.IsNil)
601+
602+ result, err := aProvisioner.MachinesWithTransientErrors()
603+ c.Assert(err, gc.IsNil)
604+ c.Assert(result, gc.DeepEquals, params.StatusResults{
605+ Results: []params.StatusResult{
606+ {Id: "1", Life: "alive", Status: "error", Info: "transient error",
607+ Data: params.StatusData{"transient": true, "foo": "bar"}},
608+ },
609+ })
610 }
611
612 func (s *withoutStateServerSuite) TestEnsureDead(c *gc.C) {
613@@ -384,11 +436,14 @@
614 c.Assert(s.machines[index].Life(), gc.Equals, expectLife)
615 }
616
617-func (s *withoutStateServerSuite) assertStatus(c *gc.C, index int, expectStatus params.Status, expectInfo string) {
618- status, info, _, err := s.machines[index].Status()
619+func (s *withoutStateServerSuite) assertStatus(c *gc.C, index int, expectStatus params.Status, expectInfo string,
620+ expectData params.StatusData) {
621+
622+ status, info, data, err := s.machines[index].Status()
623 c.Assert(err, gc.IsNil)
624 c.Assert(status, gc.Equals, expectStatus)
625 c.Assert(info, gc.Equals, expectInfo)
626+ c.Assert(data, gc.DeepEquals, expectData)
627 }
628
629 func (s *withoutStateServerSuite) TestWatchContainers(c *gc.C) {
630@@ -482,7 +537,7 @@
631 c.Assert(err, gc.IsNil)
632 err = s.machines[1].SetStatus(params.StatusStopped, "foo", nil)
633 c.Assert(err, gc.IsNil)
634- err = s.machines[2].SetStatus(params.StatusError, "not really", nil)
635+ err = s.machines[2].SetStatus(params.StatusError, "not really", params.StatusData{"foo": "bar"})
636 c.Assert(err, gc.IsNil)
637
638 args := params.Entities{Entities: []params.Entity{
639@@ -497,9 +552,9 @@
640 c.Assert(err, gc.IsNil)
641 c.Assert(result, gc.DeepEquals, params.StatusResults{
642 Results: []params.StatusResult{
643- {Status: params.StatusStarted, Info: "blah"},
644- {Status: params.StatusStopped, Info: "foo"},
645- {Status: params.StatusError, Info: "not really"},
646+ {Status: params.StatusStarted, Info: "blah", Data: params.StatusData{}},
647+ {Status: params.StatusStopped, Info: "foo", Data: params.StatusData{}},
648+ {Status: params.StatusError, Info: "not really", Data: params.StatusData{"foo": "bar"}},
649 {Error: apiservertesting.NotFoundError("machine 42")},
650 {Error: apiservertesting.ErrUnauthorized},
651 {Error: apiservertesting.ErrUnauthorized},
652@@ -650,7 +705,7 @@
653 c.Assert(err, gc.IsNil)
654 c.Assert(result, gc.DeepEquals, params.StringsWatchResult{
655 StringsWatcherId: "1",
656- Changes: []string{"0", "1", "2"},
657+ Changes: []string{"0", "1", "2", "3"},
658 })
659
660 // Verify the resources were registered and stop them when done.
661
662=== modified file 'state/apiserver/uniter/uniter_test.go'
663--- state/apiserver/uniter/uniter_test.go 2014-03-17 13:22:55 +0000
664+++ state/apiserver/uniter/uniter_test.go 2014-03-26 05:17:40 +0000
665@@ -109,7 +109,7 @@
666 c.Assert(err, gc.IsNil)
667
668 args := params.SetStatus{
669- Entities: []params.SetEntityStatus{
670+ Entities: []params.EntityStatus{
671 {Tag: "unit-mysql-0", Status: params.StatusError, Info: "not really"},
672 {Tag: "unit-wordpress-0", Status: params.StatusStopped, Info: "foobar"},
673 {Tag: "unit-foo-42", Status: params.StatusStarted, Info: "blah"},
674
675=== modified file 'state/interface.go'
676--- state/interface.go 2014-01-20 17:47:59 +0000
677+++ state/interface.go 2014-03-26 05:17:40 +0000
678@@ -37,9 +37,15 @@
679 SetStatus(status params.Status, info string, data params.StatusData) error
680 }
681
682+type StatusGetter interface {
683+ Status() (status params.Status, info string, data params.StatusData, err error)
684+}
685+
686 var (
687 _ StatusSetter = (*Machine)(nil)
688 _ StatusSetter = (*Unit)(nil)
689+ _ StatusGetter = (*Machine)(nil)
690+ _ StatusGetter = (*Unit)(nil)
691 )
692
693 // Lifer represents an entity with a life.
694
695=== modified file 'state/machine_test.go'
696=== modified file 'worker/provisioner/provisioner_task.go'
697--- worker/provisioner/provisioner_task.go 2014-03-13 05:09:14 +0000
698+++ worker/provisioner/provisioner_task.go 2014-03-26 05:17:40 +0000
699@@ -405,7 +405,7 @@
700 // time until the error is resolved, but don't return an
701 // error; just keep going with the other machines.
702 logger.Errorf("cannot start instance for machine %q: %v", machine, err)
703- if err1 := machine.SetStatus(params.StatusError, err.Error()); err1 != nil {
704+ if err1 := machine.SetStatus(params.StatusError, err.Error(), nil); err1 != nil {
705 // Something is wrong with this machine, better report it back.
706 logger.Errorf("cannot set error status for machine %q: %v", machine, err1)
707 return err1

Subscribers

People subscribed via source and target branches

to status/vote changes: