Reviewers: mp+157836_code.launchpad.net,

Message:
Please take a look.

Description:
Fix ServiceDeploy panic when called via the API.

Fix the panic that happens when calling ServiceDeploy via the juju-core
API.

Add a global charm.CacheDir variable, in order to avoid the call to
config.JujuHomePath inside the CharmStore.Get method.

charm.CacheDir is originally empty, and is then set by the
machineAgent.Run
method.

Pre-implementation call with Roger.

https://code.launchpad.net/~frankban/juju-core/bug-1166224-service-deploy-panic/+merge/157836

(do not edit description out of merge proposal)


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

Affected files:
   A [revision details]
   M charm/repo.go
   M charm/repo_test.go
   M cmd/jujud/machine.go
   M cmd/jujud/machine_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: charm/repo.go
=== modified file 'charm/repo.go'
--- charm/repo.go	2013-04-03 23:29:07 +0000
+++ charm/repo.go	2013-04-08 16:36:35 +0000
@@ -7,7 +7,6 @@
  	"fmt"
  	"io"
  	"io/ioutil"
-	"launchpad.net/juju-core/environs/config"
  	"launchpad.net/juju-core/log"
  	"net/http"
  	"net/url"
@@ -16,6 +15,9 @@
  	"strings"
  )

+// CacheDir stores the charm cache directory path.
+var CacheDir string
+
  // InfoResponse is sent by the charm store in response to charm-info  
