Skip to content

Add HTTP/2 ping options to DestinationRule#3706

Open
mpuncel wants to merge 2 commits into
istio:masterfrom
mpuncel:mpuncel/http2-ping-destination-rule
Open

Add HTTP/2 ping options to DestinationRule#3706
mpuncel wants to merge 2 commits into
istio:masterfrom
mpuncel:mpuncel/http2-ping-destination-rule

Conversation

@mpuncel
Copy link
Copy Markdown

@mpuncel mpuncel commented May 12, 2026 •
edited
Loading

Summary

  • Adds connectionPool.http.http2ProtocolOptions.connectionKeepalive.interval/timeout to DestinationRule.
  • Regenerates protobuf, JSON/deepcopy/alias, docs, and CRD outputs.
  • Adds a CRD validation fixture covering the new YAML shape.

Related

Tests

  • buf lint
  • go test ./... from tests/
  • git diff --check

@mpuncel
Add HTTP/2 ping options to DestinationRule
e14cf02 
This is required for solving istio/istio#55640. In
particular it is useful for detecting dead TCP connections and/or keeping
connections alive through intermediate proxies.
@mpuncel mpuncel requested a review from a team as a code owner May 12, 2026 16:30
@istio-testing istio-testing added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test labels May 12, 2026
@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented May 12, 2026

Hi @mpuncel. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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-sigs/prow repository.

@mpuncel mpuncel mentioned this pull request May 12, 2026
Closed
ramaraochavali
ramaraochavali reviewed May 13, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@ramaraochavali ramaraochavali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question on the proto.

Can you explain your use case more - where you want to use this?

Comment thread networking/v1alpha3/destination_rule.proto Outdated
@mpuncel
Copy link
Copy Markdown
Author

mpuncel commented May 14, 2026

@ramaraochavali the use case is that sometimes in our service mesh we see "stuck" TCP connections, where the request appears to be written to the connection successfully but no response ever comes back, all requests time out for a period of time until the connection is torn down. Envoy HTTP/2 ping can detect this faster and help recover. We have seen this resolve the issue with HTTP/2 ping directly from gRPC client libraries, but want to make it possible to enable via Istio.

@istio-testing istio-testing added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 14, 2026
ramaraochavali
ramaraochavali reviewed May 14, 2026
View reviewed changes
Comment thread networking/v1alpha3/destination_rule.proto
int32 max_concurrent_streams = 8;

// Settings for HTTP/2 PING frames.
message ConnectionKeepalive {
Copy link
Copy Markdown
Contributor

@ramaraochavali ramaraochavali May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you planning to default the jitter to envoy's default?

Copy link
Copy Markdown
Author

@mpuncel mpuncel May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, should I make that configurable here?

Copy link
Copy Markdown
Contributor

@ramaraochavali ramaraochavali May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Just checking. Envoy default is fine.

ramaraochavali
ramaraochavali reviewed May 14, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@ramaraochavali ramaraochavali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me
@istio/technical-oversight-committee any objections to this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ramaraochavali ramaraochavali ramaraochavali left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

needs-ok-to-test size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mpuncel @istio-testing @ramaraochavali