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

Ensure concierge and supervisor gracefully exit #829

Merged
merged 1 commit into from Aug 31, 2021

Conversation

enj
Copy link
Contributor

@enj enj commented Aug 30, 2021

Changes made to both components:

  1. Logs are always flushed on process exit
  2. Informer cache sync can no longer hang process start up forever

Changes made to concierge:

  1. Add pre-shutdown hook that waits for controllers to exit cleanly
  2. Informer caches are synced in post-start hook

Changes made to supervisor:

  1. Add shutdown code that waits for controllers to exit cleanly
  2. Add shutdown code that waits for active connections to become idle

Waiting for controllers to exit cleanly is critical as this allows
the leader election logic to release the lock on exit. This reduces
the time needed for the next leader to be elected.

Signed-off-by: Monis Khan mok@vmware.com

Fixes #654
xref #828

Release note:

The pinniped supervisor now waits for up to a minute for active connections to become idle before exiting when told to terminate by the kubelet.

@codecov
Copy link

codecov bot commented Aug 30, 2021

Codecov Report

Merging #829 (0d285ce) into main (e43bd59) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #829      +/-   ##
==========================================
- Coverage   78.66%   78.64%   -0.02%     
==========================================
  Files         129      129              
  Lines        8941     8945       +4     
==========================================
+ Hits         7033     7035       +2     
- Misses       1680     1682       +2     
  Partials      228      228              
Impacted Files Coverage Δ
internal/concierge/server/server.go 27.50% <0.00%> (-0.47%) ⬇️
internal/leaderelection/leaderelection.go 52.12% <0.00%> (-1.14%) ⬇️
...l/localuserauthenticator/localuserauthenticator.go 57.40% <0.00%> (+0.92%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e43bd59...0d285ce. Read the comment docs.

@enj enj enabled auto-merge August 30, 2021 18:48
Changes made to both components:

1. Logs are always flushed on process exit
2. Informer cache sync can no longer hang process start up forever

Changes made to concierge:

1. Add pre-shutdown hook that waits for controllers to exit cleanly
2. Informer caches are synced in post-start hook

Changes made to supervisor:

1. Add shutdown code that waits for controllers to exit cleanly
2. Add shutdown code that waits for active connections to become idle

Waiting for controllers to exit cleanly is critical as this allows
the leader election logic to release the lock on exit.  This reduces
the time needed for the next leader to be elected.

Signed-off-by: Monis Khan <mok@vmware.com>
@cfryanr cfryanr disabled auto-merge August 31, 2021 18:30
@cfryanr cfryanr merged commit b19af2e into vmware-tanzu:main Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Informers should timeout if initial sync blocks forever
3 participants