Skip to content

Commit

Permalink
responding to review feedback, add tests, fix the tests that were the…
Browse files Browse the repository at this point in the history 
…re but didn't run
  • Loading branch information
@davecheney
davecheney committed Jul 15, 2014
1 parent a5f0220 commit 029f5c4
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 27 deletions.
6 changes: 6 additions & 0 deletions agent/agent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,12 @@ func NewAgentConfig(configParams AgentConfigParams) (ConfigSetterWriter, error)
if configParams.Tag == nil {
return nil, errors.Trace(requiredError("entity tag"))
}
switch configParams.Tag.(type) {
case names.MachineTag, names.UnitTag:
// these are the only two type of tags that can represent an agent
default:
return nil, errors.Errorf("entity tag must be MachineTag or UnitTag, got %T", configParams.Tag)
}
if configParams.UpgradedToVersion == version.Zero {
return nil, errors.Trace(requiredError("upgradedToVersion"))
}
Expand Down
60 changes: 43 additions & 17 deletions agent/agent_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,22 @@ var agentConfigTests = []struct {
about: "missing upgraded to version",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"), // a user called omg
Tag: names.NewMachineTag("1"),
},
checkErr: "upgradedToVersion not found in configuration",
}, {
about: "missing password",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
},
checkErr: "password not found in configuration",
}, {
about: "missing CA cert",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
Password: "sekrit",
},
Expand All @@ -69,7 +69,7 @@ var agentConfigTests = []struct {
about: "need either state or api addresses",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
Password: "sekrit",
CACert: "ca cert",
Expand All @@ -79,7 +79,7 @@ var agentConfigTests = []struct {
about: "invalid state address",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
Password: "sekrit",
CACert: "ca cert",
Expand All @@ -90,7 +90,7 @@ var agentConfigTests = []struct {
about: "invalid api address",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
Password: "sekrit",
CACert: "ca cert",
Expand All @@ -101,7 +101,7 @@ var agentConfigTests = []struct {
about: "good state addresses",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
Password: "sekrit",
CACert: "ca cert",
Expand All @@ -111,7 +111,7 @@ var agentConfigTests = []struct {
about: "good api addresses",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
Password: "sekrit",
CACert: "ca cert",
Expand All @@ -121,7 +121,7 @@ var agentConfigTests = []struct {
about: "both state and api addresses",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
Password: "sekrit",
CACert: "ca cert",
Expand All @@ -132,7 +132,7 @@ var agentConfigTests = []struct {
about: "everything...",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
Password: "sekrit",
UpgradedToVersion: version.Current.Number,
CACert: "ca cert",
Expand All @@ -144,7 +144,7 @@ var agentConfigTests = []struct {
about: "missing logDir sets default",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
Password: "sekrit",
UpgradedToVersion: version.Current.Number,
CACert: "ca cert",
Expand All @@ -155,15 +155,41 @@ var agentConfigTests = []struct {
inspectConfig: func(c *gc.C, cfg agent.Config) {
c.Check(cfg.LogDir(), gc.Equals, agent.DefaultLogDir)
},
}, {
about: "agentConfig must not be a User tag",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("admin"), // this is a joke, the admin user is nil.
UpgradedToVersion: version.Current.Number,
Password: "sekrit",
},
checkErr: "entity tag must be MachineTag or UnitTag, got names.UserTag",
}, {
about: "agentConfig accepts a Unit tag",
params: agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUnitTag("ubuntu/1"),
Password: "sekrit",
UpgradedToVersion: version.Current.Number,
CACert: "ca cert",
StateAddresses: []string{"localhost:1234"},
APIAddresses: []string{"localhost:1235"},
Nonce: "a nonce",
},
inspectConfig: func(c *gc.C, cfg agent.Config) {
c.Assert(cfg.Dir(), gc.Equals, "/data/dir/agents/unit-ubuntu-1")
},
}}

func (*suite) TestNewAgentConfig(c *gc.C) {

for i, test := range agentConfigTests {
c.Logf("%v: %s", i, test.about)
_, err := agent.NewAgentConfig(test.params)
config, err := agent.NewAgentConfig(test.params)
if test.checkErr == "" {
c.Assert(err, gc.IsNil)
if test.inspectConfig != nil {
test.inspectConfig(c, config)
}
} else {
c.Assert(err, gc.ErrorMatches, test.checkErr)
}
Expand All @@ -174,7 +200,7 @@ func (*suite) TestMigrate(c *gc.C) {
initialParams := agent.AgentConfigParams{
DataDir: c.MkDir(),
LogDir: c.MkDir(),
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
Nonce: "nonce",
Password: "secret",
UpgradedToVersion: version.MustParse("1.16.5"),
Expand Down Expand Up @@ -357,7 +383,7 @@ func (*suite) TestNewStateMachineConfig(c *gc.C) {

var attributeParams = agent.AgentConfigParams{
DataDir: "/data/dir",
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
Password: "sekrit",
CACert: "ca cert",
Expand All @@ -371,8 +397,8 @@ func (*suite) TestAttributes(c *gc.C) {
c.Assert(err, gc.IsNil)
c.Assert(conf.DataDir(), gc.Equals, "/data/dir")
c.Assert(conf.SystemIdentityPath(), gc.Equals, "/data/dir/system-identity")
c.Assert(conf.Tag(), gc.Equals, names.NewUserTag("omg"))
c.Assert(conf.Dir(), gc.Equals, "/data/dir/agents/user-omg")
c.Assert(conf.Tag(), gc.Equals, names.NewMachineTag("1"))
c.Assert(conf.Dir(), gc.Equals, "/data/dir/agents/machine-1")
c.Assert(conf.Nonce(), gc.Equals, "a nonce")
c.Assert(conf.UpgradedToVersion(), jc.DeepEquals, version.Current.Number)
}
Expand Down
8 changes: 4 additions & 4 deletions agent/format_whitebox_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var _ = gc.Suite(&formatSuite{})
// The agentParams are used by the specific formatter whitebox tests, and is
// located here for easy reuse.
var agentParams = AgentConfigParams{
Tag: names.NewUserTag("omg"),
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
Jobs: []params.MachineJob{params.JobHostUnits},
Password: "sekrit",
Expand All @@ -49,9 +49,9 @@ func (*formatSuite) TestWriteCommands(c *gc.C) {
commands, err := config.WriteCommands()
c.Assert(err, gc.IsNil)
c.Assert(commands, gc.HasLen, 3)
c.Assert(commands[0], gc.Matches, `mkdir -p '\S+/agents/user-omg'`)
c.Assert(commands[1], gc.Matches, `install -m 600 /dev/null '\S+/agents/user-omg/agent.conf'`)
c.Assert(commands[2], gc.Matches, `printf '%s\\n' '(.|\n)*' > '\S+/agents/user-omg/agent.conf'`)
c.Assert(commands[0], gc.Matches, `mkdir -p '\S+/agents/machine-1'`)
c.Assert(commands[1], gc.Matches, `install -m 600 /dev/null '\S+/agents/machine-1/agent.conf'`)
c.Assert(commands[2], gc.Matches, `printf '%s\\n' '(.|\n)*' > '\S+/agents/machine-1/agent.conf'`)
}

func (*formatSuite) TestWriteAgentConfig(c *gc.C) {
Expand Down
2 changes: 1 addition & 1 deletion agent/identity_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type identitySuite struct {
var _ = gc.Suite(&identitySuite{})

var attributeParams = AgentConfigParams{
Tag: names.NewUserTag("omg"), // the omg user
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
Password: "sekrit",
CACert: "ca cert",
Expand Down
1 change: 0 additions & 1 deletion cmd/jujud/machine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,6 @@ func openState(agentConfig agent.Config, dialOpts mongo.DialOpts) (_ *state.Stat
st.Close()
}
}()
// TODO(dfc)
m0, err := st.FindEntity(agentConfig.Tag().String())
if err != nil {
if errors.IsNotFound(err) {
Expand Down
1 change: 0 additions & 1 deletion cmd/jujud/machine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ func (s *commonMachineSuite) primeAgent(
c.Assert(err, gc.IsNil)
err = m.SetPassword(initialMachinePassword)
c.Assert(err, gc.IsNil)
// TODO(dfc)
tag := m.Tag()
if m.IsManager() {
err = m.SetMongoPassword(initialMachinePassword)
Expand Down
2 changes: 1 addition & 1 deletion worker/provisioner/container_initialisation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (s *ContainerSetupSuite) assertContainerProvisionerStarted(
startProvisionerWorker := func(runner worker.Runner, containerType instance.ContainerType,
pr *apiprovisioner.State, cfg agent.Config, broker environs.InstanceBroker) error {
c.Assert(containerType, gc.Equals, ctype)
c.Assert(cfg.Tag(), gc.Equals, host.Tag().String())
c.Assert(cfg.Tag(), gc.Equals, host.Tag())
provisionerStarted = true
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion worker/provisioner/kvm-broker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (s *kvmBrokerSuite) SetUpTest(c *gc.C) {
s.agentConfig, err = agent.NewAgentConfig(
agent.AgentConfigParams{
DataDir: "/not/used/here",
Tag: names.NewUserTag("tag"), // a user called tag
Tag: names.NewUnitTag("ubuntu/1"),
UpgradedToVersion: version.Current.Number,
Password: "dummy-secret",
Nonce: "nonce",
Expand Down
2 changes: 1 addition & 1 deletion worker/provisioner/lxc-broker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (s *lxcBrokerSuite) SetUpTest(c *gc.C) {
s.agentConfig, err = agent.NewAgentConfig(
agent.AgentConfigParams{
DataDir: "/not/used/here",
Tag: names.NewUserTag("tag"),
Tag: names.NewMachineTag("1"),
UpgradedToVersion: version.Current.Number,
Password: "dummy-secret",
Nonce: "nonce",
Expand Down

0 comments on commit 029f5c4

Please sign in to comment.