Skip to content

Commit 45b0ff9

Browse files
fwereadefwereade
committed
exposed Endpoint.IsImplicit; uniter now skips hooks for implicit relations
1 parent 6ea96a8 commit 45b0ff9

File tree

6 files changed

+147
-42
lines changed

6 files changed

+147
-42
lines changed

‎state/endpoint.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (ep Endpoint) CanRelateTo(other Endpoint) bool {
5656

5757
// ImplementedBy returns whether the endpoint is implemented by the supplied charm.
5858
func (ep Endpoint) ImplementedBy(ch charm.Charm) bool {
59-
if ep.isImplicit() {
59+
if ep.IsImplicit() {
6060
return true
6161
}
6262
var m map[string]charm.Relation
@@ -87,9 +87,9 @@ func (ep Endpoint) ImplementedBy(ch charm.Charm) bool {
8787
return false
8888
}
8989

90-
// isImplicit returns whether the endpoint is supplied by juju itself,
90+
// IsImplicit returns whether the endpoint is supplied by juju itself,
9191
// rather than by a charm.
92-
func (ep Endpoint) isImplicit() bool {
92+
func (ep Endpoint) IsImplicit() bool {
9393
return (ep.RelationName == "juju-info" &&
9494
ep.Interface == "juju-info" &&
9595
ep.RelationRole == RoleProvider)

‎state/endpoint_test.go

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -68,47 +68,49 @@ func (c *dummyCharm) Meta() *charm.Meta {
6868
}
6969

7070
var implementedByTests = []struct {
71-
ifce string
72-
name string
73-
role state.RelationRole
74-
scope charm.RelationScope
75-
match bool
71+
ifce string
72+
name string
73+
role state.RelationRole
74+
scope charm.RelationScope
75+
match bool
76+
implicit bool
7677
}{
77-
{"ifce-pro", "pro", state.RoleProvider, charm.ScopeGlobal, true},
78-
{"blah", "pro", state.RoleProvider, charm.ScopeGlobal, false},
79-
{"ifce-pro", "blah", state.RoleProvider, charm.ScopeGlobal, false},
80-
{"ifce-pro", "pro", state.RoleRequirer, charm.ScopeGlobal, false},
81-
{"ifce-pro", "pro", state.RoleProvider, charm.ScopeContainer, true},
78+
{"ifce-pro", "pro", state.RoleProvider, charm.ScopeGlobal, true, false},
79+
{"blah", "pro", state.RoleProvider, charm.ScopeGlobal, false, false},
80+
{"ifce-pro", "blah", state.RoleProvider, charm.ScopeGlobal, false, false},
81+
{"ifce-pro", "pro", state.RoleRequirer, charm.ScopeGlobal, false, false},
82+
{"ifce-pro", "pro", state.RoleProvider, charm.ScopeContainer, true, false},
8283

83-
{"juju-info", "juju-info", state.RoleProvider, charm.ScopeGlobal, true},
84-
{"blah", "juju-info", state.RoleProvider, charm.ScopeGlobal, false},
85-
{"juju-info", "blah", state.RoleProvider, charm.ScopeGlobal, false},
86-
{"juju-info", "juju-info", state.RoleRequirer, charm.ScopeGlobal, false},
87-
{"juju-info", "juju-info", state.RoleProvider, charm.ScopeContainer, true},
84+
{"juju-info", "juju-info", state.RoleProvider, charm.ScopeGlobal, true, true},
85+
{"blah", "juju-info", state.RoleProvider, charm.ScopeGlobal, false, false},
86+
{"juju-info", "blah", state.RoleProvider, charm.ScopeGlobal, false, false},
87+
{"juju-info", "juju-info", state.RoleRequirer, charm.ScopeGlobal, false, false},
88+
{"juju-info", "juju-info", state.RoleProvider, charm.ScopeContainer, true, true},
8889

89-
{"ifce-req", "req", state.RoleRequirer, charm.ScopeGlobal, true},
90-
{"blah", "req", state.RoleRequirer, charm.ScopeGlobal, false},
91-
{"ifce-req", "blah", state.RoleRequirer, charm.ScopeGlobal, false},
92-
{"ifce-req", "req", state.RolePeer, charm.ScopeGlobal, false},
93-
{"ifce-req", "req", state.RoleRequirer, charm.ScopeContainer, true},
90+
{"ifce-req", "req", state.RoleRequirer, charm.ScopeGlobal, true, false},
91+
{"blah", "req", state.RoleRequirer, charm.ScopeGlobal, false, false},
92+
{"ifce-req", "blah", state.RoleRequirer, charm.ScopeGlobal, false, false},
93+
{"ifce-req", "req", state.RolePeer, charm.ScopeGlobal, false, false},
94+
{"ifce-req", "req", state.RoleRequirer, charm.ScopeContainer, true, false},
9495

95-
{"juju-info", "info", state.RoleRequirer, charm.ScopeContainer, true},
96-
{"blah", "info", state.RoleRequirer, charm.ScopeContainer, false},
97-
{"juju-info", "blah", state.RoleRequirer, charm.ScopeContainer, false},
98-
{"juju-info", "info", state.RolePeer, charm.ScopeContainer, false},
99-
{"juju-info", "info", state.RoleRequirer, charm.ScopeGlobal, false},
96+
{"juju-info", "info", state.RoleRequirer, charm.ScopeContainer, true, false},
97+
{"blah", "info", state.RoleRequirer, charm.ScopeContainer, false, false},
98+
{"juju-info", "blah", state.RoleRequirer, charm.ScopeContainer, false, false},
99+
{"juju-info", "info", state.RolePeer, charm.ScopeContainer, false, false},
100+
{"juju-info", "info", state.RoleRequirer, charm.ScopeGlobal, false, false},
100101

101-
{"ifce-peer", "peer", state.RolePeer, charm.ScopeGlobal, true},
102-
{"blah", "peer", state.RolePeer, charm.ScopeGlobal, false},
103-
{"ifce-peer", "blah", state.RolePeer, charm.ScopeGlobal, false},
104-
{"ifce-peer", "peer", state.RoleProvider, charm.ScopeGlobal, false},
105-
{"ifce-peer", "peer", state.RolePeer, charm.ScopeContainer, true},
102+
{"ifce-peer", "peer", state.RolePeer, charm.ScopeGlobal, true, false},
103+
{"blah", "peer", state.RolePeer, charm.ScopeGlobal, false, false},
104+
{"ifce-peer", "blah", state.RolePeer, charm.ScopeGlobal, false, false},
105+
{"ifce-peer", "peer", state.RoleProvider, charm.ScopeGlobal, false, false},
106+
{"ifce-peer", "peer", state.RolePeer, charm.ScopeContainer, true, false},
106107
}
107108

108109
func (s *EndpointSuite) TestImplementedBy(c *C) {
109110
for i, t := range implementedByTests {
110111
c.Logf("test %d", i)
111112
ep := state.Endpoint{"x", t.ifce, t.name, t.role, t.scope}
112113
c.Assert(ep.ImplementedBy(&dummyCharm{}), Equals, t.match)
114+
c.Assert(ep.IsImplicit(), Equals, t.implicit)
113115
}
114116
}

‎state/state.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ func (st *State) InferEndpoints(names []string) ([]Endpoint, error) {
440440
outer:
441441
for _, cand := range candidates {
442442
for _, ep := range cand {
443-
if ep.isImplicit() {
443+
if ep.IsImplicit() {
444444
continue outer
445445
}
446446
}

‎worker/uniter/modes.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,9 +301,11 @@ func modeAbideDyingLoop(u *Uniter) (next Mode, err error) {
301301
return nil, err
302302
}
303303
}
304-
for _, r := range u.relationers {
304+
for id, r := range u.relationers {
305305
if err := r.SetDying(); err != nil {
306306
return nil, err
307+
} else if r.IsImplicit() {
308+
delete(u.relationers, id)
307309
}
308310
}
309311
for {

‎worker/uniter/relationer.go

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ func (r *Relationer) Context() *ContextRelation {
3333
return r.ctx
3434
}
3535

36+
// IsImplicit returns whether the local relation endpoint is implicit. Implicit
37+
// relations do not run hooks.
38+
func (r *Relationer) IsImplicit() bool {
39+
return r.ru.Endpoint().IsImplicit()
40+
}
41+
3642
// Join initializes local state and causes the unit to enter its relation
3743
// scope, allowing its counterpart units to detect its presence and settings
3844
// changes.
@@ -50,6 +56,10 @@ func (r *Relationer) Join() error {
5056
// and that the only hooks it should send henceforth are -departed hooks,
5157
// until the relation is empty, followed by a -broken hook.
5258
func (r *Relationer) SetDying() error {
59+
if r.IsImplicit() {
60+
r.dying = true
61+
return r.die()
62+
}
5363
if r.queue != nil {
5464
if err := r.StopHooks(); err != nil {
5565
return err
@@ -60,10 +70,22 @@ func (r *Relationer) SetDying() error {
6070
return nil
6171
}
6272

73+
// die is run when the relationer has no further responsibilities; it leaves
74+
// relation scope, and removes the local relation state directory.
75+
func (r *Relationer) die() error {
76+
if err := r.ru.LeaveScope(); err != nil {
77+
return err
78+
}
79+
return r.dir.Remove()
80+
}
81+
6382
// StartHooks starts watching the relation, and sending hook.Info events on the
6483
// hooks channel. It will panic if called when already responding to relation
6584
// changes.
6685
func (r *Relationer) StartHooks() {
86+
if r.IsImplicit() {
87+
return
88+
}
6789
if r.queue != nil {
6890
panic("hooks already started!")
6991
}
@@ -90,6 +112,9 @@ func (r *Relationer) StopHooks() error {
90112
// contains the latest relation state as communicated in the hook.Info. It
91113
// returns the name of the hook that must be run.
92114
func (r *Relationer) PrepareHook(hi hook.Info) (hookName string, err error) {
115+
if r.IsImplicit() {
116+
panic("implicit relations must not run hooks")
117+
}
93118
if err = r.dir.State().Validate(hi); err != nil {
94119
return
95120
}
@@ -105,10 +130,10 @@ func (r *Relationer) PrepareHook(hi hook.Info) (hookName string, err error) {
105130

106131
// CommitHook persists the fact of the supplied hook's completion.
107132
func (r *Relationer) CommitHook(hi hook.Info) error {
108-
if hi.Kind == hook.RelationBroken {
109-
if err := r.ru.LeaveScope(); err != nil {
110-
return err
111-
}
133+
if r.IsImplicit() {
134+
panic("implicit relations must not run hooks")
135+
} else if hi.Kind == hook.RelationBroken {
136+
return r.die()
112137
}
113138
return r.dir.Write(hi)
114139
}

‎worker/uniter/relationer_test.go

Lines changed: 79 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import (
88
"launchpad.net/juju-core/worker/uniter"
99
"launchpad.net/juju-core/worker/uniter/hook"
1010
"launchpad.net/juju-core/worker/uniter/relation"
11+
"os"
12+
"path/filepath"
13+
"strconv"
1114
"strings"
1215
"time"
1316
)
@@ -112,6 +115,7 @@ func (s *RelationerSuite) TestStartStopHooks(c *C) {
112115
ru1 := s.AddRelationUnit(c, "u/1")
113116
ru2 := s.AddRelationUnit(c, "u/2")
114117
r := uniter.NewRelationer(s.ru, s.dir, s.hooks)
118+
c.Assert(r.IsImplicit(), Equals, false)
115119
err := r.Join()
116120
c.Assert(err, IsNil)
117121

@@ -330,9 +334,6 @@ func (s *RelationerSuite) TestSetDying(c *C) {
330334
// Check that the relation state has been broken.
331335
err = s.dir.State().Validate(hook.Info{Kind: hook.RelationBroken})
332336
c.Assert(err, ErrorMatches, ".*: relation is broken and cannot be changed further")
333-
334-
// TODO: when we have lifecycle handling, verify that the relation can
335-
// be destroyed. Can't see a clean way to do so currently.
336337
}
337338

338339
func (s *RelationerSuite) assertNoHook(c *C) {
@@ -368,3 +369,78 @@ func stop(c *C, s stopper) {
368369
func stopHooks(c *C, r *uniter.Relationer) {
369370
c.Assert(r.StopHooks(), IsNil)
370371
}
372+
373+
type RelationerImplicitSuite struct {
374+
testing.JujuConnSuite
375+
}
376+
377+
var _ = Suite(&RelationerImplicitSuite{})
378+
379+
func (s *RelationerImplicitSuite) TestImplicitRelationer(c *C) {
380+
// Create a relationer for an implicit endpoint (mysql:juju-info).
381+
mysql, err := s.State.AddService("mysql", s.AddTestingCharm(c, "mysql"))
382+
c.Assert(err, IsNil)
383+
u, err := mysql.AddUnit()
384+
c.Assert(err, IsNil)
385+
err = u.SetPrivateAddress("blah")
386+
c.Assert(err, IsNil)
387+
logging, err := s.State.AddService("logging", s.AddTestingCharm(c, "logging"))
388+
c.Assert(err, IsNil)
389+
eps, err := s.State.InferEndpoints([]string{"logging", "mysql"})
390+
c.Assert(err, IsNil)
391+
rel, err := s.State.AddRelation(eps...)
392+
c.Assert(err, IsNil)
393+
ru, err := rel.Unit(u)
394+
c.Assert(err, IsNil)
395+
relsDir := c.MkDir()
396+
dir, err := relation.ReadStateDir(relsDir, rel.Id())
397+
c.Assert(err, IsNil)
398+
hooks := make(chan hook.Info)
399+
r := uniter.NewRelationer(ru, dir, hooks)
400+
c.Assert(r.IsImplicit(), Equals, true)
401+
402+
// Join the relationer; check the dir was created and scope was entered.
403+
err = r.Join()
404+
c.Assert(err, IsNil)
405+
fi, err := os.Stat(filepath.Join(relsDir, strconv.Itoa(rel.Id())))
406+
c.Assert(err, IsNil)
407+
c.Assert(fi.IsDir(), Equals, true)
408+
sub, err := logging.Unit("logging/0")
409+
c.Assert(err, IsNil)
410+
err = sub.SetPrivateAddress("blah")
411+
c.Assert(err, IsNil)
412+
413+
// Join the other side; check no hooks are sent.
414+
r.StartHooks()
415+
defer func() { c.Assert(r.StopHooks(), IsNil) }()
416+
subru, err := rel.Unit(sub)
417+
c.Assert(err, IsNil)
418+
err = subru.EnterScope()
419+
c.Assert(err, IsNil)
420+
checkNoHooks := func() {
421+
s.State.StartSync()
422+
select {
423+
case <-time.After(50 * time.Millisecond):
424+
case <-hooks:
425+
c.Fatalf("unexpected hook generated")
426+
}
427+
}
428+
checkNoHooks()
429+
430+
// Set it to Dying; check that the dir is removed and no hooks are fired.
431+
err = r.SetDying()
432+
c.Assert(err, IsNil)
433+
_, err = os.Stat(filepath.Join(relsDir, strconv.Itoa(rel.Id())))
434+
c.Assert(os.IsNotExist(err), Equals, true)
435+
checkNoHooks()
436+
437+
// Check that it left scope, by leaving scope on the other side and destroying
438+
// the relation, and that still no hooks are fired.
439+
err = subru.LeaveScope()
440+
c.Assert(err, IsNil)
441+
err = rel.Destroy()
442+
c.Assert(err, IsNil)
443+
err = rel.Refresh()
444+
c.Assert(state.IsNotFound(err), Equals, true)
445+
checkNoHooks()
446+
}

0 commit comments

Comments
 (0)