-
Notifications
You must be signed in to change notification settings - Fork 595
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
Disable chaosduck on the webhook #5419
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5419 +/- ##
=======================================
Coverage 82.62% 82.62%
=======================================
Files 194 194
Lines 5987 5987
=======================================
Hits 4947 4947
Misses 717 717
Partials 323 323 Continue to review full report at Codecov.
|
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, one minor comment.
There is another webhook inmemorychannel-webhook
:
eventing/config/channels/in-memory-channel/webhooks/resource-validation.yaml
Lines 24 to 26 in 23198cd
service: | |
name: inmemorychannel-webhook | |
namespace: knative-eventing |
eventing/config/channels/in-memory-channel/webhooks/defaulting.yaml
Lines 24 to 26 in 23198cd
service: | |
name: inmemorychannel-webhook | |
namespace: knative-eventing |
which points to imc-controller
, which is already disabled due to another issue #3590
@benmoss can you do me a favour and bump the timeout in a separate PR and see if the flake still occurs after some time https://github.com/knative/eventing/blob/main/config/core/webhooks/defaulting.yaml#L30 I noticed Istio has their timeout set to 30s. |
weird that we are still seeing EOFs on this PR even, maybe this isn't the fix we need /test pull-knative-eventing-reconciler-tests |
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.
/lgtm
/retest
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pierDipi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold |
Serving has done this as well, the chaosduck killing the webhook causes errors being tracked in knative/pkg#1509
I had a subtle mistake: |
let's run this a few times and see that it seems stable |
/lgtm unhold once you're ready to merge this. |
Worked, retesting |
@benmoss any comments on Dave's suggestion: |
I don't really understand why increasing the timeout would fix things, it's not a timeout issue but an EOF on an open connection |
/test pull-knative-eventing-reconciler-tests |
/hold cancel |
Serving has done this as well, the chaosduck killing the webhook causes errors being tracked in knative/pkg#1509
ref: knative/serving#8772
Fixes #5416, probably lots of others 😄