juju-core

Merge lp:~dimitern/juju-core/002-bug-1101140-ignore-unknown-relations into lp:~juju/juju-core/trunk

  1. 002-bug-1101140-ignore-unknown-relations
  2. Merge into trunk
Proposed by Dimiter Naydenov
Status: Merged
Merged at revision: 927
Proposed branch: lp:~dimitern/juju-core/002-bug-1101140-ignore-unknown-relations
Merge into: lp:~juju/juju-core/trunk
Prerequisite: lp:~dimitern/juju-core/001-upgrade-charm-filter-restart-watchers
Diff against target: 355 lines (+111/-30)
6 files modified
worker/uniter/filter.go (+7/-7)
worker/uniter/modes.go (+5/-0)
worker/uniter/relationer.go (+5/-3)
worker/uniter/relationer_test.go (+19/-10)
worker/uniter/uniter.go (+12/-0)
worker/uniter/uniter_test.go (+63/-10)
To merge this branch: bzr merge lp:~dimitern/juju-core/002-bug-1101140-ignore-unknown-relations
Reviewer Review Type Date Requested Status
The Go Language Gophers Pending
Review via email: mp+149921@code.launchpad.net

Description of the change

uniter: skip unknown rel. endpoints

Ignore relations with unknown (not implemented by
the charm) when handling upgrades.
The uniter is now requesting all relation events
on upgrade, so they won't be missed.

This also fixes LP bug #1101140 (uniter does not
skip unimplemented relations).

https://codereview.appspot.com/7385049/

To post a comment you must log in.
Revision history for this message
William Reade (fwereade) wrote :

Tests! But look into the prereq first anyway :).

https://codereview.appspot.com/7385049/diff/1/worker/uniter/uniter.go
File worker/uniter/uniter.go (right):

https://codereview.appspot.com/7385049/diff/1/worker/uniter/uniter.go#newcode441
worker/uniter/uniter.go:441: sch, _, err := svc.Charm()
This will never catch problems. We know for *sure* that this charm
implements the endpoint... but we need to check the charm that is
actually deployed right here ;). I think you want something a bit more
like:

ch, err := corecharm.ReadDir(u.charm.Path())

...but I would also be potentially interested in smarter approaches
(like storing the *charm.Dir on the uniter, if you can think of a way to
do so cleanly and clearly... I don't *expect* you to do this, just a
suggestion).

https://codereview.appspot.com/7385049/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Reviewers: mp+149921_code.launchpad.net, fwereade,

Message:
Please take a look.

Description:
uniter: ignore unknown endpoints

Ignore relations with unknown (not implemented by
the charm) when handling upgrades.
The uniter is now requesting all relation events
on upgrade, so they won't be missed.

Partial fix for bug #1101140.

https://code.launchpad.net/~dimitern/juju-core/002-bug-1101140-ignore-unknown-relations/+merge/149921

Requires:
https://code.launchpad.net/~dimitern/juju-core/001-upgrade-charm-filter-restart-watchers/+merge/149910

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7385049/

Affected files:
   A [revision details]
   M worker/uniter/filter.go
   M worker/uniter/modes.go
   M worker/uniter/relationer.go
   M worker/uniter/relationer_test.go
   M worker/uniter/uniter.go
   M worker/uniter/uniter_test.go

Revision history for this message
William Reade (fwereade) wrote :

