Merge lp:~thumper/juju-core/move-tools-dir into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Move the Dir function from agent/tools to agent.

Part of the agent refactoring, and this is the only bit
that touched parts outside of the agent package.

https://codereview.appspot.com/13256045/

Move the Dir function from agent/tools to agent.

Part of the agent refactoring, and this is the only bit
that touched parts outside of the agent package.

https://codereview.appspot.com/13256045/

[Tim Penhey (thumper)]

Reviewers: mp+182777_code.launchpad.net,

Message:
Please take a look.

Description:
Move the Dir function from agent/tools to agent.

Part of the agent refactoring, and this is the only bit
that touched parts outside of the agent package.

https://code.launchpad.net/~thumper/juju-core/move-tools-dir/+merge/182777

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M agent/agent.go
   M agent/tools/toolsdir.go
   M worker/deployer/simple.go
   M worker/deployer/simple_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: tarmac-20130827122923-qx3dpiw3118bbub2
+New revision: <email address hidden>

Index: agent/agent.go
=== modified file 'agent/agent.go'
--- agent/agent.go 2013-08-22 23:04:04 +0000
+++ agent/agent.go 2013-08-28 22:59:45 +0000
@@ -13,7 +13,6 @@
   "launchpad.net/goyaml"
   "launchpad.net/loggo"

- "launchpad.net/juju-core/agent/tools"
   "launchpad.net/juju-core/environs/config"
   "launchpad.net/juju-core/errors"
   "launchpad.net/juju-core/state"
@@ -198,10 +197,15 @@
   return conf, nil
  }

+// Dir returns the agent-specific data directory.
+func Dir(dataDir, agentName string) string {
+ return path.Join(dataDir, "agents", agentName)
+}
+
  // ReadConf reads configuration data for the given
  // entity from the given data directory.
  func ReadConf(dataDir, tag string) (Config, error) {
- dir := tools.Dir(dataDir, tag)
+ dir := Dir(dataDir, tag)
   data, err := ioutil.ReadFile(path.Join(dir, "agent.conf"))
   if err != nil {
    return nil, err
@@ -259,7 +263,7 @@

  // Dir returns the agent's directory.
  func (c *conf) Dir() string {
- return tools.Dir(c.dataDir, c.Tag())
+ return Dir(c.dataDir, c.Tag())
  }

  // Check checks that the configuration has all the required elements.

Index: agent/tools/toolsdir.go
=== modified file 'agent/tools/toolsdir.go'
--- agent/tools/toolsdir.go 2013-08-21 05:38:38 +0000
+++ agent/tools/toolsdir.go 2013-08-28 22:59:45 +0000
@@ -33,11 +33,6 @@
   return path.Join(dataDir, "tools", agentName)
  }

-// Dir returns the agent-specific data directory.
-func Dir(dataDir, agentName string) string {
- return path.Join(dataDir, "agents", agentName)
-}
-
  // UnpackTools reads a set of juju tools in gzipped tar-archive
  // format and unpacks them into the appropriate tools directory
  // within dataDir. If a valid tools directory already exists,

Index: worker/deployer/simple.go
=== modified file 'worker/deployer/simple.go'
--- worker/deployer/simple.go 2013-08-22 03:39:33 +0000
+++ worker/deployer/simple.go 2013-08-28 22:59:45 +0000
@@ -175,7 +175,7 @@
    return err
   }
   tag := names.UnitTag(unitName)
- agentDir := tools.Dir(ctx.dataDir, tag)
+ agentDir := agent.Dir(ctx.dataDir, tag)
   if err := os.RemoveAll(agentDir); err != nil {
    return err
   }

Index: worker/deployer/simple_test.go
=== modified file 'worker/deployer/simple_test.go'
--- worker/deployer/simple_test.go 2013-08-21 04:21:06 +0000
+++ w...

[John A Meinel (jameinel)]

LGTM though it feels like we are really missing a test case that Dir is
available and reporting something useful. (moving from agent/tools to
agent and nothing breaks seems a bit surprising)

https://codereview.appspot.com/13256045/

[Go Bot (go-bot)]

The attempt to merge lp:~thumper/juju-core/move-tools-dir into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 0.686s
ok launchpad.net/juju-core/agent/tools 0.248s
ok launchpad.net/juju-core/bzr 7.006s
ok launchpad.net/juju-core/cert 2.167s
ok launchpad.net/juju-core/charm 0.606s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.021s
ok launchpad.net/juju-core/cmd 0.224s
? launchpad.net/juju-core/cmd/builddb [no test files]
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 113.522s

----------------------------------------------------------------------
FAIL: machine_test.go:377: MachineSuite.TestManageStateServesAPI

