Skip to content

Commit

Permalink
sort first by leaseID, then stable sort by expiration
Browse files Browse the repository at this point in the history
  • Loading branch information
swayne275 committed Jun 1, 2021
1 parent 33c1d4c commit 23a1082
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 22 deletions.
19 changes: 12 additions & 7 deletions vault/expiration.go
Original file line number Diff line number Diff line change
Expand Up @@ -2490,9 +2490,10 @@ func (m *ExpirationManager) getIrrevocableLeaseCounts(ctx context.Context, inclu
}

type leaseResponse struct {
LeaseID string `json:"lease_id"`
MountID string `json:"mount_id"`
ErrMsg string `json:"error"`
LeaseID string `json:"lease_id"`
MountID string `json:"mount_id"`
ErrMsg string `json:"error"`
expireTime time.Time
}

// returns a warning string, if applicable
Expand Down Expand Up @@ -2536,18 +2537,22 @@ func (m *ExpirationManager) listIrrevocableLeases(ctx context.Context, includeCh

numMatchingLeases++
matchingLeases = append(matchingLeases, &leaseResponse{
LeaseID: leaseID,
MountID: mountAccessor,
ErrMsg: leaseInfo.RevokeErr,
LeaseID: leaseID,
MountID: mountAccessor,
ErrMsg: leaseInfo.RevokeErr,
expireTime: leaseInfo.ExpireTime,
})

return true
})

// sort the results for consistent API response
// sort the results for consistent API response, with the least fresh leases first in the list
sort.Slice(matchingLeases, func(i, j int) bool {
return matchingLeases[i].LeaseID < matchingLeases[j].LeaseID
})
sort.SliceStable(matchingLeases, func(i, j int) bool {
return matchingLeases[i].expireTime.Before(matchingLeases[j].expireTime)
})

resp := make(map[string]interface{})
resp["lease_count"] = numMatchingLeases
Expand Down
18 changes: 9 additions & 9 deletions vault/expiration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3095,18 +3095,15 @@ func TestExpiration_listIrrevocableLeases(t *testing.T) {

exp := c.expiration

type basicLeaseInfo struct {
id string
mount string
}
expectedLeases := make([]*basicLeaseInfo, 0)
expectedLeases := make([]*basicLeaseTestInfo, 0)
expectedPerMount := 10
for i := 0; i < expectedPerMount; i++ {
for _, mountPrefix := range mountPrefixes {
leaseID := addIrrevocableLease(t, exp, mountPrefix, namespace.RootNamespace)
expectedLeases = append(expectedLeases, &basicLeaseInfo{
id: leaseID,
mount: pathToMount[mountPrefix],
le := addIrrevocableLease(t, exp, mountPrefix, namespace.RootNamespace)
expectedLeases = append(expectedLeases, &basicLeaseTestInfo{
id: le.id,
mount: pathToMount[mountPrefix],
expire: le.expire,
})
}
}
Expand Down Expand Up @@ -3144,6 +3141,9 @@ func TestExpiration_listIrrevocableLeases(t *testing.T) {
sort.Slice(expectedLeases, func(i, j int) bool {
return expectedLeases[i].id < expectedLeases[j].id
})
sort.SliceStable(expectedLeases, func(i, j int) bool {
return expectedLeases[i].expire.Before(expectedLeases[j].expire)
})

for i, lease := range expectedLeases {
if lease.id != leases[i].LeaseID {
Expand Down
23 changes: 17 additions & 6 deletions vault/expiration_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package vault
import (
"context"
"fmt"
"math/rand"
"testing"
"time"

Expand Down Expand Up @@ -33,9 +34,15 @@ func (m *ExpirationManager) collectLeases() (map[*namespace.Namespace][]string,
return existing, leaseCount, nil
}

type basicLeaseTestInfo struct {
id string
mount string
expire time.Time
}

// add an irrevocable lease for test purposes
// returns the lease ID for the lease
func addIrrevocableLease(t *testing.T, m *ExpirationManager, pathPrefix string, ns *namespace.Namespace) string {
// returns the lease ID and expire time
func addIrrevocableLease(t *testing.T, m *ExpirationManager, pathPrefix string, ns *namespace.Namespace) *basicLeaseTestInfo {
t.Helper()

uuid, err := uuid.GenerateUUID()
Expand All @@ -52,12 +59,13 @@ func addIrrevocableLease(t *testing.T, m *ExpirationManager, pathPrefix string,
nsSuffix = fmt.Sprintf("/blah.%s", ns.ID)
}

randomTimeDelta := time.Duration(rand.Int31n(24))
le := &leaseEntry{
LeaseID: pathPrefix + "lease" + uuid + nsSuffix,
Path: pathPrefix + nsSuffix,
namespace: ns,
IssueTime: time.Now(),
ExpireTime: time.Now().Add(time.Hour),
ExpireTime: time.Now().Add(randomTimeDelta * time.Hour),
RevokeErr: "some error message",
}

Expand All @@ -70,7 +78,10 @@ func addIrrevocableLease(t *testing.T, m *ExpirationManager, pathPrefix string,

m.updatePendingInternal(le)

return le.LeaseID
return &basicLeaseTestInfo{
id: le.LeaseID,
expire: le.ExpireTime,
}
}

// InjectIrrevocableLeases injects `count` irrevocable leases (currently to a
Expand All @@ -79,9 +90,9 @@ func addIrrevocableLease(t *testing.T, m *ExpirationManager, pathPrefix string,
func (c *Core) InjectIrrevocableLeases(t *testing.T, ctx context.Context, count int) map[string]int {
out := make(map[string]int)
for i := 0; i < count; i++ {
leaseID := addIrrevocableLease(t, c.expiration, "foo/", namespace.RootNamespace)
le := addIrrevocableLease(t, c.expiration, "foo/", namespace.RootNamespace)

mountAccessor := c.expiration.getLeaseMountAccessor(ctx, leaseID)
mountAccessor := c.expiration.getLeaseMountAccessor(ctx, le.id)
if _, ok := out[mountAccessor]; !ok {
out[mountAccessor] = 0
}
Expand Down

0 comments on commit 23a1082

Please sign in to comment.