// Relations returns a Relation for every relation the service is in.
func (s *Service) Relations() (relations []*Relation, err error) {
- defer utils.ErrorContextf(&err, "can't get relations for service %q", s)
+ return serviceRelations(s.st, s.doc.Name)
+}
+
+func serviceRelations(st *State, name string) (relations []*Relation, err
error) {
+ defer utils.ErrorContextf(&err, "can't get relations for service %q",
name)
docs := []relationDoc{}
- err = s.st.relations.Find(D{{"endpoints.servicename",
s.doc.Name}}).All(&docs)
+ err = st.relations.Find(D{{"endpoints.servicename", name}}).All(&docs)
if err != nil {
return nil, err
}
for _, v := range docs {
- relations = append(relations, newRelation(s.st, &v))
+ relations = append(relations, newRelation(st, &v))
}
return relations, nil
}
Index: state/unit.go
=== modified file 'state/unit.go'
--- state/unit.go 2013-11-07 06:44:41 +0000
+++ state/unit.go 2013-11-07 17:30:38 +0000
@@ -418,6 +418,28 @@
if u.doc.Life != Dead {
return stderrors.New("unit is not dead")
}
+
+ // Now the unit is Dead, we can be sure that it's impossible for it to
+ // enter relation scopes (once it's Dying, we can be sure of this; but
+ // EnsureDead does not require that it already be Dying, so this is the
+ // only point at which we can safely backstop lp:1233457 and mitigate
+ // the impact of unit agent bugs that leave relation scopes occupied).
+ relations, err := serviceRelations(u.st, u.doc.Service)
+ if err != nil {
+ return err
+ }
+ for _, rel := range relations {
+ ru, err := rel.Unit(u)
+ if err != nil {
+ return err
+ }
+ if err := ru.LeaveScope(); err != nil {
+ return err
+ }
+ }
+
+ // Now we're sure we haven't left any scopes occupied by this unit, we
+ // can safely remove the document.
unit := &Unit{st: u.st, doc: u.doc}
for i := 0; i < 5; i++ {
switch ops, err := unit.removeOps(isDeadDoc); err {
+func (s *UnitSuite) TestRemovePathologicalWithBuggyUniter(c *gc.C) {
+ // Add a relation between wordpress and mysql...
+ wordpress := s.service
+ wordpress0 := s.unit
+ mysql, err := s.State.AddService("mysql", s.AddTestingCharm(c, "mysql"))
+ c.Assert(err, gc.IsNil)
+ eps, err := s.State.InferEndpoints([]string{"wordpress", "mysql"})
+ c.Assert(err, gc.IsNil)
+ rel, err := s.State.AddRelation(eps...)
+ c.Assert(err, gc.IsNil)
+
+ // The relation holds a reference to wordpress, but that can't keep
+ // wordpress from being removed -- because the relation will be removed
+ // if we destroy wordpress.
+ // However, if a unit of the *other* service joins the relation, that
+ // will add an additional reference and prevent the relation -- and
+ // thus wordpress itself -- from being removed when its last unit is.
+ mysql0, err := mysql.AddUnit()
+ c.Assert(err, gc.IsNil)
+ mysql0ru, err := rel.Unit(mysql0)
+ c.Assert(err, gc.IsNil)
+ err = mysql0ru.EnterScope(nil)
+ c.Assert(err, gc.IsNil)
+
+ // Destroy wordpress, and remove its last unit.
+ err = wordpress.Destroy()
+ c.Assert(err, gc.IsNil)
+ err = wordpress0.EnsureDead()
+ c.Assert(err, gc.IsNil)
+ err = wordpress0.Remove()
+ c.Assert(err, gc.IsNil)
+
+ // Check this didn't kill the service or relation yet...
+ err = wordpress.Refresh()
+ c.Assert(err, gc.IsNil)
+ err = rel.Refresh()
+ c.Assert(err, gc.IsNil)
+
+ // ...and that when the malfunctioning unit agent on the other side
+ // sets itself to dead *without* departing the relation, the unit's
+ // removal causes the relation and the other service to be cleaned up.
+ err = mysql0.EnsureDead()
+ c.Assert(err, gc.IsNil)
+ err = mysql0.Remove()
+ c.Assert(err, gc.IsNil)
+ err = wordpress.Refresh()
+ c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
+ err = rel.Refresh()
+ c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
+}
+
func (s *UnitSuite) TestWatchSubordinates(c *gc.C) {
w := s.unit.WatchSubordinateUnits()
defer testing.AssertStop(c, w)
Reviewers: mp+194389_ code.launchpad. net,
Message:
Please take a look.
Description:
state: unit removal departs relation scopes
Fixes lp:1233457
https:/ /code.launchpad .net/~fwereade/ juju-core/ unit-remove- depart- scopes/ +merge/ 194389
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/23080045/
Affected files (+82, -3 lines):
A [revision details]
M state/service.go
M state/unit.go
M state/unit_test.go
Index: [revision details] 20131107094222- rb2jpeg465spb0q a
=== 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: tarmac-
+New revision: <email address hidden>
Index: state/service.go
=== modified file 'state/service.go'
--- state/service.go 2013-07-30 14:00:53 +0000
+++ state/service.go 2013-11-07 17:30:14 +0000
@@ -718,14 +718,18 @@
// Relations returns a Relation for every relation the service is in. extf(&err, "can't get relations for service %q", s) s(s.st, s.doc.Name) extf(&err, "can't get relations for service %q", Find(D{ {"endpoints. servicename" , }).All( &docs) Find(D{ {"endpoints. servicename" , name}}).All(&docs)
func (s *Service) Relations() (relations []*Relation, err error) {
- defer utils.ErrorCont
+ return serviceRelation
+}
+
+func serviceRelations(st *State, name string) (relations []*Relation, err
error) {
+ defer utils.ErrorCont
name)
docs := []relationDoc{}
- err = s.st.relations.
s.doc.Name}
+ err = st.relations.
if err != nil {
return nil, err
}
for _, v := range docs {
- relations = append(relations, newRelation(s.st, &v))
+ relations = append(relations, newRelation(st, &v))
}
return relations, nil
}
Index: state/unit.go s(u.st, u.doc.Service) isDeadDoc) ; err {
=== modified file 'state/unit.go'
--- state/unit.go 2013-11-07 06:44:41 +0000
+++ state/unit.go 2013-11-07 17:30:38 +0000
@@ -418,6 +418,28 @@
if u.doc.Life != Dead {
return stderrors.New("unit is not dead")
}
+
+ // Now the unit is Dead, we can be sure that it's impossible for it to
+ // enter relation scopes (once it's Dying, we can be sure of this; but
+ // EnsureDead does not require that it already be Dying, so this is the
+ // only point at which we can safely backstop lp:1233457 and mitigate
+ // the impact of unit agent bugs that leave relation scopes occupied).
+ relations, err := serviceRelation
+ if err != nil {
+ return err
+ }
+ for _, rel := range relations {
+ ru, err := rel.Unit(u)
+ if err != nil {
+ return err
+ }
+ if err := ru.LeaveScope(); err != nil {
+ return err
+ }
+ }
+
+ // Now we're sure we haven't left any scopes occupied by this unit, we
+ // can safely remove the document.
unit := &Unit{st: u.st, doc: u.doc}
for i := 0; i < 5; i++ {
switch ops, err := unit.removeOps(
Index: state/unit_test.go unit_test. go' IsNotFoundError )
=== modified file 'state/
--- state/unit_test.go 2013-11-05 07:19:16 +0000
+++ state/unit_test.go 2013-11-07 17:30:38 +0000
@@ -1127,6 +1127,57 @@
c.Assert(err, jc.Satisfies, errors.
}
+func (s *UnitSuite) TestRemovePatho logicalWithBugg yUniter( c *gc.C) { AddService( "mysql" , s.AddTestingCha rm(c, "mysql")) InferEndpoints( []string{ "wordpress" , "mysql"}) AddRelation( eps...) EnterScope( nil) EnsureDead( ) IsNotFoundError ) IsNotFoundError ) inates( c *gc.C) { WatchSubordinat eUnits( ) AssertStop( c, w)
+ // Add a relation between wordpress and mysql...
+ wordpress := s.service
+ wordpress0 := s.unit
+ mysql, err := s.State.
+ c.Assert(err, gc.IsNil)
+ eps, err := s.State.
+ c.Assert(err, gc.IsNil)
+ rel, err := s.State.
+ c.Assert(err, gc.IsNil)
+
+ // The relation holds a reference to wordpress, but that can't keep
+ // wordpress from being removed -- because the relation will be removed
+ // if we destroy wordpress.
+ // However, if a unit of the *other* service joins the relation, that
+ // will add an additional reference and prevent the relation -- and
+ // thus wordpress itself -- from being removed when its last unit is.
+ mysql0, err := mysql.AddUnit()
+ c.Assert(err, gc.IsNil)
+ mysql0ru, err := rel.Unit(mysql0)
+ c.Assert(err, gc.IsNil)
+ err = mysql0ru.
+ c.Assert(err, gc.IsNil)
+
+ // Destroy wordpress, and remove its last unit.
+ err = wordpress.Destroy()
+ c.Assert(err, gc.IsNil)
+ err = wordpress0.
+ c.Assert(err, gc.IsNil)
+ err = wordpress0.Remove()
+ c.Assert(err, gc.IsNil)
+
+ // Check this didn't kill the service or relation yet...
+ err = wordpress.Refresh()
+ c.Assert(err, gc.IsNil)
+ err = rel.Refresh()
+ c.Assert(err, gc.IsNil)
+
+ // ...and that when the malfunctioning unit agent on the other side
+ // sets itself to dead *without* departing the relation, the unit's
+ // removal causes the relation and the other service to be cleaned up.
+ err = mysql0.EnsureDead()
+ c.Assert(err, gc.IsNil)
+ err = mysql0.Remove()
+ c.Assert(err, gc.IsNil)
+ err = wordpress.Refresh()
+ c.Assert(err, jc.Satisfies, errors.
+ err = rel.Refresh()
+ c.Assert(err, jc.Satisfies, errors.
+}
+
func (s *UnitSuite) TestWatchSubord
w := s.unit.
defer testing.