-
Notifications
You must be signed in to change notification settings - Fork 774
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
fix: unreliable webhook behaviour on gatekeeper pod startup and shutdown #3780
base: master
Are you sure you want to change the base?
fix: unreliable webhook behaviour on gatekeeper pod startup and shutdown #3780
Conversation
46de743
to
937a91e
Compare
937a91e
to
fcd6e13
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
fe4bc04
to
262f5c4
Compare
@JaydipGabani Integrated your suggestion. Could you please rerun the workflows? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3780 +/- ##
==========================================
- Coverage 54.49% 47.74% -6.75%
==========================================
Files 134 235 +101
Lines 12329 19852 +7523
==========================================
+ Hits 6719 9479 +2760
- Misses 5116 9484 +4368
- Partials 494 889 +395
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
3f4870c
to
8b50591
Compare
@JaydipGabani Could you please rerun the pipelines and merge this? I missed the window where I could have merged without rebase. |
@l0wl3vel we want approval from one more maintainer to merge. cc: @ritazh @maxsmythe @sozercan PTAL. |
Signed-off-by: Benjamin Ritter <[email protected]>
Signed-off-by: Benjamin Ritter <[email protected]>
Co-authored-by: Jaydip Gabani <[email protected]> Signed-off-by: Benjamin Ritter <[email protected]>
8b50591
to
5e8f3c4
Compare
What this PR does / why we need it:
This PR fixes webhook requests getting routed to starting and stopping gatekeeper pods, that are not ready to serve requests. The implications of this behaviour are explained in #3776
Which issue(s) this PR fixes:
Fixes #3776
Special notes for your reviewer:
The grace period for handling termination will not work when using Tilt to test. It will terminate instantly, like it did before.
We tracked this down to the rerun-process-handler Tilt uses to wrap the application to test.
The start.sh script traps SIGTERM and INTERRUPTS, the two signals we handle, and replaces them with SIGKILL, which we do not intercept.
Running
make run
yields the intended behaviour.Co-authored by Nils Federle (@nilsfed) and Paweł Kukliński (@pawelkuk)