requests.
  type InfoResponse struct {
  	Revision int      `json:"revision"` // Zero is valid. Can't omitempty.
@@ -211,8 +213,11 @@

  // Get returns the charm referenced by curl.
  func (s *CharmStore) Get(curl *URL) (Charm, error) {
-	cachePath := config.JujuHomePath("cache")
-	if err := os.MkdirAll(cachePath, 0755); err != nil {
+	// MachineAgent.Run should have already set the CacheDir.
+	if CacheDir == "" {
+		panic("the charm cache directory path is empty")
+	}
+	if err := os.MkdirAll(CacheDir, 0755); err != nil {
  		return nil, err
  	}
  	rev, digest, err := s.revision(curl)
@@ -224,14 +229,14 @@
  	} else if curl.Revision != rev {
  		return nil, fmt.Errorf("charm: store returned charm with wrong revision  
for %q", curl.String())
  	}
-	path := filepath.Join(cachePath, Quote(curl.String())+".charm")
+	path := filepath.Join(CacheDir, Quote(curl.String())+".charm")
  	if verify(path, digest) != nil {
  		resp, err := http.Get(s.baseURL + "/charm/" +  
url.QueryEscape(curl.Path()))
  		if err != nil {
  			return nil, err
  		}
  		defer resp.Body.Close()
-		f, err := ioutil.TempFile(cachePath, "charm-download")
+		f, err := ioutil.TempFile(CacheDir, "charm-download")
  		if err != nil {
  			return nil, err
  		}


Index: charm/repo_test.go
=== modified file 'charm/repo_test.go'
--- charm/repo_test.go	2013-04-03 23:29:07 +0000
+++ charm/repo_test.go	2013-04-09 09:51:19 +0000
@@ -8,7 +8,6 @@
  	"io/ioutil"
  	. "launchpad.net/gocheck"
  	"launchpad.net/juju-core/charm"
-	"launchpad.net/juju-core/environs/config"
  	"launchpad.net/juju-core/log"
  	"launchpad.net/juju-core/testing"
  	"net"
@@ -150,23 +149,24 @@
  type StoreSuite struct {
  	server      *MockStore
  	store       *charm.CharmStore
-	oldJujuHome string
+	oldCacheDir string
  }

  var _ = Suite(&StoreSuite{})

  func (s *StoreSuite) SetUpSuite(c *C) {
  	s.server = NewMockStore(c)
+	s.oldCacheDir = charm.CacheDir
  }

  func (s *StoreSuite) SetUpTest(c *C) {
-	s.oldJujuHome = config.SetJujuHome(c.MkDir())
+	charm.CacheDir = c.MkDir()
  	s.store = charm.NewStore("http://127.0.0.1:4444")
  	s.server.downloads = nil
  }

  func (s *StoreSuite) TearDownSuite(c *C) {
-	config.SetJujuHome(s.oldJujuHome)
+	charm.CacheDir = s.oldCacheDir
  	s.server.lis.Close()
  }

@@ -245,12 +245,12 @@
  }

  func (s *StoreSuite) TestGetBadCache(c *C) {
-	c.Assert(os.Mkdir(config.JujuHomePath("cache"), 0777), IsNil)
+	c.Assert(os.Mkdir(filepath.Join(charm.CacheDir, "cache"), 0777), IsNil)
  	base := "cs:series/good"
  	curl := charm.MustParseURL(base)
  	revCurl := charm.MustParseURL(base + "-23")
  	name := charm.Quote(revCurl.String()) + ".charm"
-	err := ioutil.WriteFile(config.JujuHomePath("cache", name), nil, 0666)
+	err := ioutil.WriteFile(filepath.Join(charm.CacheDir, "cache", name),  
nil, 0666)
  	c.Assert(err, IsNil)
  	ch, err := s.store.Get(curl)
  	c.Assert(err, IsNil)


Index: cmd/jujud/machine.go
=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2013-04-05 13:47:17 +0000
+++ cmd/jujud/machine.go	2013-04-08 16:36:35 +0000
@@ -3,6 +3,7 @@
  import (
  	"fmt"
  	"launchpad.net/gnuflag"
+	"launchpad.net/juju-core/charm"
  	"launchpad.net/juju-core/cmd"
  	"launchpad.net/juju-core/environs/agent"
  	"launchpad.net/juju-core/log"
@@ -13,6 +14,7 @@
  	"launchpad.net/juju-core/worker/machiner"
  	"launchpad.net/juju-core/worker/provisioner"
  	"launchpad.net/tomb"
+	"path/filepath"
  	"time"
  )

@@ -58,6 +60,7 @@
  	if err := a.Conf.read(state.MachineTag(a.MachineId)); err != nil {
  		return err
  	}
+	charm.CacheDir = filepath.Join(a.Conf.DataDir, "charmcache")
  	defer log.Noticef("cmd/jujud: machine agent exiting")
  	defer a.tomb.Done()



Index: cmd/jujud/machine_test.go
=== modified file 'cmd/jujud/machine_test.go'
--- cmd/jujud/machine_test.go	2013-04-03 15:33:42 +0000
+++ cmd/jujud/machine_test.go	2013-04-09 09:51:19 +0000
@@ -3,6 +3,7 @@
  import (
  	"fmt"
  	. "launchpad.net/gocheck"
+	"launchpad.net/juju-core/charm"
  	"launchpad.net/juju-core/cmd"
  	"launchpad.net/juju-core/environs/agent"
  	"launchpad.net/juju-core/environs/dummy"
@@ -10,16 +11,26 @@
  	"launchpad.net/juju-core/state/api"
  	"launchpad.net/juju-core/state/watcher"
  	"launchpad.net/juju-core/testing"
+	"path/filepath"
  	"reflect"
  	"time"
  )

  type MachineSuite struct {
  	agentSuite
+	oldCacheDir string
  }

  var _ = Suite(&MachineSuite{})

+func (s *MachineSuite) SetUpSuite(c *C) {
+	s.oldCacheDir = charm.CacheDir
+}
+
+func (s *MachineSuite) TearDownSuite(c *C) {
+	charm.CacheDir = s.oldCacheDir
+}
+
  // primeAgent adds a new Machine to run the given jobs, and sets up the
  // machine agent's directory.  It returns the new machine, the
  // agent's configuration and the tools currently running.
@@ -72,7 +83,7 @@
  }

  func (s *MachineSuite) TestRunStop(c *C) {
-	m, _, _ := s.primeAgent(c, state.JobHostUnits)
+	m, ac, _ := s.primeAgent(c, state.JobHostUnits)
  	a := s.newAgent(c, m)
  	done := make(chan error)
  	go func() {
@@ -81,6 +92,7 @@
  	err := a.Stop()
  	c.Assert(err, IsNil)
  	c.Assert(<-done, IsNil)
+	c.Assert(charm.CacheDir, Equals, filepath.Join(ac.DataDir, "charmcache"))
  }

  func (s *MachineSuite) TestWithDeadMachine(c *C) {