api: api no longer depends on state.

As part of a previous refactoring I accedently made api directly dependent on state, which is an even worse crime than state depending on the apiserver. This proposal corrects this mistake and removes all dependencies on state from the api packages, along with additional removals of direct dependencies on state. The cost is the agent package is responsible for translating between params.StateServingInfo, which is effectively the public data type of state information, and state.StateServingInfo which is the state's own version of the same.
zhatin · Sep 12, 2014 · 40c5aaa · 40c5aaa
1 parent 4438ab8
commit 40c5aaa
Show file tree

Hide file tree

Showing 19 changed files with 75 additions and 63 deletions.
diff --git a/agent/agent.go b/agent/agent.go
@@ -26,7 +26,6 @@ import (
 	"github.com/juju/juju/juju/paths"
 	"github.com/juju/juju/mongo"
 	"github.com/juju/juju/network"
-	"github.com/juju/juju/state"
 	"github.com/juju/juju/version"
 )
 
@@ -113,7 +112,7 @@ type Config interface {
 	// StateServingInfo returns the details needed to run
 	// a state server and reports whether those details
 	// are available
-	StateServingInfo() (state.StateServingInfo, bool)
+	StateServingInfo() (params.StateServingInfo, bool)
 
 	// APIInfo returns details for connecting to the API server.
 	APIInfo() *api.Info
@@ -182,7 +181,7 @@ type ConfigSetterOnly interface {
 
 	// SetStateServingInfo sets the information needed
 	// to run a state server
-	SetStateServingInfo(info state.StateServingInfo)
+	SetStateServingInfo(info params.StateServingInfo)
 }
 
 type ConfigWriter interface {
@@ -241,7 +240,7 @@ type configInternal struct {
 	stateDetails      *connectionDetails
 	apiDetails        *connectionDetails
 	oldPassword       string
-	servingInfo       *state.StateServingInfo
+	servingInfo       *params.StateServingInfo
 	values            map[string]string
 	preferIPv6        bool
 }
@@ -325,7 +324,7 @@ func NewAgentConfig(configParams AgentConfigParams) (ConfigSetterWriter, error)
 
 // NewStateMachineConfig returns a configuration suitable for
 // a machine running the state server.
-func NewStateMachineConfig(configParams AgentConfigParams, serverInfo state.StateServingInfo) (ConfigSetterWriter, error) {
+func NewStateMachineConfig(configParams AgentConfigParams, serverInfo params.StateServingInfo) (ConfigSetterWriter, error) {
 	if serverInfo.Cert == "" {
 		return nil, errors.Trace(requiredError("state server cert"))
 	}
@@ -551,14 +550,14 @@ func (c *configInternal) PreferIPv6() bool {
 	return c.preferIPv6
 }
 
-func (c *configInternal) StateServingInfo() (state.StateServingInfo, bool) {
+func (c *configInternal) StateServingInfo() (params.StateServingInfo, bool) {
 	if c.servingInfo == nil {
-		return state.StateServingInfo{}, false
+		return params.StateServingInfo{}, false
 	}
 	return *c.servingInfo, true
 }
 
-func (c *configInternal) SetStateServingInfo(info state.StateServingInfo) {
+func (c *configInternal) SetStateServingInfo(info params.StateServingInfo) {
 	c.servingInfo = &info
 }
 

diff --git a/agent/agent_test.go b/agent/agent_test.go
@@ -16,7 +16,6 @@ import (
 	"github.com/juju/juju/apiserver/params"
 	"github.com/juju/juju/mongo"
 	"github.com/juju/juju/network"
-	"github.com/juju/juju/state"
 	"github.com/juju/juju/testing"
 	"github.com/juju/juju/version"
 )
@@ -354,7 +353,7 @@ func (*suite) TestNewStateMachineConfig(c *gc.C) {
 	type testStruct struct {
 		about         string
 		params        agent.AgentConfigParams
-		servingInfo   state.StateServingInfo
+		servingInfo   params.StateServingInfo
 		checkErr      string
 		inspectConfig func(*gc.C, agent.Config)
 	}
@@ -363,20 +362,20 @@ func (*suite) TestNewStateMachineConfig(c *gc.C) {
 		checkErr: "state server cert not found in configuration",
 	}, {
 		about: "missing state server key",
-		servingInfo: state.StateServingInfo{
+		servingInfo: params.StateServingInfo{
 			Cert: "server cert",
 		},
 		checkErr: "state server key not found in configuration",
 	}, {
 		about: "missing state port",
-		servingInfo: state.StateServingInfo{
+		servingInfo: params.StateServingInfo{
 			Cert:       "server cert",
 			PrivateKey: "server key",
 		},
 		checkErr: "state port not found in configuration",
 	}, {
 		about: "params api port",
-		servingInfo: state.StateServingInfo{
+		servingInfo: params.StateServingInfo{
 			Cert:       "server cert",
 			PrivateKey: "server key",
 			StatePort:  69,
@@ -387,7 +386,7 @@ func (*suite) TestNewStateMachineConfig(c *gc.C) {
 		tests = append(tests, testStruct{
 			about:  test.about,
 			params: test.params,
-			servingInfo: state.StateServingInfo{
+			servingInfo: params.StateServingInfo{
 				Cert:       "server cert",
 				PrivateKey: "server key",
 				StatePort:  3171,
@@ -434,7 +433,7 @@ func (*suite) TestAttributes(c *gc.C) {
 }
 
 func (*suite) TestStateServingInfo(c *gc.C) {
-	servingInfo := state.StateServingInfo{
+	servingInfo := params.StateServingInfo{
 		Cert:           "old cert",
 		PrivateKey:     "old key",
 		StatePort:      69,
@@ -447,7 +446,7 @@ func (*suite) TestStateServingInfo(c *gc.C) {
 	gotInfo, ok := conf.StateServingInfo()
 	c.Assert(ok, jc.IsTrue)
 	c.Assert(gotInfo, jc.DeepEquals, servingInfo)
-	newInfo := state.StateServingInfo{
+	newInfo := params.StateServingInfo{
 		APIPort:        147,
 		StatePort:      169,
 		Cert:           "new cert",
@@ -497,7 +496,7 @@ func (*suite) TestWriteAndRead(c *gc.C) {
 
 func (*suite) TestAPIInfoAddsLocalhostWhenServingInfoPresent(c *gc.C) {
 	attrParams := attributeParams
-	servingInfo := state.StateServingInfo{
+	servingInfo := params.StateServingInfo{
 		Cert:           "old cert",
 		PrivateKey:     "old key",
 		StatePort:      69,
@@ -522,7 +521,7 @@ func (*suite) TestAPIInfoAddsLocalhostWhenServingInfoPresent(c *gc.C) {
 func (*suite) TestAPIInfoAddsLocalhostWhenServingInfoPresentAndPreferIPv6On(c *gc.C) {
 	attrParams := attributeParams
 	attrParams.PreferIPv6 = true
-	servingInfo := state.StateServingInfo{
+	servingInfo := params.StateServingInfo{
 		Cert:           "old cert",
 		PrivateKey:     "old key",
 		StatePort:      69,
@@ -548,7 +547,7 @@ func (*suite) TestAPIInfoAddsLocalhostWhenServingInfoPresentAndPreferIPv6On(c *g
 func (*suite) TestMongoInfoHonorsPreferIPv6(c *gc.C) {
 	attrParams := attributeParams
 	attrParams.PreferIPv6 = true
-	servingInfo := state.StateServingInfo{
+	servingInfo := params.StateServingInfo{
 		Cert:           "old cert",
 		PrivateKey:     "old key",
 		StatePort:      69,
@@ -590,7 +589,7 @@ func (*suite) TestAPIInfoDoesntAddLocalhostWhenNoServingInfoPreferIPv6On(c *gc.C
 
 func (*suite) TestSetPassword(c *gc.C) {
 	attrParams := attributeParams
-	servingInfo := state.StateServingInfo{
+	servingInfo := params.StateServingInfo{
 		Cert:           "old cert",
 		PrivateKey:     "old key",
 		StatePort:      1234,

diff --git a/agent/bootstrap.go b/agent/bootstrap.go
@@ -105,7 +105,8 @@ func InitializeState(c ConfigSetter, envCfg *config.Config, machineCfg Bootstrap
 	if err = initAPIHostPorts(c, st, machineCfg.Addresses, servingInfo.APIPort); err != nil {
 		return nil, nil, err
 	}
-	if err := st.SetStateServingInfo(servingInfo); err != nil {
+	ssi := paramsStateServingInfoToStateStateServingInfo(servingInfo)
+	if err := st.SetStateServingInfo(ssi); err != nil {
 		return nil, nil, fmt.Errorf("cannot set state serving info: %v", err)
 	}
 	m, err := initConstraintsAndBootstrapMachine(c, st, machineCfg)
@@ -115,6 +116,17 @@ func InitializeState(c ConfigSetter, envCfg *config.Config, machineCfg Bootstrap
 	return st, m, nil
 }
 
+func paramsStateServingInfoToStateStateServingInfo(i params.StateServingInfo) state.StateServingInfo {
+	return state.StateServingInfo{
+		APIPort:        i.APIPort,
+		StatePort:      i.StatePort,
+		Cert:           i.Cert,
+		PrivateKey:     i.PrivateKey,
+		SharedSecret:   i.SharedSecret,
+		SystemIdentity: i.SystemIdentity,
+	}
+}
+
 func initConstraintsAndBootstrapMachine(c ConfigSetter, st *state.State, cfg BootstrapMachineConfig) (*state.Machine, error) {
 	if err := st.SetEnvironConstraints(cfg.Constraints); err != nil {
 		return nil, fmt.Errorf("cannot set initial environ constraints: %v", err)

diff --git a/agent/bootstrap_test.go b/agent/bootstrap_test.go
@@ -57,7 +57,7 @@ func (s *bootstrapSuite) TestInitializeState(c *gc.C) {
 		CACert:            testing.CACert,
 		Password:          pwHash,
 	}
-	servingInfo := state.StateServingInfo{
+	servingInfo := params.StateServingInfo{
 		Cert:           testing.ServerCert,
 		PrivateKey:     testing.ServerKey,
 		APIPort:        1234,
@@ -198,7 +198,7 @@ func (s *bootstrapSuite) TestInitializeStateFailsSecondTime(c *gc.C) {
 	}
 	cfg, err := agent.NewAgentConfig(configParams)
 	c.Assert(err, gc.IsNil)
-	cfg.SetStateServingInfo(state.StateServingInfo{
+	cfg.SetStateServingInfo(params.StateServingInfo{
 		APIPort:        5555,
 		StatePort:      s.mgoInst.Port(),
 		Cert:           "foo",

diff --git a/agent/format-1.16.go b/agent/format-1.16.go
@@ -12,7 +12,7 @@ import (
 	"github.com/juju/names"
 	goyaml "gopkg.in/yaml.v1"
 
-	"github.com/juju/juju/state"
+	"github.com/juju/juju/apiserver/params"
 	"github.com/juju/juju/version"
 )
 
@@ -113,7 +113,7 @@ func (formatter_1_16) unmarshal(data []byte) (*configInternal, error) {
 	}
 
 	if len(stateServerKey) != 0 {
-		config.servingInfo = &state.StateServingInfo{
+		config.servingInfo = &params.StateServingInfo{
 			Cert:       string(stateServerCert),
 			PrivateKey: string(stateServerKey),
 			APIPort:    format.APIPort,

diff --git a/agent/format-1.18.go b/agent/format-1.18.go
@@ -12,7 +12,6 @@ import (
 	goyaml "gopkg.in/yaml.v1"
 
 	"github.com/juju/juju/apiserver/params"
-	"github.com/juju/juju/state"
 	"github.com/juju/juju/version"
 )
 
@@ -110,7 +109,7 @@ func (formatter_1_18) unmarshal(data []byte) (*configInternal, error) {
 		}
 	}
 	if len(format.StateServerKey) != 0 {
-		config.servingInfo = &state.StateServingInfo{
+		config.servingInfo = &params.StateServingInfo{
 			Cert:           format.StateServerCert,
 			PrivateKey:     format.StateServerKey,
 			APIPort:        format.APIPort,

diff --git a/agent/format_whitebox_test.go b/agent/format_whitebox_test.go
@@ -12,7 +12,6 @@ import (
 	gc "launchpad.net/gocheck"
 
 	"github.com/juju/juju/apiserver/params"
-	"github.com/juju/juju/state"
 	"github.com/juju/juju/testing"
 	"github.com/juju/juju/version"
 )
@@ -84,7 +83,7 @@ func (*formatSuite) TestRead(c *gc.C) {
 }
 
 func (*formatSuite) TestReadWriteStateConfig(c *gc.C) {
-	servingInfo := state.StateServingInfo{
+	servingInfo := params.StateServingInfo{
 		Cert:       "some special cert",
 		PrivateKey: "a special key",
 		StatePort:  12345,

diff --git a/agent/identity_test.go b/agent/identity_test.go
@@ -10,7 +10,7 @@ import (
 	"github.com/juju/names"
 	gc "launchpad.net/gocheck"
 
-	"github.com/juju/juju/state"
+	"github.com/juju/juju/apiserver/params"
 	"github.com/juju/juju/testing"
 	"github.com/juju/juju/version"
 )
@@ -33,7 +33,7 @@ var attributeParams = AgentConfigParams{
 	Nonce:             "a nonce",
 }
 
-var servingInfo = state.StateServingInfo{
+var servingInfo = params.StateServingInfo{
 	Cert:           "old cert",
 	PrivateKey:     "old key",
 	StatePort:      69,

diff --git a/api/agent/machine_test.go b/api/agent/machine_test.go
@@ -36,14 +36,21 @@ var _ = gc.Suite(&servingInfoSuite{})
 func (s *servingInfoSuite) TestStateServingInfo(c *gc.C) {
 	st, _ := s.OpenAPIAsNewMachine(c, state.JobManageEnviron)
 
-	expected := state.StateServingInfo{
+	ssi := state.StateServingInfo{
 		PrivateKey:   "some key",
 		Cert:         "Some cert",
 		SharedSecret: "really, really secret",
 		APIPort:      33,
 		StatePort:    44,
 	}
-	s.State.SetStateServingInfo(expected)
+	expected := params.StateServingInfo{
+		PrivateKey:   ssi.PrivateKey,
+		Cert:         ssi.Cert,
+		SharedSecret: ssi.SharedSecret,
+		APIPort:      ssi.APIPort,
+		StatePort:    ssi.StatePort,
+	}
+	s.State.SetStateServingInfo(ssi)
 	info, err := st.Agent().StateServingInfo()
 	c.Assert(err, gc.IsNil)
 	c.Assert(info, jc.DeepEquals, expected)

diff --git a/api/agent/state.go b/api/agent/state.go
@@ -11,7 +11,6 @@ import (
 	"github.com/juju/juju/api/base"
 	"github.com/juju/juju/apiserver/params"
 	"github.com/juju/juju/instance"
-	"github.com/juju/juju/state"
 )
 
 // State provides access to an agent's view of the state.
@@ -44,23 +43,10 @@ func (st *State) getEntity(tag names.Tag) (*params.AgentGetEntitiesResult, error
 	return &results.Entities[0], nil
 }
 
-func (st *State) StateServingInfo() (state.StateServingInfo, error) {
+func (st *State) StateServingInfo() (params.StateServingInfo, error) {
 	var results params.StateServingInfo
 	err := st.facade.FacadeCall("StateServingInfo", nil, &results)
-	return paramsStateServingInfoToStateStateServingInfo(&results), err
-}
-
-// convert params.StateServingInfo to a state.StateServingInfo.
-// This avoids state having a dependency on api/params.
-func paramsStateServingInfoToStateStateServingInfo(si *params.StateServingInfo) state.StateServingInfo {
-	return state.StateServingInfo{
-		APIPort:        si.APIPort,
-		StatePort:      si.StatePort,
-		Cert:           si.Cert,
-		PrivateKey:     si.PrivateKey,
-		SharedSecret:   si.SharedSecret,
-		SystemIdentity: si.SystemIdentity,
-	}
+	return results, err
 }
 
 // IsMaster reports whether the connected machine

diff --git a/cmd/jujud/agent_test.go b/cmd/jujud/agent_test.go
@@ -377,7 +377,7 @@ func writeStateAgentConfig(c *gc.C, stateInfo *mongo.MongoInfo, dataDir string,
 			APIAddresses:      apiAddr,
 			CACert:            stateInfo.CACert,
 		},
-		state.StateServingInfo{
+		params.StateServingInfo{
 			Cert:       coretesting.ServerCert,
 			PrivateKey: coretesting.ServerKey,
 			StatePort:  gitjujutesting.MgoServer.Port(),

diff --git a/cmd/jujud/bootstrap_test.go b/cmd/jujud/bootstrap_test.go
@@ -177,7 +177,7 @@ func (s *BootstrapSuite) initBootstrapCommand(c *gc.C, jobs []params.MachineJob,
 			agent.MongoOplogSize: s.mongoOplogSize,
 		},
 	}
-	servingInfo := state.StateServingInfo{
+	servingInfo := params.StateServingInfo{
 		Cert:       "some cert",
 		PrivateKey: "some key",
 		APIPort:    3737,
@@ -218,7 +218,8 @@ func (s *BootstrapSuite) TestInitializeEnvironment(c *gc.C) {
 	c.Assert(len(servingInfo.SystemIdentity), gc.Not(gc.Equals), 0)
 	servingInfo.SharedSecret = ""
 	servingInfo.SystemIdentity = ""
-	c.Assert(servingInfo, jc.DeepEquals, expectInfo)
+	expect := paramsStateServingInfoToStateStateServingInfo(expectInfo)
+	c.Assert(servingInfo, jc.DeepEquals, expect)
 	expectDialAddrs := []string{fmt.Sprintf("127.0.0.1:%d", expectInfo.StatePort)}
 	gotDialAddrs := s.fakeEnsureMongo.initiateParams.DialInfo.Addrs
 	c.Assert(gotDialAddrs, gc.DeepEquals, expectDialAddrs)

diff --git a/cmd/jujud/machine.go b/cmd/jujud/machine.go
@@ -778,7 +778,8 @@ func (a *MachineAgent) ensureMongoServer(agentConfig agent.Config) (err error) {
 		if err != nil {
 			return err
 		}
-		if err := st.SetStateServingInfo(servingInfo); err != nil {
+		ssi := paramsStateServingInfoToStateStateServingInfo(servingInfo)
+		if err := st.SetStateServingInfo(ssi); err != nil {
 			st.Close()
 			return fmt.Errorf("cannot set state serving info: %v", err)
 		}
@@ -827,6 +828,17 @@ func (a *MachineAgent) ensureMongoServer(agentConfig agent.Config) (err error) {
 	return nil
 }
 
+func paramsStateServingInfoToStateStateServingInfo(i params.StateServingInfo) state.StateServingInfo {
+	return state.StateServingInfo{
+		APIPort:        i.APIPort,
+		StatePort:      i.StatePort,
+		Cert:           i.Cert,
+		PrivateKey:     i.PrivateKey,
+		SharedSecret:   i.SharedSecret,
+		SystemIdentity: i.SystemIdentity,
+	}
+}
+
 func (a *MachineAgent) ensureMongoAdminUser(agentConfig agent.Config) (added bool, err error) {
 	stateInfo, ok1 := agentConfig.MongoInfo()
 	servingInfo, ok2 := agentConfig.StateServingInfo()