Skip to content

Commit

Permalink
Merge pull request #14927 from manadart/3.0-dqlite-lease-store-interface
Browse files Browse the repository at this point in the history
[JUJU-2259] Add errors to lease store interface function returns
  • Loading branch information
manadart committed Nov 29, 2022
2 parents 1657a1d + fcbbbb1 commit a73b947
Show file tree
Hide file tree
Showing 19 changed files with 186 additions and 216 deletions.
4 changes: 4 additions & 0 deletions api/common/leadership.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ func (a *LeadershipPinningAPI) PinnedLeadership() (map[string][]names.Tag, error
return nil, errors.Trace(err)
}

if callResult.Error != nil {
return nil, errors.Trace(callResult.Error)
}

pinned := make(map[string][]names.Tag, len(callResult.Result))
for app, entities := range callResult.Result {
entityTags := make([]names.Tag, len(entities))
Expand Down
10 changes: 10 additions & 0 deletions api/common/leadership_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@ func (s *LeadershipSuite) TestPinnedLeadership(c *gc.C) {
c.Check(res, gc.DeepEquals, map[string][]names.Tag{"redis": {names.NewMachineTag("0"), names.NewMachineTag("1")}})
}

func (s *LeadershipSuite) TestPinnedLeadershipError(c *gc.C) {
defer s.setup(c).Finish()

resultSource := params.PinnedLeadershipResult{Error: apiservererrors.ServerError(errors.New("splat"))}
s.facade.EXPECT().FacadeCall("PinnedLeadership", nil, gomock.Any()).SetArg(2, resultSource)

_, err := s.client.PinnedLeadership()
c.Assert(err, gc.ErrorMatches, "splat")
}

func (s *LeadershipSuite) TestPinMachineApplicationsSuccess(c *gc.C) {
defer s.setup(c).Finish()

Expand Down
7 changes: 5 additions & 2 deletions apiserver/common/leadership.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type leadershipMachine struct {
*state.Machine
}

// LeadershipPinningBacked describes state method wrappers used by this API.
// LeadershipPinningBackend describes state method wrappers used by this API.
type LeadershipPinningBackend interface {
Machine(string) (LeadershipMachine, error)
}
Expand Down Expand Up @@ -96,7 +96,10 @@ func (a *LeadershipPinning) PinnedLeadership() (params.PinnedLeadershipResult, e
return result, apiservererrors.ErrPerm
}

result.Result = a.pinner.PinnedLeadership()
result.Result, err = a.pinner.PinnedLeadership()
if err != nil {
result.Error = apiservererrors.ServerError(err)
}
return result, nil
}

Expand Down
2 changes: 1 addition & 1 deletion apiserver/common/leadership_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (s *LeadershipSuite) TestPinnedLeadershipSuccess(c *gc.C) {
defer s.setup(c).Finish()

pinned := map[string][]string{"redis": {"machine-0", "machine-1"}}
s.pinner.EXPECT().PinnedLeadership().Return(pinned)
s.pinner.EXPECT().PinnedLeadership().Return(pinned, nil)

res, err := s.api.PinnedLeadership()
c.Assert(err, jc.ErrorIsNil)
Expand Down
8 changes: 5 additions & 3 deletions apiserver/leadership.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@ func (m leadershipPinner) UnpinLeadership(applicationName string, entity string)
// PinnedLeadership (leadership.Pinner) returns applications for which
// leadership is pinned, along with the entities requiring the
// pinned behaviour.
func (m leadershipPinner) PinnedLeadership() map[string][]string {
return m.pinner.Pinned()
func (m leadershipPinner) PinnedLeadership() (map[string][]string, error) {
pinned, err := m.pinner.Pinned()
return pinned, errors.Trace(err)
}

// leadershipReader implements leadership.Reader by wrapping a lease.Reader.
Expand All @@ -112,5 +113,6 @@ type leadershipReader struct {
// Leaders (leadership.Reader) returns all application leaders in the
// current model.
func (r leadershipReader) Leaders() (map[string]string, error) {
return r.reader.Leases(), nil
leaders, err := r.reader.Leases()
return leaders, errors.Trace(err)
}
2 changes: 1 addition & 1 deletion core/leadership/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ type Pinner interface {

// PinnedLeadership returns a map keyed on pinned application names,
// with entities that require the application's pinned behaviour.
PinnedLeadership() map[string][]string
PinnedLeadership() (map[string][]string, error)
}

// Token represents a unit's leadership of its application.
Expand Down
5 changes: 3 additions & 2 deletions core/leadership/mocks/leadership_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions core/lease/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ type Pinner interface {

// Pinned returns all names for pinned leases, with the entities requiring
// their pinned behaviour.
Pinned() map[string][]string
Pinned() (map[string][]string, error)
}

// Checker exposes facts about lease ownership.
Expand All @@ -80,7 +80,7 @@ type Token interface {
// Reader describes retrieval of all leases and holders
// for a known namespace and model.
type Reader interface {
Leases() map[string]string
Leases() (map[string]string, error)
}

// Manager describes methods for acquiring objects that manipulate and query
Expand Down
6 changes: 3 additions & 3 deletions core/lease/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ type Store interface {
// Leases returns a recent snapshot of lease state. Expiry times are
// expressed according to the Clock the store was configured with.
// Supplying any lease keys will filter the return for those requested.
Leases(keys ...Key) map[Key]Info
Leases(keys ...Key) (map[Key]Info, error)

// LeaseGroup returns a snapshot of all of the leases for a
// particular namespace/model combination. This is useful for
// reporting holder information for a model, and can often be
// implemented more efficiently than getting all leases when there
// are many models.
LeaseGroup(namespace, modelUUID string) map[Key]Info
LeaseGroup(namespace, modelUUID string) (map[Key]Info, error)

// PinLease ensures that the current holder of the lease for the input key
// will not lose the lease to expiry.
Expand All @@ -59,7 +59,7 @@ type Store interface {
// Pinned returns a snapshot of pinned leases.
// The return consists of each pinned lease and the collection of entities
// requiring its pinned behaviour.
Pinned() map[Key][]string
Pinned() (map[Key][]string, error)
}

// Key fully identifies a lease, including the namespace and
Expand Down
12 changes: 6 additions & 6 deletions core/raftlease/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,13 @@ func (s *Store) RevokeLease(key lease.Key, holder string, stop <-chan struct{})
}

// Leases is part of lease.Store.
func (s *Store) Leases(keys ...lease.Key) map[lease.Key]lease.Info {
return s.fsm.Leases(s.config.Clock.Now, keys...)
func (s *Store) Leases(keys ...lease.Key) (map[lease.Key]lease.Info, error) {
return s.fsm.Leases(s.config.Clock.Now, keys...), nil
}

// LeaseGroup is part of Lease.Store.
func (s *Store) LeaseGroup(namespace, modelUUID string) map[lease.Key]lease.Info {
return s.fsm.LeaseGroup(s.config.Clock.Now, namespace, modelUUID)
func (s *Store) LeaseGroup(namespace, modelUUID string) (map[lease.Key]lease.Info, error) {
return s.fsm.LeaseGroup(s.config.Clock.Now, namespace, modelUUID), nil
}

// PinLease is part of lease.Store.
Expand All @@ -136,8 +136,8 @@ func (s *Store) UnpinLease(key lease.Key, entity string, stop <-chan struct{}) e
}

// Pinned is part of the Store interface.
func (s *Store) Pinned() map[lease.Key][]string {
return s.fsm.Pinned()
func (s *Store) Pinned() (map[lease.Key][]string, error) {
return s.fsm.Pinned(), nil
}

func (s *Store) pinOp(operation string, key lease.Key, entity string, stop <-chan struct{}) error {
Expand Down
15 changes: 11 additions & 4 deletions core/raftlease/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,8 @@ func (s *storeSuite) TestLeases(c *gc.C) {
Holder: "mozart",
Expiry: in5Seconds,
}
result := s.store.Leases()
result, err := s.store.Leases()
c.Assert(err, jc.ErrorIsNil)
c.Assert(len(result), gc.Equals, 2)

r1 := result[lease1]
Expand All @@ -441,7 +442,9 @@ func (s *storeSuite) TestLeasesFilter(c *gc.C) {
lease1 := lease.Key{Namespace: "quam", ModelUUID: "olim", Lease: "abrahe"}
lease2 := lease.Key{Namespace: "la", ModelUUID: "cry", Lease: "mosa"}

_ = s.store.Leases(lease1, lease2)
_, err := s.store.Leases(lease1, lease2)
c.Assert(err, jc.ErrorIsNil)

s.fsm.CheckCallNames(c, "Leases")
c.Check(s.fsm.Calls()[0].Args[1], jc.SameContents, []lease.Key{lease1, lease2})
}
Expand All @@ -459,7 +462,8 @@ func (s *storeSuite) TestLeaseGroup(c *gc.C) {
Holder: "mozart",
Expiry: in5Seconds,
}
result := s.store.LeaseGroup("ns", "model")
result, err := s.store.LeaseGroup("ns", "model")
c.Assert(err, jc.ErrorIsNil)
c.Assert(len(result), gc.Equals, 2)
s.fsm.CheckCall(c, 0, "LeaseGroup", s.clock.Now(), "ns", "model")

Expand Down Expand Up @@ -678,7 +682,10 @@ func (s *storeSuite) TestUnpinTimeout(c *gc.C) {

func (s *storeSuite) TestPinned(c *gc.C) {
s.fsm.pinned = map[lease.Key][]string{}
c.Check(s.store.Pinned(), gc.DeepEquals, s.fsm.pinned)

pinned, err := s.store.Pinned()
c.Assert(err, jc.ErrorIsNil)
c.Check(pinned, gc.DeepEquals, s.fsm.pinned)
s.fsm.CheckCallNames(c, "Pinned")
}

Expand Down
16 changes: 8 additions & 8 deletions provider/dummy/leasestore.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (s *leaseStore) RevokeLease(key lease.Key, _ string, _ <-chan struct{}) err
}

// Leases is part of lease.Store.
func (s *leaseStore) Leases(keys ...lease.Key) map[lease.Key]lease.Info {
func (s *leaseStore) Leases(keys ...lease.Key) (map[lease.Key]lease.Info, error) {
s.mu.Lock()
defer s.mu.Unlock()

Expand All @@ -117,22 +117,22 @@ func (s *leaseStore) Leases(keys ...lease.Key) map[lease.Key]lease.Info {
Expiry: entry.start.Add(entry.duration),
}
}
return results
return results, nil
}

// LeaseGroup is part of lease.Store.
func (s *leaseStore) LeaseGroup(namespace, modelUUID string) map[lease.Key]lease.Info {
leases := s.Leases()
func (s *leaseStore) LeaseGroup(namespace, modelUUID string) (map[lease.Key]lease.Info, error) {
leases, _ := s.Leases()
if len(leases) == 0 {
return leases
return leases, nil
}
results := make(map[lease.Key]lease.Info)
for key, info := range leases {
if key.Namespace == namespace && key.ModelUUID == modelUUID {
results[key] = info
}
}
return results
return results, nil
}

// PinLease is part of lease.Store.
Expand All @@ -146,6 +146,6 @@ func (s *leaseStore) UnpinLease(lease.Key, string, <-chan struct{}) error {
}

// Pinned is part of the Store interface.
func (s *leaseStore) Pinned() map[lease.Key][]string {
return nil
func (s *leaseStore) Pinned() (map[lease.Key][]string, error) {
return nil, nil
}
6 changes: 5 additions & 1 deletion rpc/params/leadership.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,16 @@ type PinApplicationResult struct {
Error *Error `json:"error,omitempty"`
}

// PinnedLeadershipResults holds data representing the current applications for
// PinnedLeadershipResult holds data representing the current applications for
// which leadership is pinned
type PinnedLeadershipResult struct {
// Result has:
// - Application name keys representing the application pinned.
// - Tag slice values representing the entities requiring pinned
// behaviour for each application.
Result map[string][]string `json:"result,omitempty"`

// Error will contain a reference to an error resulting from
// reading lease data, if one occurred.
Error *Error `json:"error,omitempty"`
}
10 changes: 6 additions & 4 deletions worker/lease/bound.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@ func (b *boundManager) Token(leaseName, holderName string) lease.Token {

// Pinned (lease.Pinner) returns applications and the entities requiring their
// pinned behaviour, for pinned leases in the bound namespace/model.
func (b *boundManager) Pinned() map[string][]string {
return b.manager.pinned(b.namespace, b.modelUUID)
func (b *boundManager) Pinned() (map[string][]string, error) {
pinned, err := b.manager.pinned(b.namespace, b.modelUUID)
return pinned, errors.Trace(err)
}

// Pin (lease.Pinner) sends a pin message to the worker loop.
Expand All @@ -113,8 +114,9 @@ func (b *boundManager) Unpin(leaseName string, entity string) error {

// Leases (lease.Reader) returns all leases and holders
// in the bound namespace/model.
func (b *boundManager) Leases() map[string]string {
return b.manager.leases(b.namespace, b.modelUUID)
func (b *boundManager) Leases() (map[string]string, error) {
leases, err := b.manager.leases(b.namespace, b.modelUUID)
return leases, errors.Trace(err)
}

// pinOp creates a pin instance from the input lease name,
Expand Down
89 changes: 0 additions & 89 deletions worker/lease/dead_manager_test.go

This file was deleted.

Loading

0 comments on commit a73b947

Please sign in to comment.