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
OCPBUGS-9037: Require http 1.1 or earlier when using curl #28301
OCPBUGS-9037: Require http 1.1 or earlier when using curl #28301
Conversation
Skipping CI for Draft Pull Request. |
/test e2e-gcp-ovn |
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.
It's surprising and unfortunate that ReadResponse
cannot handle HTTP/2. I guess there isn't a handy drop-in replacement for ReadResponse
?
To add more detail, there's a conversation about this in the issue golang/go#48766. I think the key takeaway is this comment from the issue:
When debugging this issue, I saw a number of suggestions from StackOverflow and elsewhere that suggested just replacing
Option (1) requires editing the output of curl, and technically is not even correct in light of the golang issue I mentioned above, so I'm disinclined to go with it. (2) is probably the most long-term effective solution, but I'm unsure how big of a change it'd be. (3) was the easiest to implement, since it just requires an additional flag passed to curl. It also shouldn't change the behavior of this function anywhere other than in the canary case, since if they were using HTTP/2 or newer connections, we'd already be seeing tests that use |
ReadResponse from golang's net/http package cannot parse http/2 response text as generated by curl. To work around this, force http/1.1 or earlier to be used.
becdc8a
to
f90ba61
Compare
@rfredette: This pull request references Jira Issue OCPBUGS-9037, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
/jira refresh |
@rfredette: This pull request references Jira Issue OCPBUGS-9037, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
/retest |
test failures look unrelated. |
test failures are unrelated. |
Thanks! |
@rfredette: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
/assign bparees |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: knobunc, Miciah, rfredette 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 |
@rfredette: Jira Issue OCPBUGS-9037: Some pull requests linked via external trackers have merged: The following pull requests linked via external trackers have not merged: These pull request must merge or be unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with Jira Issue OCPBUGS-9037 has not been moved to the MODIFIED state. 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. |
ReadResponse from golang's net/http package cannot parse http/2 response text as generated by curl. To work around this, force http/1.1 or earlier to be used.