Merge lp:~rogpeppe/juju-core/039-init-agent-version into lp:~juju/juju-core/trunk

[Roger Peppe]

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://codereview.appspot.com/6494057/

[Roger Peppe (rogpeppe)]

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/provis...

[Gustavo Niemeyer (niemeyer)]

https://codereview.appspot.com/6494057/diff/2001/state/open.go
File state/open.go (right):

https://codereview.appspot.com/6494057/diff/2001/state/open.go#newcode120
state/open.go:120: n.Set("agent-version",
version.Current.Number.String())
It'd be nice to avoid yet another place where the environment
configuration is manipulated magically. What's wrong with doing it when
that configuration is created?

https://codereview.appspot.com/6494057/

[Roger Peppe (rogpeppe)]

Please take a look.

https://codereview.appspot.com/6494057/

[Gustavo Niemeyer (niemeyer)]

We've discussed the use of environs.BootstrapConfig on IRC.

https://codereview.appspot.com/6494057/

Unmerged revisions

452. By Roger Peppe on 2012-08-30

merge 041-config-agent-version

451. By Roger Peppe on 2012-08-30

merge trunk

450. By Roger Peppe on 2012-08-30

environs: make Bootstrap responsible for setting agent version in environ config

449. By Roger Peppe on 2012-08-30

revert to bootstrap agent version set

448. By Roger Peppe on 2012-08-30

merge 041-config-agent-version

447. By Roger Peppe on 2012-08-30

environs/dummy: reinstate strict field checking

446. By Roger Peppe on 2012-08-29

gofmt

445. By Roger Peppe on 2012-08-29

state: write agent tools into initial EnvironConfig

444. By Roger Peppe on 2012-08-29

cmd/jujud: init store agent version

443. By Roger Peppe on 2012-08-29

testing: define and use all fixture methods.

When a helper suite defines only a subset of the available
fixture methods, it invites problems if that subset changes.
By making all helper suites define all fixture methods,
we make it easy to mechanically check that
a helper suite is being used correctly without going
to look at the source for that suite.

R=niemeyer
CC=
https://codereview.appspot.com/6472053

=== modified file 'cmd/jujud/bootstrap.go'
--- cmd/jujud/bootstrap.go	2012-08-20 01:52:41 +0000
+++ cmd/jujud/bootstrap.go	2012-08-30 12:29:18 +0000
@@ -7,6 +7,7 @@
 	"launchpad.net/goyaml"
 	"launchpad.net/juju-core/cmd"
 	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/version"
 )
 
 type BootstrapCommand struct {
@@ -26,6 +27,7 @@
 	stateInfoVar(f, &c.StateInfo, "zookeeper-servers", []string{"127.0.0.1:2181"}, "address of zookeeper to initialize")
 	f.StringVar(&c.InstanceId, "instance-id", "", "instance id of this machine")
 	f.StringVar(&c.EnvType, "env-type", "", "environment type")
+	c.EnvConfig = make(map[string]interface{})
 	yamlBase64Var(f, &c.EnvConfig, "env-config", "", "initial environment configuration (yaml, base64 encoded)")
 	if err := f.Parse(true, args); err != nil {
 		return err
@@ -39,6 +41,7 @@
 	if c.EnvType == "" {
 		return requiredError("env-type")
 	}
+	c.EnvConfig["agent-version"] = version.Current.Number.String()
 	return cmd.CheckEmpty(f.Args())
 }
 

=== modified file 'cmd/jujud/bootstrap_test.go'
--- cmd/jujud/bootstrap_test.go	2012-08-17 07:58:51 +0000
+++ cmd/jujud/bootstrap_test.go	2012-08-30 12:29:18 +0000
@@ -4,6 +4,7 @@
 	"encoding/base64"
 	. "launchpad.net/gocheck"
 	"launchpad.net/juju-core/juju/testing"
+	"launchpad.net/juju-core/version"
 )
 
 type BootstrapSuite struct {
@@ -70,12 +71,12 @@
 		// no value supplied
 		nil,
 		"",
-		nil,
+		map[string]interface{}{"agent-version": "3.4.5"},
 	}, {
 		// empty 
 		[]string{"--env-config", ""},
 		"",
-		nil,
+		map[string]interface{}{"agent-version": "3.4.5"},
 	}, {
 		// wrong, should be base64
 		[]string{"--env-config", "name: banana\n"},
@@ -84,12 +85,18 @@
 	}, {
 		[]string{"--env-config", base64.StdEncoding.EncodeToString([]byte("name: banana\n"))},
 		"",
-		map[string]interface{}{"name": "banana"},
+		map[string]interface{}{"agent-version": "3.4.5", "name": "banana"},
 	},
 }
 
 func (s *BootstrapSuite) TestBase64Config(c *C) {
-	for _, t := range base64ConfigTests {
+	oldVersion := version.Current
+	defer func() {
+		version.Current = oldVersion
+	}()
+	version.Current = version.MustParseBinary("3.4.5-foo-bar")
+	for i, t := range base64ConfigTests {
+		c.Logf("test %d", i)
 		args := []string{"--zookeeper-servers"}
 		args = append(args, s.StateInfo(c).Addrs...)
 		args = append(args, "--instance-id", "over9000", "--env-type", "dummy")

=== 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-30 12:29:18 +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")

=== modified file 'environs/dummy/environs.go'
--- environs/dummy/environs.go	2012-08-30 02:34:58 +0000
+++ environs/dummy/environs.go	2012-08-30 12:29:18 +0000
@@ -29,6 +29,7 @@
 	"launchpad.net/juju-core/schema"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/testing"
+	"launchpad.net/juju-core/version"
 	"net"
 	"net/http"
 	"os"
@@ -359,6 +360,7 @@
 			"zookeeper":       true,
 			"name":            e.ecfg().Name(),
 			"authorized-keys": e.ecfg().AuthorizedKeys(),
+			"agent-version":   version.Current.Number.String(),
 		}
 		st, err := state.Initialize(info, config)
 		if err != nil {

=== modified file 'environs/jujutest/livetests.go'
--- environs/jujutest/livetests.go	2012-08-24 01:45:40 +0000
+++ environs/jujutest/livetests.go	2012-08-30 12:29:18 +0000
@@ -178,6 +178,9 @@
 	v, ok = cfg.Get("authorized-keys")
 	c.Assert(ok, Equals, true)
 	c.Assert(v, Not(Equals), "")
+	v, ok = cfg.Get("agent-version")
+	c.Assert(ok, Equals, true)
+	c.Assert(v, Equals, version.Current.Number.String())
 
 	t.checkUpgradeMachineAgent(c, m)
 

=== modified file 'state/state_test.go'
--- state/state_test.go	2012-08-24 01:45:40 +0000
+++ state/state_test.go	2012-08-30 12:29:18 +0000
@@ -79,6 +79,10 @@
 	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{}{})
 }
 
 func (s *StateSuite) TestInitalizeWithConfig(c *C) {