state: subordinate constraints cannot be got/set

state: subordinate constraints cannot be got/set

This was implemented by denormalizing charm subordinacy into the service's
new Subordinate field; since I was doing this, I also did Series, to allow
that to also be sanity-checked in SetCharm. As a bonus, addUnitOps no
longer needs to do a roundtrip to get the necessary charm information.

I also moved the various charm-related test helpers into export_test because
it seemed neater that way.

https://code.launchpad.net/~fwereade/juju-core/state-subordinate-constraints/+merge/156138

(do not edit description out of merge proposal)

[fwereade, 2013-03-29 12:16:04]

Please take a look.

[dimitern, 2013-03-29 15:09:49]

Nice changes, I like especially the sorting out AddTestingCharm stuff :) LGTM.

https://codereview.appspot.com/8140044/diff/1/state/service_test.go
File state/service_test.go (left):

https://codereview.appspot.com/8140044/diff/1/state/service_test.go#oldcode39
state/service_test.go:39: // TODO: SetCharm must validate the change (version,
relations, etc)
I believe the relations are not entirely checked right now?

[fwereade, 2013-03-30 05:04:53]

https://codereview.appspot.com/8140044/diff/1/state/service.go
File state/service.go (right):

https://codereview.appspot.com/8140044/diff/1/state/service.go#newcode453
state/service.go:453: // TODO(fwereade) check that the service's endpoint in
each of
(here)

https://codereview.appspot.com/8140044/diff/1/state/service_test.go
File state/service_test.go (left):

https://codereview.appspot.com/8140044/diff/1/state/service_test.go#oldcode39
state/service_test.go:39: // TODO: SetCharm must validate the change (version,
relations, etc)
On 2013/03/29 15:09:49, dimitern wrote:
> I believe the relations are not entirely checked right now?

compensatory TODO in service.go

[jameinel, 2013-04-01 07:16:42]

LGTM

https://codereview.appspot.com/8140044/diff/1/state/conn_test.go
File state/conn_test.go (left):

https://codereview.appspot.com/8140044/diff/1/state/conn_test.go#oldcode62
state/conn_test.go:62: }
If all callers are passing "series" for series, is there a reason to pass it in?
Are there plans to pass something else? (Our testing repo doesn't have others,
but I think you can copy it to another name.)

https://codereview.appspot.com/8140044/diff/1/state/service.go
File state/service.go (right):

https://codereview.appspot.com/8140044/diff/1/state/service.go#newcode427
state/service.go:427: return fmt.Errorf("cannot change a service's subordinacy")
So s.doc.Subordinate is "Am I a subordinate" not "what is my Subordinate" right?

[fwereade, 2013-04-01 08:52:14]

*** Submitted:

state: subordinate constraints cannot be got/set

This was implemented by denormalizing charm subordinacy into the service's
new Subordinate field; since I was doing this, I also did Series, to allow
that to also be sanity-checked in SetCharm. As a bonus, addUnitOps no
longer needs to do a roundtrip to get the necessary charm information.

I also moved the various charm-related test helpers into export_test because
it seemed neater that way.

R=dimitern, jameinel
CC=
https://codereview.appspot.com/8140044

https://codereview.appspot.com/8140044/diff/1/state/conn_test.go
File state/conn_test.go (left):

https://codereview.appspot.com/8140044/diff/1/state/conn_test.go#oldcode62
state/conn_test.go:62: }
On 2013/04/01 07:16:42, jameinel wrote:
> If all callers are passing "series" for series, is there a reason to pass it
in?
> Are there plans to pass something else? (Our testing repo doesn't have others,
> but I think you can copy it to another name.)

Not quite all callers: service_test.go needs a custom series. The structure of
the local repo isn't relevant here: the series for which a charm is rated is
purely within its URL.

https://codereview.appspot.com/8140044/diff/1/state/service.go
File state/service.go (right):

https://codereview.appspot.com/8140044/diff/1/state/service.go#newcode427
state/service.go:427: return fmt.Errorf("cannot change a service's subordinacy")
On 2013/04/01 07:16:42, jameinel wrote:
> So s.doc.Subordinate is "Am I a subordinate" not "what is my Subordinate"
right?

That is correct. Principal/subordinate services (and units) are each one/many.

https://codereview.appspot.com/8140044/diff/1/state/service_test.go
File state/service_test.go (right):

https://codereview.appspot.com/8140044/diff/1/state/service_test.go#newcode63
state/service_test.go:63: othermysql := s.AddSeriesCharm(c, "mysql",
"otherseries", 123)
jam, here's the call to AddSeriesCharm.