Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleaning of stale leases #2452

Merged
merged 47 commits into from
May 5, 2017
Merged
Changes from 1 commit
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
1e5d6e3
Added sys/tidy-leases endpoint
vishalnayak Mar 7, 2017
239bd1c
Add locking where possible while doing auth/token/tidy
vishalnayak Mar 7, 2017
14aaa0a
Merge branch 'oss' into clean-stale-leases
vishalnayak Apr 26, 2017
3477038
Added atomic lock to ensure a single tidy operation is in progress
vishalnayak Apr 26, 2017
e52625d
Revoke lease that has empty token; added logs
vishalnayak Apr 26, 2017
dca0d70
Added logger to token store and logs to tidy function
vishalnayak Apr 26, 2017
de1a2a0
Added caching of looked up tokens
vishalnayak Apr 26, 2017
65c63b4
Fix the log statements
vishalnayak Apr 26, 2017
b036478
Fix logging levels
vishalnayak Apr 26, 2017
711153d
Fix logging suggestions; put the policyStore nil check back in
vishalnayak Apr 27, 2017
0d629ff
Cache only valid tokens
vishalnayak Apr 27, 2017
0c65cd4
Some more logging updates
vishalnayak Apr 27, 2017
785177a
Merge branch 'oss' into sys-tidy-leases
vishalnayak Apr 27, 2017
3fdf38a
Distinguish valid and invalid tokens using bool value in cache
vishalnayak Apr 27, 2017
98cdb68
Use an atomic lock for tidy operation in token store
vishalnayak Apr 27, 2017
2ef62fe
refactor lock handling in token tidy function
vishalnayak Apr 27, 2017
a8ef2c0
Refactor locking code in lease tidy; add ending debug statements
vishalnayak Apr 27, 2017
0892102
Merge branch 'oss' into sys-tidy-leases
vishalnayak May 1, 2017
8c7b175
Skip checking the validity of an empty client token
vishalnayak May 2, 2017
853233a
Added a test for tidying of empty token
vishalnayak May 2, 2017
d07d3cb
Added steps to check if invalid token is properly cleaned up
vishalnayak May 2, 2017
79fc0d8
Check if multiple leases with same invalid token is getting cleaned up
vishalnayak May 2, 2017
497bebe
Do not duplicate log lines for invalid leases
vishalnayak May 2, 2017
aa08e5c
Added test to check the atomicity of the lease tidy operation
vishalnayak May 2, 2017
a3c2a42
Test to check that leases with valid tokens are not being cleaned up
vishalnayak May 2, 2017
b6843ec
Added summary logs to help better understand the consequence
vishalnayak May 3, 2017
0c02540
Less scary debugging
jefferai May 3, 2017
b3c6a56
change some logging output
jefferai May 3, 2017
415b0a2
Two things:
jefferai May 3, 2017
0bda5a7
Adhere to tainted status in salted accessor lookup
jefferai May 3, 2017
8d35f92
consistent logging
vishalnayak May 3, 2017
a2e431b
Added logs when deletion fails so we can rely on server logs
vishalnayak May 3, 2017
2d21bf6
logging updates
vishalnayak May 3, 2017
5bc47b0
Add taint flag for looking up by accessor
jefferai May 3, 2017
b0c4a7e
Add more cleanup if a lease fails to register and revoke tokens if re…
jefferai May 3, 2017
265b4cd
Merge remote-tracking branch 'oss/master' into sys-tidy-leases
jefferai May 3, 2017
2f6e924
Fix substitution of index/child in delete call
jefferai May 3, 2017
f1d2fc3
Merge branch 'master-oss' into sys-tidy-leases
jefferai May 4, 2017
5320da0
Move tidy-leases to leases/tidy
jefferai May 4, 2017
cacf072
Update commenting
jefferai May 4, 2017
4de09fb
Update Tidy function comment
vishalnayak May 4, 2017
106f08a
Fix up the tests
vishalnayak May 4, 2017
7829107
Update comments
jefferai May 4, 2017
1378dd5
Move client token check in exp register to top
jefferai May 4, 2017
5dde45d
Address feedback
jefferai May 5, 2017
e61298e
Update debugging around tidy
jefferai May 5, 2017
0e10477
Merge branch 'master-oss' into sys-tidy-leases
jefferai May 5, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Distinguish valid and invalid tokens using bool value in cache
vishalnayak committed Apr 27, 2017
commit 3fdf38a58a990d4a9cd7d148c8bc3178e55fb4d3
43 changes: 25 additions & 18 deletions vault/expiration.go
Original file line number Diff line number Diff line change
@@ -124,7 +124,7 @@ func (m *ExpirationManager) Tidy() error {
var tidyErrors *multierror.Error

// Create a cache to keep track of looked up tokens
validTokenCache := make(map[string]struct{})
tokenCache := make(map[string]bool)
i := 0

tidyFunc := func(leaseID string) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some progress logging so people can be aware things are still happening.

@@ -150,26 +150,33 @@ func (m *ExpirationManager) Tidy() error {
revokeLease = true
}

if _, ok := validTokenCache[le.ClientToken]; ok {
return
}
isValid, ok := tokenCache[le.ClientToken]
if !ok {
saltedID := m.tokenStore.SaltID(le.ClientToken)
lock := locksutil.LockForKey(m.tokenStore.tokenLocks, le.ClientToken)
lock.RLock()
te, err := m.tokenStore.lookupSalted(saltedID, true)
lock.RUnlock()

saltedID := m.tokenStore.SaltID(le.ClientToken)
lock := locksutil.LockForKey(m.tokenStore.tokenLocks, le.ClientToken)
lock.RLock()
te, err := m.tokenStore.lookupSalted(saltedID, true)
lock.RUnlock()

if err != nil {
tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to lookup token: %v", err))
return
}
if err != nil {
tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to lookup token: %v", err))
return
}

if te == nil {
m.logger.Debug("expiration: lease has an invalid token", "lease_id", leaseID)
revokeLease = true
if te == nil {
m.logger.Debug("expiration: lease has an invalid token", "lease_id", leaseID)
revokeLease = true
tokenCache[le.ClientToken] = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary, but I think you might as well add a goto REVOKE_CHECK statement here incase more branches are added in the future. Makes it slightly more extendable.

} else {
tokenCache[le.ClientToken] = true
}
} else {
validTokenCache[le.ClientToken] = struct{}{}
if isValid {
return
} else {
m.logger.Debug("expiration: lease has an invalid token", "lease_id", leaseID)
revokeLease = true
}
}

if revokeLease {