Merge lp:~thumper/juju-core/agent-config-formatters into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Add the formatter concept to agent config.

In order to decouple the internal structure of the agent config from how it is
stored on disk, we introduce formatters. They are responsible for moving the
internal config to and from a serialization format.

The existing format is called "format 1.12", as that is the stable release
that the format was introduced.

Some of the methods are purposefully trivial at this stage, and they are
flushed out in the next branch.

https://codereview.appspot.com/13237050/

Add the formatter concept to agent config.

In order to decouple the internal structure of the agent config from how it is
stored on disk, we introduce formatters. They are responsible for moving the
internal config to and from a serialization format.

The existing format is called "format 1.12", as that is the stable release
that the format was introduced.

Some of the methods are purposefully trivial at this stage, and they are
flushed out in the next branch.

https://codereview.appspot.com/13237050/

[Tim Penhey (thumper)]

Reviewers: mp+183074_code.launchpad.net,

Message:
Please take a look.

Description:
Add the formatter concept to agent config.

In order to decouple the internal structure of the agent config from how
it is
stored on disk, we introduce formatters. They are responsible for
moving the
internal config to and from a serialization format.

The existing format is called "format 1.12", as that is the stable
release
that the format was introduced.

Some of the methods are purposefully trivial at this stage, and they are
flushed out in the next branch.

https://code.launchpad.net/~thumper/juju-core/agent-config-formatters/+merge/183074

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M agent/agent.go
   A agent/format-1.12.go
   A agent/format-1.12_whitebox_test.go
   A agent/format.go
   A agent/format_whitebox_test.go

[William Reade (fwereade)]

LGTM, just quibbles

https://codereview.appspot.com/13237050/diff/1/agent/format-1.12.go
File agent/format-1.12.go (right):

https://codereview.appspot.com/13237050/diff/1/agent/format-1.12.go#newcode19
agent/format-1.12.go:19: const format112 = "format 1.12"
1_12 throughout?

https://codereview.appspot.com/13237050/diff/1/agent/format-1.12_whitebox_test.go
File agent/format-1.12_whitebox_test.go (right):

https://codereview.appspot.com/13237050/diff/1/agent/format-1.12_whitebox_test.go#newcode6
agent/format-1.12_whitebox_test.go:6: // package.
I don't personally feel we need to make a big deal out of this.

https://codereview.appspot.com/13237050/

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/13237050/diff/1/agent/format-1.12.go
File agent/format-1.12.go (right):

https://codereview.appspot.com/13237050/diff/1/agent/format-1.12.go#newcode19
agent/format-1.12.go:19: const format112 = "format 1.12"
On 2013/08/30 07:06:22, fwereade wrote:
> 1_12 throughout?

I have renamed "format112" to "format_1_12" because "format1_12" looked
too weird.

