Merge pull request juju#4539 from ericsnowcurrently/resources-unified…

…-poller Merge the resources polling worker into the charm revision updater. This patch combines the two workers. It cleans up some of the existing code in the process. (Review request: http://reviews.vapour.ws/r/3975/) Conflicts: cmd/juju/charmcmd/store.go cmd/juju/charmcmd/sub.go cmd/jujud/agent/machine.go cmd/jujud/agent/machine_test.go dependencies.tsv resource/resourceadapters/fakes.go
sinanawad · Mar 19, 2016 · e5c35f9 · e5c35f9
1 parent 06701e1
commit e5c35f9
Show file tree

Hide file tree

Showing 26 changed files with 902 additions and 652 deletions.
diff --git a/apiserver/charmrevisionupdater/handlers.go b/apiserver/charmrevisionupdater/handlers.go
@@ -0,0 +1,49 @@
+// Copyright 2016 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package charmrevisionupdater
+
+import (
+	"fmt"
+
+	"github.com/juju/errors"
+	"github.com/juju/names"
+
+	"github.com/juju/juju/charmstore"
+	"github.com/juju/juju/state"
+)
+
+// LatestCharmHandler exposes the functionality needed to deal with
+// the latest info (from the store) for a charm.
+type LatestCharmHandler interface {
+	// HandleLatest deals with the given charm info, treating it as the
+	// most up-to-date information for the charms most recent revision.
+	HandleLatest(names.ServiceTag, charmstore.CharmInfo) error
+}
+
+type newHandlerFunc func(*state.State) (LatestCharmHandler, error)
+
+var registeredHandlers = map[string]newHandlerFunc{}
+
+// RegisterLatestCharmHandler adds the factory func for the identified
+// handler to the handler registry.
+func RegisterLatestCharmHandler(name string, newHandler newHandlerFunc) error {
+	if _, ok := registeredHandlers[name]; ok {
+		msg := fmt.Sprintf(`"latest charm" handler %q already registered`, name)
+		return errors.NewAlreadyExists(nil, msg)
+	}
+	registeredHandlers[name] = newHandler
+	return nil
+}
+
+func createHandlers(st *state.State) ([]LatestCharmHandler, error) {
+	var handlers []LatestCharmHandler
+	for _, newHandler := range registeredHandlers {
+		handler, err := newHandler(st)
+		if err != nil {
+			return nil, errors.Trace(err)
+		}
+		handlers = append(handlers, handler)
+	}
+	return handlers, nil
+}
diff --git a/apiserver/charmrevisionupdater/testing/suite.go b/apiserver/charmrevisionupdater/testing/suite.go
@@ -15,6 +15,7 @@ import (
 	"gopkg.in/juju/charmstore.v5-unstable"
 
 	"github.com/juju/juju/apiserver/charmrevisionupdater"
+	jujucharmstore "github.com/juju/juju/charmstore"
 	jujutesting "github.com/juju/juju/juju/testing"
 	"github.com/juju/juju/state"
 	"github.com/juju/juju/testcharms"
@@ -66,9 +67,10 @@ func (s *CharmSuite) SetUpTest(c *gc.C) {
 	s.jcSuite.PatchValue(&charmrepo.CacheDir, c.MkDir())
 	// Patch the charm repo initializer function: it is replaced with a charm
 	// store repo pointing to the testing server.
-	s.jcSuite.PatchValue(&charmrevisionupdater.NewCharmStore, func(p charmrepo.NewCharmStoreParams) *charmrepo.CharmStore {
-		p.URL = s.Server.URL
-		return charmrepo.NewCharmStore(p)
+	s.jcSuite.PatchValue(&charmrevisionupdater.NewCharmStoreClientConfig, func() jujucharmstore.ClientConfig {
+		var config jujucharmstore.ClientConfig
+		config.URL = s.Server.URL
+		return config
 	})
 	s.charms = make(map[string]*state.Charm)
 }

diff --git a/apiserver/charmrevisionupdater/updater.go b/apiserver/charmrevisionupdater/updater.go
@@ -4,13 +4,12 @@
 package charmrevisionupdater
 
 import (
-	"github.com/juju/errors"
 	"github.com/juju/loggo"
 	"gopkg.in/juju/charm.v6-unstable"
-	"gopkg.in/juju/charmrepo.v2-unstable"
 
 	"github.com/juju/juju/apiserver/common"
 	"github.com/juju/juju/apiserver/params"
+	"github.com/juju/juju/charmstore"
 	"github.com/juju/juju/state"
 )
 
@@ -51,87 +50,117 @@ func NewCharmRevisionUpdaterAPI(
 // UpdateLatestRevisions retrieves the latest revision information from the charm store for all deployed charms
 // and records this information in state.
 func (api *CharmRevisionUpdaterAPI) UpdateLatestRevisions() (params.ErrorResult, error) {
-	// First get the uuid for the environment to use when querying the charm store.
-	env, err := api.state.Model()
-	if err != nil {
+	if err := api.updateLatestRevisions(); err != nil {
 		return params.ErrorResult{Error: common.ServerError(err)}, nil
 	}
-	uuid := env.UUID()
+	return params.ErrorResult{}, nil
+}
 
-	deployedCharms, err := fetchAllDeployedCharms(api.state)
+func (api *CharmRevisionUpdaterAPI) updateLatestRevisions() error {
+	// Get the handlers to use.
+	handlers, err := createHandlers(api.state)
 	if err != nil {
-		return params.ErrorResult{Error: common.ServerError(err)}, nil
+		return err
 	}
-	// Look up the revision information for all the deployed charms.
-	curls, err := retrieveLatestCharmInfo(deployedCharms, uuid)
+
+	// Look up the information for all the deployed charms. This is the
+	// "expensive" part.
+	latest, err := retrieveLatestCharmInfo(api.state)
 	if err != nil {
-		return params.ErrorResult{Error: common.ServerError(err)}, nil
+		return err
 	}
-	// Add the charms and latest revision info to state as charm placeholders.
-	for _, curl := range curls {
-		if err = api.state.AddStoreCharmPlaceholder(curl); err != nil {
-			return params.ErrorResult{Error: common.ServerError(err)}, nil
+
+	// Process the resulting info for each charm.
+	for _, info := range latest {
+		// First, add a charm placeholder to the model for each.
+		if err = api.state.AddStoreCharmPlaceholder(info.LatestURL()); err != nil {
+			return err
 		}
-	}
-	return params.ErrorResult{}, nil
-}
 
-// fetchAllDeployedCharms returns a map from service name to service
-// and a map from service name to unit name to unit.
-func fetchAllDeployedCharms(st *state.State) (map[string]*charm.URL, error) {
-	deployedCharms := make(map[string]*charm.URL)
-	services, err := st.AllServices()
-	if err != nil {
-		return nil, err
-	}
-	for _, s := range services {
-		url, _ := s.CharmURL()
-		// Record the basic charm information so it can be bulk processed later to
-		// get the available revision numbers from the repo.
-		baseCharm := url.WithRevision(-1)
-		deployedCharms[baseCharm.String()] = baseCharm
+		// Then run through the handlers.
+		serviceID := info.service.ServiceTag()
+		for _, handler := range handlers {
+			if err := handler.HandleLatest(serviceID, info.CharmInfo); err != nil {
+				return err
+			}
+		}
 	}
-	return deployedCharms, nil
+
+	return nil
 }
 
-// NewCharmStore instantiates a new charm store repository.
+// NewCharmStoreClientConfig returns the client config to use.
 // It is defined at top level for testing purposes.
-var NewCharmStore = charmrepo.NewCharmStore
+var NewCharmStoreClientConfig = func() charmstore.ClientConfig {
+	var config charmstore.ClientConfig
+	return config
+}
+
+// newCharmStoreClient instantiates a new charm store repository.
+func newCharmStoreClient(modelUUID string) (*charmstore.Client, error) {
+	// TODO(ericsnow) Use the Juju "HTTP context" once we have one.
+	config := NewCharmStoreClientConfig()
+	client := charmstore.NewClient(config)
+	return client.WithMetadata(charmstore.JujuMetadata{
+		ModelUUID: modelUUID,
+	})
+}
+
+type latestCharmInfo struct {
+	charmstore.CharmInfo
+	service *state.Service
+}
 
 // retrieveLatestCharmInfo looks up the charm store to return the charm URLs for the
 // latest revision of the deployed charms.
-func retrieveLatestCharmInfo(deployedCharms map[string]*charm.URL, uuid string) ([]*charm.URL, error) {
+func retrieveLatestCharmInfo(st *state.State) ([]latestCharmInfo, error) {
+	// First get the uuid for the environment to use when querying the charm store.
+	env, err := st.Model()
+	if err != nil {
+		return nil, err
+	}
+	modelUUID := env.UUID()
+
+	services, err := st.AllServices()
+	if err != nil {
+		return nil, err
+	}
+
 	var curls []*charm.URL
-	for _, curl := range deployedCharms {
+	var resultsIndexedServices []*state.Service
+	for _, service := range services {
+		curl, _ := service.CharmURL()
 		if curl.Schema == "local" {
 			// Version checking for charms from local repositories is not
 			// currently supported, since we don't yet support passing in
 			// a path to the local repo. This may change if the need arises.
 			continue
 		}
 		curls = append(curls, curl)
+		resultsIndexedServices = append(resultsIndexedServices, service)
 	}
 
-	// Do a bulk call to get the revision info for all charms.
-	logger.Infof("retrieving revision information for %d charms", len(curls))
-	repo := NewCharmStore(charmrepo.NewCharmStoreParams{})
-	repo = repo.WithJujuAttrs(map[string]string{
-		"environment_uuid": uuid,
-	})
-	revInfo, err := repo.Latest(curls...)
+	client, err := newCharmStoreClient(modelUUID)
+	if err != nil {
+		return nil, err
+	}
+
+	results, err := charmstore.LatestCharmInfo(client, curls)
 	if err != nil {
-		err = errors.Annotate(err, "finding charm revision info")
-		logger.Infof(err.Error())
 		return nil, err
 	}
-	var latestCurls []*charm.URL
-	for i, info := range revInfo {
-		curl := curls[i]
-		if info.Err == nil {
-			latestCurls = append(latestCurls, curl.WithRevision(info.Revision))
-		} else {
-			logger.Errorf("retrieving charm info for %s: %v", curl, info.Err)
+
+	var latest []latestCharmInfo
+	for i, result := range results {
+		if result.Error != nil {
+			logger.Errorf("retrieving charm info for %s: %v", curls[i], result.Error)
+			continue
 		}
+		service := resultsIndexedServices[i]
+		latest = append(latest, latestCharmInfo{
+			CharmInfo: result.CharmInfo,
+			service:   service,
+		})
 	}
-	return latestCurls, nil
+	return latest, nil
 }
diff --git a/apiserver/charmrevisionupdater/updater_test.go b/apiserver/charmrevisionupdater/updater_test.go
@@ -17,6 +17,7 @@ import (
 	"github.com/juju/juju/apiserver/charmrevisionupdater/testing"
 	"github.com/juju/juju/apiserver/common"
 	apiservertesting "github.com/juju/juju/apiserver/testing"
+	"github.com/juju/juju/charmstore"
 	jujutesting "github.com/juju/juju/juju/testing"
 	"github.com/juju/juju/state"
 )
@@ -164,9 +165,10 @@ func (s *charmVersionSuite) TestEnvironmentUUIDUsed(c *gc.C) {
 	defer srv.Close()
 
 	// Point the charm repo initializer to the testing server.
-	s.PatchValue(&charmrevisionupdater.NewCharmStore, func(p charmrepo.NewCharmStoreParams) *charmrepo.CharmStore {
-		p.URL = srv.URL
-		return charmrepo.NewCharmStore(p)
+	s.PatchValue(&charmrevisionupdater.NewCharmStoreClientConfig, func() charmstore.ClientConfig {
+		var config charmstore.ClientConfig
+		config.URL = srv.URL
+		return config
 	})
 
 	result, err := s.charmrevisionupdater.UpdateLatestRevisions()