Merge lp://staging/~rogpeppe/juju-core/312-api-jobs into lp://staging/~juju/juju-core/trunk

Reviewers: mp+165705_code.launchpad.net,

Message:
Please take a look.

Description:
api: implement Machine.Jobs

https://code.launchpad.net/~rogpeppe/juju-core/312-api-jobs/+merge/165705

Requires:
https://code.launchpad.net/~rogpeppe/juju-core/311-juju-bootstrap-state-change-password-1.5/+merge/165675

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M state/api/machine.go
   M state/api/params/params.go
   M state/apiserver/machine_test.go
   M state/apiserver/utils.go
   M state/machine.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: state/machine.go
=== modified file 'state/machine.go'
--- state/machine.go 2013-05-21 16:40:29 +0000
+++ state/machine.go 2013-05-24 21:38:13 +0000
@@ -36,7 +36,7 @@
   JobServeAPI
  )

-var jobNames = []string{
+var jobNames = []params.MachineJob{
   JobHostUnits: "JobHostUnits",
   JobManageEnviron: "JobManageEnviron",
   JobServeAPI: "JobServeAPI",
@@ -47,7 +47,7 @@
   if j <= 0 || j >= len(jobNames) {
    return fmt.Sprintf("<unknown job %d>", j)
   }
- return jobNames[j]
+ return string(jobNames[j])
  }

  // machineDoc represents the internal state of a machine in MongoDB.

Index: state/api/machine.go
=== modified file 'state/api/machine.go'
--- state/api/machine.go 2013-05-24 19:03:39 +0000
+++ state/api/machine.go 2013-05-24 21:38:13 +0000
@@ -139,6 +139,11 @@
   return m.doc.Life
  }

+// Jobs returns the responsibilities that must be fulfilled by m's agent.
+func (m *Machine) Jobs() []params.MachineJob {
+ return m.doc.Jobs
+}
+
  // Series returns the operating system series running on the machine.
  func (m *Machine) Series() string {
   return m.doc.Series

Index: state/apiserver/machine_test.go
=== modified file 'state/apiserver/machine_test.go'
--- state/apiserver/machine_test.go 2013-05-24 19:03:39 +0000
+++ state/apiserver/machine_test.go 2013-05-24 21:38:13 +0000
@@ -212,6 +212,29 @@
   c.Assert(string(life), Equals, "dead")
  }

+func (s *suite) TestMachineJobs(c *C) {
+ stm, err := s.State.AddMachine(
+ "series",
+ state.JobHostUnits,
+ state.JobManageEnviron,
+ state.JobServeAPI,
+ )
+ c.Assert(err, IsNil)
+ setDefaultPassword(c, stm)
+
+ st := s.openAs(c, stm.Tag())
+ defer st.Close()
+
+ m, err := st.Machine(stm.Id())
+ c.Assert(err, IsNil)
+
+ c.Assert(m.Jobs(), DeepEquals, []params.MachineJob{
+ params.JobHostUnits,
+ params.JobManageEnviron,
+ params.JobServeAPI,
+ })
+}
+
  func (s *suite) TestMachineEnsureDead(c *C) {
   stm, err := s.State.AddMachine("series", state.JobHostUnits)
   c.Assert(err, IsNil)

Index: state/apiserver/utils.go
=== modified file 'state/apiserver/utils.go'
--- state/apiserver/utils.go 2013-05-24 19:11:19 +0000
+++ state/apiserver/utils.go 2013-05-24 21:38:13 +0000
@@ -44,10 +44,16 @@
    return nil
   }
   instId, _ := stm.InstanceId()
+ jobs := stm....

Please take a look.

https://codereview.appspot.com/9754043/

Please take a look.

https://codereview.appspot.com/9754043/

NOT LGTM. Int jobs aren't great, but they're better than baking bad
names into the API forever, I think.

https://codereview.appspot.com/9754043/diff/13001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/9754043/diff/13001/state/machine.go#newcode38
state/machine.go:38:
// jobNames MUST NEVER BE CHANGED.

...or maybe we could just use ints, as designed originally, and not
lumber the API with guaranteed-inaccurate names. (JobServeAPI is not
sensible; it conflates jobs with tasks.)

https://codereview.appspot.com/9754043/

Please take a look.

https://codereview.appspot.com/9754043/diff/13001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/9754043/diff/13001/state/machine.go#newcode38
state/machine.go:38:
On 2013/05/27 20:23:20, fwereade wrote:
> // jobNames MUST NEVER BE CHANGED.

> ...or maybe we could just use ints, as designed originally, and not
lumber the
> API with guaranteed-inaccurate names. (JobServeAPI is not sensible; it
conflates
> jobs with tasks.)

renamed to JobManageState as agreed in discussion.

https://codereview.appspot.com/9754043/

LGTM, thanks

https://codereview.appspot.com/9754043/

Please do not commit this without refactoring it into a facade. We don't
have api/machine.go or apiserver/machine.go anymore.

https://codereview.appspot.com/9754043/

superseded by https://code.launchpad.net/~rogpeppe/juju-core/312-alt-api-jobs/+merge/170135