Thanks -- LGTM assuming the below.

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/modes.go
File worker/uniter/modes.go (right):

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/modes.go#newcode145
worker/uniter/modes.go:145: // handle the relations for the new unit
Now the upgrade is complete, we'll need to check all relations again:
some might previously have been skipped (if they involved endpoints only
implemented in the new charm).

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter.go
File worker/uniter/uniter.go (right):

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter.go#newcode383
worker/uniter/uniter.go:383: // possibly even panic?
Nope, this is just a straight-up error, I think. We don't have any way
of resolving it, or reporting it, it's true; but panicking won't improve
matters so... ;).

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter.go#newcode389
worker/uniter/uniter.go:389: log.Printf("worker/uniter: ignoring not
implemented relation endpoint %q", ep)
"skipping relation with unknown endpoint"?

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter_test.go
File worker/uniter/uniter_test.go (right):

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter_test.go#newcode323
worker/uniter/uniter_test.go:323: customize: func(c *C, path string, ctx
*context) {
(c, ctx, path) feels neater to me, but YMMV

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter_test.go#newcode620
worker/uniter/uniter_test.go:620: // doing upgradeCharm before
addRelation (?!)
This test does an add-relation as quickly as possible after an
upgrade-charm, in the hope that the scheduler will deliver the events in
the wrong order. The observed behaviour should be the same in either
case.

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter_test.go#newcode636
worker/uniter/uniter_test.go:636: waitHooks{"db2-relation-joined mysql/0
db2:0"},
May as well stick those steps together.

https://codereview.appspot.com/7385049/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/modes.go
File worker/uniter/modes.go (right):

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/modes.go#newcode145
worker/uniter/modes.go:145: // handle the relations for the new unit
On 2013/02/25 10:31:57, fwereade wrote:
> Now the upgrade is complete, we'll need to check all relations again:
 some might
> previously have been skipped (if they involved endpoints only
 implemented in the
> new charm).

Done.

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter.go
File worker/uniter/uniter.go (right):

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter.go#newcode383
worker/uniter/uniter.go:383: // possibly even panic?
On 2013/02/25 10:31:57, fwereade wrote:
> Nope, this is just a straight-up error, I think. We don't have any way
 of
> resolving it, or reporting it, it's true; but panicking won't improve
 matters
> so... ;).

Done.

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter.go#newcode383
worker/uniter/uniter.go:383: // possibly even panic?
On 2013/02/25 10:31:57, fwereade wrote:
> Nope, this is just a straight-up error, I think. We don't have any way
 of
> resolving it, or reporting it, it's true; but panicking won't improve
 matters
> so... ;).

Done.

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter.go#newcode389
worker/uniter/uniter.go:389: log.Printf("worker/uniter: ignoring not
implemented relation endpoint %q", ep)
On 2013/02/25 10:31:57, fwereade wrote:
> "skipping relation with unknown endpoint"?

Done.

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter_test.go
File worker/uniter/uniter_test.go (right):

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter_test.go#newcode323
worker/uniter/uniter_test.go:323: customize: func(c *C, path string, ctx
*context) {
On 2013/02/25 10:31:57, fwereade wrote:
> (c, ctx, path) feels neater to me, but YMMV

Done.

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter_test.go#newcode620
worker/uniter/uniter_test.go:620: // doing upgradeCharm before
addRelation (?!)
On 2013/02/25 10:31:57, fwereade wrote:
> This test does an add-relation as quickly as possible after an
 upgrade-charm, in
> the hope that the scheduler will deliver the events in the wrong
 order. The
> observed behaviour should be the same in either case.

Done.

https://codereview.appspot.com/7385049/diff/7001/worker/uniter/uniter_test.go#newcode636
worker/uniter/uniter_test.go:636: waitHooks{"db2-relation-joined mysql/0
db2:0"},
On 2013/02/25 10:31:57, fwereade wrote:
> May as well stick those steps together.

Done.

https://codereview.appspot.com/7385049/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

*** Submitted:

uniter: skip unknown rel. endpoints

Ignore relations with unknown (not implemented by
the charm) when handling upgrades.
The uniter is now requesting all relation events
on upgrade, so they won't be missed.

This also fixes LP bug #1101140 (uniter does not
skip unimplemented relations).

R=fwereade, TheMue
CC=
https://codereview.appspot.com/7385049

https://codereview.appspot.com/7385049/diff/2002/worker/uniter/relationer.go
File worker/uniter/relationer.go (right):

https://codereview.appspot.com/7385049/diff/2002/worker/uniter/relationer.go#newcode54
worker/uniter/relationer.go:54: // No need to check the r.dir exists yet
On 2013/02/25 11:20:47, TheMue wrote:
> IMHO not needed comment, only relates to removed code part.

Done.

https://codereview.appspot.com/7385049/diff/2002/worker/uniter/relationer.go#newcode124
worker/uniter/relationer.go:124: // We are about to use the dir, ensure
it's there
On 2013/02/25 11:20:47, TheMue wrote:
> Punctuation. ;)

Done.

https://codereview.appspot.com/7385049/diff/2002/worker/uniter/relationer_test.go
File worker/uniter/relationer_test.go (right):

https://codereview.appspot.com/7385049/diff/2002/worker/uniter/relationer_test.go#newcode102
worker/uniter/relationer_test.go:102: // Verify PrepareHook created the
dir
On 2013/02/25 11:20:47, TheMue wrote:
> Punctuation.

Done.

https://codereview.appspot.com/7385049/diff/2002/worker/uniter/relationer_test.go#newcode355
worker/uniter/relationer_test.go:355: // We're no longer doing this in
Join, so we must do it here
On 2013/02/25 11:20:47, TheMue wrote:
> Needless comment, it's related to a change.

Not exactly, the change breaks this unless dir.Ensure() is run. Updated
the comment to reflect this.

https://codereview.appspot.com/7385049/diff/2002/worker/uniter/relationer_test.go#newcode409
worker/uniter/relationer_test.go:409: // Join the relationer; the dir
won't be created until necessary
On 2013/02/25 11:20:47, TheMue wrote:
> Ditto.

Actually, this is now part of the Join's behavior and it's worth the
comment, even more so, because it changes the test logic.

https://codereview.appspot.com/7385049/diff/2002/worker/uniter/uniter.go
File worker/uniter/uniter.go (right):

https://codereview.appspot.com/7385049/diff/2002/worker/uniter/uniter.go#newcode318
worker/uniter/uniter.go:318: // TODO: Get these from state, not from
disk
On 2013/02/25 11:20:47, TheMue wrote:
> // TODO(who): ..., and punctuation.

Done.

https://codereview.appspot.com/7385049/diff/2002/worker/uniter/uniter_test.go
File worker/uniter/uniter_test.go (right):

https://codereview.appspot.com/7385049/diff/2002/worker/uniter/uniter_test.go#newcode619
worker/uniter/uniter_test.go:619: // after an upgrade-charm, in the hope
that the scheduler will
On 2013/02/25 11:20:47, TheMue wrote:
> "in the hope"? Sounds a bit weak to me. Any way to force the delivery
 in a wrong
> order?

No ways I can think of to tweak the scheduling at run-time.

https://codereview.appspot.com/7385049/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'worker/uniter/filter.go'
--- worker/uniter/filter.go 2013-02-22 14:11:34 +0000
+++ worker/uniter/filter.go 2013-02-25 11:02:25 +0000
@@ -78,7 +78,7 @@
78 go func() {78 go func() {
79 defer f.tomb.Done()79 defer f.tomb.Done()
80 err := f.loop(unitName)80 err := f.loop(unitName)
81 log.Printf("worker/uniter: filter error: %v", err)81 log.Printf("worker/uniter/filter: error: %v", err)
82 f.tomb.Kill(err)82 f.tomb.Kill(err)
83 }()83 }()
84 return f, nil84 return f, nil
@@ -229,7 +229,7 @@
229 f.outConfig = f.outConfigOn229 f.outConfig = f.outConfigOn
230 discardConfig = f.discardConfig230 discardConfig = f.discardConfig
231 case ids, ok := <-relationsw.Changes():231 case ids, ok := <-relationsw.Changes():
232 log.Debugf("filter: got relations change")232 log.Debugf("worker/uniter/filter: got relations change")
233 if !ok {233 if !ok {
234 return watcher.MustErr(relationsw)234 return watcher.MustErr(relationsw)
235 }235 }
@@ -244,10 +244,10 @@
244 log.Debugf("worker/uniter/filter: sent resolved event")244 log.Debugf("worker/uniter/filter: sent resolved event")
245 f.outResolved = nil245 f.outResolved = nil
246 case f.outConfig <- nothing:246 case f.outConfig <- nothing:
247 log.Debugf("filter: sent config event")247 log.Debugf("worker/uniter/filter: sent config event")
248 f.outConfig = nil248 f.outConfig = nil
249 case f.outRelations <- f.relations:249 case f.outRelations <- f.relations:
250 log.Debugf("filter: sent relations event")250 log.Debugf("worker/uniter/filter: sent relations event")
251 f.outRelations = nil251 f.outRelations = nil
252 f.relations = nil252 f.relations = nil
253253
@@ -269,7 +269,7 @@
269 watcher.Stop(relationsw, &f.tomb)269 watcher.Stop(relationsw, &f.tomb)
270 relationsw = f.service.WatchRelations()270 relationsw = f.service.WatchRelations()
271 case <-discardConfig:271 case <-discardConfig:
272 log.Debugf("filter: discarded config event")272 log.Debugf("worker/uniter/filter: discarded config event")
273 f.outConfig = nil273 f.outConfig = nil
274 }274 }
275 }275 }
@@ -287,11 +287,11 @@
287 if f.life != f.unit.Life() {287 if f.life != f.unit.Life() {
288 switch f.life = f.unit.Life(); f.life {288 switch f.life = f.unit.Life(); f.life {
289 case state.Dying:289 case state.Dying:
290 log.Printf("worker/uniter: unit is dying")290 log.Printf("worker/uniter/filter: unit is dying")
291 close(f.outUnitDying)291 close(f.outUnitDying)
292 f.outUpgrade = nil292 f.outUpgrade = nil
293 case state.Dead:293 case state.Dead:
294 log.Printf("worker/uniter: unit is dead")294 log.Printf("worker/uniter/filter: unit is dead")
295 return worker.ErrDead295 return worker.ErrDead
296 }296 }
297 }297 }
298298
=== modified file 'worker/uniter/modes.go'
--- worker/uniter/modes.go 2013-02-15 10:09:00 +0000
+++ worker/uniter/modes.go 2013-02-25 11:02:25 +0000
@@ -141,6 +141,11 @@
141 } else if err != nil {141 } else if err != nil {
142 return nil, err142 return nil, err
143 }143 }
144 // Now the upgrade is complete, we'll need to check all
145 // relations again: some might previously have been skipped
146 // (if they involved endpoints only implemented in the new
147 // charm).
148 u.f.WantAllRelationsEvents()
144 return ModeContinue, nil149 return ModeContinue, nil
145 }150 }
146}151}
147152
=== modified file 'worker/uniter/relationer.go'
--- worker/uniter/relationer.go 2013-02-15 10:09:00 +0000
+++ worker/uniter/relationer.go 2013-02-25 11:02:25 +0000
@@ -51,9 +51,7 @@
51 if !ok {51 if !ok {
52 return fmt.Errorf("cannot enter scope: private-address not set")52 return fmt.Errorf("cannot enter scope: private-address not set")
53 }53 }
54 if err := r.dir.Ensure(); err != nil {54 // No need to check the r.dir exists yet
55 return err
56 }
57 return r.ru.EnterScope(map[string]interface{}{"private-address": address})55 return r.ru.EnterScope(map[string]interface{}{"private-address": address})
58}56}
5957
@@ -123,6 +121,10 @@
123 if err = r.dir.State().Validate(hi); err != nil {121 if err = r.dir.State().Validate(hi); err != nil {
124 return122 return
125 }123 }
124 // We are about to use the dir, ensure it's there
125 if err = r.dir.Ensure(); err != nil {
126 return
127 }
126 r.ctx.UpdateMembers(hi.Members)128 r.ctx.UpdateMembers(hi.Members)
127 if hi.Kind == hooks.RelationDeparted {129 if hi.Kind == hooks.RelationDeparted {
128 r.ctx.DeleteMember(hi.RemoteUnit)130 r.ctx.DeleteMember(hi.RemoteUnit)
129131
=== modified file 'worker/uniter/relationer_test.go'
--- worker/uniter/relationer_test.go 2013-02-15 10:09:00 +0000
+++ worker/uniter/relationer_test.go 2013-02-25 11:02:25 +0000
@@ -17,11 +17,12 @@
1717
18type RelationerSuite struct {18type RelationerSuite struct {
19 testing.JujuConnSuite19 testing.JujuConnSuite
20 hooks chan hook.Info20 hooks chan hook.Info
21 svc *state.Service21 svc *state.Service
22 rel *state.Relation22 rel *state.Relation
23 ru *state.RelationUnit23 ru *state.RelationUnit
24 dir *relation.StateDir24 dir *relation.StateDir
25 dirPath string
25}26}
2627
27var _ = Suite(&RelationerSuite{})28var _ = Suite(&RelationerSuite{})
@@ -36,7 +37,8 @@
36 c.Assert(rels, HasLen, 1)37 c.Assert(rels, HasLen, 1)
37 s.rel = rels[0]38 s.rel = rels[0]
38 s.ru = s.AddRelationUnit(c, "u/0")39 s.ru = s.AddRelationUnit(c, "u/0")
39 s.dir, err = relation.ReadStateDir(c.MkDir(), s.rel.Id())40 s.dirPath = c.MkDir()
41 s.dir, err = relation.ReadStateDir(s.dirPath, s.rel.Id())
40 c.Assert(err, IsNil)42 c.Assert(err, IsNil)
41 s.hooks = make(chan hook.Info)43 s.hooks = make(chan hook.Info)
42}44}
@@ -96,6 +98,12 @@
96 hi := hook.Info{Kind: hooks.RelationBroken}98 hi := hook.Info{Kind: hooks.RelationBroken}
97 _, err = r.PrepareHook(hi)99 _, err = r.PrepareHook(hi)
98 c.Assert(err, IsNil)100 c.Assert(err, IsNil)
101
102 // Verify PrepareHook created the dir
103 fi, err := os.Stat(filepath.Join(s.dirPath, strconv.Itoa(s.rel.Id())))
104 c.Assert(err, IsNil)
105 c.Assert(fi.IsDir(), Equals, true)
106
99 err = r.CommitHook(hi)107 err = r.CommitHook(hi)
100 c.Assert(err, IsNil)108 c.Assert(err, IsNil)
101 s.State.StartSync()109 s.State.StartSync()
@@ -344,6 +352,8 @@
344352
345func (s *RelationerSuite) assertHook(c *C, expect hook.Info) {353func (s *RelationerSuite) assertHook(c *C, expect hook.Info) {
346 s.State.StartSync()354 s.State.StartSync()
355 // We're no longer doing this in Join, so we must do it here
356 c.Assert(s.dir.Ensure(), IsNil)
347 select {357 select {
348 case hi, ok := <-s.hooks:358 case hi, ok := <-s.hooks:
349 c.Assert(ok, Equals, true)359 c.Assert(ok, Equals, true)
@@ -396,12 +406,11 @@
396 r := uniter.NewRelationer(ru, dir, hooks)406 r := uniter.NewRelationer(ru, dir, hooks)
397 c.Assert(r.IsImplicit(), Equals, true)407 c.Assert(r.IsImplicit(), Equals, true)
398408
399 // Join the relationer; check the dir was created and scope was entered.409 // Join the relationer; the dir won't be created until necessary
400 err = r.Join()410 err = r.Join()
401 c.Assert(err, IsNil)411 c.Assert(err, IsNil)
402 fi, err := os.Stat(filepath.Join(relsDir, strconv.Itoa(rel.Id())))412 _, err = os.Stat(filepath.Join(relsDir, strconv.Itoa(rel.Id())))
403 c.Assert(err, IsNil)413 c.Assert(err, NotNil)
404 c.Assert(fi.IsDir(), Equals, true)
405 sub, err := logging.Unit("logging/0")414 sub, err := logging.Unit("logging/0")
406 c.Assert(err, IsNil)415 c.Assert(err, IsNil)
407 err = sub.SetPrivateAddress("blah")416 err = sub.SetPrivateAddress("blah")
408417
=== modified file 'worker/uniter/uniter.go'
--- worker/uniter/uniter.go 2013-02-15 10:09:00 +0000
+++ worker/uniter/uniter.go 2013-02-25 11:02:25 +0000
@@ -315,6 +315,7 @@
315// restoreRelations reconciles the supplied relation state dirs with the315// restoreRelations reconciles the supplied relation state dirs with the
316// remote state of the corresponding relations.316// remote state of the corresponding relations.
317func (u *Uniter) restoreRelations() error {317func (u *Uniter) restoreRelations() error {
318 // TODO: Get these from state, not from disk
318 dirs, err := relation.ReadAllStateDirs(u.relationsDir)319 dirs, err := relation.ReadAllStateDirs(u.relationsDir)
319 if err != nil {320 if err != nil {
320 return err321 return err
@@ -376,6 +377,17 @@
376 if rel.Life() != state.Alive {377 if rel.Life() != state.Alive {
377 continue378 continue
378 }379 }
380 // Make sure we ignore relations not implemented by the unit's charm
381 ch, err := corecharm.ReadDir(u.charm.Path())
382 if err != nil {
383 return nil, err
384 }
385 if ep, err := rel.Endpoint(u.unit.ServiceName()); err != nil {
386 return nil, err
387 } else if !ep.ImplementedBy(ch) {
388 log.Printf("worker/uniter: skipping relation with unknown endpoint %q", ep)
389 continue
390 }
379 dir, err := relation.ReadStateDir(u.relationsDir, id)391 dir, err := relation.ReadStateDir(u.relationsDir, id)
380 if err != nil {392 if err != nil {
381 return nil, err393 return nil, err
382394
=== modified file 'worker/uniter/uniter_test.go'
--- worker/uniter/uniter_test.go 2013-02-18 13:18:29 +0000
+++ worker/uniter/uniter_test.go 2013-02-25 11:02:25 +0000
@@ -143,13 +143,13 @@
143func (ctx *context) matchLogHooks(c *C) (match bool, overshoot bool) {143func (ctx *context) matchLogHooks(c *C) (match bool, overshoot bool) {
144 // hookPattern matches juju-log calls as generated by writeHook.144 // hookPattern matches juju-log calls as generated by writeHook.
145 hookPattern := fmt.Sprintf(`^.* JUJU `+145 hookPattern := fmt.Sprintf(`^.* JUJU `+
146 `u/0(| [a-z-]+:[0-9]+)`+ // juju-log badge; group matches relation id146 `u/0(| [a-z0-9-]+:[0-9]+)`+ // juju-log badge; group matches relation id
147 `: UniterSuite-%d`+ // test badge; prevents cross-pollution147 `: UniterSuite-%d`+ // test badge; prevents cross-pollution
148 ` ([0-9a-z-/ ]+)$`, // foo-relation-joined bar/123148 ` ([0-9a-z-/ ]+)$`, // foo-relation-joined bar/123
149 ctx.id,149 ctx.id,
150 )150 )
151 // donePattern matches uniter logging that indicates a hook has run.151 // donePattern matches uniter logging that indicates a hook has run.
152 donePattern := `^.* JUJU worker/uniter: (ran "[a-z-]+" hook|hook failed)`152 donePattern := `^.* JUJU worker/uniter: (ran "[a-z0-9-]+" hook|hook failed)`
153 hookRegexp := regexp.MustCompile(hookPattern)153 hookRegexp := regexp.MustCompile(hookPattern)
154 doneRegexp := regexp.MustCompile(donePattern)154 doneRegexp := regexp.MustCompile(donePattern)
155155
@@ -319,7 +319,7 @@
319 ut(319 ut(
320 "steady state config change with config-get verification",320 "steady state config change with config-get verification",
321 createCharm{321 createCharm{
322 customize: func(c *C, path string) {322 customize: func(c *C, ctx *context, path string) {
323 appendHook(c, path, "config-changed", "config-get --format yaml --output config.out")323 appendHook(c, path, "config-changed", "config-get --format yaml --output config.out")
324 },324 },
325 },325 },
@@ -520,7 +520,7 @@
520 ), ut(520 ), ut(
521 `upgrade: conflicting directories`,521 `upgrade: conflicting directories`,
522 createCharm{522 createCharm{
523 customize: func(c *C, path string) {523 customize: func(c *C, ctx *context, path string) {
524 err := os.Mkdir(filepath.Join(path, "data"), 0755)524 err := os.Mkdir(filepath.Join(path, "data"), 0755)
525 c.Assert(err, IsNil)525 c.Assert(err, IsNil)
526 appendHook(c, path, "start", "echo DATA > data/newfile")526 appendHook(c, path, "start", "echo DATA > data/newfile")
@@ -536,7 +536,7 @@
536536
537 createCharm{537 createCharm{
538 revision: 1,538 revision: 1,
539 customize: func(c *C, path string) {539 customize: func(c *C, ctx *context, path string) {
540 data := filepath.Join(path, "data")540 data := filepath.Join(path, "data")
541 err := ioutil.WriteFile(data, []byte("<nelson>ha ha</nelson>"), 0644)541 err := ioutil.WriteFile(data, []byte("<nelson>ha ha</nelson>"), 0644)
542 c.Assert(err, IsNil)542 c.Assert(err, IsNil)
@@ -563,7 +563,7 @@
563 startUpgradeError{},563 startUpgradeError{},
564 createCharm{564 createCharm{
565 revision: 2,565 revision: 2,
566 customize: func(c *C, path string) {566 customize: func(c *C, ctx *context, path string) {
567 otherdata := filepath.Join(path, "otherdata")567 otherdata := filepath.Join(path, "otherdata")
568 err := ioutil.WriteFile(otherdata, []byte("blah"), 0644)568 err := ioutil.WriteFile(otherdata, []byte("blah"), 0644)
569 c.Assert(err, IsNil)569 c.Assert(err, IsNil)
@@ -614,6 +614,27 @@
614 unitDead,614 unitDead,
615 waitUniterDead{},615 waitUniterDead{},
616 waitHooks{},616 waitHooks{},
617 ), ut(
618 // This test does and add-relation as quickly as possible
619 // after an upgrade-charm, in the hope that the scheduler will
620 // deliver the events in the wrong order. The observed
621 // behaviour should be the same in either case.
622 "ignore unknown relations until upgrade is done",
623 quickStart{},
624 createCharm{
625 revision: 2,
626 customize: func(c *C, ctx *context, path string) {
627 renameRelation(c, path, "db", "db2")
628 hpath := filepath.Join(path, "hooks", "db2-relation-joined")
629 ctx.writeHook(c, hpath, true)
630 },
631 },
632 serveCharm{},
633 upgradeCharm{revision: 2},
634 addRelation{},
635 addRelationUnit{},
636 waitHooks{"upgrade-charm", "config-changed", "db2-relation-joined mysql/0 db2:0"},
637 verifyCharm{revision: 2},
617 ),638 ),
618639
619 // Relations.640 // Relations.
@@ -783,7 +804,7 @@
783type createCharm struct {804type createCharm struct {
784 revision int805 revision int
785 badHooks []string806 badHooks []string
786 customize func(*C, string)807 customize func(*C, *context, string)
787}808}
788809
789var charmHooks = []string{810var charmHooks = []string{
@@ -805,7 +826,7 @@
805 ctx.writeHook(c, path, good)826 ctx.writeHook(c, path, good)
806 }827 }
807 if s.customize != nil {828 if s.customize != nil {
808 s.customize(c, base)829 s.customize(c, ctx, base)
809 }830 }
810 dir, err := charm.ReadDir(base)831 dir, err := charm.ReadDir(base)
811 c.Assert(err, IsNil)832 c.Assert(err, IsNil)
@@ -1199,7 +1220,7 @@
1199func (s startUpgradeError) step(c *C, ctx *context) {1220func (s startUpgradeError) step(c *C, ctx *context) {
1200 steps := []stepper{1221 steps := []stepper{
1201 createCharm{1222 createCharm{
1202 customize: func(c *C, path string) {1223 customize: func(c *C, ctx *context, path string) {
1203 appendHook(c, path, "start", "echo STARTDATA > data")1224 appendHook(c, path, "start", "echo STARTDATA > data")
1204 },1225 },
1205 },1226 },
@@ -1213,7 +1234,7 @@
12131234
1214 createCharm{1235 createCharm{
1215 revision: 1,1236 revision: 1,
1216 customize: func(c *C, path string) {1237 customize: func(c *C, ctx *context, path string) {
1217 data := filepath.Join(path, "data")1238 data := filepath.Join(path, "data")
1218 err := ioutil.WriteFile(data, []byte("<nelson>ha ha</nelson>"), 0644)1239 err := ioutil.WriteFile(data, []byte("<nelson>ha ha</nelson>"), 0644)
1219 c.Assert(err, IsNil)1240 c.Assert(err, IsNil)
@@ -1472,3 +1493,35 @@
1472 _, err = f.Write([]byte(data))1493 _, err = f.Write([]byte(data))
1473 c.Assert(err, IsNil)1494 c.Assert(err, IsNil)
1474}1495}
1496
1497func renameRelation(c *C, charmPath, oldName, newName string) {
1498 path := filepath.Join(charmPath, "metadata.yaml")
1499 f, err := os.Open(path)
1500 c.Assert(err, IsNil)
1501 defer f.Close()
1502 meta, err := charm.ReadMeta(f)
1503 c.Assert(err, IsNil)
1504
1505 replace := func(what map[string]charm.Relation) bool {
1506 for relName, relation := range what {
1507 if relName == oldName {
1508 what[newName] = relation
1509 delete(what, oldName)
1510 return true
1511 }
1512 }
1513 return false
1514 }
1515 replaced := replace(meta.Provides) || replace(meta.Requires) || replace(meta.Peers)
1516 c.Assert(replaced, Equals, true, Commentf("charm %q does not implement relation %q", charmPath, oldName))
1517
1518 newmeta, err := goyaml.Marshal(meta)
1519 c.Assert(err, IsNil)
1520 ioutil.WriteFile(path, newmeta, 0644)
1521
1522 f, err = os.Open(path)
1523 c.Assert(err, IsNil)
1524 defer f.Close()
1525 meta, err = charm.ReadMeta(f)
1526 c.Assert(err, IsNil)
1527}

Subscribers

People subscribed via source and target branches