Fire upgrade charm when resources change

api/uniter/service.go

@@ -84,6 +84,29 @@ func (s *Service) Refresh() error {
 	return nil
 }
 
+// CharmModifiedVersion increments every time the charm, or any part of it, is
+// changed in some way.
+func (s *Service) CharmModifiedVersion() (int, error) {
+	var results params.IntResults
+	args := params.Entities{
+		Entities: []params.Entity{{Tag: s.tag.String()}},
+	}
+	err := s.st.facade.FacadeCall("CharmModifiedVersion", args, &results)
+	if err != nil {
+		return -1, err
+	}
+
+	if len(results.Results) != 1 {
+		return -1, fmt.Errorf("expected 1 result, got %d", len(results.Results))
+	}
+	result := results.Results[0]
+	if result.Error != nil {
+		return -1, result.Error
+	}
+
+	return result.Value, nil
+}
+
 // CharmURL returns the service's charm URL, and whether units should
 // upgrade to the charm with that URL even if they are in an error
 // state (force flag).

apiserver/params/internal.go

@@ -166,6 +166,18 @@ type BoolResults struct {
 	Results []BoolResult
 }
 
+// IntResults holds multiple results with an int in each.
+type IntResults struct {
+	Results []IntResult
+}
+
+// IntResult holds the result of an API call that returns a
+// int or an error.
+type IntResult struct {
+	Error *Error
+	Value int
+}
+
 // Settings holds relation settings names and values.
 type Settings map[string]string
 

apiserver/uniter/uniter.go

@@ -556,6 +556,56 @@ func (u *UniterAPIV3) HasSubordinates(args params.Entities) (params.BoolResults,
 	return result, nil
 }
 
