Code review comment for lp:~rogpeppe/juju-core/039-init-agent-version

Reviewers: mp+122019_code.launchpad.net,

Message:
Please take a look.

Description:
state: write agent tools version into initial EnvironConfig

We will use this mechanism to trigger upgrades.
Note that because we use the EnvironConfig to hold
the version, environment providers can no longer be
strict about the fields they accept.

https://code.launchpad.net/~rogpeppe/juju-core/039-init-agent-version/+merge/122019

Requires:
https://code.launchpad.net/~rogpeppe/juju-core/041-config-agent-version/+merge/122016

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M cmd/jujud/provisioning_test.go
   M state/open.go
   M state/state_test.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/open.go
=== modified file 'state/open.go'
--- state/open.go 2012-08-20 01:19:38 +0000
+++ state/open.go 2012-08-29 17:42:03 +0000
@@ -3,9 +3,9 @@
  import (
   "errors"
   "fmt"
- "launchpad.net/goyaml"
   "launchpad.net/gozk/zookeeper"
   "launchpad.net/juju-core/log"
+ "launchpad.net/juju-core/version"
   "strings"
   "time"
  )
@@ -110,16 +110,16 @@
   if _, err := s.zk.Create("/relations", "", 0, zkPermAll); err != nil {
    return err
   }
- // TODO Create node for bootstrap machine.
-
- if config != nil {
- yaml, err := goyaml.Marshal(config)
- if err != nil {
- return err
- }
- if _, err := s.zk.Create(zkEnvironmentPath, string(yaml), 0, zkPermAll);
err != nil {
- return err
- }
+ if _, err = s.zk.Create(zkEnvironmentPath, "", 0, zkPermAll); err != nil {
+ return err
+ }
+ n, err := createConfigNode(s.zk, zkEnvironmentPath, config)
+ if err != nil {
+ return err
+ }
+ n.Set("agent-version", version.Current.Number.String())
+ if _, err = n.Write(); err != nil {
+ return err
   }

   // Finally creation of /initialized as marker.

Index: state/state_test.go
=== modified file 'state/state_test.go'
--- state/state_test.go 2012-08-24 01:45:40 +0000
+++ state/state_test.go 2012-08-29 17:44:53 +0000
@@ -8,6 +8,7 @@
   "launchpad.net/juju-core/juju/testing"
   "launchpad.net/juju-core/state"
   coretesting "launchpad.net/juju-core/testing"
+ "launchpad.net/juju-core/version"
   "net/url"
   "sort"
   stdtesting "testing"
@@ -79,6 +80,12 @@
   case <-time.After(1e9):
    c.Fatalf("state.Open blocked forever")
   }
+
+ cfg, err := st.EnvironConfig()
+ c.Assert(err, IsNil)
+ c.Assert(cfg.Map(), DeepEquals, map[string]interface{}{
+ "agent-version": version.Current.Number.String(),
+ })
  }

  func (s *StateSuite) TestInitalizeWithConfig(c *C) {
@@ -90,6 +97,7 @@
    "type": "dummy",
    "zookeeper": true,
    "authorized-keys": "i-am-a-key",
+ "agent-version": version.Current.Number.String(),
   }
   st, err := state.Initialize(s.StateInfo(c), m)
   c.Assert(err, IsNil)

Index: cmd/jujud/provisioning_test.go
=== modified file 'cmd/jujud/provisioning_test.go'
--- cmd/jujud/provisioning_test.go 2012-08-27 05:56:21 +0000
+++ cmd/jujud/provisioning_test.go 2012-08-29 17:42:03 +0000
@@ -86,7 +86,7 @@
     }
     c.Logf("discarding unknown event %#v", op)
    case <-time.After(2 * time.Second):
- c.Errorf("time out wating for operation")
+ c.Fatalf("time out wating for operation")
    }
   }
   panic("not reached")