Use exec in registryctl so signals are passed properly #16642
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Mac Chaffee <machaffe@renci.org>
|
Friendly bump :) |
Codecov Report
@@ Coverage Diff @@
## main #16642 +/- ##
==========================================
+ Coverage 67.31% 67.33% +0.01%
==========================================
Files 953 968 +15
Lines 79028 81132 +2104
Branches 2331 2550 +219
==========================================
+ Hits 53198 54630 +1432
- Misses 22259 22823 +564
- Partials 3571 3679 +108
Flags with carried forward coverage won't be shown. Click here to find out more.
|
sluetze
pushed a commit
to sluetze/harbor
that referenced
this pull request
Oct 29, 2022
Signed-off-by: Mac Chaffee <machaffe@renci.org>
|
So it seems that code for handing graceful shutdown might be bugged: harbor/src/registryctl/main.go Line 72 in 77df4d9 When registryctl receives SIGTERM, it just logs the following but doesn't actually shut down: The bug isn't obvious to me as a golang novice, but is there some context we're not cancelling or a goroutine still running or something? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When containers are terminated in Kubernetes, Kubernetes will send a SIGTERM to PID1, wait for terminationGracePeriodSeconds, then SIGKILL it.
But since PID1 in registryctl is
bash, that SIGTERM signal is not automatically passed to the registryctl process, forcing Kubernetes to wait the whole terminationGracePeriodSeconds to update the registry Pod.This PR fixes that issue by using
exec, which makesregistryctlitself be PID1 so that it receives the SIGTERM signal.Wasn't able to test this locally, I think the build system needs some TLC, especially for testing individual components. Like for example, I need a local copy of
registryto build the registryctl image which is downloaded fromhttps://storage.googleapis.com/harbor-builds/bin/registry/release-v2.8.0-patch-redis/registry, but that URL 404s.