-
Notifications
You must be signed in to change notification settings - Fork 136
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: bad connection handling for in cluster dialer #1800
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1800 +/- ##
==========================================
+ Coverage 62.50% 63.81% +1.31%
==========================================
Files 106 106
Lines 13417 13508 +91
==========================================
+ Hits 8386 8620 +234
+ Misses 4244 4077 -167
- Partials 787 811 +24
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Connections were closed from wrong end of io.Pipe which resulted in confusing error logs. Signed-off-by: Matej Vasek <mvasek@redhat.com>
Now DialContext() returns some errors immediately instead of deferring it Read/Write operation on the returned connection. Signed-off-by: Matej Vasek <mvasek@redhat.com>
Now ContextDial() tries to parse socat's stderr and translate it to Go's net.OpError instead of just creating error with whole stderr embedded in it. Signed-off-by: Matej Vasek <mvasek@redhat.com>
I recommend review each commit separately. |
@jrangelramos @lance PTAL |
Co-authored-by: Lance Ball <lball@redhat.com>
PTAL @zroubalik |
@zroubalik @lance we need to override the linter error here. |
@lkingland PTAL |
/override "style / suggester / shell" |
@lance: Overrode contexts on behalf of lance: style / suggester / github_actions, style / suggester / shell, style / suggester / yaml In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
/test unit-tests_func_main |
@matejvasek: No presubmit jobs available for knative/func@main In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lkingland, matejvasek 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 |
* fix: bad connection handling for in cluster dialer Connections were closed from wrong end of io.Pipe which resulted in confusing error logs. Signed-off-by: Matej Vasek <mvasek@redhat.com> * feat: DialContext() better error handling Now DialContext() returns some errors immediately instead of deferring it Read/Write operation on the returned connection. Signed-off-by: Matej Vasek <mvasek@redhat.com> * feat: DialContext() more better error handling Now ContextDial() tries to parse socat's stderr and translate it to Go's net.OpError instead of just creating error with whole stderr embedded in it. Signed-off-by: Matej Vasek <mvasek@redhat.com> * Apply suggestions from code review Co-authored-by: Lance Ball <lball@redhat.com> --------- Signed-off-by: Matej Vasek <mvasek@redhat.com> Co-authored-by: Lance Ball <lball@redhat.com>
…647) * fix: bad connection handling for in cluster dialer (knative#1800) * fix: bad connection handling for in cluster dialer Connections were closed from wrong end of io.Pipe which resulted in confusing error logs. Signed-off-by: Matej Vasek <mvasek@redhat.com> * feat: DialContext() better error handling Now DialContext() returns some errors immediately instead of deferring it Read/Write operation on the returned connection. Signed-off-by: Matej Vasek <mvasek@redhat.com> * feat: DialContext() more better error handling Now ContextDial() tries to parse socat's stderr and translate it to Go's net.OpError instead of just creating error with whole stderr embedded in it. Signed-off-by: Matej Vasek <mvasek@redhat.com> * Apply suggestions from code review Co-authored-by: Lance Ball <lball@redhat.com> --------- Signed-off-by: Matej Vasek <mvasek@redhat.com> Co-authored-by: Lance Ball <lball@redhat.com> * fixup: update golangci-lit version Signed-off-by: Matej Vasek <mvasek@redhat.com> * fixup: update CI Go version Signed-off-by: Matej Vasek <mvasek@redhat.com> --------- Signed-off-by: Matej Vasek <mvasek@redhat.com> Co-authored-by: Lance Ball <lball@redhat.com>
Changes
in cluster dialer
which caused confusing error messages (although functionality was not affected). Connections were closed from wrong end of io.Pipewhich resulted in confusing error logs.
ContextDial()
returns some errors immediately instead of deferring them onRead/Write
operation on returned connection.ContextDial()
tries to parsesocat
's stderr and translate it toGo
'snet.OpError
instead of just creating error with whole stderr embedded in it./kind bug
resolves: #1792