Code review comment for lp://staging/~dimitern/juju-core/040-machine-api-calls-needed-by-machiner

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

*** Submitted:

state/api: Machine API needed by machiner

This introduces several new API calls, needed by the
worker/machiner to handle machines through the API:
* Life
* EnsureDead
* SetAgentAlive
* SetStatus

In the course of this some refactoring of the API
server was in order:
* Introduced presence.Pinger support
* Watchers and pingers are now both handled as resources
   which have a Stop() method
* Added a couple of root-level permission checking calls:
- authOwner(), taking an entity with a Tag() method and returning
   true if the authenticated user's tag matches the entity's tag
- authEnvironManager() returning true if the authenticated user
   matches the environment manager (machine with JobManageEnviron)

Permission checks are performed to ensure only agents (or managers)
can access the new machine API calls.

A few other drive-by changes done as well as suggested.

R=rog, TheMue, gz
CC=
https://codereview.appspot.com/9614043

https://codereview.appspot.com/9614043/diff/3002/state/apiserver/api_test.go
File state/apiserver/api_test.go (right):

https://codereview.appspot.com/9614043/diff/3002/state/apiserver/api_test.go#newcode304
state/apiserver/api_test.go:304: c.Check(api.ErrCode(err), Equals,
api.CodeHasAssignedUnits)
On 2013/05/21 16:52:33, rog wrote:
> this isn't the right place for the errcode check, i think.
> there should be a separate test that checks the err code.

Done.

https://codereview.appspot.com/9614043/diff/3002/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/9614043/diff/3002/state/machine.go#newcode210
state/machine.go:210: func IsHasAssignedUnitsError(err error) bool {
On 2013/05/21 16:52:33, rog wrote:
> is has :-)

Well.. it has has :)

https://codereview.appspot.com/9614043/diff/3002/state/presence/presence.go
File state/presence/presence.go (right):

https://codereview.appspot.com/9614043/diff/3002/state/presence/presence.go#newcode416
state/presence/presence.go:416: // Pinger periodically reports that a
specific key is alive, so that
On 2013/05/21 16:52:33, rog wrote:
> I think this can remain as it was. As long as it's a full sentence,
the rule
> isn't hard and fast.

Nah, let's keep to our defined standards.
A lot more stupid cases come to mind.

https://codereview.appspot.com/9614043/

« Back to merge proposal