+// CharmModifiedVersion returns the most CharmModifiedVersion for all given
+// units or services.
+func (u *UniterAPIV3) CharmModifiedVersion(args params.Entities) (params.IntResults, error) {
+	results := params.IntResults{
+		Results: make([]params.IntResult, len(args.Entities)),
+	}
+
+	accessUnitOrService := common.AuthEither(u.accessUnit, u.accessService)
+	canAccess, err := accessUnitOrService()
+	if err != nil {
+		return results, err
+	}
+	for i, entity := range args.Entities {
+		ver, err := u.charmModifiedVersion(entity.Tag, canAccess)
+		if err != nil {
+			results.Results[i].Error = common.ServerError(err)
+			continue
+		}
+		results.Results[i].Value = ver
+	}
+	return results, nil
+}
+
+func (u *UniterAPIV3) charmModifiedVersion(tagStr string, canAccess func(names.Tag) bool) (int, error) {
+	tag, err := names.ParseTag(tagStr)
+	if err != nil {
+		return -1, common.ErrPerm
+	}
+	if !canAccess(tag) {
+		return -1, common.ErrPerm
+	}
+	unitOrService, err := u.st.FindEntity(tag)
+	if err != nil {
+		return -1, err
+	}
+	var service *state.Service
+	switch entity := unitOrService.(type) {
+	case *state.Service:
+		service = entity
+	case *state.Unit:
+		service, err = entity.Service()
+		if err != nil {
+			return -1, err
+		}
+	default:
+		return -1, errors.BadRequestf("type %t does not have a CharmModifiedVersion", entity)
+	}
+	return service.CharmModifiedVersion(), nil
+}
+
 // CharmURL returns the charm URL for all given units or services.
 func (u *UniterAPIV3) CharmURL(args params.Entities) (params.StringBoolResults, error) {
 	result := params.StringBoolResults{

resource/persistence/persistence.go

@@ -29,6 +29,10 @@ type PersistenceBase interface {
 	// Run runs the transaction generated by the provided factory
 	// function. It may be retried several times.
 	Run(transactions jujutxn.TransactionSource) error
+
+	// IncCharmModifiedVersionOps returns the operations necessary to increment
+	// the CharmModifiedVersion field for the given service.
+	IncCharmModifiedVersionOps(serviceID string) []txn.Op
 }
 
 // Persistence provides the persistence functionality for the
@@ -183,6 +187,13 @@ func (p Persistence) SetResource(res resource.Resource) error {
 			// Either insert or update will work so we should not get here.
 			return nil, errors.New("setting the resource failed")
 		}
+		// for any update to a non-pending resource, we increment the
+		// CharmModifiedVersion on the service, since resources are integral to
+		// the high level "version" of the charm.
+		if res.PendingID == "" {
+			incOps := p.base.IncCharmModifiedVersionOps(res.ServiceID)
+			ops = append(ops, incOps...)
+		}
 		return ops, nil
 	}
 	if err := p.base.Run(buildTxn); err != nil {

resource/persistence/persistence_test.go

@@ -239,9 +239,11 @@ func (s *PersistenceSuite) TestSetResourceOkay(c *gc.C) {
 	s.stub.CheckCallNames(c,
 		"One",
 		"Run",
+		"IncCharmModifiedVersionOps",
 		"RunTransaction",
 	)
-	s.stub.CheckCall(c, 2, "RunTransaction", []txn.Op{{
+	s.stub.CheckCall(c, 2, "IncCharmModifiedVersionOps", servicename)
+	s.stub.CheckCall(c, 3, "RunTransaction", []txn.Op{{
 		C:      "resources",
 		Id:     "resource#a-service/spam",
 		Assert: txn.DocMissing,
@@ -266,9 +268,10 @@ func (s *PersistenceSuite) TestSetResourceNotFound(c *gc.C) {
 	s.stub.CheckCallNames(c,
 		"One",
 		"Run",
+		"IncCharmModifiedVersionOps",
 		"RunTransaction",
 	)
-	s.stub.CheckCall(c, 2, "RunTransaction", []txn.Op{{
+	s.stub.CheckCall(c, 3, "RunTransaction", []txn.Op{{
 		C:      "resources",
 		Id:     "resource#a-service/spam",
 		Assert: txn.DocMissing,

resource/persistence/staged.go

@@ -76,6 +76,15 @@ func (staged StagedResource) Activate() error {
 		}
 		// No matter what, we always remove any staging.
 		ops = append(ops, newRemoveStagedOps(staged.id)...)
+
+		// When we activate a non-pending staged resource, we increment the
+		// CharmModifiedVersion on the service, since resources are integral to
+		// the high level "version" of the charm.
+		if staged.stored.PendingID == "" {
+			incOps := staged.base.IncCharmModifiedVersionOps(staged.stored.ServiceID)
+			ops = append(ops, incOps...)
+		}
+
 		return ops, nil
 	}
 	if err := staged.base.Run(buildTxn); err != nil {

resource/persistence/staged_test.go

@@ -106,8 +106,9 @@ func (s *StagedResourceSuite) TestActivateOkay(c *gc.C) {
 	err := staged.Activate()
 	c.Assert(err, jc.ErrorIsNil)
 
-	s.stub.CheckCallNames(c, "Run", "RunTransaction")
-	s.stub.CheckCall(c, 1, "RunTransaction", []txn.Op{{
+	s.stub.CheckCallNames(c, "Run", "IncCharmModifiedVersionOps", "RunTransaction")
+	s.stub.CheckCall(c, 1, "IncCharmModifiedVersionOps", "a-service")
+	s.stub.CheckCall(c, 2, "RunTransaction", []txn.Op{{
 		C:      "resources",
 		Id:     "resource#a-service/spam",
 		Assert: txn.DocMissing,
@@ -127,8 +128,9 @@ func (s *StagedResourceSuite) TestActivateExists(c *gc.C) {
 	err := staged.Activate()
 	c.Assert(err, jc.ErrorIsNil)
 
-	s.stub.CheckCallNames(c, "Run", "RunTransaction", "RunTransaction")
-	s.stub.CheckCall(c, 1, "RunTransaction", []txn.Op{{
+	s.stub.CheckCallNames(c, "Run", "IncCharmModifiedVersionOps", "RunTransaction", "IncCharmModifiedVersionOps", "RunTransaction")
+	s.stub.CheckCall(c, 1, "IncCharmModifiedVersionOps", "a-service")
+	s.stub.CheckCall(c, 2, "RunTransaction", []txn.Op{{
 		C:      "resources",
 		Id:     "resource#a-service/spam",
 		Assert: txn.DocMissing,
@@ -138,7 +140,8 @@ func (s *StagedResourceSuite) TestActivateExists(c *gc.C) {
 		Id:     "resource#a-service/spam#staged",
 		Remove: true,
 	}})
-	s.stub.CheckCall(c, 2, "RunTransaction", []txn.Op{{
+	s.stub.CheckCall(c, 3, "IncCharmModifiedVersionOps", "a-service")
+	s.stub.CheckCall(c, 4, "RunTransaction", []txn.Op{{
 		C:      "resources",
 		Id:     "resource#a-service/spam",
 		Assert: txn.DocExists,

resource/persistence/stubs_test.go

@@ -15,6 +15,7 @@ type stubStatePersistence struct {
 
 	docs      []resourceDoc
 	ReturnOne resourceDoc
+	incOps    []txn.Op
 }
 
 func (s stubStatePersistence) One(collName, id string, doc interface{}) error {
@@ -52,6 +53,12 @@ func (s stubStatePersistence) Run(buildTxn jujutxn.TransactionSource) error {
 	return nil
 }
 
+func (s stubStatePersistence) IncCharmModifiedVersionOps(serviceID string) []txn.Op {
+	s.stub.AddCall("IncCharmModifiedVersionOps", serviceID)
+
+	return s.incOps
+}
+
 // See github.com/juju/txn.transactionRunner.Run.
 func (s stubStatePersistence) run(buildTxn jujutxn.TransactionSource) error {
 	for i := 0; ; i++ {

state/persistence.go

@@ -7,6 +7,7 @@ import (
 	"github.com/juju/errors"
 	jujutxn "github.com/juju/txn"
 	"gopkg.in/mgo.v2"
+	"gopkg.in/mgo.v2/txn"
 
 	"github.com/juju/juju/state/storage"
 )
@@ -27,6 +28,10 @@ type Persistence interface {
 
 	// NewStorage returns a new blob storage for the environment.
 	NewStorage() storage.Storage
+
+	// IncCharmModifiedVersionOps returns the operations necessary to increment
+	// the CharmModifiedVersion field for the given service.
+	IncCharmModifiedVersionOps(serviceID string) []txn.Op
 }
 
 type statePersistence struct {
@@ -80,3 +85,9 @@ func (sp *statePersistence) NewStorage() storage.Storage {
 	store := storage.NewStorage(envUUID, session)
 	return store
 }
+
+// IncCharmModifiedVersionOps returns the operations necessary to increment the
+// CharmModifiedVersion field for the given service.
+func (sp *statePersistence) IncCharmModifiedVersionOps(serviceID string) []txn.Op {
+	return incCharmModifiedVersionOps(serviceID)
+}

state/service.go

@@ -33,21 +33,22 @@ type Service struct {
 // serviceDoc represents the internal state of a service in MongoDB.
 // Note the correspondence with ServiceInfo in apiserver.
 type serviceDoc struct {
-	DocID             string     `bson:"_id"`
-	Name              string     `bson:"name"`
-	ModelUUID         string     `bson:"model-uuid"`
-	Series            string     `bson:"series"`
-	Subordinate       bool       `bson:"subordinate"`
-	CharmURL          *charm.URL `bson:"charmurl"`
-	ForceCharm        bool       `bson:"forcecharm"`
-	Life              Life       `bson:"life"`
-	UnitCount         int        `bson:"unitcount"`
-	RelationCount     int        `bson:"relationcount"`
-	Exposed           bool       `bson:"exposed"`
-	MinUnits          int        `bson:"minunits"`
-	OwnerTag          string     `bson:"ownertag"`
-	TxnRevno          int64      `bson:"txn-revno"`
-	MetricCredentials []byte     `bson:"metric-credentials"`
+	DocID                string     `bson:"_id"`
+	Name                 string     `bson:"name"`
+	ModelUUID            string     `bson:"model-uuid"`
+	Series               string     `bson:"series"`
+	Subordinate          bool       `bson:"subordinate"`
+	CharmURL             *charm.URL `bson:"charmurl"`
+	CharmModifiedVersion int        `bson:"charmmodifiedversion"`
+	ForceCharm           bool       `bson:"forcecharm"`
+	Life                 Life       `bson:"life"`
+	UnitCount            int        `bson:"unitcount"`
+	RelationCount        int        `bson:"relationcount"`
+	Exposed              bool       `bson:"exposed"`
+	MinUnits             int        `bson:"minunits"`
+	OwnerTag             string     `bson:"ownertag"`
+	TxnRevno             int64      `bson:"txn-revno"`
+	MetricCredentials    []byte     `bson:"metric-credentials"`
 }
 
 func newService(st *State, doc *serviceDoc) *Service {
@@ -286,6 +287,12 @@ func (s *Service) IsPrincipal() bool {
 	return !s.doc.Subordinate
 }
 
+// CharmModifiedVersion increases whenever the service's charm is changed in any
+// way.
+func (s *Service) CharmModifiedVersion() int {
+	return s.doc.CharmModifiedVersion
+}
+
 // CharmURL returns the service's charm URL, and whether units should upgrade
 // to the charm with that URL even if they are in an error state.
 func (s *Service) CharmURL() (curl *charm.URL, force bool) {
@@ -524,6 +531,9 @@ func (s *Service) changeCharmOps(ch *Charm, forceUnits bool) ([]txn.Op, error) {
 			Update: bson.D{{"$set", bson.D{{"charmurl", ch.URL()}, {"forcecharm", forceUnits}}}},
 		},
 	}...)
+
+	ops = append(ops, incCharmModifiedVersionOps(s.doc.DocID)...)
+
 	// Add any extra peer relations that need creation.
 	newPeers := s.extraPeerRelations(ch.Meta())
 	peerOps, err := s.st.addPeerRelationsOps(s.doc.Name, newPeers)
@@ -637,6 +647,7 @@ func (s *Service) SetCharm(ch *Charm, forceSeries, forceUnits bool) error {
 	services, closer := s.st.getCollection(servicesC)
 	defer closer()
 
+	var charmModifiedVersion int
 	buildTxn := func(attempt int) ([]txn.Op, error) {
 		if attempt > 0 {
 			// NOTE: We're explicitly allowing SetCharm to succeed
@@ -652,31 +663,45 @@ func (s *Service) SetCharm(ch *Charm, forceSeries, forceUnits bool) error {
 		}
 		// Make sure the service doesn't have this charm already.
 		sel := bson.D{{"_id", s.doc.DocID}, {"charmurl", ch.URL()}}
-		var ops []txn.Op
-		if count, err := services.Find(sel).Count(); err != nil {
+		count, err := services.Find(sel).Count()
+		if err != nil {
 			return nil, errors.Trace(err)
-		} else if count == 1 {
+		}
+		var doc serviceDoc
+		if err := services.FindId(s.doc.DocID).One(&doc); err != nil {
+			return nil, errors.Trace(err)
+		}
+		charmModifiedVersion = doc.CharmModifiedVersion
+		ops := []txn.Op{{
+			C:      servicesC,
+			Id:     s.doc.DocID,
+			Assert: bson.D{{"charmmodifiedversion", charmModifiedVersion}},
+		}}
+		if count > 0 {
 			// Charm URL already set; just update the force flag.
 			sameCharm := bson.D{{"charmurl", ch.URL()}}
-			ops = []txn.Op{{
+			ops = append(ops, []txn.Op{{
 				C:      servicesC,
 				Id:     s.doc.DocID,
 				Assert: append(notDeadDoc, sameCharm...),
 				Update: bson.D{{"$set", bson.D{{"forcecharm", forceUnits}}}},
-			}}
+			}}...)
 		} else {
 			// Change the charm URL.
-			ops, err = s.changeCharmOps(ch, forceUnits)
+			chng, err := s.changeCharmOps(ch, forceUnits)
 			if err != nil {
 				return nil, errors.Trace(err)
 			}
+			ops = append(ops, chng...)
 		}
+
 		return ops, nil
 	}
 	err := s.st.run(buildTxn)
 	if err == nil {
 		s.doc.CharmURL = ch.URL()
 		s.doc.ForceCharm = forceUnits
+		s.doc.CharmModifiedVersion = charmModifiedVersion + 1
 	}
 	return err
 }
@@ -1468,3 +1493,14 @@ var statusServerities = map[Status]int{
 	StatusActive:      50,
 	StatusUnknown:     40,
 }
+
+// incCharmModifiedVersionOps returns the operations necessary to increment
+// the CharmModifiedVersion field for the given service.
+func incCharmModifiedVersionOps(serviceID string) []txn.Op {
+	return []txn.Op{{
+		C:      servicesC,
+		Id:     serviceID,
+		Assert: txn.DocExists,
+		Update: bson.D{{"$inc", bson.D{{"charmmodifiedversion", 1}}}},
+	}}
+}