state: service API changes

state/service.go

 package state
 
 import (
         "errors"
         "fmt"
         "labix.org/v2/mgo"
         "labix.org/v2/mgo/bson"
         "labix.org/v2/mgo/txn"
         "launchpad.net/juju-core/charm"
         "launchpad.net/juju-core/trivial"
@@ skipping 35 matching lines @@
 
 // Life returns whether the service is Alive, Dying or Dead.
 func (s *Service) Life() Life {
         return s.doc.Life
 }
 
 var errRefresh = errors.New("cannot determine relation destruction operations; please refresh the service")
 
 // Destroy ensures that the service and all its relations will be removed at
 // some point; if the service has no units, and no relation involving the
-// service has any units in scope, they will all be removed immediately.
+// service has any units in scope, they are all removed immediately.
 func (s *Service) Destroy() (err error) {
         defer trivial.ErrorContextf(&err, "cannot destroy service %q", s)
         defer func() {
                 if err != nil {
                         // This is a white lie; the document might actually be removed.
                         s.doc.Life = Dying
                 }
         }()
         svc := &Service{s.st, s.doc}
         for i := 0; i < 5; i++ {
                 ops, err := svc.destroyOps()
                 switch {
                 case err == errRefresh:
+                case err == errAlreadyDying:
+                        return nil
                 case err != nil:
                         return err
-                case len(ops) == 0:
-                        return nil
                 default:
                         if err := svc.st.runner.Run(ops, "", nil); err != txn.ErrAborted {
                                 return err
                         }
                 }
                 if err := svc.Refresh(); IsNotFound(err) {
                         return nil
                 } else if err != nil {
                         return err
                 }
         }
         return ErrExcessiveContention
 }
 
 // destroyOps returns the operations required to destroy the service. If it
 // returns errRefresh, the service should be refreshed and the destruction
 // operations recalculated.
 func (s *Service) destroyOps() ([]txn.Op, error) {
         if s.doc.Life == Dying {
-                return nil, nil
+                return nil, errAlreadyDying
         }
         rels, err := s.Relations()
         if err != nil {
                 return nil, err
         }
         if len(rels) != s.doc.RelationCount {
+                // This is just an early bail out. The relations obtained may still
+                // be wrong, but that situation will be caught by a combination of
+                // asserts on relationcount and on each known relation, below.
                 return nil, errRefresh
         }
         var ops []txn.Op
         removeCount := 0
         for _, rel := range rels {
                 relOps, isRemove, err := rel.destroyOps(s.doc.Name)
-                if err != nil {
+                if err == errAlreadyDying {
+                        relOps = []txn.Op{{
+                                C:      s.st.relations.Name,
+                                Id:     rel.doc.Key,
+                                Assert: D{{"life", Dying}},
+                        }}
+                } else if err != nil {
                         return nil, err
                 }
+                if isRemove {
+                        removeCount++
+                }
                 ops = append(ops, relOps...)
-                if isRemove {
-                        removeCount += 1

[rog, 2013/01/16 16:17:58]

removeCount++

[fwereade, 2013/01/17 09:12:21]

SGTM

-                }
-        }
-        asserts := D{{"life", Alive}, {"txn-revno", s.doc.TxnRevno}}
+        }
+        // If the service has no units, and all its known relations will be
+        // removed, the service can also be removed.
         if s.doc.UnitCount == 0 && s.doc.RelationCount == removeCount {
-                return append(ops, s.removeOps(asserts)...), nil
+                hasLastRefs := D{{"life", Alive}, {"unitcount", 0}, {"relationcount", removeCount}}
+                return append(ops, s.removeOps(hasLastRefs)...), nil
+        }
+        // If any units of the service exist, or if any known relation was not
+        // removed (because it had units in scope, or because it was Dying, which
+        // implies the same condition), service removal will be handled as a
+        // consequence of the removal of the last unit or relation referencing it.
+        notLastRefs := D{
+                {"life", Alive},
+                {"$or", []D{
+                        {{"unitcount", D{{"$gt", 0}}}},
+                        {{"relationcount", s.doc.RelationCount}},
+                }},
         }
         update := D{{"$set", D{{"life", Dying}}}}
         if removeCount != 0 {
                 decref := D{{"$inc", D{{"relationcount", -removeCount}}}}
                 update = append(update, decref...)
         }
         return append(ops, txn.Op{
                 C:      s.st.services.Name,
                 Id:     s.doc.Name,
-                Assert: asserts,
+                Assert: notLastRefs,
                 Update: update,
         }), nil
 }
 
-// removeOps returns the oprations required to remove the service. Supplied
-// asserts will be applied to the service document.
+// removeOps returns the operations required to remove the service. Supplied
+// asserts will be included in the operation on the service document.
 func (s *Service) removeOps(asserts D) []txn.Op {
         return []txn.Op{{
                 C:      s.st.services.Name,
                 Id:     s.doc.Name,
                 Assert: asserts,
                 Remove: true,
         }, {
                 C:      s.st.settings.Name,
                 Id:     s.globalKey(),
                 Remove: true,
@@ skipping 114 matching lines @@
 // String returns the service name.
 func (s *Service) String() string {
         return s.doc.Name
 }
 
 // Refresh refreshes the contents of the Service from the underlying
 // state. It returns a NotFoundError if the service has been removed.
 func (s *Service) Refresh() error {
         err := s.st.services.FindId(s.doc.Name).One(&s.doc)
         if err == mgo.ErrNotFound {
-                return notFound("service %q", s)
+                return notFoundf("service %q", s)
         }
         if err != nil {
                 return fmt.Errorf("cannot refresh service %q: %v", s, err)
         }
         return nil
 }
 
 // newUnitName returns the next unit name.
 func (s *Service) newUnitName() (string, error) {
         change := mgo.Change{Update: D{{"$inc", D{{"unitseq", 1}}}}}
         result := serviceDoc{}
         if _, err := s.st.services.Find(D{{"_id", s.doc.Name}}).Apply(change, &result); err == mgo.ErrNotFound {
-                return "", notFound("service %q", s)
+                return "", notFoundf("service %q", s)
         } else if err != nil {
                 return "", fmt.Errorf("cannot increment unit sequence: %v", err)
         }
         name := s.doc.Name + "/" + strconv.Itoa(result.UnitSeq)
         return name, nil
 }
 
 // addUnitOps returns a unique name for a new unit, and a list of txn operations
 // necessary to create that unit. The principalName param must be non-empty if
 // and only if s is a subordinate service. If s is a subordinate and strictSubordinates
@@ skipping 121 matching lines @@
                         Update: D{{"$pull", D{{"principals", u.doc.Name}}}},
                 })
         }
         ops = append(ops, txn.Op{
                 C:      s.st.units.Name,
                 Id:     u.doc.Name,
                 Assert: txn.DocExists,
                 Remove: true,
         })
         if s.doc.Life == Dying && s.doc.RelationCount == 0 && s.doc.UnitCount == 1 {
-                return append(ops, s.removeOps(nil)...)
+                hasLastRef := D{{"life", Dying}, {"relationcount", 0}, {"unitcount", 1}}
+                return append(ops, s.removeOps(hasLastRef)...)
         }
         svcOp := txn.Op{
                 C:      s.st.services.Name,
                 Id:     s.doc.Name,
                 Update: D{{"$inc", D{{"unitcount", -1}}}},
         }
         if s.doc.Life == Alive {
                 svcOp.Assert = D{{"life", Alive}, {"unitcount", D{{"$gt", 0}}}}
         } else {
                 svcOp.Assert = D{{"life", Dying}, {"unitcount", D{{"$gt", 1}}}}
@@ skipping 42 matching lines @@
 }
 
 // Config returns the configuration node for the service.
 func (s *Service) Config() (config *Settings, err error) {
         config, err = readSettings(s.st, s.globalKey())
         if err != nil {
                 return nil, fmt.Errorf("cannot get configuration of service %q: %v", s, err)
         }
         return config, nil
 }