Rebase from master, fix conflicts, rework session closing

apiserver/images.go

@@ -88,11 +88,12 @@ func (h *imagesDownloadHandler) processGet(r *http.Request, resp http.ResponseWr
 	if err != nil {
 		return errors.Annotate(err, "error getting image from storage")
 	}
+	defer imageReader.Close()
 
 	// Stream the image to the caller.
 	logger.Debugf("streaming image from state blobstore: %+v", metadata)
 	resp.Header().Set("Content-Type", "application/x-tar-gz")
-	resp.Header().Set("Digest", fmt.Sprintf("%s=%s", apihttp.DIGEST_SHA, metadata.SHA256))
+	resp.Header().Set("Digest", fmt.Sprintf("%s=%s", apihttp.DigestSHA, metadata.SHA256))
 	resp.Header().Set("Content-Length", fmt.Sprint(metadata.Size))
 	resp.WriteHeader(http.StatusOK)
 	if _, err := io.Copy(resp, imageReader); err != nil {

apiserver/images_test.go

@@ -127,7 +127,7 @@ func (s *imageSuite) testDownload(c *gc.C, kind, series, arch, uuid string) []by
 	resp, err := s.downloadRequest(c, uuid, kind, series, arch)
 	c.Assert(err, gc.IsNil)
 	c.Check(resp.StatusCode, gc.Equals, http.StatusOK)
-	c.Check(resp.Header.Get("Digest"), gc.Equals, string(apihttp.DIGEST_SHA)+"="+s.imageChecksum)
+	c.Check(resp.Header.Get("Digest"), gc.Equals, string(apihttp.DigestSHA)+"="+s.imageChecksum)
 	c.Check(resp.Header.Get("Content-Type"), gc.Equals, s.archiveContentType)
 	c.Check(resp.Header.Get("Content-Length"), gc.Equals, fmt.Sprintf("%v", len(s.imageData)))
 

environs/cloudinit/cloudinit.go

@@ -214,6 +214,7 @@ func AddAptCommands(
 		// leave it to the networker worker.
 		c.AddPackage("bridge-utils")
 		c.AddPackage("rsyslog-gnutls")
+		c.AddPackage("cloud-image-utils")
 	}
 
 	// Write out the apt proxy settings

state/export_test.go

@@ -31,17 +31,18 @@ const (
 )
 
 var (
-	GetManagedStorage     = (*State).getManagedStorage
-	ToolstorageNewStorage = &toolstorageNewStorage
-	MachineIdLessThan     = machineIdLessThan
-	NewAddress            = newAddress
-	StateServerAvailable  = &stateServerAvailable
-	GetOrCreatePorts      = getOrCreatePorts
-	GetPorts              = getPorts
-	PortsGlobalKey        = portsGlobalKey
-	CurrentUpgradeId      = currentUpgradeId
-	NowToTheSecond        = nowToTheSecond
-	PickAddress           = &pickAddress
+	GetManagedStorage      = (*State).getManagedStorage
+	ToolstorageNewStorage  = &toolstorageNewStorage
+	ImageStorageNewStorage = &imageStorageNewStorage
+	MachineIdLessThan      = machineIdLessThan
+	NewAddress             = newAddress
+	StateServerAvailable   = &stateServerAvailable
+	GetOrCreatePorts       = getOrCreatePorts
+	GetPorts               = getPorts
+	PortsGlobalKey         = portsGlobalKey
+	CurrentUpgradeId       = currentUpgradeId
+	NowToTheSecond         = nowToTheSecond
+	PickAddress            = &pickAddress
 )
 
 type (

state/imagestorage/export_test.go

@@ -10,8 +10,8 @@ import (
 )
 
 // ManagedStorage returns the managedStorage attribute for the storage.
-func ManagedStorage(s Storage) blobstore.ManagedStorage {
-	return s.(*imageStorage).getManagedStorage()
+func ManagedStorage(s Storage, session *mgo.Session) blobstore.ManagedStorage {
+	return s.(*imageStorage).getManagedStorage(session)
 }
 
 // MetadataCollection returns the metadataCollection attribute for the storage.

state/imagestorage/image.go

@@ -61,15 +61,14 @@ var getManagedStorage = func(session *mgo.Session) blobstore.ManagedStorage {
 	return blobstore.NewManagedStorage(metadataDb, rs)
 }
 
-func (s *imageStorage) getManagedStorage() blobstore.ManagedStorage {
-	return getManagedStorage(s.blobDb.Session.Copy())
+func (s *imageStorage) getManagedStorage(session *mgo.Session) blobstore.ManagedStorage {
+	return getManagedStorage(session)
 }
 
-func (s *imageStorage) txnRunnerWithSession() (jujutxn.Runner, *mgo.Session) {
+func (s *imageStorage) txnRunner(session *mgo.Session) jujutxn.Runner {
 	db := s.metadataCollection.Database
-	session := db.Session.Copy()
 	runnerDb := db.With(session)
-	return txnRunner(runnerDb), session
+	return txnRunner(runnerDb)
 }
 
 // Override for testing.
@@ -79,7 +78,9 @@ var txnRunner = func(db *mgo.Database) jujutxn.Runner {
 
 // AddImage is defined on the Storage interface.
 func (s *imageStorage) AddImage(r io.Reader, metadata *Metadata) (resultErr error) {
-	managedStorage := s.getManagedStorage()
+	session := s.blobDb.Session.Copy()
+	defer session.Close()
+	managedStorage := s.getManagedStorage(session)
 	path := imagePath(metadata.Kind, metadata.Series, metadata.Arch, metadata.SHA256)
 	if err := managedStorage.PutForEnvironment(s.envUUID, path, r, metadata.Size); err != nil {
 		return errors.Annotate(err, "cannot store image")
@@ -140,8 +141,7 @@ func (s *imageStorage) AddImage(r io.Reader, metadata *Metadata) (resultErr erro
 		}
 		return []txn.Op{op}, nil
 	}
-	txnRunner, session := s.txnRunnerWithSession()
-	defer session.Close()
+	txnRunner := s.txnRunner(session)
 	err := txnRunner.Run(buildTxn)
 	if err != nil {
 		return errors.Annotate(err, "cannot store image metadata")
@@ -161,8 +161,11 @@ func (s *imageStorage) AddImage(r io.Reader, metadata *Metadata) (resultErr erro
 
 // DeleteImage is defined on the Storage interface.
 func (s *imageStorage) DeleteImage(metadata *Metadata) (resultErr error) {
+	session := s.blobDb.Session.Copy()
+	defer session.Close()
+	managedStorage := s.getManagedStorage(session)
 	path := imagePath(metadata.Kind, metadata.Series, metadata.Arch, metadata.SHA256)
-	if err := s.getManagedStorage().RemoveForEnvironment(s.envUUID, path); err != nil {
+	if err := managedStorage.RemoveForEnvironment(s.envUUID, path); err != nil {
 		return errors.Annotate(err, "cannot remove image blob")
 	}
 	// Remove the metadata.
@@ -174,8 +177,7 @@ func (s *imageStorage) DeleteImage(metadata *Metadata) (resultErr error) {
 		}
 		return []txn.Op{op}, nil
 	}
-	txnRunner, session := s.txnRunnerWithSession()
-	defer session.Close()
+	txnRunner := s.txnRunner(session)
 	err := txnRunner.Run(buildTxn)
 	// Metadata already removed, we don't care.
 	if err == mgo.ErrNotFound {
@@ -184,6 +186,18 @@ func (s *imageStorage) DeleteImage(metadata *Metadata) (resultErr error) {
 	return errors.Annotate(err, "cannot remove image metadata")
 }
 
+// imageCloser encapsulates an image reader and session
+// so that both are closed together.
+type imageCloser struct {
+	io.ReadCloser
+	session *mgo.Session
+}
+
+func (c *imageCloser) Close() error {
+	c.session.Close()
+	return c.ReadCloser.Close()
+}
+
 // Image is defined on the Storage interface.
 func (s *imageStorage) Image(kind, series, arch string) (*Metadata, io.ReadCloser, error) {
 	metadataDoc, err := s.imageMetadataDoc(s.envUUID, kind, series, arch)
@@ -194,7 +208,9 @@ func (s *imageStorage) Image(kind, series, arch string) (*Metadata, io.ReadClose
 	if err != nil {
 		return nil, nil, err
 	}
-	image, err := s.imageBlob(s.getManagedStorage(), metadataDoc.Path)
+	session := s.blobDb.Session.Copy()
+	managedStorage := s.getManagedStorage(session)
+	image, err := s.imageBlob(managedStorage, metadataDoc.Path)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -207,7 +223,11 @@ func (s *imageStorage) Image(kind, series, arch string) (*Metadata, io.ReadClose
 		SHA256:  metadataDoc.SHA256,
 		Created: created,
 	}
-	return metadata, image, nil
+	imageResult := &imageCloser{
+		image,
+		session,
+	}
+	return metadata, imageResult, nil
 }
 
 type imageMetadataDoc struct {

state/imagestorage/image_test.go

@@ -12,7 +12,6 @@ import (
 	stdtesting "testing"
 	"time"
 
-	"github.com/juju/blobstore"
 	"github.com/juju/errors"
 	gitjujutesting "github.com/juju/testing"
 	jc "github.com/juju/testing/checkers"
@@ -37,7 +36,6 @@ type ImageSuite struct {
 	mongo              *gitjujutesting.MgoInstance
 	session            *mgo.Session
 	storage            imagestorage.Storage
-	managedStorage     blobstore.ManagedStorage
 	metadataCollection *mgo.Collection
 	txnRunner          jujutxn.Runner
 }
@@ -51,7 +49,6 @@ func (s *ImageSuite) SetUpTest(c *gc.C) {
 	s.session, err = s.mongo.Dial()
 	c.Assert(err, gc.IsNil)
 	s.storage = imagestorage.NewStorage(s.session, "my-uuid")
-	s.managedStorage = imagestorage.ManagedStorage(s.storage)
 	s.metadataCollection = imagestorage.MetadataCollection(s.storage)
 	s.txnRunner = jujutxn.NewRunner(jujutxn.RunnerParams{Database: s.metadataCollection.Database})
 	s.patchTransactionRunner()
@@ -118,7 +115,8 @@ func (s *ImageSuite) TestImage(c *gc.C) {
 	c.Assert(err, jc.Satisfies, errors.IsNotFound)
 	c.Assert(err, gc.ErrorMatches, `resource at path "environs/my-uuid/path" not found`)
 
-	err = s.managedStorage.PutForEnvironment("my-uuid", "path", strings.NewReader("blah"), 4)
+	managedStorage := imagestorage.ManagedStorage(s.storage, s.session)
+	err = managedStorage.PutForEnvironment("my-uuid", "path", strings.NewReader("blah"), 4)
 	c.Assert(err, gc.IsNil)
 
 	metadata, r, err := s.storage.Image("lxc", "trusty", "amd64")
@@ -142,7 +140,8 @@ func (s *ImageSuite) TestAddImageRemovesExisting(c *gc.C) {
 	// Add a metadata doc and a blob at a known path, then
 	// call AddImage and ensure the original blob is removed.
 	s.addMetadataDoc(c, "lxc", "trusty", "amd64", 3, "hash(abc)", "path")
-	err := s.managedStorage.PutForEnvironment("my-uuid", "path", strings.NewReader("blah"), 4)
+	managedStorage := imagestorage.ManagedStorage(s.storage, s.session)
+	err := managedStorage.PutForEnvironment("my-uuid", "path", strings.NewReader("blah"), 4)
 	c.Assert(err, gc.IsNil)
 
 	addedMetadata := &imagestorage.Metadata{
@@ -157,7 +156,7 @@ func (s *ImageSuite) TestAddImageRemovesExisting(c *gc.C) {
 	c.Assert(err, gc.IsNil)
 
 	// old blob should be gone
-	_, _, err = s.managedStorage.GetForEnvironment("my-uuid", "path")
+	_, _, err = managedStorage.GetForEnvironment("my-uuid", "path")
 	c.Assert(err, jc.Satisfies, errors.IsNotFound)
 
 	s.assertImage(c, addedMetadata, "xyzzzz")
@@ -169,7 +168,8 @@ func (s *ImageSuite) TestAddImageRemovesExistingRemoveFails(c *gc.C) {
 	// the original blob, but does not return an error if it
 	// fails.
 	s.addMetadataDoc(c, "lxc", "trusty", "amd64", 3, "hash(abc)", "path")
-	err := s.managedStorage.PutForEnvironment("my-uuid", "path", strings.NewReader("blah"), 4)
+	managedStorage := imagestorage.ManagedStorage(s.storage, s.session)
+	err := managedStorage.PutForEnvironment("my-uuid", "path", strings.NewReader("blah"), 4)
 	c.Assert(err, gc.IsNil)
 
 	storage := imagestorage.NewStorage(s.session, "my-uuid")
@@ -186,7 +186,7 @@ func (s *ImageSuite) TestAddImageRemovesExistingRemoveFails(c *gc.C) {
 	c.Assert(err, gc.IsNil)
 
 	// old blob should still be there
-	r, _, err := s.managedStorage.GetForEnvironment("my-uuid", "path")
+	r, _, err := managedStorage.GetForEnvironment("my-uuid", "path")
 	c.Assert(err, gc.IsNil)
 	r.Close()
 
@@ -217,7 +217,8 @@ func (s *ImageSuite) TestAddImageRemovesBlobOnFailure(c *gc.C) {
 
 	path := fmt.Sprintf(
 		"images/%s-%s-%s:%s", addedMetadata.Kind, addedMetadata.Series, addedMetadata.Arch, addedMetadata.SHA256)
-	_, _, err = s.managedStorage.GetForEnvironment("my-uuid", path)
+	managedStorage := imagestorage.ManagedStorage(s.storage, s.session)
+	_, _, err = managedStorage.GetForEnvironment("my-uuid", path)
 	c.Assert(err, jc.Satisfies, errors.IsNotFound)
 }
 
@@ -239,7 +240,8 @@ func (s *ImageSuite) TestAddImageRemovesBlobOnFailureRemoveFails(c *gc.C) {
 	// blob should still be there, because the removal failed.
 	path := fmt.Sprintf(
 		"images/%s-%s-%s:%s", addedMetadata.Kind, addedMetadata.Series, addedMetadata.Arch, addedMetadata.SHA256)
-	r, _, err := s.managedStorage.GetForEnvironment("my-uuid", path)
+	managedStorage := imagestorage.ManagedStorage(s.storage, s.session)
+	r, _, err := managedStorage.GetForEnvironment("my-uuid", path)
 	c.Assert(err, gc.IsNil)
 	r.Close()
 }
@@ -284,7 +286,8 @@ func (s *ImageSuite) TestAddImageConcurrent(c *gc.C) {
 	addMetadata := func() {
 		err := s.storage.AddImage(strings.NewReader("0"), metadata0)
 		c.Assert(err, gc.IsNil)
-		r, _, err := s.managedStorage.GetForEnvironment("my-uuid", "images/lxc-trusty-amd64:0")
+		managedStorage := imagestorage.ManagedStorage(s.storage, s.session)
+		r, _, err := managedStorage.GetForEnvironment("my-uuid", "images/lxc-trusty-amd64:0")
 		c.Assert(err, gc.IsNil)
 		r.Close()
 	}
@@ -294,7 +297,8 @@ func (s *ImageSuite) TestAddImageConcurrent(c *gc.C) {
 	c.Assert(err, gc.IsNil)
 
 	// Blob added in before-hook should be removed.
-	_, _, err = s.managedStorage.GetForEnvironment("my-uuid", "images/lxc-trusty-amd64:0")
+	managedStorage := imagestorage.ManagedStorage(s.storage, s.session)
+	_, _, err = managedStorage.GetForEnvironment("my-uuid", "images/lxc-trusty-amd64:0")
 	c.Assert(err, jc.Satisfies, errors.IsNotFound)
 
 	s.assertImage(c, metadata1, "1")
@@ -322,7 +326,8 @@ func (s *ImageSuite) TestAddImageExcessiveContention(c *gc.C) {
 	// There should be no blobs apart from the last one added by the before-hook.
 	for _, metadata := range metadata[:3] {
 		path := fmt.Sprintf("images/%s-%s-%s:%s", metadata.Kind, metadata.Series, metadata.Arch, metadata.SHA256)
-		_, _, err = s.managedStorage.GetForEnvironment("my-uuid", path)
+		managedStorage := imagestorage.ManagedStorage(s.storage, s.session)
+		_, _, err = managedStorage.GetForEnvironment("my-uuid", path)
 		c.Assert(err, jc.Satisfies, errors.IsNotFound)
 	}
 
@@ -331,7 +336,8 @@ func (s *ImageSuite) TestAddImageExcessiveContention(c *gc.C) {
 
 func (s *ImageSuite) TestDeleteImage(c *gc.C) {
 	s.addMetadataDoc(c, "lxc", "trusty", "amd64", 3, "hash(abc)", "images/lxc-trusty-amd64:sha256")
-	err := s.managedStorage.PutForEnvironment("my-uuid", "images/lxc-trusty-amd64:sha256", strings.NewReader("blah"), 4)
+	managedStorage := imagestorage.ManagedStorage(s.storage, s.session)
+	err := managedStorage.PutForEnvironment("my-uuid", "images/lxc-trusty-amd64:sha256", strings.NewReader("blah"), 4)
 	c.Assert(err, gc.IsNil)
 
 	_, rc, err := s.storage.Image("lxc", "trusty", "amd64")
@@ -349,7 +355,7 @@ func (s *ImageSuite) TestDeleteImage(c *gc.C) {
 	err = s.storage.DeleteImage(metadata)
 	c.Assert(err, gc.IsNil)
 
-	_, _, err = s.managedStorage.GetForEnvironment("my-uuid", "images/lxc-trusty-amd64:sha256")
+	_, _, err = managedStorage.GetForEnvironment("my-uuid", "images/lxc-trusty-amd64:sha256")
 	c.Assert(err, jc.Satisfies, errors.IsNotFound)
 
 	_, _, err = s.storage.Image("lxc", "trusty", "amd64")

state/tools.go

@@ -14,14 +14,6 @@ var (
 	toolstorageNewStorage = toolstorage.NewStorage
 )
 
-// getManagedStorage returns a blobstore.ManagedStorage using the
-// specified UUID and mgo.Session.
-func (st *State) getManagedStorage(uuid string, session *mgo.Session) blobstore.ManagedStorage {
-	rs := blobstore.NewGridFS(blobstoreDB, uuid, session)
-	db := st.db.With(session)
-	return blobstore.NewManagedStorage(db, rs)
-}
-
 // ToolsStorage returns a new toolstorage.StorageCloser
 // that stores tools metadata in the "juju" database''
 // "toolsmetadata" collection.

worker/certupdater/certupdater.go

@@ -98,10 +98,10 @@ func (c *CertificateUpdater) Handle() error {
 		return errors.Annotate(err, "cannot add CA private key to environment config")
 	}
 
-	// We only want to include externally accessible addresses, so exclude local
-	// host. For backwards compatibility, we must include "juju-apiserver" as a
+	// For backwards compatibility, we must include "juju-apiserver" as a
 	// hostname as that is what clients specify as the hostname for verification.
-	serverAddrs := []string{"juju-apiserver"}
+	// We also explicitly include localhost.
+	serverAddrs := []string{"localhost", "juju-apiserver"}
 	for _, addr := range addresses {
 		if addr.Value == "localhost" {
 			continue