LGTM with the below trivial points addressed. really good stuff, thanks! https://codereview.appspot.com/7526043/diff/8001/state/api/apiclient.go File state/api/apiclient.go (right): https://codereview.appspot.com/7526043/diff/8001/state/api/apiclient.go#newcode146 state/api/apiclient.go:146: func (c *Client) GetAnnotations(id string) (*Annotations, error) { i think there's not really a point in having a new type for annotations here, just to contain a map. i'd just return the map directly, like the state Annotations method does. something like this, perhaps? (note the variable name change, just to make it obvious what kind of id we're talking about here). // SetAnnotations returns annotations that have been // set on the given entity. func (c *Client) SetAnnotations(entityId string) (map[string]string, error) https://codereview.appspot.com/7526043/diff/8001/state/api/apiclient.go#newcode156 state/api/apiclient.go:156: // SetAnnotation stores an annotation about a given entity. // SetAnnotation sets the annotation with the given key // on the given entity to the given value. // Currently annotations are supported on machines, // services, units and the environment itself. func (c *Client) SetAnnotation(entityId, key, value string) error { ? https://codereview.appspot.com/7526043/diff/8001/state/api/params/params.go File state/api/params/params.go (right): https://codereview.appspot.com/7526043/diff/8001/state/api/params/params.go#newcode76 state/api/params/params.go:76: Id string s/Id/EntityId/ https://codereview.appspot.com/7526043/diff/8001/state/api/params/params.go#newcode81 state/api/params/params.go:81: Id string s/Id/EntityId/ https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go File state/apiserver/api_test.go (right): https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode301 state/apiserver/api_test.go:301: return func() {}, nil the returned function should reset the annotation to its original value - all these op* functions should leave the state unchanged. https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode641 state/apiserver/api_test.go:641: // Annotations are correctly set. s/Annotations/Check annotations/ ? https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode642 state/apiserver/api_test.go:642: entity.Refresh() check error https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode647 state/apiserver/api_test.go:647: // Annotations are correctly returned. s/Annotations/Check annotations/ ? https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode648 state/apiserver/api_test.go:648: entity.Refresh() check error https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode652 state/apiserver/api_test.go:652: err = entity.SetAnnotation(key, "") check error https://codereview.appspot.com/7526043/diff/8001/state/apiserver/apiserver.go File state/apiserver/apiserver.go (right): https://codereview.appspot.com/7526043/diff/8001/state/apiserver/apiserver.go#newcode308 state/apiserver/apiserver.go:308: func (c *srvClient) GetAnnotations(args params.GetAnnotations) (api.Annotations, error) { if the api.Annotations type goes away (as i suggest elsewhere) then this should return params.GetAnnotationsResults. https://codereview.appspot.com/7526043/