[LOG] 6.53576 INFO juju environs/testing: uploading FAKE tools 1.13.3-precise-amd64
[LOG] 6.55261 INFO juju.environs.boostrap bootstrapping environment "dummyenv"
[LOG] 6.55263 INFO juju.environs.tools reading tools with major version 1
[LOG] 6.55270 INFO juju.environs.tools filtering tools by version: 1.13.3
[LOG] 6.55271 INFO juju.environs.tools filtering tools by series: precise
[LOG] 6.55272 DEBUG juju.environs.tools reading v1.* tools
[LOG] 6.55274 INFO juju.environs.tools falling back to public bucket
[LOG] 6.55275 DEBUG juju.environs.tools reading v1.* tools
[LOG] 6.55278 DEBUG juju.environs.tools found 1.13.3-precise-amd64
[LOG] 6.55282 INFO juju environs/dummy: would pick tools from 1.13.3-precise-amd64
[LOG] 6.57570 INFO juju.state opening state; mongo addresses: ["localhost:59092"]; entity ""
[LOG] 6.59128 INFO juju.state connection established
[LOG] 6.62404 INFO juju.state initializing environment
[LOG] 6.64511 INFO juju state/api: listening on "127.0.0.1:47458"
[LOG] 6.66289 INFO juju.state opening state; mongo addresses: ["localhost:59092"]; entity ""
[LOG] 6.66566 INFO juju.state connection established
[LOG] 6.68398 INFO juju juju: authorization error while connecting to state server; retrying
[LOG] 6.68406 INFO juju.state opening state; mongo addresses: ["localhost:59092"]; entity ""
[LOG] 6.68962 INFO juju.state connection established
[LOG] 6.71678 INFO juju state/api: dialing "wss://127.0.0.1:47458/"
[LOG] 6.71966 INFO juju state/api: connection established
[LOG] 6.71977 DEBUG juju rpc/jsoncodec: <- {"RequestId":1,"Type":"Admin","Request":"Login","Params":{"AuthTag":"user-admin","Password":"dummy-secret","Nonce":""}}
[LOG] 6.72009 DEBUG juju rpc/jsoncodec: -> {"RequestId":1,"Response":{}}
[LOG] 6.72036 INFO juju.container.lxc lxcObjectFactory replaced with mock lxc factory
[LOG] 6.72149 DEBUG juju rpc/jsoncodec: <- {"RequestId":2,"Type":"Pinger","Request":"Ping","Params":{}}
[LOG] 6.72155 DEBUG juju rpc/jsoncodec: -> {"RequestId":2,"Response":{}}
[LOG] 6.80245 INFO juju machine agent machine-0 start
[LOG] 6.81650 INFO juju Starting StateWorker for machine-0
[LOG] 6.81656 INFO juju worker: start "state"
[LOG] 6.81658 INFO juju.state opening state; mongo addresses: ["localhost:59092"]; entity "machine-0"
[LOG] 6.81685 INFO juju worker: start "api"
[LOG] 6.81718 INFO juju state/api: dialing "wss://local...

=== modified file 'agent/agent.go'
--- agent/agent.go	2013-08-22 23:04:04 +0000
+++ agent/agent.go	2013-08-28 23:07:19 +0000
@@ -13,7 +13,6 @@
 	"launchpad.net/goyaml"
 	"launchpad.net/loggo"
 
-	"launchpad.net/juju-core/agent/tools"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/errors"
 	"launchpad.net/juju-core/state"
@@ -198,10 +197,15 @@
 	return conf, nil
 }
 
+// Dir returns the agent-specific data directory.
+func Dir(dataDir, agentName string) string {
+	return path.Join(dataDir, "agents", agentName)
+}
+
 // ReadConf reads configuration data for the given
 // entity from the given data directory.
 func ReadConf(dataDir, tag string) (Config, error) {
-	dir := tools.Dir(dataDir, tag)
+	dir := Dir(dataDir, tag)
 	data, err := ioutil.ReadFile(path.Join(dir, "agent.conf"))
 	if err != nil {
 		return nil, err
@@ -259,7 +263,7 @@
 
 // Dir returns the agent's directory.
 func (c *conf) Dir() string {
-	return tools.Dir(c.dataDir, c.Tag())
+	return Dir(c.dataDir, c.Tag())
 }
 
 // Check checks that the configuration has all the required elements.

=== modified file 'agent/tools/toolsdir.go'
--- agent/tools/toolsdir.go	2013-08-21 05:38:38 +0000
+++ agent/tools/toolsdir.go	2013-08-28 23:07:19 +0000
@@ -33,11 +33,6 @@
 	return path.Join(dataDir, "tools", agentName)
 }
 
-// Dir returns the agent-specific data directory.
-func Dir(dataDir, agentName string) string {
-	return path.Join(dataDir, "agents", agentName)
-}
-
 // UnpackTools reads a set of juju tools in gzipped tar-archive
 // format and unpacks them into the appropriate tools directory
 // within dataDir. If a valid tools directory already exists,

=== modified file 'worker/deployer/simple.go'
--- worker/deployer/simple.go	2013-08-22 03:39:33 +0000
+++ worker/deployer/simple.go	2013-08-28 23:07:19 +0000
@@ -175,7 +175,7 @@
 		return err
 	}
 	tag := names.UnitTag(unitName)
-	agentDir := tools.Dir(ctx.dataDir, tag)
+	agentDir := agent.Dir(ctx.dataDir, tag)
 	if err := os.RemoveAll(agentDir); err != nil {
 		return err
 	}

=== modified file 'worker/deployer/simple_test.go'
--- worker/deployer/simple_test.go	2013-08-21 04:21:06 +0000
+++ worker/deployer/simple_test.go	2013-08-28 23:07:19 +0000
@@ -189,7 +189,7 @@
 func (fix *SimpleToolsFixture) paths(tag string) (confPath, agentDir, toolsDir, syslogConfPath string) {
 	confName := fmt.Sprintf("jujud-%s.conf", tag)
 	confPath = filepath.Join(fix.initDir, confName)
-	agentDir = tools.Dir(fix.dataDir, tag)
+	agentDir = agent.Dir(fix.dataDir, tag)
 	toolsDir = tools.ToolsDir(fix.dataDir, tag)
 	syslogConfPath = filepath.Join(fix.syslogConfigDir, fmt.Sprintf("26-juju-%s.conf", tag))
 	return