Skip to content

Commit

Permalink
Merge pull request #10301 from manadart/2.5-write-leader-settings-guard
Browse files Browse the repository at this point in the history
#10301

## Description of change

This is a reversion of a single commit from #9353.

The commit logs a warning instead of returning an error when `LeaderSettingsChanged` is called by a non-leader unit.

That commit should not have been retained in the patch, which ultimately focussed on re-checking leader status when preparing to fire the _leader-elected_ hook.

## QA steps

- Bootstrap.
- `juju deploy ubuntu -n 2`
- `juju run --unit <whichever is not the leader> leader-set`.
- Result should be: `ERROR cannot write leadership settings: cannot write settings: not the leader`.

## Documentation changes

None.

## Bug reference

Original: https://bugs.launchpad.net/juju-core/+bug/1723184
  • Loading branch information
jujubot committed Jun 10, 2019
2 parents 890482e + a9a4ead commit 72d9064
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 9 deletions.
4 changes: 0 additions & 4 deletions worker/uniter/runner/context/leader.go
Expand Up @@ -74,10 +74,6 @@ func (ctx *leadershipContext) WriteLeaderSettings(settings map[string]string) er
// `apiserver/leadership.LeadershipSettingsAccessor.Merge`, and as of
// 2015-02-19 it's better to stay eager.
err := ctx.ensureLeader()
if err == errIsMinion {
logger.Warningf("skipping write settings; not the leader")
return nil
}
if err == nil {
// Clear local settings; if we need them again we should use the values
// as merged by the server. But we don't need to get them again right now;
Expand Down
9 changes: 4 additions & 5 deletions worker/uniter/runner/context/leader_test.go
Expand Up @@ -201,17 +201,16 @@ func (s *LeaderSuite) TestWriteLeaderSettingsMinion(c *gc.C) {
s.CheckCalls(c, []testing.StubCall{{
FuncName: "ClaimLeader",
}}, func() {
// We are not the leader.
// The first call fails...
s.tracker.results = []StubTicket{false}
err := s.context.WriteLeaderSettings(map[string]string{"blah": "blah"})
// No error, no call to Merge.
c.Check(err, jc.ErrorIsNil)
c.Check(err, gc.ErrorMatches, "cannot write settings: not the leader")
})

s.CheckCalls(c, nil, func() {
// ctx.isMinion is now true. No call to claim leader.
// The second doesn't even try.
err := s.context.WriteLeaderSettings(map[string]string{"blah": "blah"})
c.Check(err, jc.ErrorIsNil)
c.Check(err, gc.ErrorMatches, "cannot write settings: not the leader")
})
}

Expand Down

0 comments on commit 72d9064

Please sign in to comment.