Skip to content
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

configuration: Add support for Envoys MaxStreamDuration configuration #6895

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nickburgin
Copy link

@nickburgin nickburgin commented Feb 12, 2025

Fixes #6873

Signed-off-by: Nick Burgin [email protected]

Copy link

Hi @nickburgin! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

@nickburgin nickburgin force-pushed the support_max_stream_duration_config branch from f572f60 to 7b6bd26 Compare February 12, 2025 00:47
@nickburgin nickburgin force-pushed the support_max_stream_duration_config branch from 7b6bd26 to ee8ebee Compare February 12, 2025 02:13
@nickburgin nickburgin marked this pull request as ready for review February 12, 2025 02:18
@nickburgin nickburgin requested a review from a team as a code owner February 12, 2025 02:18
@nickburgin nickburgin requested review from tsaarni and sunjayBhatia and removed request for a team February 12, 2025 02:18
@sunjayBhatia sunjayBhatia requested review from a team, davinci26 and clayton-gonsalves and removed request for a team February 12, 2025 02:18
@nickburgin
Copy link
Author

nickburgin commented Feb 12, 2025

I wasn't able to add labels, but this should be considered release-note/small unless one of the maintainers thinks it should be a minor (In which case, I will need to change the release notes entry)

@tsaarni tsaarni added the release-note/small A small change that needs one line of explanation in the release notes. label Feb 12, 2025
Copy link

codecov bot commented Feb 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.79%. Comparing base (39c7cb9) to head (18151c3).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6895      +/-   ##
==========================================
+ Coverage   80.70%   80.79%   +0.09%     
==========================================
  Files         131      131              
  Lines       19805    19828      +23     
==========================================
+ Hits        15983    16021      +38     
+ Misses       3532     3522      -10     
+ Partials      290      285       -5     
Files with missing lines Coverage Δ
cmd/contour/servecontext.go 87.77% <100.00%> (+3.43%) ⬆️
internal/contourconfig/contourconfiguration.go 98.23% <100.00%> (+0.06%) ⬆️
internal/dag/dag.go 98.78% <ø> (ø)
internal/dag/policy.go 95.72% <100.00%> (+0.04%) ⬆️
internal/envoy/v3/route.go 80.53% <100.00%> (+0.11%) ⬆️
pkg/config/parameters.go 87.76% <100.00%> (+0.11%) ⬆️

…uration

Adds entirely missing test coverage for the timeouts configuration section in servecontext

Signed-off-by: Nick Burgin <[email protected]>
@nickburgin
Copy link
Author

Codecov Report

Attention: Patch coverage is 86.95652% with 3 lines in your changes missing coverage. Please review.

...

There were no tests for any of the timeout settings. I have added a basic test case for those to ensure they are all passed through as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/small A small change that needs one line of explanation in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support setting MaxStreamDuration of both specific routes and globally
2 participants