https://codereview.appspot.com/13237050/diff/1/agent/format-1.12.go#newcode22
agent/format-1.12.go:22: type formatter112 struct {
formatter112 becomes formatter_1_12

https://codereview.appspot.com/13237050/diff/1/agent/format-1.12.go#newcode26
agent/format-1.12.go:26: type format112Serialization struct {
format112Serialization becomes format_1_12Serialization

https://codereview.appspot.com/13237050/diff/1/agent/format-1.12_whitebox_test.go
File agent/format-1.12_whitebox_test.go (right):

https://codereview.appspot.com/13237050/diff/1/agent/format-1.12_whitebox_test.go#newcode6
agent/format-1.12_whitebox_test.go:6: // package.
On 2013/08/30 07:06:22, fwereade wrote:
> I don't personally feel we need to make a big deal out of this.

Removed then.

https://codereview.appspot.com/13237050/

=== modified file 'agent/agent.go'
--- agent/agent.go	2013-08-28 22:59:45 +0000
+++ agent/agent.go	2013-09-02 21:47:29 +0000
@@ -5,12 +5,9 @@
 
 import (
 	"fmt"
-	"io/ioutil"
-	"os"
 	"path"
 	"regexp"
 
-	"launchpad.net/goyaml"
 	"launchpad.net/loggo"
 
 	"launchpad.net/juju-core/environs/config"
@@ -69,46 +66,27 @@
 	APIServerDetails() (port int, cert, key []byte)
 }
 
-// conf holds information for a given agent.
-type conf struct {
-	// DataDir specifies the path of the data directory used by all
-	// agents
-	dataDir string
-
-	// StateServerCert and StateServerKey hold the state server
-	// certificate and private key in PEM format.
-	StateServerCert []byte `yaml:",omitempty"`
-	StateServerKey  []byte `yaml:",omitempty"`
-
-	StatePort int `yaml:",omitempty"`
-	APIPort   int `yaml:",omitempty"`
-
-	// OldPassword specifies a password that should be
-	// used to connect to the state if StateInfo.Password
-	// is blank or invalid.
-	OldPassword string
-
-	// MachineNonce is set at provisioning/bootstrap time and used to
-	// ensure the agent is running on the correct instance.
-	MachineNonce string
-
-	// StateInfo specifies how the agent should connect to the
-	// state.  The password may be empty if an old password is
-	// specified, or when bootstrapping.
-	StateInfo *state.Info `yaml:",omitempty"`
-
-	// OldAPIPassword specifies a password that should
-	// be used to connect to the API if APIInfo.Password
-	// is blank or invalid.
-	OldAPIPassword string
-
-	// APIInfo specifies how the agent should connect to the
-	// state through the API.
-	APIInfo *api.Info `yaml:",omitempty"`
-}
-
-// Ensure that conf implements Config.
-var _ Config = (*conf)(nil)
+// Ensure that the configInternal struct implements the Config interface.
+var _ Config = (*configInternal)(nil)
+
+type connectionDetails struct {
+	addresses []string
+	password  string
+}
+
+type configInternal struct {
+	dataDir         string
+	tag             string
+	nonce           string
+	caCert          []byte
+	stateDetails    *connectionDetails
+	apiDetails      *connectionDetails
+	oldPassword     string
+	stateServerCert []byte
+	stateServerKey  []byte
+	statePort       int
+	apiPort         int
+}
 
 type AgentConfigParams struct {
 	DataDir        string
@@ -120,7 +98,7 @@
 	CACert         []byte
 }
 
-func newConfig(params AgentConfigParams) (*conf, error) {
+func newConfig(params AgentConfigParams) (*configInternal, error) {
 	if params.DataDir == "" {
 		return nil, requiredError("data directory")
 	}
@@ -135,33 +113,27 @@
 	}
 	// Note that the password parts of the state and api information are
 	// blank.  This is by design.
-	var stateInfo *state.Info
+	config := &configInternal{
+		dataDir:     params.DataDir,
+		tag:         params.Tag,
+		nonce:       params.Nonce,
+		caCert:      params.CACert,
+		oldPassword: params.Password,
+	}
 	if len(params.StateAddresses) > 0 {
-		stateInfo = &state.Info{
-			Addrs:  params.StateAddresses,
-			Tag:    params.Tag,
-			CACert: params.CACert,
+		config.stateDetails = &connectionDetails{
+			addresses: params.StateAddresses,
 		}
 	}
-	var apiInfo *api.Info
 	if len(params.APIAddresses) > 0 {
-		apiInfo = &api.Info{
-			Addrs:  params.APIAddresses,
-			Tag:    params.Tag,
-			CACert: params.CACert,
+		config.apiDetails = &connectionDetails{
+			addresses: params.APIAddresses,
 		}
 	}
-	conf := &conf{
-		dataDir:      params.DataDir,
-		OldPassword:  params.Password,
-		StateInfo:    stateInfo,
-		APIInfo:      apiInfo,
-		MachineNonce: params.Nonce,
-	}
-	if err := conf.check(); err != nil {
+	if err := config.check(); err != nil {
 		return nil, err
 	}
-	return conf, nil
+	return config, nil
 }
 
 // NewAgentConfig returns a new config object suitable for use for a unit
@@ -186,15 +158,15 @@
 	if params.StateServerKey == nil {
 		return nil, requiredError("state server key")
 	}
-	conf, err := newConfig(params.AgentConfigParams)
+	config, err := newConfig(params.AgentConfigParams)
 	if err != nil {
 		return nil, err
 	}
-	conf.StateServerCert = params.StateServerCert
-	conf.StateServerKey = params.StateServerKey
-	conf.StatePort = params.StatePort
-	conf.APIPort = params.APIPort
-	return conf, nil
+	config.stateServerCert = params.StateServerCert
+	config.stateServerKey = params.StateServerKey
+	config.statePort = params.StatePort
+	config.apiPort = params.APIPort
+	return config, nil
 }
 
 // Dir returns the agent-specific data directory.
@@ -206,25 +178,20 @@
 // entity from the given data directory.
 func ReadConf(dataDir, tag string) (Config, error) {
 	dir := Dir(dataDir, tag)
-	data, err := ioutil.ReadFile(path.Join(dir, "agent.conf"))
+	config, err := currentFormatter.read(dir)
 	if err != nil {
 		return nil, err
 	}
-	var c conf
-	if err := goyaml.Unmarshal(data, &c); err != nil {
-		return nil, err
-	}
-	c.dataDir = dataDir
-	if err := c.check(); err != nil {
-		return nil, err
-	}
-	if c.StateInfo != nil {
-		c.StateInfo.Tag = tag
-	}
-	if c.APIInfo != nil {
-		c.APIInfo.Tag = tag
-	}
-	return &c, nil
+	config.dataDir = dataDir
+	if err := config.check(); err != nil {
+		return nil, err
+	}
+	return config, nil
+	// Read the format for the dir.
+	// Create a formatter for the format
+	// Read in the config
+	// If the format isn't the current format, call the upgrade function, and
+	// write out using the new formatter.
 }
 
 func requiredError(what string) error {
@@ -232,53 +199,45 @@
 }
 
 // File returns the path of the given file in the agent's directory.
-func (c *conf) File(name string) string {
+func (c *configInternal) File(name string) string {
 	return path.Join(c.Dir(), name)
 }
 
-func (c *conf) confFile() string {
-	return c.File("agent.conf")
-}
-
-func (c *conf) DataDir() string {
+func (c *configInternal) DataDir() string {
 	return c.dataDir
 }
 
-func (c *conf) Nonce() string {
-	return c.MachineNonce
+func (c *configInternal) Nonce() string {
+	return c.nonce
 }
 
-func (c *conf) APIServerDetails() (port int, cert, key []byte) {
-	return c.APIPort, c.StateServerCert, c.StateServerKey
+func (c *configInternal) APIServerDetails() (port int, cert, key []byte) {
+	return c.apiPort, c.stateServerCert, c.stateServerKey
 }
 
 // Tag returns the tag of the entity on whose behalf the state connection will
 // be made.
-func (c *conf) Tag() string {
-	if c.StateInfo != nil {
-		return c.StateInfo.Tag
-	}
-	return c.APIInfo.Tag
+func (c *configInternal) Tag() string {
+	return c.tag
 }
 
 // Dir returns the agent's directory.
-func (c *conf) Dir() string {
-	return Dir(c.dataDir, c.Tag())
+func (c *configInternal) Dir() string {
+	return Dir(c.dataDir, c.tag)
 }
 
 // Check checks that the configuration has all the required elements.
-func (c *conf) check() error {
-	if c.StateInfo == nil && c.APIInfo == nil {
+func (c *configInternal) check() error {
+	if c.stateDetails == nil && c.apiDetails == nil {
 		return requiredError("state or API addresses")
 	}
-	if c.StateInfo != nil {
-		if err := checkAddrs(c.StateInfo.Addrs, "state server address"); err != nil {
+	if c.stateDetails != nil {
+		if err := checkAddrs(c.stateDetails.addresses, "state server address"); err != nil {
 			return err
 		}
 	}
-	// TODO(rog) make APIInfo mandatory
-	if c.APIInfo != nil {
-		if err := checkAddrs(c.APIInfo.Addrs, "API server address"); err != nil {
+	if c.apiDetails != nil {
+		if err := checkAddrs(c.apiDetails.addresses, "API server address"); err != nil {
 			return err
 		}
 	}
@@ -299,17 +258,17 @@
 	return nil
 }
 
-func (c *conf) PasswordHash() string {
+func (c *configInternal) PasswordHash() string {
 	var password string
-	if c.StateInfo == nil {
-		password = c.APIInfo.Password
+	if c.stateDetails == nil {
+		password = c.apiDetails.password
 	} else {
-		password = c.StateInfo.Password
+		password = c.stateDetails.password
 	}
 	return utils.PasswordHash(password)
 }
 
-func (c *conf) GenerateNewPassword() (string, error) {
+func (c *configInternal) GenerateNewPassword() (string, error) {
 	newPassword, err := utils.RandomPassword()
 	if err != nil {
 		return "", err
@@ -318,15 +277,15 @@
 	// to write the configuration file, the configuration will
 	// still be valid.
 	other := *c
-	if c.StateInfo != nil {
-		stateInfo := *c.StateInfo
-		stateInfo.Password = newPassword
-		other.StateInfo = &stateInfo
+	if c.stateDetails != nil {
+		stateDetails := *c.stateDetails
+		stateDetails.password = newPassword
+		other.stateDetails = &stateDetails
 	}
-	if c.APIInfo != nil {
-		apiInfo := *c.APIInfo
-		apiInfo.Password = newPassword
-		other.APIInfo = &apiInfo
+	if c.apiDetails != nil {
+		apiDetails := *c.apiDetails
+		apiDetails.password = newPassword
+		other.apiDetails = &apiDetails
 	}
 
 	if err := other.Write(); err != nil {
@@ -337,47 +296,15 @@
 }
 
 // Write writes the agent configuration.
-func (c *conf) Write() error {
-	if err := c.check(); err != nil {
-		return err
-	}
-	data, err := goyaml.Marshal(c)
-	if err != nil {
-		return err
-	}
-	if err := os.MkdirAll(c.Dir(), 0755); err != nil {
-		return err
-	}
-	f := c.File("agent.conf-new")
-	if err := ioutil.WriteFile(f, data, 0600); err != nil {
-		return err
-	}
-	if err := os.Rename(f, c.confFile()); err != nil {
-		return err
-	}
-	return nil
+func (c *configInternal) Write() error {
+	return currentFormatter.write(c)
 }
 
 // WriteCommands returns shell commands to write the agent
 // configuration.  It returns an error if the configuration does not
 // have all the right elements.
-func (c *conf) WriteCommands() ([]string, error) {
-	if err := c.check(); err != nil {
-		return nil, err
-	}
-	data, err := goyaml.Marshal(c)
-	if err != nil {
-		return nil, err
-	}
-	var cmds []string
-	addCmd := func(f string, a ...interface{}) {
-		cmds = append(cmds, fmt.Sprintf(f, a...))
-	}
-	f := utils.ShQuote(c.confFile())
-	addCmd("mkdir -p %s", utils.ShQuote(c.Dir()))
-	addCmd("install -m %o /dev/null %s", 0600, f)
-	addCmd(`printf '%%s\n' %s > %s`, utils.ShQuote(string(data)), f)
-	return cmds, nil
+func (c *configInternal) WriteCommands() ([]string, error) {
+	return currentFormatter.writeCommands(c)
 }
 
 // OpenAPI tries to open the state using the given Conf.  If it
@@ -385,9 +312,14 @@
 // to the state should be changed accordingly - the caller should write the
 // configuration with StateInfo.Password set to newPassword, then
 // set the entity's password accordingly.
-func (c *conf) OpenAPI(dialOpts api.DialOpts) (st *api.State, newPassword string, err error) {
-	info := *c.APIInfo
-	info.Nonce = c.MachineNonce
+func (c *configInternal) OpenAPI(dialOpts api.DialOpts) (st *api.State, newPassword string, err error) {
+	info := api.Info{
+		Addrs:    c.apiDetails.addresses,
+		Password: c.apiDetails.password,
+		CACert:   c.caCert,
+		Tag:      c.tag,
+		Nonce:    c.nonce,
+	}
 	if info.Password != "" {
 		st, err := api.Open(&info, dialOpts)
 		if err == nil {
@@ -401,7 +333,7 @@
 		// password but before changing it, so we'll try again
 		// with the old password.
 	}
-	info.Password = c.OldPassword
+	info.Password = c.oldPassword
 	st, err = api.Open(&info, dialOpts)
 	if err != nil {
 		return nil, "", err
@@ -418,8 +350,13 @@
 }
 
 // OpenState tries to open the state using the given Conf.
-func (c *conf) OpenState() (*state.State, error) {
-	info := *c.StateInfo
+func (c *configInternal) OpenState() (*state.State, error) {
+	info := state.Info{
+		Addrs:    c.stateDetails.addresses,
+		Password: c.stateDetails.password,
+		CACert:   c.caCert,
+		Tag:      c.tag,
+	}
 	if info.Password != "" {
 		st, err := state.Open(&info, state.DefaultDialOpts())
 		if err == nil {
@@ -431,14 +368,16 @@
 			return nil, err
 		}
 	}
-	info.Password = c.OldPassword
+	info.Password = c.oldPassword
 	return state.Open(&info, state.DefaultDialOpts())
 }
 
 func InitialStateConfiguration(agentConfig Config, cfg *config.Config, timeout state.DialOpts) (*state.State, error) {
-	c := agentConfig.(*conf)
-	info := *c.StateInfo
-	info.Tag = ""
+	c := agentConfig.(*configInternal)
+	info := state.Info{
+		Addrs:  c.stateDetails.addresses,
+		CACert: c.caCert,
+	}
 	logger.Debugf("initializing address %v", info.Addrs)
 	st, err := state.Initialize(&info, cfg, timeout)
 	if err != nil {
@@ -451,7 +390,7 @@
 	}
 	logger.Debugf("state initialized")
 
-	if err := bootstrapUsers(st, cfg, c.OldPassword); err != nil {
+	if err := bootstrapUsers(st, cfg, c.oldPassword); err != nil {
 		st.Close()
 		return nil, err
 	}
 
=== added file 'agent/format-1.12.go'
--- agent/format-1.12.go	1970-01-01 00:00:00 +0000
+++ agent/format-1.12.go	2013-09-02 21:47:29 +0000
@@ -0,0 +1,175 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package agent
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path"
+
+	"launchpad.net/goyaml"
+
+	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/state/api"
+	"launchpad.net/juju-core/utils"
+)
+
+const format_1_12 = "format 1.12"
+
+// formatter_1_12 is the formatter for the 1.12 format.
+type formatter_1_12 struct {
+}
+
+// format_1_12Serialization holds information stored in the agent.conf file.
+type format_1_12Serialization struct {
+	// StateServerCert and StateServerKey hold the state server
+	// certificate and private key in PEM format.
+	StateServerCert []byte `yaml:",omitempty"`
+	StateServerKey  []byte `yaml:",omitempty"`
+
+	StatePort int `yaml:",omitempty"`
+	APIPort   int `yaml:",omitempty"`
+
+	// OldPassword specifies a password that should be
+	// used to connect to the state if StateInfo.Password
+	// is blank or invalid.
+	OldPassword string
+
+	// MachineNonce is set at provisioning/bootstrap time and used to
+	// ensure the agent is running on the correct instance.
+	MachineNonce string
+
+	// StateInfo specifies how the agent should connect to the
+	// state.  The password may be empty if an old password is
+	// specified, or when bootstrapping.
+	StateInfo *state.Info `yaml:",omitempty"`
+
+	// OldAPIPassword specifies a password that should
+	// be used to connect to the API if APIInfo.Password
+	// is blank or invalid.
+	OldAPIPassword string
+
+	// APIInfo specifies how the agent should connect to the
+	// state through the API.
+	APIInfo *api.Info `yaml:",omitempty"`
+}
+
+// Ensure that the formatter_1_12 struct implements the formatter interface.
+var _ formatter = (*formatter_1_12)(nil)
+
+func (*formatter_1_12) configFile(dirName string) string {
+	return path.Join(dirName, "agent.conf")
+}
+
+func (formatter *formatter_1_12) read(dirName string) (*configInternal, error) {
+	data, err := ioutil.ReadFile(formatter.configFile(dirName))
+	if err != nil {
+		return nil, err
+	}
+	var conf format_1_12Serialization
+	if err := goyaml.Unmarshal(data, &conf); err != nil {
+		return nil, err
+	}
+
+	var stateDetails *connectionDetails
+	var caCert []byte
+	var tag string
+	if conf.StateInfo != nil {
+		stateDetails = &connectionDetails{
+			conf.StateInfo.Addrs,
+			conf.StateInfo.Password,
+		}
+		tag = conf.StateInfo.Tag
+		caCert = conf.StateInfo.CACert
+	}
+	var apiDetails *connectionDetails
+	if conf.APIInfo != nil {
+		apiDetails = &connectionDetails{
+			conf.APIInfo.Addrs,
+			conf.APIInfo.Password,
+		}
+		tag = conf.APIInfo.Tag
+		caCert = conf.APIInfo.CACert
+	}
+	return &configInternal{
+		tag:             tag,
+		nonce:           conf.MachineNonce,
+		caCert:          caCert,
+		stateDetails:    stateDetails,
+		apiDetails:      apiDetails,
+		oldPassword:     conf.OldPassword,
+		stateServerCert: conf.StateServerCert,
+		stateServerKey:  conf.StateServerKey,
+		statePort:       conf.StatePort,
+		apiPort:         conf.APIPort,
+	}, nil
+}
+
+func (formatter *formatter_1_12) makeAgentConf(config *configInternal) *format_1_12Serialization {
+	format := &format_1_12Serialization{
+		StateServerCert: config.stateServerCert,
+		StateServerKey:  config.stateServerKey,
+		StatePort:       config.statePort,
+		APIPort:         config.apiPort,
+		OldPassword:     config.oldPassword,
+		MachineNonce:    config.nonce,
+	}
+	if config.stateDetails != nil {
+		// It is fine that we are copying the slices for the addresses.
+		format.StateInfo = &state.Info{
+			Addrs:    config.stateDetails.addresses,
+			Password: config.stateDetails.password,
+			Tag:      config.tag,
+			CACert:   config.caCert,
+		}
+	}
+	if config.apiDetails != nil {
+		format.APIInfo = &api.Info{
+			Addrs:    config.apiDetails.addresses,
+			Password: config.apiDetails.password,
+			Tag:      config.tag,
+			CACert:   config.caCert,
+		}
+	}
+	return format
+}
+
+func (formatter *formatter_1_12) write(config *configInternal) error {
+	dirName := config.Dir()
+	conf := formatter.makeAgentConf(config)
+	data, err := goyaml.Marshal(conf)
+	if err != nil {
+		return err
+	}
+	if err := os.MkdirAll(dirName, 0755); err != nil {
+		return err
+	}
+	newFile := path.Join(dirName, "agent.conf-new")
+	if err := ioutil.WriteFile(newFile, data, 0600); err != nil {
+		return err
+	}
+	if err := os.Rename(newFile, formatter.configFile(dirName)); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (formatter *formatter_1_12) writeCommands(config *configInternal) ([]string, error) {
+	dirName := config.Dir()
+	conf := formatter.makeAgentConf(config)
+	data, err := goyaml.Marshal(conf)
+	if err != nil {
+		return nil, err
+	}
+	var commands []string
+	addCommand := func(f string, a ...interface{}) {
+		commands = append(commands, fmt.Sprintf(f, a...))
+	}
+	filename := utils.ShQuote(formatter.configFile(dirName))
+	addCommand("mkdir -p %s", utils.ShQuote(dirName))
+	addCommand("install -m %o /dev/null %s", 0600, filename)
+	addCommand(`printf '%%s\n' %s > %s`, utils.ShQuote(string(data)), filename)
+	return commands, nil
+}
 
=== added file 'agent/format-1.12_whitebox_test.go'
--- agent/format-1.12_whitebox_test.go	1970-01-01 00:00:00 +0000
+++ agent/format-1.12_whitebox_test.go	2013-09-02 21:47:29 +0000
@@ -0,0 +1,96 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package agent
+
+import (
+	"os"
+	"path"
+
+	gc "launchpad.net/gocheck"
+
+	"launchpad.net/juju-core/testing"
+	jc "launchpad.net/juju-core/testing/checkers"
+)
+
+type format_1_12Suite struct {
+	testing.LoggingSuite
+	formatter formatter_1_12
+}
+
+var _ = gc.Suite(&format_1_12Suite{})
+
+var agentParams = AgentConfigParams{
+	Tag:            "omg",
+	Password:       "sekrit",
+	CACert:         []byte("ca cert"),
+	StateAddresses: []string{"localhost:1234"},
+	APIAddresses:   []string{"localhost:1235"},
+	Nonce:          "a nonce",
+}
+
+func (s *format_1_12Suite) newConfig(c *gc.C) *configInternal {
+	params := agentParams
+	params.DataDir = c.MkDir()
+	config, err := newConfig(params)
+	c.Assert(err, gc.IsNil)
+	return config
+}
+
+func (s *format_1_12Suite) TestWriteAgentConfig(c *gc.C) {
+	config := s.newConfig(c)
+	err := s.formatter.write(config)
+	c.Assert(err, gc.IsNil)
+
+	expectedLocation := path.Join(config.Dir(), "agent.conf")
+	fileInfo, err := os.Stat(expectedLocation)
+	c.Assert(err, gc.IsNil)
+	c.Assert(fileInfo.Mode().IsRegular(), jc.IsTrue)
+	c.Assert(fileInfo.Mode().Perm(), gc.Equals, os.FileMode(0600))
+	c.Assert(fileInfo.Size(), jc.GreaterThan, 0)
+}
+
+func (s *format_1_12Suite) assertWriteAndRead(c *gc.C, config *configInternal) {
+	err := s.formatter.write(config)
+	c.Assert(err, gc.IsNil)
+	// The readConfig is missing the dataDir initially.
+	readConfig, err := s.formatter.read(config.Dir())
+	c.Assert(err, gc.IsNil)
+	c.Assert(readConfig.dataDir, gc.Equals, "")
+	// This is put in by the ReadConf method that we are avoiding using
+	// becuase it will have side-effects soon around migrating configs.
+	readConfig.dataDir = config.dataDir
+	c.Assert(readConfig, gc.DeepEquals, config)
+}
+
+func (s *format_1_12Suite) TestRead(c *gc.C) {
+	config := s.newConfig(c)
+	s.assertWriteAndRead(c, config)
+}
+
+func (s *format_1_12Suite) TestWriteCommands(c *gc.C) {
+	config := s.newConfig(c)
+	commands, err := s.formatter.writeCommands(config)
+	c.Assert(err, gc.IsNil)
+	c.Assert(commands, gc.HasLen, 3)
+	c.Assert(commands[0], gc.Matches, `mkdir -p '\S+/agents/omg'`)
+	c.Assert(commands[1], gc.Matches, `install -m 600 /dev/null '\S+/agents/omg/agent.conf'`)
+	c.Assert(commands[2], gc.Matches, `printf '%s\\n' '(.|\n)*' > '\S+/agents/omg/agent.conf'`)
+}
+
+func (s *format_1_12Suite) TestReadWriteStateConfig(c *gc.C) {
+	stateParams := StateMachineConfigParams{
+		AgentConfigParams: agentParams,
+		StateServerCert:   []byte("some special cert"),
+		StateServerKey:    []byte("a special key"),
+		StatePort:         12345,
+		APIPort:           23456,
+	}
+	stateParams.DataDir = c.MkDir()
+	configInterface, err := NewStateMachineConfig(stateParams)
+	c.Assert(err, gc.IsNil)
+	config, ok := configInterface.(*configInternal)
+	c.Assert(ok, jc.IsTrue)
+
+	s.assertWriteAndRead(c, config)
+}
 
=== added file 'agent/format.go'
--- agent/format.go	1970-01-01 00:00:00 +0000
+++ agent/format.go	2013-09-02 21:47:29 +0000
@@ -0,0 +1,51 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package agent
+
+import (
+	"fmt"
+)
+
+// The format file in the agent config directory is used to identify the
+// method of serialization.  This is used by individual format readers and
+// writers to be able to translate from the file format to the in-memory
+// structure.
+//
+// Juju only supports upgrading from single steps, so Juju only needs to know
+// about the current format and the format of the previous stable release.
+// For convenience, the format name includes the version number of the stable
+// release that it will be released with.  Once this release has happened, the
+// format should be considered FIXED, and should no longer be modified.  If
+// changes are necessary to the format, a new format should be created.
+//
+// We don't need to create new formats for each release, the version number is
+// just a convenience for us to know which stable release introduced that
+// format.
+
+const (
+	formatFilename = "format"
+	currentFormat  = format_1_12
+)
+
+var currentFormatter = &formatter_1_12{}
+
+// The formatter defines the two methods needed by the formatters for
+// translating to and from the internal, format agnostic, structure.
+type formatter interface {
+	read(dirName string) (*configInternal, error)
+	write(config *configInternal) error
+	writeCommands(config *configInternal) ([]string, error)
+}
+
+func readFormat(dirName string) (string, error) {
+	return currentFormat, nil
+}
+
+func newFormatter(format string) (formatter, error) {
+	switch format {
+	case currentFormat:
+		return currentFormatter, nil
+	}
+	return nil, fmt.Errorf("unknown agent config format")
+}
 
=== added file 'agent/format_whitebox_test.go'
--- agent/format_whitebox_test.go	1970-01-01 00:00:00 +0000
+++ agent/format_whitebox_test.go	2013-09-02 21:47:29 +0000
@@ -0,0 +1,32 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package agent
+
+import (
+	gc "launchpad.net/gocheck"
+
+	"launchpad.net/juju-core/testing"
+)
+
+type formatSuite struct {
+	testing.LoggingSuite
+}
+
+var _ = gc.Suite(&formatSuite{})
+
+func (*formatSuite) TestReadFormat(c *gc.C) {
+	format, err := readFormat("ignored")
+	c.Assert(format, gc.Equals, currentFormat)
+	c.Assert(err, gc.IsNil)
+}
+
+func (*formatSuite) TestNewFormatter(c *gc.C) {
+	formatter, err := newFormatter(currentFormat)
+	c.Assert(formatter, gc.NotNil)
+	c.Assert(err, gc.IsNil)
+
+	formatter, err = newFormatter("other")
+	c.Assert(formatter, gc.IsNil)
+	c.Assert(err, gc.ErrorMatches, "unknown agent config format")
+}